2018-10-19 16:08:59

by Jianhong Yin

[permalink] [raw]
Subject: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'

From: "Jianhong.Yin" <[email protected]>

see: https://bugzilla.redhat.com/show_bug.cgi?id=1629705
mount.nfs4 -o context=system_u:object_r:user_home_dir_t:s0,sharecache $serv:$expdir $nfsmp
mount.nfs4 -o context=system_u:object_r:xferlog_t:s0,sharecache $serv:$expdir $nfsmp2
^^^ here mount fail, but return true. it confuse user!

according: https://patchwork.kernel.org/patch/10602607/#22234047
add function is_mounted_already()
- if (errno == EBUSY)
+ if (errno == EBUSY && is_mounted_already(mi->spec, mi->node))
return EX_SUCCESS;

Signed-off-by: Jianhong Yin <[email protected]>
---
utils/mount/stropts.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 4d2e37e..4be7e61 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -48,6 +48,7 @@
#include "version.h"
#include "parse_dev.h"
#include "conffile.h"
+#include <mntent.h>

#ifndef NFS_PROGRAM
#define NFS_PROGRAM (100003)
@@ -1056,6 +1057,27 @@ static int nfs_is_permanent_error(int error)
}
}

+static int is_mounted_already(const char *fsname, const char *dir)
+{
+ struct mntent *ent;
+ FILE *fp;
+ int ret = 0;
+
+ fp = setmntent("/proc/mounts", "r");
+ if (fp == NULL) {
+ perror("[unlikely] setmntent(3) fail");
+ exit(1);
+ }
+ while (NULL != (ent = getmntent(fp))) {
+ if (!strcmp(ent->mnt_fsname, fsname) && !strcmp(ent->mnt_dir, dir)) {
+ ret = 1;
+ break;
+ }
+ }
+ endmntent(fp);
+ return ret;
+}
+
/*
* Handle "foreground" NFS mounts.
*
@@ -1078,13 +1100,8 @@ static int nfsmount_fg(struct nfsmount_info *mi)
if (nfs_try_mount(mi))
return EX_SUCCESS;

- if (errno == EBUSY)
- /* The only cause of EBUSY is if exactly the desired
- * filesystem is already mounted. That can arguably
- * be seen as success. "mount -a" tries to optimise
- * out this case but sometimes fails. Help it out
- * by pretending everything is rosy
- */
+ /* if EBUSY is caused by re-mount, ignore the error */
+ if (errno == EBUSY && is_mounted_already(mi->spec, mi->node))
return EX_SUCCESS;

if (nfs_is_permanent_error(errno))
--
2.17.1


2018-10-20 01:44:06

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'



On 10/19/18 4:03 AM, [email protected] wrote:
> From: "Jianhong.Yin" <[email protected]>
>
> see: https://bugzilla.redhat.com/show_bug.cgi?id=1629705
> mount.nfs4 -o context=system_u:object_r:user_home_dir_t:s0,sharecache $serv:$expdir $nfsmp
> mount.nfs4 -o context=system_u:object_r:xferlog_t:s0,sharecache $serv:$expdir $nfsmp2
> ^^^ here mount fail, but return true. it confuse user!
Why should it fail? Two different mounts are being used and using -o sharecache
which is the default the way...

steved.
>
> according: https://patchwork.kernel.org/patch/10602607/#22234047
> add function is_mounted_already()
> - if (errno == EBUSY)
> + if (errno == EBUSY && is_mounted_already(mi->spec, mi->node))
> return EX_SUCCESS;
>
> Signed-off-by: Jianhong Yin <[email protected]>
> ---
> utils/mount/stropts.c | 31 ++++++++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index 4d2e37e..4be7e61 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -48,6 +48,7 @@
> #include "version.h"
> #include "parse_dev.h"
> #include "conffile.h"
> +#include <mntent.h>
>
> #ifndef NFS_PROGRAM
> #define NFS_PROGRAM (100003)
> @@ -1056,6 +1057,27 @@ static int nfs_is_permanent_error(int error)
> }
> }
>
> +static int is_mounted_already(const char *fsname, const char *dir)
> +{
> + struct mntent *ent;
> + FILE *fp;
> + int ret = 0;
> +
> + fp = setmntent("/proc/mounts", "r");
> + if (fp == NULL) {
> + perror("[unlikely] setmntent(3) fail");
> + exit(1);
> + }
> + while (NULL != (ent = getmntent(fp))) {
> + if (!strcmp(ent->mnt_fsname, fsname) && !strcmp(ent->mnt_dir, dir)) {
> + ret = 1;
> + break;
> + }
> + }
> + endmntent(fp);
> + return ret;
> +}
> +
> /*
> * Handle "foreground" NFS mounts.
> *
> @@ -1078,13 +1100,8 @@ static int nfsmount_fg(struct nfsmount_info *mi)
> if (nfs_try_mount(mi))
> return EX_SUCCESS;
>
> - if (errno == EBUSY)
> - /* The only cause of EBUSY is if exactly the desired
> - * filesystem is already mounted. That can arguably
> - * be seen as success. "mount -a" tries to optimise
> - * out this case but sometimes fails. Help it out
> - * by pretending everything is rosy
> - */
> + /* if EBUSY is caused by re-mount, ignore the error */
> + if (errno == EBUSY && is_mounted_already(mi->spec, mi->node))
> return EX_SUCCESS;
>
> if (nfs_is_permanent_error(errno))
>

