[PATCH v2] common/rc: add global mounted option for _scratch_remount

[PATCH v2] common/rc: add global mounted option for _scratch_remount

[XiaoLi Feng]

From: Xiaoli Feng <xifeng@redhat.com>

When call _scratch_remount for cifs , it always requires to input
password. This will make generic/306 generic/452 failed because
cifs remount failed.
---
 common/rc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index 1c42515f..ea1eaf6a 100644
--- a/common/rc
+++ b/common/rc
@@ -357,7 +357,7 @@ _scratch_remount()
     local opts="$1"
 
     if test -n "$opts"; then
-	mount -o "remount,$opts" $SCRATCH_MNT
+	mount $MOUNT_OPTIONS -o "remount,$opts" $SCRATCH_MNT
     fi
 }
 
-- 
2.18.1

Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount

[Eryu Guan]

On Sun, Mar 31, 2019 at 08:31:30PM +0800, Xiaoli Feng wrote:
> From: Xiaoli Feng <xifeng@redhat.com>
> 
> When call _scratch_remount for cifs , it always requires to input
> password. This will make generic/306 generic/452 failed because
> cifs remount failed.
> ---
>  common/rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/rc b/common/rc
> index 1c42515f..ea1eaf6a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -357,7 +357,7 @@ _scratch_remount()
>      local opts="$1"
>  
>      if test -n "$opts"; then
> -	mount -o "remount,$opts" $SCRATCH_MNT
> +	mount $MOUNT_OPTIONS -o "remount,$opts" $SCRATCH_MNT

IMHO, your v1 patch looks better, just need to update overlay/035 to use
bare $MOUNT_PROG to do the first remount.

Thanks,
Eryu

>      fi
>  }
>  
> -- 
> 2.18.1
> 

Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount

[Steve French]

I also verified that test 452 now works to cifs (SMB3 mounts to
multiple types of test targets, Azure, Windows etc.)

On Sun, Mar 31, 2019 at 7:31 AM XiaoLi Feng <xifeng@redhat.com> wrote:
>
> From: Xiaoli Feng <xifeng@redhat.com>
>
> When call _scratch_remount for cifs , it always requires to input
> password. This will make generic/306 generic/452 failed because
> cifs remount failed.
> ---
>  common/rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index 1c42515f..ea1eaf6a 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -357,7 +357,7 @@ _scratch_remount()
>      local opts="$1"
>
>      if test -n "$opts"; then
> -       mount -o "remount,$opts" $SCRATCH_MNT
> +       mount $MOUNT_OPTIONS -o "remount,$opts" $SCRATCH_MNT
>      fi
>  }
>
> --
> 2.18.1
>


-- 
Thanks,

Steve

Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount

[Steve French]

And 306 also works with cifs (as long as mounting with "sfu" mount
option) with this patch

On Mon, Apr 1, 2019 at 10:29 AM Steve French <smfrench@gmail.com> wrote:
>
> I also verified that test 452 now works to cifs (SMB3 mounts to
> multiple types of test targets, Azure, Windows etc.)
>
> On Sun, Mar 31, 2019 at 7:31 AM XiaoLi Feng <xifeng@redhat.com> wrote:
> >
> > From: Xiaoli Feng <xifeng@redhat.com>
> >
> > When call _scratch_remount for cifs , it always requires to input
> > password. This will make generic/306 generic/452 failed because
> > cifs remount failed.
> > ---
> >  common/rc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/rc b/common/rc
> > index 1c42515f..ea1eaf6a 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -357,7 +357,7 @@ _scratch_remount()
> >      local opts="$1"
> >
> >      if test -n "$opts"; then
> > -       mount -o "remount,$opts" $SCRATCH_MNT
> > +       mount $MOUNT_OPTIONS -o "remount,$opts" $SCRATCH_MNT
> >      fi
> >  }
> >
> > --
> > 2.18.1
> >
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount

[Xiaoli Feng]

Thanks Steve for the feedback.

----- Original Message -----
> From: "Steve French" <smfrench@gmail.com>
> To: "XiaoLi Feng" <xifeng@redhat.com>, fstests@vger.kernel.org
> Sent: Tuesday, April 2, 2019 8:33:12 AM
> Subject: Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount
> 
> And 306 also works with cifs (as long as mounting with "sfu" mount
> option) with this patch
> 
> On Mon, Apr 1, 2019 at 10:29 AM Steve French <smfrench@gmail.com> wrote:
> >
> > I also verified that test 452 now works to cifs (SMB3 mounts to
> > multiple types of test targets, Azure, Windows etc.)
> >
> > On Sun, Mar 31, 2019 at 7:31 AM XiaoLi Feng <xifeng@redhat.com> wrote:
> > >
> > > From: Xiaoli Feng <xifeng@redhat.com>
> > >
> > > When call _scratch_remount for cifs , it always requires to input
> > > password. This will make generic/306 generic/452 failed because
> > > cifs remount failed.
> > > ---
> > >  common/rc | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/common/rc b/common/rc
> > > index 1c42515f..ea1eaf6a 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -357,7 +357,7 @@ _scratch_remount()
> > >      local opts="$1"
> > >
> > >      if test -n "$opts"; then
> > > -       mount -o "remount,$opts" $SCRATCH_MNT
> > > +       mount $MOUNT_OPTIONS -o "remount,$opts" $SCRATCH_MNT
> > >      fi
> > >  }
> > >
> > > --
> > > 2.18.1
> > >
> >
> >
> > --
> > Thanks,
> >
> > Steve
> 
> 
> 
> --
> Thanks,
> 
> Steve
> 

Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount

[Xiaoli Feng]

Hi Eryu,

----- Original Message -----
> From: "Eryu Guan" <guaneryu@gmail.com>
> To: "XiaoLi Feng" <xifeng@redhat.com>
> Cc: fstests@vger.kernel.org, smfrench@gmail.com
> Sent: Monday, April 1, 2019 7:15:54 PM
> Subject: Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount
> 
> On Sun, Mar 31, 2019 at 08:31:30PM +0800, Xiaoli Feng wrote:
> > From: Xiaoli Feng <xifeng@redhat.com>
> > 
> > When call _scratch_remount for cifs , it always requires to input
> > password. This will make generic/306 generic/452 failed because
> > cifs remount failed.
> > ---
> >  common/rc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/common/rc b/common/rc
> > index 1c42515f..ea1eaf6a 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -357,7 +357,7 @@ _scratch_remount()
> >      local opts="$1"
> >  
> >      if test -n "$opts"; then
> > -	mount -o "remount,$opts" $SCRATCH_MNT
> > +	mount $MOUNT_OPTIONS -o "remount,$opts" $SCRATCH_MNT
> 
> IMHO, your v1 patch looks better, just need to update overlay/035 to use
> bare $MOUNT_PROG to do the first remount.

The v1 patch use "_try_scratch_mount "-o remount,$opts"". And actually it will
call like this "mount -o $option -o remount $dev $mountpoint". But this remounting
operation seems is different from "mount -o $option -o remount $mountpoint"(just use
strace to check). One difference is that it won't check /etc/fstab. That's why I give
up v1 patch. What do you think about it? If you consider we can ignore the difference.
Then I will use v1 patch and modify overlay/035.

Thanks.

> 
> Thanks,
> Eryu
> 
> >      fi
> >  }
> >  
> > --
> > 2.18.1
> > 
> 

Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount

[Eryu Guan]

On Tue, Apr 02, 2019 at 04:58:45AM -0400, Xiaoli Feng wrote:
> Hi Eryu,
> 
> ----- Original Message -----
> > From: "Eryu Guan" <guaneryu@gmail.com>
> > To: "XiaoLi Feng" <xifeng@redhat.com>
> > Cc: fstests@vger.kernel.org, smfrench@gmail.com
> > Sent: Monday, April 1, 2019 7:15:54 PM
> > Subject: Re: [PATCH v2] common/rc: add global mounted option for _scratch_remount
> > 
> > On Sun, Mar 31, 2019 at 08:31:30PM +0800, Xiaoli Feng wrote:
> > > From: Xiaoli Feng <xifeng@redhat.com>
> > > 
> > > When call _scratch_remount for cifs , it always requires to input
> > > password. This will make generic/306 generic/452 failed because
> > > cifs remount failed.
> > > ---
> > >  common/rc | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/common/rc b/common/rc
> > > index 1c42515f..ea1eaf6a 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -357,7 +357,7 @@ _scratch_remount()
> > >      local opts="$1"
> > >  
> > >      if test -n "$opts"; then
> > > -	mount -o "remount,$opts" $SCRATCH_MNT
> > > +	mount $MOUNT_OPTIONS -o "remount,$opts" $SCRATCH_MNT
> > 
> > IMHO, your v1 patch looks better, just need to update overlay/035 to use
> > bare $MOUNT_PROG to do the first remount.
> 
> The v1 patch use "_try_scratch_mount "-o remount,$opts"". And actually it will
> call like this "mount -o $option -o remount $dev $mountpoint". But this remounting
> operation seems is different from "mount -o $option -o remount $mountpoint"(just use
> strace to check). One difference is that it won't check /etc/fstab. That's why I give
> up v1 patch. What do you think about it? If you consider we can ignore the difference.
> Then I will use v1 patch and modify overlay/035.

I think we could ignore that difference. All you need for CIFS to work
is the password information set in $MOUNT_OPTIONS when remounting, and
_try_scratch_mount takes other mount options, e.g. selinux context, to
remount as well, not just $MOUNT_OPTIONS.

Thanks,
Eryu