https://bugzilla.redhat.com/show_bug.cgi?id=1485458 --- Comment #9 from Martin Brandenburg <martin@xxxxxxxxxxxxxxxxxxxxx> --- Dave, yes we're interested in EPEL as well. The problem is the third-party kernel module. Fortunately we've solved this problem for newer kernels. I'm sorry to say that I haven't looked through your packages much, but I hope that we won't duplicate too much work going forward. There are a lot of options, and many of them are mutually exclusive and compile-time only which makes it hard to decide which are appropriate defaults. Jonathan, thanks for reviewing. (In reply to Jonathan Dieter from comment #8) > (In reply to Martin Brandenburg from comment #0) > > This is my first package. I represent the upstream developers. I have > > fixed many problems since our last release 2.9.6 while making this > > package. We intend to fix any further problems uncovered by this review > > and make a new release 2.9.7 if this package is accepted. As such, my > > RPM currently builds our SVN trunk. > > As Alexander pointed out, at least until 2.9.7 has been built you really > need to use proper pre-release versioning > (https://fedoraproject.org/wiki/ > Package_Versioning_Examples#Complex_versioning_examples, scroll down to pkg > pre-release svn checkout) > > This allows us to verify that the code submitted matches the upstream code. That's easy enough. > If it's not built, you don't need to list the licenses. But I would wonder > why you're not building these modules (obviously excepting the kernel > module), and splitting them into subpackages so your users can easily get > the parts of OrangeFS that they want. > > I noticed the following options not being set by configure: > PVFS2 configured to build karma gui : no > PVFS2 configured to build visualization tools : no > PVFS2 configured to perform coverage analysis : no > PVFS2 configured to use FUSE : no > PVFS2 configured for the 2.6/3 kernel module : no > PVFS2 configured for the 2.4.x kernel module : no > PVFS2 configured for using the ra-cache : no > PVFS2 configured for resetting file position : no > PVFS2 will use workaround for redhat 2.4 kernels : no > PVFS2 will use workaround for buggy NPTL : no > PVFS2 server capability cache enabled : no > PVFS2 server credential cache enabled : no > PVFS2 server certificate cache enabled : no > PVFS2 configured with key-based security : no > PVFS2 configured with certificate-based security : no > PVFS2 configured with profiling enabled : no > PVFS2 user interface libraries will be built : no > PVFS2 symbolic libraries will be built : no > PVFS2 user environment variables enabled : no > PVFS2 user interface library cache enabled : no > PVFS2 client JNI enabled : no > > Obviously some of those are irrelevant to Fedora, but what about the karma > gui, visualization tools, ra-cache, server caches, key-based security, > certificate-based security, etc. Are these all obsolete or irrelevant to > Fedora, or would some of those be useful to your users? Some of these have not been dealt with in a long time, but you're certainly right that some of it may be useful. It turns out that the "visualization tools" don't build. Obviously the kernel modules are unnecessary. You can see the age of this stuff. We actually don't ship the 2.4 code. I wonder why these messages survived. I wonder what these workarounds are. They're before my time. The ra-cache is new and not very well tested. I don't know what "resetting file position" is, but I have an idea. It causes read/write to falsely avoid returning partial reads/writes. It was a hack for someone who would not fix their application, and I hope nobody else wants it. It should just be deleted. The caches affect the security modes. OrangeFS supports three security modes, which are dealt with at compile time. It would be nice to build all three as mutually exclusive packages. (It would be nicer to make it a runtime option, but that's difficult.) The user interface library is the libc interposer I mentioned before. I don't know what a symbolic library is. We put a lot of effort into JNI, and it would be nice to build it, but I personally don't know much about it. It breaks down in to obsolete things which should just be deleted, and useful but non-core features. I wasn't going to tackle non-core stuff yet, but it's definitely on the to-do list before I'd want the package released in Fedora. Honestly for some of this, just getting it regularly built (and tested!) will be an improvement to the status quo. It at least forces us to make the decision whether to remove old crufty bits or keep them working. So I've enabled Karma, FUSE, and (stopped disabling) the usrint. That leaves JNI and the webpack as far as I can see as optional things that can be enabled. Then there's stuff like the caches where we have to consider stability with and without. I will discuss this with other OrangeFS developers. I've enabled Infiniband as well. > > I am not sure how to handle this. I assume that the BSD/MIT style > > licenses will not pose a problem, but I don't know where to document it. > > Normally like this: GPLv3 and BSD and ... > > > OrangeFS includes a copy of LMDB. It does not link against any external > > package. Does this need to be changed in upstream? > > According to the Guidelines, if upstream *can't* be built against the system > version of LMDB, then you can bundle it, while following the guidelines on > how to do that > (https://fedoraproject.org/wiki/Packaging: > Guidelines#Bundling_and_Duplication_of_system_libraries), but, given that > you are upstream, it would be *much* better if you could make the changes > required to build against the system library. You really don't want to be > in a position where your bundled library has a vulnerability that's been > patched in the system library, but not in your bundled library. I've been >?deve there. It sucks. I've fixed upstream to allow linking against the system LMDB. > > OrangeFS requires some configuration to start and creates some files at > > runtime. A simple configuration file that configures a one-machine file > > system still requires the local hostname. We generally use a tool to > > generate configuration files. Then the storage database must be > > initialized. I have a commented postinstall script. How do I handle > > this? > > My recommendation would be to put pvfs2-genconfig in /usr/bin, as you > already have, but possibly throw a simple config in > /etc/orangefs/orangefs.conf. If the config could contain minimal (even if > it doesn't work out of the box) configuration and a comment explaining the > best way to properly generate it using pvfs2-genconfig, that would make it > easy enough for even a non-documentation-reading sysadmin like myself to > figure out what to do. > > Whatever you do, it looks like there are enough configuration files > (certificates, etc as well as orangefs.conf) that it would be worth creating > and owning /etc/orangefs/ and using that as your default configuration > location. I've done this now. Most of the client applications look for /etc/pvfs2tab. Should I patch it to look for /etc/orangefs/pvfs2tab? The server configuration is given on the command line, but this is hardcoded (or settable by environment variable). > > The storage database is currently /usr/storage (i.e. $PREFIX/storage). > > Obviously this is not right. I suppose it should go in /var somewhere. > > Where should it go? I also don't know how to mark this directory as > > owned by the package. Should it be deleted on package uninstall since > > it contains user data? > > /var/lib/orangefs or /var/lib/orangefs/storage if there are other state > files kept in /var/lib/orangefs. I'm assuming the admin can change this > location in the config file. > > > The storage database contains a number called the FSID (file system > > identifier). It is generally different for each installation > > (calculated randomly), so distributing one base storage directory would > > be impractical. > > I'm assuming storage database generation is manual. If so, I would use > /var/lib/orangefs as the default location (as generated by pvfs2-genconfig), > but when the sysadmin runs pvfs2-genconfig, they will obviously be given the > chance to change this. All this makes sense. We have encouraged people to chose paths ad hoc in the past, and I would really like to move away from this. I have patched genconfig to default to Fedora-appropriate paths and options. I've also produced the "example" configuration file mentioned above and ensured that genconfig produces the same output (modulo the random ID). The administrator can set whatever path is preferred. > > I have written a systemd file for the server. I'm not really sure how > > to make it not run if the file system has not yet been initialized. > > If the server exits with a sane enough message ("Your file system has not > yet been initialized. Please read the configuration file/documentation for > more details"), then I wouldn't worry about it. It does. > > The utility programs distributed and others which can be linked against > > our libraries will run against the server. It is also possible to use a > > userspace client program along with the kernel module (distributed by > > kernel.org and already present in Fedora). This would require running > > the client program and mounting the filesystem through the kernel. I > > suppose systemd scripts are needed, but I'm not sure what to distribute. > > If the client needs to be run with arguments to mount the filesystem, I'd > probably not bother with a systemd service, but if it reads the > configuration from /etc, a service might make sense. It doesn't need any arguments. The mount request comes in from the kernel with all the information it needs. I've added a systemd unit for it. I'm not sure how to arrange things so that systemd doesn't try to mount the filesystem before the daemon is started. > > The server logs to /var/log/orangefs-server.log and the client logs to > > /tmp/pvfs2-client.log. Obviously /tmp/pvfs2-client.log should be moved > > to /var/log. The packaging system does not know about either file yet. > > Can both log to the journal? Or to syslog (which will get forwarded to the > journal)? I believe that would be the most sane option. I've patched genconfig to product configuration which logs to syslog by default. It'll probably be better to clean up and upstream this. > > We have a component called the usrint which is a libc interposer. > > Several of our utility programs require it now. It does not build on > > all platforms. It is not required to run either the server or the > > client. I have disabled it completely. > > If some of your utilities require it and it's important, I would probably > check that it builds on all primary architectures. If it does, I'd enable > it, and then use conditionals to disable building it on the arches it > crashes on. Then open a bug for each arch it crashes on, (similar to what's > described at > https://fedoraproject.org/wiki/Packaging: > Guidelines#Architecture_Build_Failures). It's possible someone who knows > those arches may fix the bugs. If it doesn't build on all primary > architectures (which I believe are currently ARM and x86_64), then I'd try > to get it working at least for those, but it's not required by the > guidelines. > > The one other thing I noticed is that you're packaging .a files in your > -devel package. I'd remove them as part of your build process. I figure somebody may want to link statically, but I have no preference either way, and this is easy to change. I've left it in for now, but will remove if there's a Fedora policy or anyone has a strong opinion. > There are other issues, but I'd like to see the above dealt with before we > start moving through the detailed review. I will update next week with an updated spec and SRPM fixing at least some of this. I have some arch-specific stuff now. On armv7hl, we disable Infiniband since there is no package libibverbs-devel. I've disabled aarch64. The other option is to disable the usrint only on aarch64, but I don't like cutting out a big part of the package only on one architecture. Here is a new build: Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=21890828 Spec: http://dev.orangefs.org/2017/marbran/0915/orangefs.spec SRPM: http://dev.orangefs.org/2017/marbran/0915/orangefs-2.9.6-0.1.20170904svn.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
