diff mbox

What does rpc.mountd dlopen() libnfsjunct.so rather than libnfsjunct.so.0

Message ID 20140226161646.1520358b@notabene.brown (mailing list archive)
State New, archived
Headers show

Commit Message

NeilBrown Feb. 26, 2014, 5:16 a.m. UTC
See $SUBJ

Shared libraries are usually versioned so you can release a new version with
an incompatible API and gradually transition to it.

A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
prohibited from ever changing the API in an incompatible way.

Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
"-devel" package and so trip over this library which clearly needs to be
installed even if you aren't doing 'devel'opment.

I would like to change mountd as per the patch below to use the ".0" file.
I believe this will not break any installation as the ".so" is installed as a
symlink to the ".0" (or maybe ".0.0.0").

Would this be acceptable?

There is a bit of a discussion about this here:
  https://bugzilla.redhat.com/show_bug.cgi?id=889174

but I either don't understand it or don't agree with it.

Thanks,
NeilBrown

Comments

Chuck Lever Feb. 26, 2014, 6:06 a.m. UTC | #1
> On Feb 25, 2014, at 9:16 PM, NeilBrown <neilb@suse.de> wrote:
> 
> 
> See $SUBJ
> 
> Shared libraries are usually versioned so you can release a new version with
> an incompatible API and gradually transition to it.
> 
> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
> prohibited from ever changing the API in an incompatible way.

There is an API version field that mountd can examine before using the plug-in.  .so symlinks to whichever library version is the blessed one.

> 
> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
> "-devel" package and so trip over this library which clearly needs to be
> installed even if you aren't doing 'devel'opment.

I've seen the rpmlint warning for some time.  Never been sure exactly what to do about it, or whether it matters.

> 
> I would like to change mountd as per the patch below to use the ".0" file.
> I believe this will not break any installation as the ".so" is installed as a
> symlink to the ".0" (or maybe ".0.0.0").
> 
> Would this be acceptable?

That dlopen(3) should follow whatever packaging guidelines are set out by distributors.  Do what you guys agree is correct.