2018-10-20 22:46:30

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'

Hello,

On 10/19/18 11:31 PM, 尹剑虹 wrote:
>
> Hi Steved
>
> The scenario is: these two mountings use different "context=" and sharecache option, in this case mount(2) return fail with EBUSY.
Fair enough.... I did miss the different contexts...

But we already have a routine that check mount points
is_mountpoint() used my mountd. I would rather use that.

I'll resend the patch...

Thanks!!

steved.

>
> Jianhong
>
>
>
> 尹剑虹
> 邮箱:[email protected]
>
> <https://maas.mail.163.com/dashi-web-extend/html/proSignature.html?iconUrl=https%3A%2F%2Fmail-online.nosdn.127.net%2Fqiyelogo%2FdefaultAvatar.png&name=%E5%B0%B9%E5%89%91%E8%99%B9&uid=yin-jianhong%40163.com&ftlId=1&items=%5B%22%E9%82%AE%E7%AE%B1%EF%BC%9Ayin-jianhong%40163.com%22%5D>
>
> 签名由 网易邮箱大师 <https://mail.163.com/dashi/dlpro.html?from=mail88> 定制
>
> On 10/20/2018 01:36, Steve Dickson <mailto:[email protected]> wrote:
>
>
> On 10/19/18 4:03 AM, [email protected] wrote:
> > From: "Jianhong.Yin" <[email protected]>
> >
> > see: https://bugzilla.redhat.com/show_bug.cgi?id=1629705
> > mount.nfs4 -o context=system_u:object_r:user_home_dir_t:s0,sharecache $serv:$expdir $nfsmp
> > mount.nfs4 -o context=system_u:object_r:xferlog_t:s0,sharecache $serv:$expdir $nfsmp2
> > ^^^ here mount fail, but return true. it confuse user!
> Why should it fail? Two different mounts are being used and using -o sharecache
> which is the default the way...
>
> steved.
> >
> > according: https://patchwork.kernel.org/patch/10602607/#22234047
> > add function is_mounted_already()
> > -          if (errno == EBUSY)
> > +          if (errno == EBUSY && is_mounted_already(mi->spec, mi->node))
> >                return EX_SUCCESS;
> >
> > Signed-off-by: Jianhong Yin <[email protected]>
> > ---
> >  utils/mount/stropts.c | 31 ++++++++++++++++++++++++-------
> >  1 file changed, 24 insertions(+), 7 deletions(-)
> >
> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > index 4d2e37e..4be7e61 100644
> > --- a/utils/mount/stropts.c
> > +++ b/utils/mount/stropts.c
> > @@ -48,6 +48,7 @@
> >  #include "version.h"
> >  #include "parse_dev.h"
> >  #include "conffile.h"
> > +#include <mntent.h>
> >  
> >  #ifndef NFS_PROGRAM
> >  #define NFS_PROGRAM     (100003)
> > @@ -1056,6 +1057,27 @@ static int nfs_is_permanent_error(int error)
> >       }
> >  }
> >  
> > +static int is_mounted_already(const char *fsname, const char *dir)
> > +{
> > +     struct mntent *ent;
> > +     FILE *fp;
> > +     int ret = 0;
> > +
> > +     fp = setmntent("/proc/mounts", "r");
> > +     if (fp == NULL) {
> > +          perror("[unlikely] setmntent(3) fail");
> > +          exit(1);
> > +     }
> > +     while (NULL != (ent = getmntent(fp))) {
> > +          if (!strcmp(ent->mnt_fsname, fsname) && !strcmp(ent->mnt_dir, dir)) {
> > +               ret = 1;
> > +               break;
> > +          }
> > +     }
> > +     endmntent(fp);
> > +     return ret;
> > +}
> > +
> >  /*
> >   * Handle "foreground" NFS mounts.
> >   *
> > @@ -1078,13 +1100,8 @@ static int nfsmount_fg(struct nfsmount_info *mi)
> >            if (nfs_try_mount(mi))
> >                 return EX_SUCCESS;
> >  
> > -          if (errno == EBUSY)
> > -               /* The only cause of EBUSY is if exactly the desired
> > -                * filesystem is already mounted.  That can arguably
> > -                * be seen as success.  "mount -a" tries to optimise
> > -                * out this case but sometimes fails.  Help it out
> > -                * by pretending everything is rosy
> > -                */
> > +          /* if EBUSY is caused by re-mount, ignore the error */
> > +          if (errno == EBUSY && is_mounted_already(mi->spec, mi->node))
> >                 return EX_SUCCESS;
> >  
> >            if (nfs_is_permanent_error(errno))
> >
>

2018-10-22 02:56:34

by Jianhong Yin

[permalink] [raw]
Subject: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'


----- 原始邮件 -----
> 发件人: "Steve Dickson" <[email protected]>
> 收件人: "尹剑虹" <[email protected]>
> 抄送: [email protected], [email protected]
> 发送时间: 星期六, 2018年 10 月 20日 下午 10:35:48
> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
>
> Hello,
>
> On 10/19/18 11:31 PM, 尹剑虹 wrote:
> >
> > Hi Steved
> >
> > The scenario is: these two mountings use different "context=" and
> > sharecache option, in this case mount(2) return fail with EBUSY.
> Fair enough.... I did miss the different contexts...
>
> But we already have a routine that check mount points
> is_mountpoint() used my mountd. I would rather use that.
Got it, good to know.

>
> I'll resend the patch...
>
> Thanks!!
>
> steved.
>
> >
> > Jianhong
> >
> >
> >
> > 尹剑虹
> > 邮箱:[email protected]
> >
> > <https://maas.mail.163.com/dashi-web-extend/html/proSignature.html?iconUrl=https%3A%2F%2Fmail-online.nosdn.127.net%2Fqiyelogo%2FdefaultAvatar.png&name=%E5%B0%B9%E5%89%91%E8%99%B9&uid=yin-jianhong%40163.com&ftlId=1&items=%5B%22%E9%82%AE%E7%AE%B1%EF%BC%9Ayin-jianhong%40163.com%22%5D>
> >
> > 签名由 网易邮箱大师 <https://mail.163.com/dashi/dlpro.html?from=mail88> 定制
> >
> > On 10/20/2018 01:36, Steve Dickson <mailto:[email protected]>
> > wrote:
> >
> >
> > On 10/19/18 4:03 AM, [email protected] wrote:
> > > From: "Jianhong.Yin" <[email protected]>
> > >
> > > see: https://bugzilla.redhat.com/show_bug.cgi?id=1629705
> > > mount.nfs4 -o
> > > context=system_u:object_r:user_home_dir_t:s0,sharecache
> > > $serv:$expdir $nfsmp
> > > mount.nfs4 -o context=system_u:object_r:xferlog_t:s0,sharecache
> > > $serv:$expdir $nfsmp2
> > > ^^^ here mount fail, but return true. it confuse user!
> > Why should it fail? Two different mounts are being used and using
> > -o sharecache
> > which is the default the way...
> >
> > steved.
> > >
> > > according: https://patchwork.kernel.org/patch/10602607/#22234047
> > > add function is_mounted_already()
> > > -          if (errno == EBUSY)
> > > +          if (errno == EBUSY && is_mounted_already(mi->spec,
> > > mi->node))
> > >                return EX_SUCCESS;
> > >
> > > Signed-off-by: Jianhong Yin <[email protected]>
> > > ---
> > >  utils/mount/stropts.c | 31 ++++++++++++++++++++++++-------
> > >  1 file changed, 24 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > > index 4d2e37e..4be7e61 100644
> > > --- a/utils/mount/stropts.c
> > > +++ b/utils/mount/stropts.c
> > > @@ -48,6 +48,7 @@
> > >  #include "version.h"
> > >  #include "parse_dev.h"
> > >  #include "conffile.h"
> > > +#include <mntent.h>
> > >  
> > >  #ifndef NFS_PROGRAM
> > >  #define NFS_PROGRAM     (100003)
> > > @@ -1056,6 +1057,27 @@ static int nfs_is_permanent_error(int
> > > error)
> > >       }
> > >  }
> > >  
> > > +static int is_mounted_already(const char *fsname, const char
> > > *dir)
> > > +{
> > > +     struct mntent *ent;
> > > +     FILE *fp;
> > > +     int ret = 0;
> > > +
> > > +     fp = setmntent("/proc/mounts", "r");
> > > +     if (fp == NULL) {
> > > +          perror("[unlikely] setmntent(3) fail");
> > > +          exit(1);
> > > +     }
> > > +     while (NULL != (ent = getmntent(fp))) {
> > > +          if (!strcmp(ent->mnt_fsname, fsname) &&
> > > !strcmp(ent->mnt_dir, dir)) {
> > > +               ret = 1;
> > > +               break;
> > > +          }
> > > +     }
> > > +     endmntent(fp);
> > > +     return ret;
> > > +}
> > > +
> > >  /*
> > >   * Handle "foreground" NFS mounts.
> > >   *
> > > @@ -1078,13 +1100,8 @@ static int nfsmount_fg(struct
> > > nfsmount_info *mi)
> > >            if (nfs_try_mount(mi))
> > >                 return EX_SUCCESS;
> > >  
> > > -          if (errno == EBUSY)
> > > -               /* The only cause of EBUSY is if exactly the
> > > desired
> > > -                * filesystem is already mounted.  That can
> > > arguably
> > > -                * be seen as success.  "mount -a" tries to
> > > optimise
> > > -                * out this case but sometimes fails.  Help it
> > > out
> > > -                * by pretending everything is rosy
> > > -                */
> > > +          /* if EBUSY is caused by re-mount, ignore the error */
> > > +          if (errno == EBUSY && is_mounted_already(mi->spec,
> > > mi->node))
> > >                 return EX_SUCCESS;
> > >  
> > >            if (nfs_is_permanent_error(errno))
> > >
> >
>

2018-10-22 03:14:19

by Jianhong Yin

[permalink] [raw]
Subject: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'


----- 原始邮件 -----
> 发件人: "Jianhong Yin" <[email protected]>
> 收件人: "Steve Dickson" <[email protected]>
> 抄送: "尹剑虹" <[email protected]>, [email protected]
> 发送时间: 星期一, 2018年 10 月 22日 上午 10:56:31
> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
>
>
> ----- 原始邮件 -----
> > 发件人: "Steve Dickson" <[email protected]>
> > 收件人: "尹剑虹" <[email protected]>
> > 抄送: [email protected], [email protected]
> > 发送时间: 星期六, 2018年 10 月 20日 下午 10:35:48
> > 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but
> > return 'true'
> >
> > Hello,
> >
> > On 10/19/18 11:31 PM, 尹剑虹 wrote:
> > >
> > > Hi Steved
> > >
> > > The scenario is: these two mountings use different "context=" and
> > > sharecache option, in this case mount(2) return fail with EBUSY.
> > Fair enough.... I did miss the different contexts...
> >
> > But we already have a routine that check mount points
> > is_mountpoint() used my mountd. I would rather use that.

Hi Sir

I reviewed function is_mountpoint(), seems it just check if the mountpoint is a root-fs,
our goal is check if special dev has been mounted on specail path.

WDYT?

> Got it, good to know.
>
> >
> > I'll resend the patch...
> >
> > Thanks!!
> >
> > steved.
> >
> > >
> > > Jianhong
> > >
> > >
> > >
> > > 尹剑虹
> > > 邮箱:[email protected]
> > >
> > > <https://maas.mail.163.com/dashi-web-extend/html/proSignature.html?iconUrl=https%3A%2F%2Fmail-online.nosdn.127.net%2Fqiyelogo%2FdefaultAvatar.png&name=%E5%B0%B9%E5%89%91%E8%99%B9&uid=yin-jianhong%40163.com&ftlId=1&items=%5B%22%E9%82%AE%E7%AE%B1%EF%BC%9Ayin-jianhong%40163.com%22%5D>
> > >
> > > 签名由 网易邮箱大师 <https://mail.163.com/dashi/dlpro.html?from=mail88> 定制
> > >
> > > On 10/20/2018 01:36, Steve Dickson <mailto:[email protected]>
> > > wrote:
> > >
> > >
> > > On 10/19/18 4:03 AM, [email protected] wrote:
> > > > From: "Jianhong.Yin" <[email protected]>
> > > >
> > > > see: https://bugzilla.redhat.com/show_bug.cgi?id=1629705
> > > > mount.nfs4 -o
> > > > context=system_u:object_r:user_home_dir_t:s0,sharecache
> > > > $serv:$expdir $nfsmp
> > > > mount.nfs4 -o context=system_u:object_r:xferlog_t:s0,sharecache
> > > > $serv:$expdir $nfsmp2
> > > > ^^^ here mount fail, but return true. it confuse user!
> > > Why should it fail? Two different mounts are being used and using
> > > -o sharecache
> > > which is the default the way...
> > >
> > > steved.
> > > >
> > > > according:
> > > > https://patchwork.kernel.org/patch/10602607/#22234047
> > > > add function is_mounted_already()
> > > > -          if (errno == EBUSY)
> > > > +          if (errno == EBUSY && is_mounted_already(mi->spec,
> > > > mi->node))
> > > >                return EX_SUCCESS;
> > > >
> > > > Signed-off-by: Jianhong Yin <[email protected]>
> > > > ---
> > > >  utils/mount/stropts.c | 31 ++++++++++++++++++++++++-------
> > > >  1 file changed, 24 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> > > > index 4d2e37e..4be7e61 100644
> > > > --- a/utils/mount/stropts.c
> > > > +++ b/utils/mount/stropts.c
> > > > @@ -48,6 +48,7 @@
> > > >  #include "version.h"
> > > >  #include "parse_dev.h"
> > > >  #include "conffile.h"
> > > > +#include <mntent.h>
> > > >  
> > > >  #ifndef NFS_PROGRAM
> > > >  #define NFS_PROGRAM     (100003)
> > > > @@ -1056,6 +1057,27 @@ static int nfs_is_permanent_error(int
> > > > error)
> > > >       }
> > > >  }
> > > >  
> > > > +static int is_mounted_already(const char *fsname, const char
> > > > *dir)
> > > > +{
> > > > +     struct mntent *ent;
> > > > +     FILE *fp;
> > > > +     int ret = 0;
> > > > +
> > > > +     fp = setmntent("/proc/mounts", "r");
> > > > +     if (fp == NULL) {
> > > > +          perror("[unlikely] setmntent(3) fail");
> > > > +          exit(1);
> > > > +     }
> > > > +     while (NULL != (ent = getmntent(fp))) {
> > > > +          if (!strcmp(ent->mnt_fsname, fsname) &&
> > > > !strcmp(ent->mnt_dir, dir)) {
> > > > +               ret = 1;
> > > > +               break;
> > > > +          }
> > > > +     }
> > > > +     endmntent(fp);
> > > > +     return ret;
> > > > +}
> > > > +
> > > >  /*
> > > >   * Handle "foreground" NFS mounts.
> > > >   *
> > > > @@ -1078,13 +1100,8 @@ static int nfsmount_fg(struct
> > > > nfsmount_info *mi)
> > > >            if (nfs_try_mount(mi))
> > > >                 return EX_SUCCESS;
> > > >  
> > > > -          if (errno == EBUSY)
> > > > -               /* The only cause of EBUSY is if exactly the
> > > > desired
> > > > -                * filesystem is already mounted.  That can
> > > > arguably
> > > > -                * be seen as success.  "mount -a" tries to
> > > > optimise
> > > > -                * out this case but sometimes fails.  Help it
> > > > out
> > > > -                * by pretending everything is rosy
> > > > -                */
> > > > +          /* if EBUSY is caused by re-mount, ignore the error
> > > > */
> > > > +          if (errno == EBUSY && is_mounted_already(mi->spec,
> > > > mi->node))
> > > >                 return EX_SUCCESS;
> > > >  
> > > >            if (nfs_is_permanent_error(errno))
> > > >
> > >
> >
>

2018-10-22 14:11:01

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'



On 10/21/18 11:14 PM, Jianhong Yin wrote:
>
> ----- 原始邮件 -----
>> 发件人: "Jianhong Yin" <[email protected]>
>> 收件人: "Steve Dickson" <[email protected]>
>> 抄送: "尹剑虹" <[email protected]>, [email protected]
>> 发送时间: 星期一, 2018年 10 月 22日 上午 10:56:31
>> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
>>
>>
>> ----- 原始邮件 -----
>>> 发件人: "Steve Dickson" <[email protected]>
>>> 收件人: "尹剑虹" <[email protected]>
>>> 抄送: [email protected], [email protected]
>>> 发送时间: 星期六, 2018年 10 月 20日 下午 10:35:48
>>> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but
>>> return 'true'
>>>
>>> Hello,
>>>
>>> On 10/19/18 11:31 PM, 尹剑虹 wrote:
>>>>
>>>> Hi Steved
>>>>
>>>> The scenario is: these two mountings use different "context=" and
>>>> sharecache option, in this case mount(2) return fail with EBUSY.
>>> Fair enough.... I did miss the different contexts...
>>>
>>> But we already have a routine that check mount points
>>> is_mountpoint() used my mountd. I would rather use that.
>
> Hi Sir
>
> I reviewed function is_mountpoint(), seems it just check if the mountpoint is a root-fs,
> our goal is check if special dev has been mounted on specail path.
No. It check to see if the directory above the path (aka /..)
is on the same device (or filesystem) as path is. When the
device numbers are the same, both paths are on the same filesystem.
When they are not the same, the paths are on different filesystems

steved.

>
> WDYT?
>
>> Got it, good to know.
>>
>>>
>>> I'll resend the patch...
>>>
>>> Thanks!!
>>>
>>> steved.
>>>
>>>>
>>>> Jianhong
>>>>
>>>>
>>>>
>>>> 尹剑虹
>>>> 邮箱:[email protected]
>>>>
>>>> <https://maas.mail.163.com/dashi-web-extend/html/proSignature.html?iconUrl=https%3A%2F%2Fmail-online.nosdn.127.net%2Fqiyelogo%2FdefaultAvatar.png&name=%E5%B0%B9%E5%89%91%E8%99%B9&uid=yin-jianhong%40163.com&ftlId=1&items=%5B%22%E9%82%AE%E7%AE%B1%EF%BC%9Ayin-jianhong%40163.com%22%5D>
>>>>
>>>> 签名由 网易邮箱大师 <https://mail.163.com/dashi/dlpro.html?from=mail88> 定制
>>>>
>>>> On 10/20/2018 01:36, Steve Dickson <mailto:[email protected]>
>>>> wrote:
>>>>
>>>>
>>>> On 10/19/18 4:03 AM, [email protected] wrote:
>>>> > From: "Jianhong.Yin" <[email protected]>
>>>> >
>>>> > see: https://bugzilla.redhat.com/show_bug.cgi?id=1629705
>>>> > mount.nfs4 -o
>>>> > context=system_u:object_r:user_home_dir_t:s0,sharecache
>>>> > $serv:$expdir $nfsmp
>>>> > mount.nfs4 -o context=system_u:object_r:xferlog_t:s0,sharecache
>>>> > $serv:$expdir $nfsmp2
>>>> > ^^^ here mount fail, but return true. it confuse user!
>>>> Why should it fail? Two different mounts are being used and using
>>>> -o sharecache
>>>> which is the default the way...
>>>>
>>>> steved.
>>>> >
>>>> > according:
>>>> > https://patchwork.kernel.org/patch/10602607/#22234047
>>>> > add function is_mounted_already()
>>>> > -          if (errno == EBUSY)
>>>> > +          if (errno == EBUSY && is_mounted_already(mi->spec,
>>>> > mi->node))
>>>> >                return EX_SUCCESS;
>>>> >
>>>> > Signed-off-by: Jianhong Yin <[email protected]>
>>>> > ---
>>>> >  utils/mount/stropts.c | 31 ++++++++++++++++++++++++-------
>>>> >  1 file changed, 24 insertions(+), 7 deletions(-)
>>>> >
>>>> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
>>>> > index 4d2e37e..4be7e61 100644
>>>> > --- a/utils/mount/stropts.c
>>>> > +++ b/utils/mount/stropts.c
>>>> > @@ -48,6 +48,7 @@
>>>> >  #include "version.h"
>>>> >  #include "parse_dev.h"
>>>> >  #include "conffile.h"
>>>> > +#include <mntent.h>
>>>> >  
>>>> >  #ifndef NFS_PROGRAM
>>>> >  #define NFS_PROGRAM     (100003)
>>>> > @@ -1056,6 +1057,27 @@ static int nfs_is_permanent_error(int
>>>> > error)
>>>> >       }
>>>> >  }
>>>> >  
>>>> > +static int is_mounted_already(const char *fsname, const char
>>>> > *dir)
>>>> > +{
>>>> > +     struct mntent *ent;
>>>> > +     FILE *fp;
>>>> > +     int ret = 0;
>>>> > +
>>>> > +     fp = setmntent("/proc/mounts", "r");
>>>> > +     if (fp == NULL) {
>>>> > +          perror("[unlikely] setmntent(3) fail");
>>>> > +          exit(1);
>>>> > +     }
>>>> > +     while (NULL != (ent = getmntent(fp))) {
>>>> > +          if (!strcmp(ent->mnt_fsname, fsname) &&
>>>> > !strcmp(ent->mnt_dir, dir)) {
>>>> > +               ret = 1;
>>>> > +               break;
>>>> > +          }
>>>> > +     }
>>>> > +     endmntent(fp);
>>>> > +     return ret;
>>>> > +}
>>>> > +
>>>> >  /*
>>>> >   * Handle "foreground" NFS mounts.
>>>> >   *
>>>> > @@ -1078,13 +1100,8 @@ static int nfsmount_fg(struct
>>>> > nfsmount_info *mi)
>>>> >            if (nfs_try_mount(mi))
>>>> >                 return EX_SUCCESS;
>>>> >  
>>>> > -          if (errno == EBUSY)
>>>> > -               /* The only cause of EBUSY is if exactly the
>>>> > desired
>>>> > -                * filesystem is already mounted.  That can
>>>> > arguably
>>>> > -                * be seen as success.  "mount -a" tries to
>>>> > optimise
>>>> > -                * out this case but sometimes fails.  Help it
>>>> > out
>>>> > -                * by pretending everything is rosy
>>>> > -                */
>>>> > +          /* if EBUSY is caused by re-mount, ignore the error
>>>> > */
>>>> > +          if (errno == EBUSY && is_mounted_already(mi->spec,
>>>> > mi->node))
>>>> >                 return EX_SUCCESS;
>>>> >  
>>>> >            if (nfs_is_permanent_error(errno))
>>>> >
>>>>
>>>
>>

