All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
@ 2018-10-19  8:03 jiyin
  2018-10-19 17:36 ` Steve Dickson
  0 siblings, 1 reply; 7+ messages in thread
From: jiyin @ 2018-10-19  8:03 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs, Jianhong.Yin

From: "Jianhong.Yin" <yin-jianhong@163.com>

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 <yin-jianhong@163.com>
---
 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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
  2018-10-19  8:03 [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true' jiyin
@ 2018-10-19 17:36 ` Steve Dickson
       [not found]   ` <17c11e3b.5987.1668f873fb0.Coremail.yin-jianhong@163.com>
  0 siblings, 1 reply; 7+ messages in thread
From: Steve Dickson @ 2018-10-19 17:36 UTC (permalink / raw)
  To: jiyin; +Cc: linux-nfs, Jianhong.Yin



On 10/19/18 4:03 AM, jiyin@redhat.com wrote:
> From: "Jianhong.Yin" <yin-jianhong@163.com>
> 
> 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 <yin-jianhong@163.com>
> ---
>  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))
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
       [not found]   ` <17c11e3b.5987.1668f873fb0.Coremail.yin-jianhong@163.com>
@ 2018-10-20 14:35     ` Steve Dickson
  2018-10-22  2:56       ` Jianhong Yin
  0 siblings, 1 reply; 7+ messages in thread
From: Steve Dickson @ 2018-10-20 14:35 UTC (permalink / raw)
  To: 尹剑虹; +Cc: jiyin, linux-nfs

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
> 
> 
> 	
> 尹剑虹
> 邮箱:yin-jianhong@163.com
> 
> <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:SteveD@RedHat.com> wrote:
> 
> 
>         On 10/19/18 4:03 AM, jiyin@redhat.com wrote:
>         > From: "Jianhong.Yin" <yin-jianhong@163.com>
>         >
>         > 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 <yin-jianhong@163.com>
>         > ---
>         >  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))
>         >
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
  2018-10-20 14:35     ` Steve Dickson
@ 2018-10-22  2:56       ` Jianhong Yin
  2018-10-22  3:14         ` Jianhong Yin
  0 siblings, 1 reply; 7+ messages in thread
From: Jianhong Yin @ 2018-10-22  2:56 UTC (permalink / raw)
  To: Steve Dickson; +Cc: 尹剑虹, linux-nfs


----- 原始邮件 -----
> 发件人: "Steve Dickson" <SteveD@RedHat.com>
> 收件人: "尹剑虹" <yin-jianhong@163.com>
> 抄送: jiyin@redhat.com, linux-nfs@vger.kernel.org
> 发送时间: 星期六, 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
> > 
> > 
> > 	
> > 尹剑虹
> > 邮箱:yin-jianhong@163.com
> > 
> > <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:SteveD@RedHat.com>
> >         wrote:
> > 
> > 
> >         On 10/19/18 4:03 AM, jiyin@redhat.com wrote:
> >         > From: "Jianhong.Yin" <yin-jianhong@163.com>
> >         >
> >         > 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 <yin-jianhong@163.com>
> >         > ---
> >         >  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))
> >         >
> > 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
  2018-10-22  2:56       ` Jianhong Yin
@ 2018-10-22  3:14         ` Jianhong Yin
  2018-10-22 14:10           ` Steve Dickson
  0 siblings, 1 reply; 7+ messages in thread
From: Jianhong Yin @ 2018-10-22  3:14 UTC (permalink / raw)
  To: Steve Dickson; +Cc: 尹剑虹, linux-nfs


----- 原始邮件 -----
> 发件人: "Jianhong Yin" <jiyin@redhat.com>
> 收件人: "Steve Dickson" <SteveD@RedHat.com>
> 抄送: "尹剑虹" <yin-jianhong@163.com>, linux-nfs@vger.kernel.org
> 发送时间: 星期一, 2018年 10 月 22日 上午 10:56:31
> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
> 
> 
> ----- 原始邮件 -----
> > 发件人: "Steve Dickson" <SteveD@RedHat.com>
> > 收件人: "尹剑虹" <yin-jianhong@163.com>
> > 抄送: jiyin@redhat.com, linux-nfs@vger.kernel.org
> > 发送时间: 星期六, 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
> > > 
> > > 
> > > 	
> > > 尹剑虹
> > > 邮箱:yin-jianhong@163.com
> > > 
> > > <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:SteveD@RedHat.com>
> > >         wrote:
> > > 
> > > 
> > >         On 10/19/18 4:03 AM, jiyin@redhat.com wrote:
> > >         > From: "Jianhong.Yin" <yin-jianhong@163.com>
> > >         >
> > >         > 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 <yin-jianhong@163.com>
> > >         > ---
> > >         >  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))
> > >         >
> > > 
> > 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
  2018-10-22  3:14         ` Jianhong Yin
@ 2018-10-22 14:10           ` Steve Dickson
  2018-10-23  2:40             ` Jianhong Yin
  0 siblings, 1 reply; 7+ messages in thread
From: Steve Dickson @ 2018-10-22 14:10 UTC (permalink / raw)
  To: Jianhong Yin; +Cc: 尹剑虹, linux-nfs



On 10/21/18 11:14 PM, Jianhong Yin wrote:
> 
> ----- 原始邮件 -----
>> 发件人: "Jianhong Yin" <jiyin@redhat.com>
>> 收件人: "Steve Dickson" <SteveD@RedHat.com>
>> 抄送: "尹剑虹" <yin-jianhong@163.com>, linux-nfs@vger.kernel.org
>> 发送时间: 星期一, 2018年 10 月 22日 上午 10:56:31
>> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
>>
>>
>> ----- 原始邮件 -----
>>> 发件人: "Steve Dickson" <SteveD@RedHat.com>
>>> 收件人: "尹剑虹" <yin-jianhong@163.com>
>>> 抄送: jiyin@redhat.com, linux-nfs@vger.kernel.org
>>> 发送时间: 星期六, 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
>>>>
>>>>
>>>> 	
>>>> 尹剑虹
>>>> 邮箱:yin-jianhong@163.com
>>>>
>>>> <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:SteveD@RedHat.com>
>>>>         wrote:
>>>>
>>>>
>>>>         On 10/19/18 4:03 AM, jiyin@redhat.com wrote:
>>>>         > From: "Jianhong.Yin" <yin-jianhong@163.com>
>>>>         >
>>>>         > 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 <yin-jianhong@163.com>
>>>>         > ---
>>>>         >  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))
>>>>         >
>>>>
>>>
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true'
  2018-10-22 14:10           ` Steve Dickson
@ 2018-10-23  2:40             ` Jianhong Yin
  0 siblings, 0 replies; 7+ messages in thread
From: Jianhong Yin @ 2018-10-23  2:40 UTC (permalink / raw)
  To: Steve Dickson; +Cc: 尹剑虹, linux-nfs



----- 原始邮件 -----
> 发件人: "Steve Dickson" <SteveD@RedHat.com>
> 收件人: "Jianhong Yin" <jiyin@redhat.com>
> 抄送: "尹剑虹" <yin-jianhong@163.com>, linux-nfs@vger.kernel.org
> 发送时间: 星期一, 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" <jiyin@redhat.com>
> >> 收件人: "Steve Dickson" <SteveD@RedHat.com>
> >> 抄送: "尹剑虹" <yin-jianhong@163.com>, linux-nfs@vger.kernel.org
> >> 发送时间: 星期一, 2018年 10 月 22日 上午 10:56:31
> >> 主题: Re: [PATCH] [nfs-utils] fix issue: mount -osharecache failure but
> >> return 'true'
> >>
> >>
> >> ----- 原始邮件 -----
> >>> 发件人: "Steve Dickson" <SteveD@RedHat.com>
> >>> 收件人: "尹剑虹" <yin-jianhong@163.com>
> >>> 抄送: jiyin@redhat.com, linux-nfs@vger.kernel.org
> >>> 发送时间: 星期六, 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
> >>>>
> >>>>
> >>>> 	
> >>>> 尹剑虹
> >>>> 邮箱:yin-jianhong@163.com
> >>>>
> >>>> <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:SteveD@RedHat.com>
> >>>>         wrote:
> >>>>
> >>>>
> >>>>         On 10/19/18 4:03 AM, jiyin@redhat.com wrote:
> >>>>         > From: "Jianhong.Yin" <yin-jianhong@163.com>
> >>>>         >
> >>>>         > 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 <yin-jianhong@163.com>
> >>>>         > ---
> >>>>         >  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))
> >>>>         >
> >>>>
> >>>
> >>
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-10-23  2:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-19  8:03 [PATCH] [nfs-utils] fix issue: mount -osharecache failure but return 'true' jiyin
2018-10-19 17:36 ` Steve Dickson
     [not found]   ` <17c11e3b.5987.1668f873fb0.Coremail.yin-jianhong@163.com>
2018-10-20 14:35     ` Steve Dickson
2018-10-22  2:56       ` Jianhong Yin
2018-10-22  3:14         ` Jianhong Yin
2018-10-22 14:10           ` Steve Dickson
2018-10-23  2:40             ` Jianhong Yin

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.