| Message ID | 20241202203046.1436990-1-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [nfs-utils] exports: Fix referrals when --enable-junction=no | expand |
On Mon, Dec 2, 2024 at 9:30 PM Scott Mayhew <smayhew@redhat.com> wrote: > Commit 15dc0bea ("exportd: Moved cache upcalls routines into > libexport.a") caused write_fsloc() to be elided when junction support is > disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so > that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should > be around actual junction code). > > Fixes: 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") > Signed-off-by: Scott Mayhew <smayhew@redhat.com> [snip] Reviewed-by: Roland Mainz <roland.mainz@nrubsig.org> ---- Bye, Roland
On Mon, Dec 02, 2024 at 03:30:46PM -0500, Scott Mayhew wrote: > Commit 15dc0bea ("exportd: Moved cache upcalls routines into > libexport.a") caused write_fsloc() to be elided when junction support is > disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so > that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should > be around actual junction code). I agree, this looks like an unintended regression in 15dc0bea. Reviewed-by: Chuck Lever <chuck.lever@oracle.com> I suggest adding: Link: https://bugs.debian.org/1035908 Link: https://bugs.debian.org/1083098 > Fixes: 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > --- > support/export/cache.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/support/export/cache.c b/support/export/cache.c > index 6c0a44a3..3a8e57cf 100644 > --- a/support/export/cache.c > +++ b/support/export/cache.c > @@ -34,10 +34,7 @@ > #include "pseudoflavors.h" > #include "xcommon.h" > #include "reexport.h" > - > -#ifdef HAVE_JUNCTION_SUPPORT > #include "fsloc.h" > -#endif > > #ifdef USE_BLKID > #include "blkid/blkid.h" > @@ -999,7 +996,6 @@ static void nfsd_retry_fh(struct delayed *d) > *dp = d; > } > > -#ifdef HAVE_JUNCTION_SUPPORT > static void write_fsloc(char **bp, int *blen, struct exportent *ep) > { > struct servers *servers; > @@ -1022,7 +1018,6 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep) > qword_addint(bp, blen, servers->h_referral); > release_replicas(servers); > } > -#endif > > static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask, int extra_flag) > { > @@ -1120,9 +1115,7 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain, > qword_addint(&bp, &blen, exp->e_anongid); > qword_addint(&bp, &blen, fsidnum); > > -#ifdef HAVE_JUNCTION_SUPPORT > write_fsloc(&bp, &blen, exp); > -#endif > write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0); > if (exp->e_uuid == NULL || different_fs) { > char u[16]; > -- > 2.46.2 > >
Hey, On 12/2/24 3:30 PM, Scott Mayhew wrote: > Commit 15dc0bea ("exportd: Moved cache upcalls routines into > libexport.a") caused write_fsloc() to be elided when junction support is > disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so > that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should > be around actual junction code). Why not just take the enable_junction config variable out of configure.ac as well? If we want junctions/referrals (which are the same) IMHO... on all the time... Lets not be able to turn them off at all? Point being... if we are going remove 3 of the 4 HAVE_JUNCTION_SUPPORT ifdefs... let get ride of all of them. steved. > > Fixes: 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > --- > support/export/cache.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/support/export/cache.c b/support/export/cache.c > index 6c0a44a3..3a8e57cf 100644 > --- a/support/export/cache.c > +++ b/support/export/cache.c > @@ -34,10 +34,7 @@ > #include "pseudoflavors.h" > #include "xcommon.h" > #include "reexport.h" > - > -#ifdef HAVE_JUNCTION_SUPPORT > #include "fsloc.h" > -#endif > > #ifdef USE_BLKID > #include "blkid/blkid.h" > @@ -999,7 +996,6 @@ static void nfsd_retry_fh(struct delayed *d) > *dp = d; > } > > -#ifdef HAVE_JUNCTION_SUPPORT > static void write_fsloc(char **bp, int *blen, struct exportent *ep) > { > struct servers *servers; > @@ -1022,7 +1018,6 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep) > qword_addint(bp, blen, servers->h_referral); > release_replicas(servers); > } > -#endif > > static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask, int extra_flag) > { > @@ -1120,9 +1115,7 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain, > qword_addint(&bp, &blen, exp->e_anongid); > qword_addint(&bp, &blen, fsidnum); > > -#ifdef HAVE_JUNCTION_SUPPORT > write_fsloc(&bp, &blen, exp); > -#endif > write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0); > if (exp->e_uuid == NULL || different_fs) { > char u[16];
On 12/2/24 4:41 PM, Chuck Lever wrote: > On Mon, Dec 02, 2024 at 03:30:46PM -0500, Scott Mayhew wrote: >> Commit 15dc0bea ("exportd: Moved cache upcalls routines into >> libexport.a") caused write_fsloc() to be elided when junction support is >> disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so >> that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should >> be around actual junction code). > > I agree, this looks like an unintended regression in 15dc0bea. > > Reviewed-by: Chuck Lever <chuck.lever@oracle.com> > > I suggest adding: > > Link: https://bugs.debian.org/1035908 > Link: https://bugs.debian.org/1083098 > >> Fixes: 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") >> Signed-off-by: Scott Mayhew <smayhew@redhat.com> Fair enough... I'm also going to tone down the "bogus" to "unnecessary"... Lets keep it civil. steved. >> --- >> support/export/cache.c | 7 ------- >> 1 file changed, 7 deletions(-) >> >> diff --git a/support/export/cache.c b/support/export/cache.c >> index 6c0a44a3..3a8e57cf 100644 >> --- a/support/export/cache.c >> +++ b/support/export/cache.c >> @@ -34,10 +34,7 @@ >> #include "pseudoflavors.h" >> #include "xcommon.h" >> #include "reexport.h" >> - >> -#ifdef HAVE_JUNCTION_SUPPORT >> #include "fsloc.h" >> -#endif >> >> #ifdef USE_BLKID >> #include "blkid/blkid.h" >> @@ -999,7 +996,6 @@ static void nfsd_retry_fh(struct delayed *d) >> *dp = d; >> } >> >> -#ifdef HAVE_JUNCTION_SUPPORT >> static void write_fsloc(char **bp, int *blen, struct exportent *ep) >> { >> struct servers *servers; >> @@ -1022,7 +1018,6 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep) >> qword_addint(bp, blen, servers->h_referral); >> release_replicas(servers); >> } >> -#endif >> >> static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask, int extra_flag) >> { >> @@ -1120,9 +1115,7 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain, >> qword_addint(&bp, &blen, exp->e_anongid); >> qword_addint(&bp, &blen, fsidnum); >> >> -#ifdef HAVE_JUNCTION_SUPPORT >> write_fsloc(&bp, &blen, exp); >> -#endif >> write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0); >> if (exp->e_uuid == NULL || different_fs) { >> char u[16]; >> -- >> 2.46.2 >> >> >
On Mon, 02 Dec 2024, Steve Dickson wrote: > Hey, > > On 12/2/24 3:30 PM, Scott Mayhew wrote: > > Commit 15dc0bea ("exportd: Moved cache upcalls routines into > > libexport.a") caused write_fsloc() to be elided when junction support is > > disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so > > that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should > > be around actual junction code). > Why not just take the enable_junction config variable > out of configure.ac as well? > > If we want junctions/referrals (which are the same) > IMHO... on all the time... Lets not be able to > turn them off at all? > > Point being... if we are going remove 3 of the 4 > HAVE_JUNCTION_SUPPORT ifdefs... let get ride of > all of them. Junctions and referrals are _not_ the same. A junction is one mechanism that can be used to generate a referral. The other way to generate a referral is with an export entry, and that is the method that stopped working after 15dc0bea. When you set up a referral via an export entry you use the refer= export option, and the directory must be a mountpoint so that nfsd will consult the export cache when the client tries to access the directory. For example, I set up the following in /etc/exports: /export *(rw,insecure,no_root_squash) /export/ref *(rw,insecure,no_root_squash,refer=/export@192.168.124.66) After the client tries to access /export/ref, this is what I see when I dump the export cache without my fix: [root@rawhide ~]# cat /proc/net/rpc/nfsd.export/content #path domain(flags) / *(ro,insecure,no_root_squash,sync,no_wdelay,no_subtree_check,v4root,fsid=0,sec=1) /export *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) /export/ref *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) Notice there's no refer= option. So when the client does a LOOKUP of /export/ref, the server treats it as a normal directory... it doesn't return NFS4ERR_MOVED and so the client doesn't know to query fs_locations. Here is what the export cache looks like with my fix: [root@rawhide ~]# cat /proc/net/rpc/nfsd.export/content #path domain(flags) /export *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) / *(ro,insecure,no_root_squash,sync,no_wdelay,no_subtree_check,v4root,fsid=0,sec=1) /export/ref *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,refer=/export@192.168.124.66,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) Note the refer= option is present, and the referral works normally. A junction is basically a fancy directory that has the user/group/other mode bits set to 0 and the sticky bit turned on. The original mode bits are stored in the trusted.junction.mode extended attribute and the referral information is stored in the trusted.junction.nfs extended attribute. Continuing with my previous example, I have this in my /etc/exports [root@rawhide ~]# cat /etc/exports /export *(rw,insecure,no_root_squash) /export/ref *(rw,insecure,no_root_squash,refer=/export@192.168.124.66) Let's add a referral using a junction. [root@rawhide ~]# nfsref add /export/junc 192.168.124.66 /export Created junction /export/junc In this case, /export/junc didn't previously exist, so the nfsref tool created it. If /export/junc did already exist, then the original mode would be stored in the trusted.junction.mode and the original contents of the directory would be hidden from the client (as well as non-privileged users on the server). You can look up the referral info using 'nfsref lookup': [root@rawhide ~]# nfsref lookup /export/junc 192.168.124.66:/export NFS port: 2049 Valid for: 0 Currency: -1 Flags: varsub(false) GenFlags: writable(false), going(false), split(true) TransFlags: rdma(true) Class: simul(0), handle(0), fileid(0) Class: writever(0), change(0), readdir(0) Read: rank(0), order(0) Write: rank(0), order(0) Or you can just use getfattr if you want to see the raw xml: [root@rawhide ~]# getfattr --only-values -d -m trusted.junction.nfs /export/junc getfattr: Removing leading '/' from absolute path names <?xml version="1.0" encoding="UTF-8"?> <junction> <savedmode bits="755"/> <fileset> <location> <host name="192.168.124.66"/> <path> <component>export</component> </path> <currency>-1</currency> <genflags writable="false" going="false" split="true"/> <transflags rdma="true"/> <class simul="0" handle="0" fileid="0" writever="0" change="0" readdir="0"/> <read rank="0" order="0"/> <write rank="0" order="0"/> <flags varsub="false"/> <validfor>0</validfor> </location> </fileset> </junction> Note that since the /export/junc referral is stored in a junction, it doesn't appear in the export info: [root@rawhide ~]# exportfs -v /export <world>(sync,wdelay,hide,no_subtree_check,sec=sys,rw,insecure,no_root_squash,no_all_squash) /export/ref <world>(sync,wdelay,hide,no_subtree_check,refer=/export@192.168.124.66,sec=sys,rw,insecure,no_root_squash,no_all_squash) From the client's standpoint, both style referrals work the same: root@aion:~# mount 192.168.124.26:/export /mnt/t root@aion:~# ls /mnt/t junc ref root@aion:~# ls /mnt/t/ref file root@aion:~# cat /mnt/t/ref/file I am on the referral server. root@aion:~# grep nfs4 /proc/mounts 192.168.124.26:/export /mnt/t nfs4 rw,relatime,vers=4.2,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.26 0 0 192.168.124.66:/export /mnt/t/ref nfs4 rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.66 0 0 root@aion:~# ls /mnt/t/junc file root@aion:~# cat /mnt/t/junc/file I am on the referral server. root@aion:~# grep nfs4 /proc/mounts 192.168.124.26:/export /mnt/t nfs4 rw,relatime,vers=4.2,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.26 0 0 192.168.124.66:/export /mnt/t/ref nfs4 rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.66 0 0 192.168.124.66:/export /mnt/t/junc nfs4 rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.66 0 0 So if you want to get rid of that last #ifdef HAVE_JUNCTION_SUPPORT then you have 2 options: a) get rid of junctions entirely, leaving users with only the old (relatively speaking) method for configuring referrals b) force all packagers of nfs-utils to pull in the extra dependencies needed to support junctions, which is the exact opposite of what the Debian folks are requesting. Or you can take the patch and we can continue to have both style referrals. -Scott > > steved. > > > > Fixes: 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") > > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > > --- > > support/export/cache.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/support/export/cache.c b/support/export/cache.c > > index 6c0a44a3..3a8e57cf 100644 > > --- a/support/export/cache.c > > +++ b/support/export/cache.c > > @@ -34,10 +34,7 @@ > > #include "pseudoflavors.h" > > #include "xcommon.h" > > #include "reexport.h" > > - > > -#ifdef HAVE_JUNCTION_SUPPORT > > #include "fsloc.h" > > -#endif > > #ifdef USE_BLKID > > #include "blkid/blkid.h" > > @@ -999,7 +996,6 @@ static void nfsd_retry_fh(struct delayed *d) > > *dp = d; > > } > > -#ifdef HAVE_JUNCTION_SUPPORT > > static void write_fsloc(char **bp, int *blen, struct exportent *ep) > > { > > struct servers *servers; > > @@ -1022,7 +1018,6 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep) > > qword_addint(bp, blen, servers->h_referral); > > release_replicas(servers); > > } > > -#endif > > static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask, int extra_flag) > > { > > @@ -1120,9 +1115,7 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain, > > qword_addint(&bp, &blen, exp->e_anongid); > > qword_addint(&bp, &blen, fsidnum); > > -#ifdef HAVE_JUNCTION_SUPPORT > > write_fsloc(&bp, &blen, exp); > > -#endif > > write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0); > > if (exp->e_uuid == NULL || different_fs) { > > char u[16]; >
On 12/3/24 7:43 AM, Scott Mayhew wrote: > On Mon, 02 Dec 2024, Steve Dickson wrote: > >> Hey, >> >> On 12/2/24 3:30 PM, Scott Mayhew wrote: >>> Commit 15dc0bea ("exportd: Moved cache upcalls routines into >>> libexport.a") caused write_fsloc() to be elided when junction support is >>> disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so >>> that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should >>> be around actual junction code). >> Why not just take the enable_junction config variable >> out of configure.ac as well? >> >> If we want junctions/referrals (which are the same) >> IMHO... on all the time... Lets not be able to >> turn them off at all? >> >> Point being... if we are going remove 3 of the 4 >> HAVE_JUNCTION_SUPPORT ifdefs... let get ride of >> all of them. > > Junctions and referrals are _not_ the same. A junction is one mechanism > that can be used to generate a referral. The other way to generate a > referral is with an export entry, and that is the method that stopped > working after 15dc0bea. > > When you set up a referral via an export entry you use the refer= > export option, and the directory must be a mountpoint so that nfsd will > consult the export cache when the client tries to access the directory. > > For example, I set up the following in /etc/exports: > > /export *(rw,insecure,no_root_squash) > /export/ref *(rw,insecure,no_root_squash,refer=/export@192.168.124.66) > > After the client tries to access /export/ref, this is what I see when I > dump the export cache without my fix: > > [root@rawhide ~]# cat /proc/net/rpc/nfsd.export/content > #path domain(flags) > / *(ro,insecure,no_root_squash,sync,no_wdelay,no_subtree_check,v4root,fsid=0,sec=1) > /export *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) > /export/ref *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) > > Notice there's no refer= option. So when the client does a LOOKUP of > /export/ref, the server treats it as a normal directory... it doesn't > return NFS4ERR_MOVED and so the client doesn't know to query > fs_locations. > > Here is what the export cache looks like with my fix: > > [root@rawhide ~]# cat /proc/net/rpc/nfsd.export/content > #path domain(flags) > /export *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) > / *(ro,insecure,no_root_squash,sync,no_wdelay,no_subtree_check,v4root,fsid=0,sec=1) > /export/ref *(rw,insecure,no_root_squash,sync,wdelay,no_subtree_check,refer=/export@192.168.124.66,uuid=c4eeda84:ea1a4dcd:a043fdc1:372d7878,sec=1) > > Note the refer= option is present, and the referral works normally. > > A junction is basically a fancy directory that has the user/group/other > mode bits set to 0 and the sticky bit turned on. The original mode bits > are stored in the trusted.junction.mode extended attribute and the > referral information is stored in the trusted.junction.nfs extended > attribute. > > Continuing with my previous example, I have this in my /etc/exports > > [root@rawhide ~]# cat /etc/exports > /export *(rw,insecure,no_root_squash) > /export/ref *(rw,insecure,no_root_squash,refer=/export@192.168.124.66) > > Let's add a referral using a junction. > > [root@rawhide ~]# nfsref add /export/junc 192.168.124.66 /export > Created junction /export/junc > > In this case, /export/junc didn't previously exist, so the nfsref tool > created it. If /export/junc did already exist, then the original mode > would be stored in the trusted.junction.mode and the original contents > of the directory would be hidden from the client (as well as > non-privileged users on the server). > > You can look up the referral info using 'nfsref lookup': > > [root@rawhide ~]# nfsref lookup /export/junc > 192.168.124.66:/export > > NFS port: 2049 > Valid for: 0 > Currency: -1 > Flags: varsub(false) > GenFlags: writable(false), going(false), split(true) > TransFlags: rdma(true) > Class: simul(0), handle(0), fileid(0) > Class: writever(0), change(0), readdir(0) > Read: rank(0), order(0) > Write: rank(0), order(0) > > Or you can just use getfattr if you want to see the raw xml: > > [root@rawhide ~]# getfattr --only-values -d -m trusted.junction.nfs /export/junc > getfattr: Removing leading '/' from absolute path names > <?xml version="1.0" encoding="UTF-8"?> > <junction> > <savedmode bits="755"/> > <fileset> > <location> > <host name="192.168.124.66"/> > <path> > <component>export</component> > </path> > <currency>-1</currency> > <genflags writable="false" going="false" split="true"/> > <transflags rdma="true"/> > <class simul="0" handle="0" fileid="0" writever="0" change="0" readdir="0"/> > <read rank="0" order="0"/> > <write rank="0" order="0"/> > <flags varsub="false"/> > <validfor>0</validfor> > </location> > </fileset> > </junction> > > Note that since the /export/junc referral is stored in a junction, it > doesn't appear in the export info: > > [root@rawhide ~]# exportfs -v > /export <world>(sync,wdelay,hide,no_subtree_check,sec=sys,rw,insecure,no_root_squash,no_all_squash) > /export/ref <world>(sync,wdelay,hide,no_subtree_check,refer=/export@192.168.124.66,sec=sys,rw,insecure,no_root_squash,no_all_squash) > > From the client's standpoint, both style referrals work the same: > > root@aion:~# mount 192.168.124.26:/export /mnt/t > root@aion:~# ls /mnt/t > junc ref > root@aion:~# ls /mnt/t/ref > file > root@aion:~# cat /mnt/t/ref/file > I am on the referral server. > root@aion:~# grep nfs4 /proc/mounts > 192.168.124.26:/export /mnt/t nfs4 rw,relatime,vers=4.2,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.26 0 0 > 192.168.124.66:/export /mnt/t/ref nfs4 rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.66 0 0 > root@aion:~# ls /mnt/t/junc > file > root@aion:~# cat /mnt/t/junc/file > I am on the referral server. > root@aion:~# grep nfs4 /proc/mounts > 192.168.124.26:/export /mnt/t nfs4 rw,relatime,vers=4.2,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.26 0 0 > 192.168.124.66:/export /mnt/t/ref nfs4 rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.66 0 0 > 192.168.124.66:/export /mnt/t/junc nfs4 rw,relatime,vers=4.2,rsize=262144,wsize=262144,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=192.168.124.1,local_lock=none,addr=192.168.124.66 0 0 > > > So if you want to get rid of that last #ifdef HAVE_JUNCTION_SUPPORT > then you have 2 options: > > a) get rid of junctions entirely, leaving users with only the old > (relatively speaking) method for configuring referrals > b) force all packagers of nfs-utils to pull in the extra dependencies > needed to support junctions, which is the exact opposite of what the > Debian folks are requesting. > > Or you can take the patch and we can continue to have both style > referrals. Very good explanation... we should put this on linux-nfs.org on how to uses both junctions and referrals... I'll let the distros decide how they want to deal with this. thanks! steved. > > -Scott >> >> steved. >>> >>> Fixes: 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") >>> Signed-off-by: Scott Mayhew <smayhew@redhat.com> >>> --- >>> support/export/cache.c | 7 ------- >>> 1 file changed, 7 deletions(-) >>> >>> diff --git a/support/export/cache.c b/support/export/cache.c >>> index 6c0a44a3..3a8e57cf 100644 >>> --- a/support/export/cache.c >>> +++ b/support/export/cache.c >>> @@ -34,10 +34,7 @@ >>> #include "pseudoflavors.h" >>> #include "xcommon.h" >>> #include "reexport.h" >>> - >>> -#ifdef HAVE_JUNCTION_SUPPORT >>> #include "fsloc.h" >>> -#endif >>> #ifdef USE_BLKID >>> #include "blkid/blkid.h" >>> @@ -999,7 +996,6 @@ static void nfsd_retry_fh(struct delayed *d) >>> *dp = d; >>> } >>> -#ifdef HAVE_JUNCTION_SUPPORT >>> static void write_fsloc(char **bp, int *blen, struct exportent *ep) >>> { >>> struct servers *servers; >>> @@ -1022,7 +1018,6 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep) >>> qword_addint(bp, blen, servers->h_referral); >>> release_replicas(servers); >>> } >>> -#endif >>> static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask, int extra_flag) >>> { >>> @@ -1120,9 +1115,7 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain, >>> qword_addint(&bp, &blen, exp->e_anongid); >>> qword_addint(&bp, &blen, fsidnum); >>> -#ifdef HAVE_JUNCTION_SUPPORT >>> write_fsloc(&bp, &blen, exp); >>> -#endif >>> write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0); >>> if (exp->e_uuid == NULL || different_fs) { >>> char u[16]; >> >
> On Dec 2, 2024, at 10:19 PM, Steve Dickson <SteveD@redhat.com> wrote: > > Hey, > > On 12/2/24 3:30 PM, Scott Mayhew wrote: >> Commit 15dc0bea ("exportd: Moved cache upcalls routines into >> libexport.a") caused write_fsloc() to be elided when junction support is >> disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so >> that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should >> be around actual junction code). > Why not just take the enable_junction config variable > out of configure.ac as well? It's not generally good practice, but I will break up your sentence below to reply to each bit. There is something to unpack in each part. > If we want junctions/referrals (which are the same) > IMHO... Junctions and refer= are related, but they aren't the same. As Scott demonstrated, a junction is a file system object that stores NFSv4 referral information. The "refer=" export option stores that information in /etc/exports. The common part of these two mechanisms resides in NFSD, which turns that information into the response to a GETATTR(fs_locations). > on all the time... We want "refer=" on all the time, yes. Junction support has to be enabled manually. This is because it depends on libxml2, which not every distro wants to, or can, pull into its nfs-utils package. That is in fact exactly how Salvatore is using this option. The stable version of Debian's nfs-utils package does not want libxml2, so junction support is disabled there. But they /do/ want "refer=" support. > Lets not be able to turn them off at all? That would be nice, but it's not yet practical for every distro to enable it. I am told that Debian unstable's nfs-utils will enable junction support, and has added the libxml2 dependency successfully. We will get there eventually. > Point being... if we are going remove 3 of the 4 > HAVE_JUNCTION_SUPPORT ifdefs... let get ride of > all of them. -- Chuck Lever
Hey! On 12/3/24 9:28 AM, Chuck Lever III wrote: > > >> On Dec 2, 2024, at 10:19 PM, Steve Dickson <SteveD@redhat.com> wrote: >> >> Hey, >> >> On 12/2/24 3:30 PM, Scott Mayhew wrote: >>> Commit 15dc0bea ("exportd: Moved cache upcalls routines into >>> libexport.a") caused write_fsloc() to be elided when junction support is >>> disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so >>> that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should >>> be around actual junction code). >> Why not just take the enable_junction config variable >> out of configure.ac as well? > > It's not generally good practice, but I will break up your > sentence below to reply to each bit. There is something to > unpack in each part. I agree not being a good practice... But sometimes config switches out live their usefulness... Basically that's what I was thinking > > >> If we want junctions/referrals (which are the same) >> IMHO... > > Junctions and refer= are related, but they aren't > the same. As Scott demonstrated, a junction is a file > system object that stores NFSv4 referral information. > The "refer=" export option stores that information in > /etc/exports. Is there a point to have both ways? What is the advantage of one way over the other? > > The common part of these two mechanisms resides in > NFSD, which turns that information into the response > to a GETATTR(fs_locations). Right... both ways use the same protocol. > > >> on all the time... > > We want "refer=" on all the time, yes. Fine.. Scott's patch will be in the coming releases. > > Junction support has to be enabled manually. This is > because it depends on libxml2, which not every distro > wants to, or can, pull into its nfs-utils package. Yeah... libxml2 seems to be an Fedora only thing. > > That is in fact exactly how Salvatore is using this > option. The stable version of Debian's nfs-utils > package does not want libxml2, so junction support is > disabled there. But they /do/ want "refer=" support. Yeah... The bug has been around for a while (pre-pandemic ;-) ) so it appears somebody is starting to use referrals... > > >> Lets not be able to turn them off at all? > > That would be nice, but it's not yet practical for > every distro to enable it. Understood. > > I am told that Debian unstable's nfs-utils will > enable junction support, and has added the libxml2 > dependency successfully. > > We will get there eventually. Patches are welcome! :-) steved.
> On Dec 3, 2024, at 11:02 AM, Steve Dickson <steved@redhat.com> wrote: > > Hey! > > On 12/3/24 9:28 AM, Chuck Lever III wrote: >>> On Dec 2, 2024, at 10:19 PM, Steve Dickson <SteveD@redhat.com> wrote: >>> >>> Hey, >>> >>> On 12/2/24 3:30 PM, Scott Mayhew wrote: >>>> Commit 15dc0bea ("exportd: Moved cache upcalls routines into >>>> libexport.a") caused write_fsloc() to be elided when junction support is >>>> disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so >>>> that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should >>>> be around actual junction code). >>> Why not just take the enable_junction config variable >>> out of configure.ac as well? >> It's not generally good practice, but I will break up your >> sentence below to reply to each bit. There is something to >> unpack in each part. > I agree not being a good practice... But sometimes > config switches out live their usefulness... > Basically that's what I was thinking Sometimes. But --enable-junctions is not there yet. >>> If we want junctions/referrals (which are the same) >>> IMHO... >> Junctions and refer= are related, but they aren't >> the same. As Scott demonstrated, a junction is a file >> system object that stores NFSv4 referral information. >> The "refer=" export option stores that information in >> /etc/exports. > Is there a point to have both ways? "refer=" is the classic way of doing this, but was added only as an experiment, back in the day. I added junction support as part of the FedFS effort, a decade ago. At that time, we decided that junctions, via the nfsref command, would be the "one way". We have to wait for distros to pick up junction support before deprecating "refer=" because there are still active users of "refer=". Debian stable is one of the distros that is blocking the complete deprecation of "refer=". We also want a better mechanism for kernel/user space interaction to enable full support for IPv6 addresses and alternate ports in referrals. So, work in progress. We want to have "one way" but there are always some road blocks that make it slower than just toggling a switch. -- Chuck Lever
diff --git a/support/export/cache.c b/support/export/cache.c index 6c0a44a3..3a8e57cf 100644 --- a/support/export/cache.c +++ b/support/export/cache.c @@ -34,10 +34,7 @@ #include "pseudoflavors.h" #include "xcommon.h" #include "reexport.h" - -#ifdef HAVE_JUNCTION_SUPPORT #include "fsloc.h" -#endif #ifdef USE_BLKID #include "blkid/blkid.h" @@ -999,7 +996,6 @@ static void nfsd_retry_fh(struct delayed *d) *dp = d; } -#ifdef HAVE_JUNCTION_SUPPORT static void write_fsloc(char **bp, int *blen, struct exportent *ep) { struct servers *servers; @@ -1022,7 +1018,6 @@ static void write_fsloc(char **bp, int *blen, struct exportent *ep) qword_addint(bp, blen, servers->h_referral); release_replicas(servers); } -#endif static void write_secinfo(char **bp, int *blen, struct exportent *ep, int flag_mask, int extra_flag) { @@ -1120,9 +1115,7 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain, qword_addint(&bp, &blen, exp->e_anongid); qword_addint(&bp, &blen, fsidnum); -#ifdef HAVE_JUNCTION_SUPPORT write_fsloc(&bp, &blen, exp); -#endif write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0); if (exp->e_uuid == NULL || different_fs) { char u[16];
Commit 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") caused write_fsloc() to be elided when junction support is disabled. Get rid of the bogus #ifdef HAVE_JUNCTION_SUPPORT blocks so that referrals work again (the only #ifdef HAVE_JUNCTION_SUPPORT should be around actual junction code). Fixes: 15dc0bea ("exportd: Moved cache upcalls routines into libexport.a") Signed-off-by: Scott Mayhew <smayhew@redhat.com> --- support/export/cache.c | 7 ------- 1 file changed, 7 deletions(-)