* [PATCH] nfs-server-generator: handle 'noauto' mounts correctly. @ 2017-02-08 2:03 NeilBrown 2017-02-16 10:23 ` Steve Dickson 2017-04-04 10:26 ` ChunYu Wang 0 siblings, 2 replies; 4+ messages in thread From: NeilBrown @ 2017-02-08 2:03 UTC (permalink / raw) To: steved; +Cc: linux-nfs [-- Attachment #1: Type: text/plain, Size: 2033 bytes --] When this code was written, the systemd documentation stated that "RequiresMountsFor" ignored mountpoints marked as "noauto". Unfortunately this is incorrect. Consquently a filesystem marked as noauto that is also NFS exported will currently be mounted when the NFS server is started. This is not what people expect. So add a check for the noauto flag. If any ancestor of a given export point has the noauto flag, no RequiresMountsFor will be generated for that point. Also skip RequiresMountsFor for exports marked 'mountpoint', as their absence is, theoretically, already handled by mountd. URL: https://github.com/systemd/systemd/issues/5249 Signed-off-by: NeilBrown <neilb@suse.com> --- systemd/nfs-server-generator.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/systemd/nfs-server-generator.c b/systemd/nfs-server-generator.c index cc99969e9922..4aa65094ca07 100644 --- a/systemd/nfs-server-generator.c +++ b/systemd/nfs-server-generator.c @@ -84,6 +84,28 @@ static void systemd_escape(FILE *f, char *path) } } +static int has_noauto_flag(char *path) +{ + FILE *fstab; + struct mntent *mnt; + + fstab = setmntent("/etc/fstab", "r"); + if (!fstab) + return 0; + + while ((mnt = getmntent(fstab)) != NULL) { + int l = strlen(mnt->mnt_dir); + if (strncmp(mnt->mnt_dir, path, l) != 0) + continue; + if (path[l] && path[l] != '/') + continue; + if (hasmntopt(mnt, "noauto")) + break; + } + fclose(fstab); + return mnt != NULL; +} + int main(int argc, char *argv[]) { char *path; @@ -124,6 +146,10 @@ int main(int argc, char *argv[]) for (exp = exportlist[i].p_head; exp; exp = exp->m_next) { if (!is_unique(&list, exp->m_export.e_path)) continue; + if (exp->m_export.e_mountpoint) + continue; + if (has_noauto_flag(exp->m_export.e_path)) + continue; if (strchr(exp->m_export.e_path, ' ')) fprintf(f, "RequiresMountsFor=\"%s\"\n", exp->m_export.e_path); -- 2.11.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nfs-server-generator: handle 'noauto' mounts correctly. 2017-02-08 2:03 [PATCH] nfs-server-generator: handle 'noauto' mounts correctly NeilBrown @ 2017-02-16 10:23 ` Steve Dickson 2017-04-04 10:26 ` ChunYu Wang 1 sibling, 0 replies; 4+ messages in thread From: Steve Dickson @ 2017-02-16 10:23 UTC (permalink / raw) To: NeilBrown; +Cc: linux-nfs On 02/07/2017 09:03 PM, NeilBrown wrote: > When this code was written, the systemd documentation stated > that "RequiresMountsFor" ignored mountpoints marked as "noauto". > Unfortunately this is incorrect. Consquently a filesystem marked > as noauto that is also NFS exported will currently be mounted when > the NFS server is started. This is not what people expect. > > So add a check for the noauto flag. If any ancestor of a given > export point has the noauto flag, no RequiresMountsFor will be > generated for that point. > > Also skip RequiresMountsFor for exports marked 'mountpoint', as their > absence is, theoretically, already handled by mountd. > > URL: https://github.com/systemd/systemd/issues/5249 > Signed-off-by: NeilBrown <neilb@suse.com> Committed... steved. > --- > systemd/nfs-server-generator.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/systemd/nfs-server-generator.c b/systemd/nfs-server-generator.c > index cc99969e9922..4aa65094ca07 100644 > --- a/systemd/nfs-server-generator.c > +++ b/systemd/nfs-server-generator.c > @@ -84,6 +84,28 @@ static void systemd_escape(FILE *f, char *path) > } > } > > +static int has_noauto_flag(char *path) > +{ > + FILE *fstab; > + struct mntent *mnt; > + > + fstab = setmntent("/etc/fstab", "r"); > + if (!fstab) > + return 0; > + > + while ((mnt = getmntent(fstab)) != NULL) { > + int l = strlen(mnt->mnt_dir); > + if (strncmp(mnt->mnt_dir, path, l) != 0) > + continue; > + if (path[l] && path[l] != '/') > + continue; > + if (hasmntopt(mnt, "noauto")) > + break; > + } > + fclose(fstab); > + return mnt != NULL; > +} > + > int main(int argc, char *argv[]) > { > char *path; > @@ -124,6 +146,10 @@ int main(int argc, char *argv[]) > for (exp = exportlist[i].p_head; exp; exp = exp->m_next) { > if (!is_unique(&list, exp->m_export.e_path)) > continue; > + if (exp->m_export.e_mountpoint) > + continue; > + if (has_noauto_flag(exp->m_export.e_path)) > + continue; > if (strchr(exp->m_export.e_path, ' ')) > fprintf(f, "RequiresMountsFor=\"%s\"\n", > exp->m_export.e_path); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfs-server-generator: handle 'noauto' mounts correctly. 2017-02-08 2:03 [PATCH] nfs-server-generator: handle 'noauto' mounts correctly NeilBrown 2017-02-16 10:23 ` Steve Dickson @ 2017-04-04 10:26 ` ChunYu Wang 2017-04-04 21:57 ` NeilBrown 1 sibling, 1 reply; 4+ messages in thread From: ChunYu Wang @ 2017-04-04 10:26 UTC (permalink / raw) To: NeilBrown; +Cc: linux-nfs, Steve Dickson, Jianhong Yin, Yongcheng Yang Hi, Neil, Thanks for your patch! But seems there still exists same problem on my fedora 25 with a patched nfs-utils version, am I made something wrong or we just need more patches for fedora family..? -- I just try it with most simple steps: [root@bootp-73-5-219 ~]# cat /etc/fstab |grep boot UUID=932047db-24e7-4043-8373-6e87333b9ce1 /boot ext4 noauto 1 2 ^^ Set flag to 'noauto' [root@bootp-73-5-219 ~]# umount /boot; mount -a; mountpoint /boot /boot is not a mountpoint ^^ umount ready [root@bootp-73-5-219 ~]# cat /lib/systemd/system/nfs-server.service |grep boot RequiresMountsFor= /boot [root@bootp-73-5-219 ~]# systemctl restart nfs-server [root@bootp-73-5-219 ~]# mountpoint /boot/ /boot/ is a mountpoint [root@bootp-73-5-219 ~]# mount |grep boot /dev/sda1 on /boot type ext4 (rw,relatime,seclabel,data=ordered) ^^ mount again automatically -- -- Already patched: [root@bootp-73-5-219 ~]# rpm -q nfs-utils nfs-utils-2.1.1-2.rc1.fc25.x86_64 [root@bootp-73-5-219 nfs-utils]# grep nfs-server-generator -r ./*.patch ./nfs-utils-2.1.2-rc1.patch:diff --git a/systemd/nfs-server-generator.c b/systemd/nfs-server-generator.c ... [root@bootp-73-5-219 nfs-utils]# cat ./nfs-utils.spec |grep -i patch001 Patch001: nfs-utils-2.1.2-rc1.patch %patch001 -p1 -- Thanks, ChunYu Wang On Wed, Feb 8, 2017 at 10:03 AM, NeilBrown <neilb@suse.com> wrote: > > When this code was written, the systemd documentation stated > that "RequiresMountsFor" ignored mountpoints marked as "noauto". > Unfortunately this is incorrect. Consquently a filesystem marked > as noauto that is also NFS exported will currently be mounted when > the NFS server is started. This is not what people expect. > > So add a check for the noauto flag. If any ancestor of a given > export point has the noauto flag, no RequiresMountsFor will be > generated for that point. > > Also skip RequiresMountsFor for exports marked 'mountpoint', as their > absence is, theoretically, already handled by mountd. > > URL: https://github.com/systemd/systemd/issues/5249 > Signed-off-by: NeilBrown <neilb@suse.com> > --- > systemd/nfs-server-generator.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/systemd/nfs-server-generator.c b/systemd/nfs-server-generator.c > index cc99969e9922..4aa65094ca07 100644 > --- a/systemd/nfs-server-generator.c > +++ b/systemd/nfs-server-generator.c > @@ -84,6 +84,28 @@ static void systemd_escape(FILE *f, char *path) > } > } > > +static int has_noauto_flag(char *path) > +{ > + FILE *fstab; > + struct mntent *mnt; > + > + fstab = setmntent("/etc/fstab", "r"); > + if (!fstab) > + return 0; > + > + while ((mnt = getmntent(fstab)) != NULL) { > + int l = strlen(mnt->mnt_dir); > + if (strncmp(mnt->mnt_dir, path, l) != 0) > + continue; > + if (path[l] && path[l] != '/') > + continue; > + if (hasmntopt(mnt, "noauto")) > + break; > + } > + fclose(fstab); > + return mnt != NULL; > +} > + > int main(int argc, char *argv[]) > { > char *path; > @@ -124,6 +146,10 @@ int main(int argc, char *argv[]) > for (exp = exportlist[i].p_head; exp; exp = exp->m_next) { > if (!is_unique(&list, exp->m_export.e_path)) > continue; > + if (exp->m_export.e_mountpoint) > + continue; > + if (has_noauto_flag(exp->m_export.e_path)) > + continue; > if (strchr(exp->m_export.e_path, ' ')) > fprintf(f, "RequiresMountsFor=\"%s\"\n", > exp->m_export.e_path); > -- > 2.11.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfs-server-generator: handle 'noauto' mounts correctly. 2017-04-04 10:26 ` ChunYu Wang @ 2017-04-04 21:57 ` NeilBrown 0 siblings, 0 replies; 4+ messages in thread From: NeilBrown @ 2017-04-04 21:57 UTC (permalink / raw) To: ChunYu Wang; +Cc: linux-nfs, Steve Dickson, Jianhong Yin, Yongcheng Yang [-- Attachment #1: Type: text/plain, Size: 4401 bytes --] On Tue, Apr 04 2017, ChunYu Wang wrote: > Hi, Neil, > > Thanks for your patch! But seems there still exists same problem on my > fedora 25 with a patched nfs-utils version, am I made something wrong > or we just need more patches for fedora family..? > > -- I just try it with most simple steps: > [root@bootp-73-5-219 ~]# cat /etc/fstab |grep boot > UUID=932047db-24e7-4043-8373-6e87333b9ce1 /boot ext4 > noauto 1 2 > ^^ Set flag to 'noauto' > > [root@bootp-73-5-219 ~]# umount /boot; mount -a; mountpoint /boot > /boot is not a mountpoint > ^^ umount ready > > [root@bootp-73-5-219 ~]# cat /lib/systemd/system/nfs-server.service |grep boot > RequiresMountsFor= /boot Why do you have this line in this file? This is what is causing /boot to be mounted. It seems that you don't understand the point of the patch. The patch modifies the nfs-server systemd generator to *not* including this line in the generated file if the noauto option is present. NeilBrown > > [root@bootp-73-5-219 ~]# systemctl restart nfs-server > > [root@bootp-73-5-219 ~]# mountpoint /boot/ > /boot/ is a mountpoint > [root@bootp-73-5-219 ~]# mount |grep boot > /dev/sda1 on /boot type ext4 (rw,relatime,seclabel,data=ordered) > ^^ mount again automatically > -- > > -- Already patched: > [root@bootp-73-5-219 ~]# rpm -q nfs-utils > nfs-utils-2.1.1-2.rc1.fc25.x86_64 > [root@bootp-73-5-219 nfs-utils]# grep nfs-server-generator -r ./*.patch > ./nfs-utils-2.1.2-rc1.patch:diff --git > a/systemd/nfs-server-generator.c b/systemd/nfs-server-generator.c > ... > [root@bootp-73-5-219 nfs-utils]# cat ./nfs-utils.spec |grep -i patch001 > Patch001: nfs-utils-2.1.2-rc1.patch > %patch001 -p1 > -- > > Thanks, > ChunYu Wang > > > > > > On Wed, Feb 8, 2017 at 10:03 AM, NeilBrown <neilb@suse.com> wrote: >> >> When this code was written, the systemd documentation stated >> that "RequiresMountsFor" ignored mountpoints marked as "noauto". >> Unfortunately this is incorrect. Consquently a filesystem marked >> as noauto that is also NFS exported will currently be mounted when >> the NFS server is started. This is not what people expect. >> >> So add a check for the noauto flag. If any ancestor of a given >> export point has the noauto flag, no RequiresMountsFor will be >> generated for that point. >> >> Also skip RequiresMountsFor for exports marked 'mountpoint', as their >> absence is, theoretically, already handled by mountd. >> >> URL: https://github.com/systemd/systemd/issues/5249 >> Signed-off-by: NeilBrown <neilb@suse.com> >> --- >> systemd/nfs-server-generator.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/systemd/nfs-server-generator.c b/systemd/nfs-server-generator.c >> index cc99969e9922..4aa65094ca07 100644 >> --- a/systemd/nfs-server-generator.c >> +++ b/systemd/nfs-server-generator.c >> @@ -84,6 +84,28 @@ static void systemd_escape(FILE *f, char *path) >> } >> } >> >> +static int has_noauto_flag(char *path) >> +{ >> + FILE *fstab; >> + struct mntent *mnt; >> + >> + fstab = setmntent("/etc/fstab", "r"); >> + if (!fstab) >> + return 0; >> + >> + while ((mnt = getmntent(fstab)) != NULL) { >> + int l = strlen(mnt->mnt_dir); >> + if (strncmp(mnt->mnt_dir, path, l) != 0) >> + continue; >> + if (path[l] && path[l] != '/') >> + continue; >> + if (hasmntopt(mnt, "noauto")) >> + break; >> + } >> + fclose(fstab); >> + return mnt != NULL; >> +} >> + >> int main(int argc, char *argv[]) >> { >> char *path; >> @@ -124,6 +146,10 @@ int main(int argc, char *argv[]) >> for (exp = exportlist[i].p_head; exp; exp = exp->m_next) { >> if (!is_unique(&list, exp->m_export.e_path)) >> continue; >> + if (exp->m_export.e_mountpoint) >> + continue; >> + if (has_noauto_flag(exp->m_export.e_path)) >> + continue; >> if (strchr(exp->m_export.e_path, ' ')) >> fprintf(f, "RequiresMountsFor=\"%s\"\n", >> exp->m_export.e_path); >> -- >> 2.11.0 >> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-04 21:57 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-08 2:03 [PATCH] nfs-server-generator: handle 'noauto' mounts correctly NeilBrown 2017-02-16 10:23 ` Steve Dickson 2017-04-04 10:26 ` ChunYu Wang 2017-04-04 21:57 ` NeilBrown
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.