2018-10-23 02:40:48

by Jianhong Yin

[permalink] [raw]
Subject: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'



----- 原始邮件 -----
> 发件人: "Steve Dickson" <[email protected]>
> 收件人: "Jianhong Yin" <[email protected]>
> 抄送: "尹剑虹" <[email protected]>, [email protected]
> 发送时间: 星期一, 2018年 10 月 22日 下午 10:10:57
> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
>
>
>
> On 10/21/18 11:14 PM, Jianhong Yin wrote:
> >
> > ----- 原始邮件 -----
> >> 发件人: "Jianhong Yin" <[email protected]>
> >> 收件人: "Steve Dickson" <[email protected]>
> >> 抄送: "尹剑虹" <[email protected]>, [email protected]
> >> 发送时间: 星期一, 2018年 10 月 22日 上午 10:56:31
> >> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but
> >> return 'true'
> >>
> >>
> >> ----- 原始邮件 -----
> >>> 发件人: "Steve Dickson" <[email protected]>
> >>> 收件人: "尹剑虹" <[email protected]>
> >>> 抄送: [email protected], [email protected]
> >>> 发送时间: 星期六, 2018年 10 月 20日 下午 10:35:48
> >>> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but
> >>> return 'true'
> >>>
> >>> Hello,
> >>>
> >>> On 10/19/18 11:31 PM, 尹剑虹 wrote:
> >>>>
> >>>> Hi Steved
> >>>>
> >>>> The scenario is: these two mountings use different "context=" and
> >>>> sharecache option, in this case mount(2) return fail with EBUSY.
> >>> Fair enough.... I did miss the different contexts...
> >>>
> >>> But we already have a routine that check mount points
> >>> is_mountpoint() used my mountd. I would rather use that.
> >
> > Hi Sir
> >
> > I reviewed function is_mountpoint(), seems it just check if the
> > mountpoint is a root-fs,
> > our goal is check if special dev has been mounted on specail path.
> No. It check to see if the directory above the path (aka /..)
> is on the same device (or filesystem) as path is. When the
> device numbers are the same, both paths are on the same filesystem.
> When they are not the same, the paths are on different filesystems
I know.. ah sorry I used wrong word root-fs(not top rootfs, just means mountpoint).

What I want to say is:
Two(or more) devices could be mounted on same mountpoint on Linux,
only check "mi->node" is a mountpoint is not enough
we have to check if "mi->node" is a mountpoint *and* mounted on "mi->spec"

Jianhong

