[Bug 1485458] Review Request: orangefs - parallel network file system ( formerly PVFS2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1485458



--- Comment #13 from Jonathan Dieter <jdieter@xxxxxxxxx> ---
(In reply to Martin Brandenburg from comment #12)
> (In reply to Jonathan Dieter from comment #10)
> > Ok, a quick note on the licensing.  The NTP license is actually a variant of
> > MIT, and, at least according to
> > https://lists.fedoraproject.org/archives/list/legal@xxxxxxxxxxxxxxxxxxxxxxx/
> > thread/JM7YJILE4GIFRD3J636EAT2PBOEND7WP/, should be listed as such.
> > 
> > And, according https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses,
> > you can list LGPLv2.1 as LGPLv2.
> 
> And LGPLv2 and LGPLv2+?  They're still listed separately.

Yes

> 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).

> > 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.

Ok, that's fine.  It's not a blocker.

> > * 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.

> 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.

> > * Remove the Provides: (but combinatorial builds should maybe provide
> >   "orangefs")

You've already done this.

> > * 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.

> > * 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).

> > * 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.

> 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.

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.

What exactly is /usr/lib64/liborangefsposix.so.2.9.6?
Its contents on my system:
GROUP ( -lofs -lorangefs -lpvfs2 -lrt -lm -llmdb  -ldl -lpthread -lpthread 
-lssl -lcrypto -L/usr/lib64 -libverbs )

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.

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?

Finally, for the server, the man page is for pvfs2.conf instead of
orangefs.conf.  This should be changed.

-- 
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




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux