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

[Jianhong Yin <jiyin@redhat.com>]

----- 原始邮件 -----
> 发件人: "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))
> >>>>         >
> >>>>
> >>>
> >>
>