> steved.
>
> >
> > WDYT?
> >
> >> Got it, good to know.
> >>
> >>>
> >>> I'll resend the patch...
> >>>
> >>> Thanks!!
> >>>
> >>> steved.
> >>>
> >>>>
> >>>> Jianhong
> >>>>
> >>>>
> >>>>
> >>>> 尹剑虹
> >>>> 邮箱:[email protected]
> >>>>
> >>>> <https://maas.mail.163.com/dashi-web-extend/html/proSignature.html?iconUrl=https%3A%2F%2Fmail-online.nosdn.127.net%2Fqiyelogo%2FdefaultAvatar.png&name=%E5%B0%B9%E5%89%91%E8%99%B9&uid=yin-jianhong%40163.com&ftlId=1&items=%5B%22%E9%82%AE%E7%AE%B1%EF%BC%9Ayin-jianhong%40163.com%22%5D>
> >>>>
> >>>> 签名由 网易邮箱大师 <https://mail.163.com/dashi/dlpro.html?from=mail88> 定制
> >>>>
> >>>> On 10/20/2018 01:36, Steve Dickson <mailto:[email protected]>
> >>>> wrote:
> >>>>
> >>>>
> >>>> On 10/19/18 4:03 AM, [email protected] wrote:
> >>>> > From: "Jianhong.Yin" <[email protected]>
> >>>> >
> >>>> > see: https://bugzilla.redhat.com/show_bug.cgi?id=1629705
> >>>> > mount.nfs4 -o
> >>>> > context=system_u:object_r:user_home_dir_t:s0,sharecache
> >>>> > $serv:$expdir $nfsmp
> >>>> > mount.nfs4 -o
> >>>> > context=system_u:object_r:xferlog_t:s0,sharecache
> >>>> > $serv:$expdir $nfsmp2
> >>>> > ^^^ here mount fail, but return true. it confuse user!
> >>>> Why should it fail? Two different mounts are being used and
> >>>> using
> >>>> -o sharecache
> >>>> which is the default the way...
> >>>>
> >>>> steved.
> >>>> >
> >>>> > according:
> >>>> > https://patchwork.kernel.org/patch/10602607/#22234047
> >>>> > add function is_mounted_already()
> >>>> > -          if (errno == EBUSY)
> >>>> > +          if (errno == EBUSY && is_mounted_already(mi->spec,
> >>>> > mi->node))
> >>>> >                return EX_SUCCESS;
> >>>> >
> >>>> > Signed-off-by: Jianhong Yin <[email protected]>
> >>>> > ---
> >>>> >  utils/mount/stropts.c | 31 ++++++++++++++++++++++++-------
> >>>> >  1 file changed, 24 insertions(+), 7 deletions(-)
> >>>> >
> >>>> > diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> >>>> > index 4d2e37e..4be7e61 100644
> >>>> > --- a/utils/mount/stropts.c
> >>>> > +++ b/utils/mount/stropts.c
> >>>> > @@ -48,6 +48,7 @@
> >>>> >  #include "version.h"
> >>>> >  #include "parse_dev.h"
> >>>> >  #include "conffile.h"
> >>>> > +#include <mntent.h>
> >>>> >  
> >>>> >  #ifndef NFS_PROGRAM
> >>>> >  #define NFS_PROGRAM     (100003)
> >>>> > @@ -1056,6 +1057,27 @@ static int nfs_is_permanent_error(int
> >>>> > error)
> >>>> >       }
> >>>> >  }
> >>>> >  
> >>>> > +static int is_mounted_already(const char *fsname, const char
> >>>> > *dir)
> >>>> > +{
> >>>> > +     struct mntent *ent;
> >>>> > +     FILE *fp;
> >>>> > +     int ret = 0;
> >>>> > +
> >>>> > +     fp = setmntent("/proc/mounts", "r");
> >>>> > +     if (fp == NULL) {
> >>>> > +          perror("[unlikely] setmntent(3) fail");
> >>>> > +          exit(1);
> >>>> > +     }
> >>>> > +     while (NULL != (ent = getmntent(fp))) {
> >>>> > +          if (!strcmp(ent->mnt_fsname, fsname) &&
> >>>> > !strcmp(ent->mnt_dir, dir)) {
> >>>> > +               ret = 1;
> >>>> > +               break;
> >>>> > +          }
> >>>> > +     }
> >>>> > +     endmntent(fp);
> >>>> > +     return ret;
> >>>> > +}
> >>>> > +
> >>>> >  /*
> >>>> >   * Handle "foreground" NFS mounts.
> >>>> >   *
> >>>> > @@ -1078,13 +1100,8 @@ static int nfsmount_fg(struct
> >>>> > nfsmount_info *mi)
> >>>> >            if (nfs_try_mount(mi))
> >>>> >                 return EX_SUCCESS;
> >>>> >  
> >>>> > -          if (errno == EBUSY)
> >>>> > -               /* The only cause of EBUSY is if exactly the
> >>>> > desired
> >>>> > -                * filesystem is already mounted.  That can
> >>>> > arguably
> >>>> > -                * be seen as success.  "mount -a" tries to
> >>>> > optimise
> >>>> > -                * out this case but sometimes fails.  Help it
> >>>> > out
> >>>> > -                * by pretending everything is rosy
> >>>> > -                */
> >>>> > +          /* if EBUSY is caused by re-mount, ignore the error
> >>>> > */
> >>>> > +          if (errno == EBUSY && is_mounted_already(mi->spec,
> >>>> > mi->node))
> >>>> >                 return EX_SUCCESS;
> >>>> >  
> >>>> >            if (nfs_is_permanent_error(errno))
> >>>> >
> >>>>
> >>>
> >>
>