> 
> There is a bit of a discussion about this here:
>  https://bugzilla.redhat.com/show_bug.cgi?id=889174
> 
> but I either don't understand it or don't agree with it.
> 
> Thanks,
> NeilBrown
> 
> 
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index ca35de28847a..f6d78490954f 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -1139,7 +1139,7 @@ static struct exportent *lookup_junction(char *dom, const char *pathname,
>    struct link_map *map;
>    void *handle;
> 
> -    handle = dlopen("libnfsjunct.so", RTLD_NOW);
> +    handle = dlopen("libnfsjunct.so.0", RTLD_NOW);
>    if (handle == NULL) {
>        xlog(D_GENERAL, "%s: dlopen: %s", __func__, dlerror());
>        return NULL;
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
NeilBrown Feb. 26, 2014, 7:01 a.m. UTC | #2
On Tue, 25 Feb 2014 22:06:58 -0800 Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> 
> > On Feb 25, 2014, at 9:16 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > 
> > See $SUBJ
> > 
> > Shared libraries are usually versioned so you can release a new version with
> > an incompatible API and gradually transition to it.
> > 
> > A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
> > prohibited from ever changing the API in an incompatible way.
> 
> There is an API version field that mountd can examine before using the plug-in.  .so symlinks to whichever library version is the blessed one.

What exactly does "blessed one" mean...

Maybe the question I want to ask is:
 Is libnfsjunct.so something that only rpc.mountd would ever use and it could
 not possibly make any sense for anything else to ever use it?

If so, then we probably don't want to install the .so.0 or .so.0.0.0 files
and maybe should give libnfsjunct.so a different suffix because it is not
"shared" in any sense (I wanted to suggest ".lo" for loadable object, but I
think that is already used).

If not, then you could conceivably have two tool installed which use
different version, so it doesn't make sense for either to be "blessed".
The bare ".so" name would be "the version that should be used when building
anything from source".

Does the question have a simple answer?

Thanks,
NeilBrown
Simo Sorce Feb. 26, 2014, 2:39 p.m. UTC | #3
On Wed, 2014-02-26 at 16:16 +1100, NeilBrown wrote:
> See $SUBJ
> 
> Shared libraries are usually versioned so you can release a new version with
> an incompatible API and gradually transition to it.
> 
> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
> prohibited from ever changing the API in an incompatible way.
> 
> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
> "-devel" package and so trip over this library which clearly needs to be
> installed even if you aren't doing 'devel'opment.

Keep in mind this rule is there only for real shared libraries that are
loaded by the the system loader.

however it is waived for 'modules' that are opened dynamically but are
private to the application.

> I would like to change mountd as per the patch below to use the ".0" file.
> I believe this will not break any installation as the ".so" is installed as a
> symlink to the ".0" (or maybe ".0.0.0").
> 
> Would this be acceptable?

It looks to me like this is an internal module for mountd that is not
for use by other apps (which is why it is not versioned and can be
changed at will as it is deployed at the same time mountd is ?

Or am I wrong here ?

If I am not wrong I would be against this change personally and would
rather move the .so file in a private library dir (if it is not already
there) to make it clear it is a private module.

> There is a bit of a discussion about this here:
>   https://bugzilla.redhat.com/show_bug.cgi?id=889174
> 
> but I either don't understand it or don't agree with it.

Which part of it, there are different opinions stated in the bugzilla at
different times.

Simo.
Simo Sorce Feb. 26, 2014, 2:40 p.m. UTC | #4
On Wed, 2014-02-26 at 18:01 +1100, NeilBrown wrote:
> On Tue, 25 Feb 2014 22:06:58 -0800 Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> > 
> > 
> > > On Feb 25, 2014, at 9:16 PM, NeilBrown <neilb@suse.de> wrote:
> > > 
> > > 
> > > See $SUBJ
> > > 
> > > Shared libraries are usually versioned so you can release a new version with
> > > an incompatible API and gradually transition to it.
> > > 
> > > A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
> > > prohibited from ever changing the API in an incompatible way.
> > 
> > There is an API version field that mountd can examine before using the plug-in.  .so symlinks to whichever library version is the blessed one.
> 
> What exactly does "blessed one" mean...
> 
> Maybe the question I want to ask is:
>  Is libnfsjunct.so something that only rpc.mountd would ever use and it could
>  not possibly make any sense for anything else to ever use it?
> 
> If so, then we probably don't want to install the .so.0 or .so.0.0.0 files
> and maybe should give libnfsjunct.so a different suffix because it is not
> "shared" in any sense (I wanted to suggest ".lo" for loadable object, but I
> think that is already used).
> 
> If not, then you could conceivably have two tool installed which use
> different version, so it doesn't make sense for either to be "blessed".
> The bare ".so" name would be "the version that should be used when building
> anything from source".
> 
> Does the question have a simple answer?

Shared objects are always called .so, however private modules SHOULD be
installed in a package private directory and no in /usr/lib[64]

Simo.
Chuck Lever Feb. 26, 2014, 2:51 p.m. UTC | #5
> On Feb 25, 2014, at 11:01 PM, NeilBrown <neilb@suse.de> wrote:
> 
>> On Tue, 25 Feb 2014 22:06:58 -0800 Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>> 
>> 
>>> On Feb 25, 2014, at 9:16 PM, NeilBrown <neilb@suse.de> wrote:
>>> 
>>> 
>>> See $SUBJ
>>> 
>>> Shared libraries are usually versioned so you can release a new version with
>>> an incompatible API and gradually transition to it.
>>> 
>>> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
>>> prohibited from ever changing the API in an incompatible way.
>> 
>> There is an API version field that mountd can examine before using the plug-in.  .so symlinks to whichever library version is the blessed one.
> 
> What exactly does "blessed one" mean...

It means the one the local administrator chooses to use.  The only reason to have more than one plug-in version installed is when testing an upgrade.

> 
> Maybe the question I want to ask is:
> Is libnfsjunct.so something that only rpc.mountd would ever use and it could
> not possibly make any sense for anything else to ever use it?

There is a public header in /usr/include that contains the API definition.  In the future the plug-in might also be able to perform administrative operations on junctions, and other applications could invoke this functionality via the plug-in.

I don't think it's applicability is narrow, even though mountd is its only user today.

> 
> If so, then we probably don't want to install the .so.0 or .so.0.0.0 files
> and maybe should give libnfsjunct.so a different suffix because it is not
> "shared" in any sense (I wanted to suggest ".lo" for loadable object, but I
> think that is already used).
> 
> If not, then you could conceivably have two tool installed which use
> different version, so it doesn't make sense for either to be "blessed".
> The bare ".so" name would be "the version that should be used when building
> anything from source".

I don't understand this last bit.  The plug-in can get bug fixes and new features and mountd does not need to be rebuilt from source to use them.

> 
> Does the question have a simple answer?

I'm afraid I don't yet understand the problem you are trying to solve.

> 
> Thanks,
> NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever Feb. 26, 2014, 4:02 p.m. UTC | #6
On Feb 26, 2014, at 6:39 AM, Simo Sorce <simo@redhat.com> wrote:

> On Wed, 2014-02-26 at 16:16 +1100, NeilBrown wrote:
>> See $SUBJ
>> 
>> Shared libraries are usually versioned so you can release a new version with
>> an incompatible API and gradually transition to it.
>> 
>> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
>> prohibited from ever changing the API in an incompatible way.
>> 
>> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
>> "-devel" package and so trip over this library which clearly needs to be
>> installed even if you aren't doing 'devel'opment.
> 
> Keep in mind this rule is there only for real shared libraries that are
> loaded by the the system loader.
> 
> however it is waived for 'modules' that are opened dynamically but are
> private to the application.
> 
>> I would like to change mountd as per the patch below to use the ".0" file.
>> I believe this will not break any installation as the ".so" is installed as a
>> symlink to the ".0" (or maybe ".0.0.0").
>> 
>> Would this be acceptable?
> 
> It looks to me like this is an internal module for mountd that is not
> for use by other apps (which is why it is not versioned and can be
> changed at will as it is deployed at the same time mountd is ?

The plug-in API is versioned internally, but maybe I got that wrong, and should remove the API version field in favor of having consumers load via a specific .so number.

> Or am I wrong here ?
> 
> If I am not wrong I would be against this change personally and would
> rather move the .so file in a private library dir (if it is not already
> there) to make it clear it is a private module.

rpc.mountd is the only user currently, but it’s not necessarily private to mountd.  A generic storage manager tool might use it to resolve NFS and FedFS referrals for display, for example.  We could add plug-in API functions for creating and removing referrals to enable generic tools to perform these operations.

A separate directory makes sense if there’s more than one thing to put in it.  Right now we just have the plug-in library, and no plans to add more.

I took an expedient approach when implementing the plug-in, and could have gotten it wrong.  I’m open to make this mechanism fit packaging guidelines and requirements.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simo Sorce Feb. 26, 2014, 4:25 p.m. UTC | #7
On Wed, 2014-02-26 at 08:02 -0800, Chuck Lever wrote:
> On Feb 26, 2014, at 6:39 AM, Simo Sorce <simo@redhat.com> wrote:
> 
> > On Wed, 2014-02-26 at 16:16 +1100, NeilBrown wrote:
> >> See $SUBJ
> >> 
> >> Shared libraries are usually versioned so you can release a new version with
> >> an incompatible API and gradually transition to it.
> >> 
> >> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
> >> prohibited from ever changing the API in an incompatible way.
> >> 
> >> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
> >> "-devel" package and so trip over this library which clearly needs to be
> >> installed even if you aren't doing 'devel'opment.
> > 
> > Keep in mind this rule is there only for real shared libraries that are
> > loaded by the the system loader.
> > 
> > however it is waived for 'modules' that are opened dynamically but are
> > private to the application.
> > 
> >> I would like to change mountd as per the patch below to use the ".0" file.
> >> I believe this will not break any installation as the ".so" is installed as a
> >> symlink to the ".0" (or maybe ".0.0.0").
> >> 
> >> Would this be acceptable?
> > 
> > It looks to me like this is an internal module for mountd that is not
> > for use by other apps (which is why it is not versioned and can be
> > changed at will as it is deployed at the same time mountd is ?
> 
> The plug-in API is versioned internally, but maybe I got that wrong,
> and should remove the API version field in favor of having consumers
> load via a specific .so number.

Either way works the same, it just changes what component makes the
determination (app code vs linker)

> > Or am I wrong here ?
> > 
> > If I am not wrong I would be against this change personally and would
> > rather move the .so file in a private library dir (if it is not already
> > there) to make it clear it is a private module.
> 
> rpc.mountd is the only user currently, but it’s not necessarily
> private to mountd.  A generic storage manager tool might use it to
> resolve NFS and FedFS referrals for display, for example.  We could
> add plug-in API functions for creating and removing referrals to
> enable generic tools to perform these operations.

If it is a generic library why is it dlopened() instead of being simply
linked in at build time ?

> A separate directory makes sense if there’s more than one thing to put
> in it.  Right now we just have the plug-in library, and no plans to
> add more.

directories are cheap, don't fear them :)

> I took an expedient approach when implementing the plug-in, and could
> have gotten it wrong.  I’m open to make this mechanism fit packaging
> guidelines and requirements.

Packaging guidelines vary depending on whether the library is public or
private and therefore you need to guarantee ABI compatibility or not.

I think you need to make that determination first.

Simo.
Chuck Lever Feb. 26, 2014, 4:54 p.m. UTC | #8
On Feb 26, 2014, at 8:25 AM, Simo Sorce <simo@redhat.com> wrote:

> On Wed, 2014-02-26 at 08:02 -0800, Chuck Lever wrote:
>> On Feb 26, 2014, at 6:39 AM, Simo Sorce <simo@redhat.com> wrote:
>> 
>>> On Wed, 2014-02-26 at 16:16 +1100, NeilBrown wrote:
>>>> See $SUBJ
>>>> 
>>>> Shared libraries are usually versioned so you can release a new version with
>>>> an incompatible API and gradually transition to it.
>>>> 
>>>> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
>>>> prohibited from ever changing the API in an incompatible way.
>>>> 
>>>> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
>>>> "-devel" package and so trip over this library which clearly needs to be
>>>> installed even if you aren't doing 'devel'opment.
>>> 
>>> Keep in mind this rule is there only for real shared libraries that are
>>> loaded by the the system loader.
>>> 
>>> however it is waived for 'modules' that are opened dynamically but are
>>> private to the application.
>>> 
>>>> I would like to change mountd as per the patch below to use the ".0" file.
>>>> I believe this will not break any installation as the ".so" is installed as a
>>>> symlink to the ".0" (or maybe ".0.0.0").
>>>> 
>>>> Would this be acceptable?
>>> 
>>> It looks to me like this is an internal module for mountd that is not
>>> for use by other apps (which is why it is not versioned and can be
>>> changed at will as it is deployed at the same time mountd is ?
>> 
>> The plug-in API is versioned internally, but maybe I got that wrong,
>> and should remove the API version field in favor of having consumers
>> load via a specific .so number.
> 
> Either way works the same, it just changes what component makes the
> determination (app code vs linker)
> 
>>> Or am I wrong here ?
>>> 
>>> If I am not wrong I would be against this change personally and would
>>> rather move the .so file in a private library dir (if it is not already
>>> there) to make it clear it is a private module.
>> 
>> rpc.mountd is the only user currently, but it’s not necessarily
>> private to mountd.  A generic storage manager tool might use it to
>> resolve NFS and FedFS referrals for display, for example.  We could
>> add plug-in API functions for creating and removing referrals to
>> enable generic tools to perform these operations.
> 
> If it is a generic library why is it dlopened() instead of being simply
> linked in at build time ?

Handling NFS and FedFS junctions requires support for sqlite3, LDAP, and XML, among others.  The maintainer of nfs-utils preferred to add zero new build dependencies when we introduced this functionality.  The design we came up with was to dlopen() a library that would pull in everything that was needed at run time.

If the plug-in is not installed, mountd simply skips trying to resolve junctions.  This would be the case for embedded NFS servers, for example.

> 
>> A separate directory makes sense if there’s more than one thing to put
>> in it.  Right now we just have the plug-in library, and no plans to
>> add more.
> 
> directories are cheap, don't fear them :)
> 
>> I took an expedient approach when implementing the plug-in, and could
>> have gotten it wrong.  I’m open to make this mechanism fit packaging
>> guidelines and requirements.
> 
> Packaging guidelines vary depending on whether the library is public or
> private and therefore you need to guarantee ABI compatibility or not.
> 
> I think you need to make that determination first.

I attempted to guarantee API compatibility using the API version field and by publishing the API definition in a header under /usr/include.  By that definition it is a public API that happens to have only one current user.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simo Sorce Feb. 26, 2014, 5:10 p.m. UTC | #9
On Wed, 2014-02-26 at 08:54 -0800, Chuck Lever wrote:
> On Feb 26, 2014, at 8:25 AM, Simo Sorce <simo@redhat.com> wrote:
> 
> > On Wed, 2014-02-26 at 08:02 -0800, Chuck Lever wrote:
> >> On Feb 26, 2014, at 6:39 AM, Simo Sorce <simo@redhat.com> wrote:
> >> 
> >>> On Wed, 2014-02-26 at 16:16 +1100, NeilBrown wrote:
> >>>> See $SUBJ
> >>>> 
> >>>> Shared libraries are usually versioned so you can release a new version with
> >>>> an incompatible API and gradually transition to it.
> >>>> 
> >>>> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
> >>>> prohibited from ever changing the API in an incompatible way.
> >>>> 
> >>>> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
> >>>> "-devel" package and so trip over this library which clearly needs to be
> >>>> installed even if you aren't doing 'devel'opment.
> >>> 
> >>> Keep in mind this rule is there only for real shared libraries that are
> >>> loaded by the the system loader.
> >>> 
> >>> however it is waived for 'modules' that are opened dynamically but are
> >>> private to the application.
> >>> 
> >>>> I would like to change mountd as per the patch below to use the ".0" file.
> >>>> I believe this will not break any installation as the ".so" is installed as a
> >>>> symlink to the ".0" (or maybe ".0.0.0").
> >>>> 
> >>>> Would this be acceptable?
> >>> 
> >>> It looks to me like this is an internal module for mountd that is not
> >>> for use by other apps (which is why it is not versioned and can be
> >>> changed at will as it is deployed at the same time mountd is ?
> >> 
> >> The plug-in API is versioned internally, but maybe I got that wrong,
> >> and should remove the API version field in favor of having consumers
> >> load via a specific .so number.
> > 
> > Either way works the same, it just changes what component makes the
> > determination (app code vs linker)
> > 
> >>> Or am I wrong here ?
> >>> 
> >>> If I am not wrong I would be against this change personally and would
> >>> rather move the .so file in a private library dir (if it is not already
> >>> there) to make it clear it is a private module.
> >> 
> >> rpc.mountd is the only user currently, but it’s not necessarily
> >> private to mountd.  A generic storage manager tool might use it to
> >> resolve NFS and FedFS referrals for display, for example.  We could
> >> add plug-in API functions for creating and removing referrals to
> >> enable generic tools to perform these operations.
> > 
> > If it is a generic library why is it dlopened() instead of being simply
> > linked in at build time ?
> 
> Handling NFS and FedFS junctions requires support for sqlite3, LDAP,
> and XML, among others.  The maintainer of nfs-utils preferred to add
> zero new build dependencies when we introduced this functionality.
> The design we came up with was to dlopen() a library that would pull
> in everything that was needed at run time.
> 
> If the plug-in is not installed, mountd simply skips trying to resolve
> junctions.  This would be the case for embedded NFS servers, for
> example.
> 
Therefore this is an intimate library and the separation is a mere
exercise in keeping the ability to not drag in dependencies in some
install scenarios.

> >> A separate directory makes sense if there’s more than one thing to put
> >> in it.  Right now we just have the plug-in library, and no plans to
> >> add more.
> > 
> > directories are cheap, don't fear them :)
> > 
> >> I took an expedient approach when implementing the plug-in, and could
> >> have gotten it wrong.  I’m open to make this mechanism fit packaging
> >> guidelines and requirements.
> > 
> > Packaging guidelines vary depending on whether the library is public or
> > private and therefore you need to guarantee ABI compatibility or not.
> > 
> > I think you need to make that determination first.

I think based on the above that we are looking at a library that is
currently just a private plugin. The best course of action IMHO is to
move it to /usr/lib[64]/nfs-mountd or something so that it is clear that
it is a private plugin. At least until mountd is the only user.

> I attempted to guarantee API compatibility using the API version field
> and by publishing the API definition in a header under /usr/include.
> By that definition it is a public API that happens to have only one
> current user.

API compatibility is all you need for a private plugin indeed, and
perhaps not even that.

However for a public library what would matter is ABI compatibility, not
API compatibility.

Given it is a lot of effort to guarantee a public API and that there
really is no other user on the horizon I would recommend to consider
this code a private plugin and treat it as such.

That is:
1. consider it tightly integrated with rpc.mountd and to be installed in
lockstep
2. consider it's API/ABI not stable and a private contract within
rpc.mountd
3. package it accordingly

Simo.
Steve Dickson March 3, 2014, 5:23 p.m. UTC | #10
Sorry for getting into this so late... 

On 02/26/2014 11:54 AM, Chuck Lever wrote:
> 
> On Feb 26, 2014, at 8:25 AM, Simo Sorce <simo@redhat.com> wrote:
> 
>> On Wed, 2014-02-26 at 08:02 -0800, Chuck Lever wrote:
>>> On Feb 26, 2014, at 6:39 AM, Simo Sorce <simo@redhat.com> wrote:
>>>
>>>> On Wed, 2014-02-26 at 16:16 +1100, NeilBrown wrote:
>>>>> See $SUBJ
>>>>>
>>>>> Shared libraries are usually versioned so you can release a new version with
>>>>> an incompatible API and gradually transition to it.
>>>>>
>>>>> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
>>>>> prohibited from ever changing the API in an incompatible way.
>>>>>
>>>>> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
>>>>> "-devel" package and so trip over this library which clearly needs to be
>>>>> installed even if you aren't doing 'devel'opment.
>>>>
>>>> Keep in mind this rule is there only for real shared libraries that are
>>>> loaded by the the system loader.
>>>>
>>>> however it is waived for 'modules' that are opened dynamically but are
>>>> private to the application.
>>>>
>>>>> I would like to change mountd as per the patch below to use the ".0" file.
>>>>> I believe this will not break any installation as the ".so" is installed as a
>>>>> symlink to the ".0" (or maybe ".0.0.0").
>>>>>
>>>>> Would this be acceptable?
>>>>
>>>> It looks to me like this is an internal module for mountd that is not
>>>> for use by other apps (which is why it is not versioned and can be
>>>> changed at will as it is deployed at the same time mountd is ?
>>>
>>> The plug-in API is versioned internally, but maybe I got that wrong,
>>> and should remove the API version field in favor of having consumers
>>> load via a specific .so number.
>>
>> Either way works the same, it just changes what component makes the
>> determination (app code vs linker)
>>
>>>> Or am I wrong here ?
>>>>
>>>> If I am not wrong I would be against this change personally and would
>>>> rather move the .so file in a private library dir (if it is not already
>>>> there) to make it clear it is a private module.
>>>
>>> rpc.mountd is the only user currently, but it’s not necessarily
>>> private to mountd.  A generic storage manager tool might use it to
>>> resolve NFS and FedFS referrals for display, for example.  We could
>>> add plug-in API functions for creating and removing referrals to
>>> enable generic tools to perform these operations.
>>
>> If it is a generic library why is it dlopened() instead of being simply
>> linked in at build time ?
> 
> Handling NFS and FedFS junctions requires support for sqlite3, LDAP, 
> and XML, among others.  The maintainer of nfs-utils preferred to add zero 
> new build dependencies when we introduced this functionality.  The design 
> we came up with was to dlopen() a library that would pull in everything 
> that was needed at run time.
Right... The dependency list is way too long now and the last thing
I wanted was make nfs-utils depend on things it will probably
never use...  
 
> 
> If the plug-in is not installed, mountd simply skips trying to resolve junctions.  
> This would be the case for embedded NFS servers, for example.
I think this works well... 

I guess I don't care about the version-ing or where the library 
lives... but I don't want rpc.mountd to be depending on libnfsjunct.so
just use it when it exists. 

steved.
> 
>>
>>> A separate directory makes sense if there’s more than one thing to put
>>> in it.  Right now we just have the plug-in library, and no plans to
>>> add more.
>>
>> directories are cheap, don't fear them :)
>>
>>> I took an expedient approach when implementing the plug-in, and could
>>> have gotten it wrong.  I’m open to make this mechanism fit packaging
>>> guidelines and requirements.
>>
>> Packaging guidelines vary depending on whether the library is public or
>> private and therefore you need to guarantee ABI compatibility or not.
>>
>> I think you need to make that determination first.
> 
> I attempted to guarantee API compatibility using the API version field and by publishing the API definition in a header under /usr/include.  By that definition it is a public API that happens to have only one current user.
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Dickson March 3, 2014, 5:30 p.m. UTC | #11
On 02/26/2014 12:10 PM, Simo Sorce wrote:
> On Wed, 2014-02-26 at 08:54 -0800, Chuck Lever wrote:
>> On Feb 26, 2014, at 8:25 AM, Simo Sorce <simo@redhat.com> wrote:
>>
>>> On Wed, 2014-02-26 at 08:02 -0800, Chuck Lever wrote:
>>>> On Feb 26, 2014, at 6:39 AM, Simo Sorce <simo@redhat.com> wrote:
>>>>
>>>>> On Wed, 2014-02-26 at 16:16 +1100, NeilBrown wrote:
>>>>>> See $SUBJ
>>>>>>
>>>>>> Shared libraries are usually versioned so you can release a new version with
>>>>>> an incompatible API and gradually transition to it.
>>>>>>
>>>>>> A rpc.mountd dlopens libnfsjunct.so with no version it is effectively
>>>>>> prohibited from ever changing the API in an incompatible way.
>>>>>>
>>>>>> Both Fedora and openSUSE get upset about packaging a libFOO.so in a non
>>>>>> "-devel" package and so trip over this library which clearly needs to be
>>>>>> installed even if you aren't doing 'devel'opment.
>>>>>
>>>>> Keep in mind this rule is there only for real shared libraries that are
>>>>> loaded by the the system loader.
>>>>>
>>>>> however it is waived for 'modules' that are opened dynamically but are
>>>>> private to the application.
>>>>>
>>>>>> I would like to change mountd as per the patch below to use the ".0" file.
>>>>>> I believe this will not break any installation as the ".so" is installed as a
>>>>>> symlink to the ".0" (or maybe ".0.0.0").
>>>>>>
>>>>>> Would this be acceptable?
>>>>>
>>>>> It looks to me like this is an internal module for mountd that is not
>>>>> for use by other apps (which is why it is not versioned and can be
>>>>> changed at will as it is deployed at the same time mountd is ?
>>>>
>>>> The plug-in API is versioned internally, but maybe I got that wrong,
>>>> and should remove the API version field in favor of having consumers
>>>> load via a specific .so number.
>>>
>>> Either way works the same, it just changes what component makes the
>>> determination (app code vs linker)
>>>
>>>>> Or am I wrong here ?
>>>>>
>>>>> If I am not wrong I would be against this change personally and would
>>>>> rather move the .so file in a private library dir (if it is not already
>>>>> there) to make it clear it is a private module.
>>>>
>>>> rpc.mountd is the only user currently, but it’s not necessarily
>>>> private to mountd.  A generic storage manager tool might use it to
>>>> resolve NFS and FedFS referrals for display, for example.  We could
>>>> add plug-in API functions for creating and removing referrals to
>>>> enable generic tools to perform these operations.
>>>
>>> If it is a generic library why is it dlopened() instead of being simply
>>> linked in at build time ?
>>
>> Handling NFS and FedFS junctions requires support for sqlite3, LDAP,
>> and XML, among others.  The maintainer of nfs-utils preferred to add
>> zero new build dependencies when we introduced this functionality.
>> The design we came up with was to dlopen() a library that would pull
>> in everything that was needed at run time.
>>
>> If the plug-in is not installed, mountd simply skips trying to resolve
>> junctions.  This would be the case for embedded NFS servers, for
>> example.
>>
> Therefore this is an intimate library and the separation is a mere
> exercise in keeping the ability to not drag in dependencies in some
> install scenarios.
> 
>>>> A separate directory makes sense if there’s more than one thing to put
>>>> in it.  Right now we just have the plug-in library, and no plans to
>>>> add more.
>>>
>>> directories are cheap, don't fear them :)
>>>
>>>> I took an expedient approach when implementing the plug-in, and could
>>>> have gotten it wrong.  I’m open to make this mechanism fit packaging
>>>> guidelines and requirements.
>>>
>>> Packaging guidelines vary depending on whether the library is public or
>>> private and therefore you need to guarantee ABI compatibility or not.
>>>
>>> I think you need to make that determination first.
> 
> I think based on the above that we are looking at a library that is
> currently just a private plugin. The best course of action IMHO is to
> move it to /usr/lib[64]/nfs-mountd or something so that it is clear that
> it is a private plugin. At least until mountd is the only user.
> 
>> I attempted to guarantee API compatibility using the API version field
>> and by publishing the API definition in a header under /usr/include.
>> By that definition it is a public API that happens to have only one
>> current user.
> 
> API compatibility is all you need for a private plugin indeed, and
> perhaps not even that.
> 
> However for a public library what would matter is ABI compatibility, not
> API compatibility.
> 
> Given it is a lot of effort to guarantee a public API and that there
> really is no other user on the horizon I would recommend to consider
> this code a private plugin and treat it as such.
> 
> That is:
> 1. consider it tightly integrated with rpc.mountd and to be installed in
> lockstep
> 2. consider it's API/ABI not stable and a private contract within
> rpc.mountd
> 3. package it accordingly
No... I do not want to make rpc.mount and the libnfsjunct.so tightly
coupled... Again, the use of it... if it exists... paradigm is good...

steved.

> 
> Simo.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index ca35de28847a..f6d78490954f 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -1139,7 +1139,7 @@  static struct exportent *lookup_junction(char *dom, const char *pathname,
 	struct link_map *map;
 	void *handle;
 
-	handle = dlopen("libnfsjunct.so", RTLD_NOW);
+	handle = dlopen("libnfsjunct.so.0", RTLD_NOW);
 	if (handle == NULL) {
 		xlog(D_GENERAL, "%s: dlopen: %s", __func__, dlerror());
 		return NULL;