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

Steve Dickson
2018-10-22T14:11:01Z

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))
>>>> >
>>>>
>>>
>>