https://bugzilla.redhat.com/show_bug.cgi?id=1485458 --- Comment #16 from Martin Brandenburg <martin@xxxxxxxxxxxxxxxxxxxxx> --- (In reply to Jonathan Dieter from comment #13) > (In reply to Martin Brandenburg from comment #12) > > (In reply to Jonathan Dieter from comment #10) > > > No you've misunderstood me, though you bring up another good point. If > > a pvfs2 filesystem appears in fstab, it cannot be mounted before the > > pvfs2-client (i.e. orangefs-client.service) is running. The mount will > > hang until either a timeout expires or the client starts running. Given > > systemd's parallel nature this might not actually cause a problem, but > > certainly isn't very clean. > > Ok, I get it. I have no idea what the answer is. You could ask on #systemd > on IRC, and see what they tell you. Beyond that, I don't know that I have a > solution (though, as a sysadmin, I find it a bit odd that you have to start > a local daemon before mounting a remote filesystem). Yeah it's a little obtuse. > > > * Use unbundled lmdb (see patch) > > > > I fixed this a while ago, though we are still shipping a copy of LMDB > > for those who want it. > > That's great, but please deal with it as required at > https://fedoraproject.org/wiki/ > Bundled_Libraries#Treatment_of_Bundled_Libraries. Namely, please delete the > lmdb directory in %prep so there's no possible way you could accidentally > build against it. Sounds good. Done. > > Do you use any of the security modes? We built all that stuff, but > > haven't really heard from anyone who uses it. I agree it would be nice > > to have, and I don't want to rule it out this early. > > FWIW, if I was going to deploy OrangeFS where I work, at least one of the > security modes would have to work for me. Quite. The advice I got from rdieter in #fedora-devel is to compile is three times within the specfile and then use subpackages. If there's no better solution, I may just declare it out of scope for this package and that if we want it, we'll have to make security a run-time option rather than a compile-time option. > > > * Fix strange aarch64-specific build failure, but conditionalize it > > > for now -- ask about it on devel list? > > > > > > src/client/usrint/pvfs-path.c: In function 'PVFS_expand_path': > > > src/client/usrint/pvfs-path.c:266:25: error: 'SYS_readlink' undeclared > > > (first use in this function); did you mean 'SYS_readlinkat'? > > > n = syscall(SYS_readlink, > > > ^~~~~~~~~~~~ > > > SYS_readlinkat > > > > They dropped obsolete syscalls in the kernel for the new architecture. > > That code is somewhat fragile (can't you tell from the fact it calls > > syscall instead of readlink?), so I'm not going to attempt to fix this, > > but I will forward the bug upstream. > > Please do, though it is not a blocker. There are test machines available to > Fedora maintainers for this kind of thing, but according to the list at > https://fedoraproject.org/wiki/ > Test_Machine_Resources_For_Package_Maintainers, none are aarch64. Yes, unfortunately the exotic architectures are harder to find. But we know what the problem is at least. > > > * Don't try to configure the server/client; just provide -example files > > > > Honestly that probably makes more sense than what I'm doing. I'm going > > to go ahead and do it. > > Ok, I think the way you did this probably wasn't the best choice (and I'm > not convinced this is the best direction to go for your users). > > Your two options are to put the examples in /usr/share/doc or to put the > nonfunctioning config files (marked as config in your spec) in the usual > locations (/etc and /etc/orangefs). I personally prefer the latter, but the > former is acceptable as well. Your current approach of putting *-example > into etc pollutes /etc with extra unused config files. > > The reason I prefer putting nonfunctioning config files straight into the > right locations is that, as a user, that's the first place I look when I > want to configure them. Copying the example config files from > /usr/share/doc/ is an extra step. I would recommend commenting out the line > in /etc/pvfs2tab with a comment above it briefly explaining the correct way > to configure it or at least pointing to a man page (see /etc/fstab for an > example of the kind of comment I'm looking for). Sounds good. For the server config, shall I put in a DeleteMe option that the server will throw a syntax error on? Well even without, the server will throw an error on the missing storage space, and the user will have to read the documentation or at least look in the config file to learn how to create it. > > > * pvfs2-{start,stop}-all should be %config as they need editing? > > > > They're also in /usr/sbin. Putting them somewhere as an example or > > extending them to take command line arguments may make more sense. > > If they require editing, you can't leave them in /usr/sbin as is. I'd mark > them as documentation. Right. I could move them to /usr/share/doc/orangefs, but I don't see where they require editing. > > There's some argument here that we should not package Karma, because it > > may not work. I will test it. I've also been advised to add > > --disable-threaded, which is non-default but supposedly the thread does > > not help performance. I'm less excited about enabling a bunch of > > non-default (and thus poorly tested) options, though. > > FWIW, I briefly tested it on a single-node filesystem, and it seemed to work > just fine. I thought so too, but see my response to Dave below. > Some other comments: > If possible, I think pvfs2-genconfig should output to > /etc/orangefs/orangefs.conf by default instead of requiring the user to > specify the location. This is not a blocker, but I think it will make > life easier for your users, especially if you choose to put orangefs.conf in > /usr/share/doc as orangefs-example.conf. I'll put in a default. It'll prompt, so the user can specify whatever, but the user can press return to get the default like some of the other prompts. This seems like something to submit upstream as well. > The orangefs-client systemd service isn't logging correctly. If you're > going to use a "simple" systemd service, you really need the following > flags: ExecStart=/usr/sbin/pvfs2-client -f --logtype=syslog > > Also, when pvfs2-client-core exits with an error, it doesn't get passed up > the stack, so systemd never realizes that the service errored out. I run the pvfs2-client-core from systemd now. > Starting the orangefs-client service on a system without the kernel module > enabled (I tested on a F26 machine) causes the client to die. If > orangefs-fuse is installed, how hard would it be to automatically fall back > to the fuse library? For that matter, is the orangefs-client service > required to use the FUSE module? It absolutely needs to print an error message if the kernel module is unavailable. That's been on my todo list for a while, but thanks for bringing it up. I don't think it'll be possible to fallback to FUSE. > Finally, for the server, the man page is for pvfs2.conf instead of > orangefs.conf. This should be changed. Done. (In reply to Dave Love from comment #14) > > > Hopefully this will sort out some of the rpmlint errors I'm seeing. I am a > > > bit concerned by the shared-lib-calls-exit warning I'm getting. Do you > > > intend for your library to actually exit the calling program as opposed to > > > returning an error? > > > > Unfortunately, probably yes. I've done some quick grepping, and many of > > them look like places where an assert wouldn't be unreasonable. > > For what it's worth, there are plenty of packages that won't worry > about that. > > > > * Use ./prepare in %prep on straight exported source > > > > Should we? The release tarballs will not require it. > > If you package from a release, no, it's not required. I don't know if > it's actually required otherwise, but it's normal ("best"?) practice. I've got to do it to remove LMDB, so it's in. > > > * Use interface-related soname (see patch); the interface was > > > sufficiently stable over some previous versions I checked with > > > abipkgdiff to keep a major version. > > > > Thanks for this. I assume you are good with me merging your > > orangefs-soname.patch upstream? > > Sure, if it's wanted. > > > > * It needs an IB version (unless you can configure for both IB and TCP > > > now) > > > > It complains loudly that performance will suffer, but it's wrong or at > > least misleading. Performance will only suffer if you actually attempt > > to use both at the same time. Just having the code built and available > > for run-time configuration doesn't harm anything. > > Oh, fine. I thought it was a real problem last time I asked. Can you > build with both and try to silence the complaint? We'll remove the complaint since it's wrong. > > > * Remove the Provides: (but combinatorial builds should maybe provide > > > "orangefs") > > > > > > * Make -libs-<transport> packages for use with mpi-io etc. > > > > I have to admit I'm not entirely sure what you're talking about with > > either of these. > > I think there was an explicit Provides of the libraries, which there > shouldn't be. With multiple packages for different security modes > (assuming you don't need them for different transports), you probably > want to be able to require something like "orangefs" to get one of > them. I think I fixed that. I'm going to look into the security modes though. > Assuming the -<transport> bit isn't relevant now, I can't remember > what was there, but to build MPI-IO support, you need a -devel package > and a package just providing relevant libraries without pulling in > executables. Okay MPI-IO. I do know about that. But OpenMPI or MPICH or something else? I don't have a preference, but I'll ask around here. > > > > > * Fix strange aarch64-specific build failure, but conditionalize it > > > for now -- ask about it on devel list? > > > > > > src/client/usrint/pvfs-path.c: In function 'PVFS_expand_path': > > > src/client/usrint/pvfs-path.c:266:25: error: 'SYS_readlink' undeclared > > > (first use in this function); did you mean 'SYS_readlinkat'? > > > n = syscall(SYS_readlink, > > > ^~~~~~~~~~~~ > > > SYS_readlinkat > > > > They dropped obsolete syscalls in the kernel for the new architecture. > > That code is somewhat fragile (can't you tell from the fact it calls > > syscall instead of readlink?), so I'm not going to attempt to fix this, > > but I will forward the bug upstream. > > You can at least conditionalize it so that you can build most of > orangefs on those platforms. I think I tested that. Jonathan, what do you think? The options are to drop support for aarch64 entirely since part of it will be missing or build what will work. > > > * Expand description? > > > > Not a bad idea. May I take (and probably tweak) your description? > > Sure, if its helpful. > > > > * Include pacemaker stuff > > > > Again I have to admit I don't know what this is. > > It's for high-availability support (not that I've used it). It's > trivial to put in, anyhow. It turns out we call it heartbeat, but it looks like the same thing. I'll look into this. > > > * pvfs2-{start,stop}-all should be %config as they need editing? > > > > They're also in /usr/sbin. Putting them somewhere as an example or > > extending them to take command line arguments may make more sense. > > Yes, or somehow make them extensible. See above. I don't see where they need editing, but I could be missing something. > > There's some argument here that we should not package Karma, because it > > may not work. > > I had a note that it had been replaced by grafana support, which I > assume I got from the list, but it sounds as if that's wrong. I had the same note. I don't think it's readily available. > > I will test it. I've also been advised to add > > --disable-threaded, which is non-default but supposedly the thread does > > not help performance. I'm less excited about enabling a bunch of > > non-default (and thus poorly tested) options, though. > > Right, if there's a good reason they're not defaulted other than build > requirements. I think a lot of stuff isn't tweaked out of the box simply because we haven't tested one way or the other. Git: https://github.com/omnibond/orangefs-fedora Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=22006250 Spec: http://dev.orangefs.org/2017/marbran/1002/orangefs.spec SRPM: http://dev.orangefs.org/2017/marbran/1002/orangefs-2.9.6-0.3.20171002.svn.fc26.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
