| Message ID | 87poitv3p4.fsf@notabene.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); -- 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
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 > -- 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
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 >>
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);
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(+)