All of lore.kernel.org
 help / color / mirror / Atom feed
* "mount -o remount,rw" sometimes doesn't work as expected.
@ 2017-05-19  7:14 NeilBrown
  2017-05-19  9:11 ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: NeilBrown @ 2017-05-19  7:14 UTC (permalink / raw)
  To: util-linux

[-- Attachment #1: Type: text/plain, Size: 1926 bytes --]



Suppose /foo and /bar are bind mounts to the same
filesystem which is currently mounted read-only,
and suppose the /etc/fstab contains

  /dev/sda1 /bar ext4 defaults 0 1
  /bar /foo none bind 0 0

Now if I want /foo to be writeable I might try:

  mount /foo -o remount,rw

and would then be surprised that this doesn't work.

What is happening is that because only one path has been given, mount
needs to find the other and goes looking in /etc/fstab.
If finds an appropriate line and parses out the options.
Then the mount system call used is

 mount( "/bar", "/foo", ..., MS_REMOUNT | MS_BIND ,....)

This changes the per-mountpoint ro flag to rw, but doesn't change the
filesystem itself.  This can be seen in /proc/self/mountinfo.  There
are two r[wo] flags, and they are different.

Had I run:

  mount /foo -o remount,bind,rw

I would have expected this.  But as I didn't explicitly ask for "bind",
it is confusing.

I think it might be good to ignore "bind" in /etc/fstab when "remount"
is used.
However.... when "remount" is used, the "device" is ignored, so there
isn't a lot of point hunting through /etc/fstab to find it.
If mount is given just one path and the "remount" option, then maybe
it shouldn't try to find an /etc/fstab entry at all?
I guess you might want to remount a device??
Is
   mount -o remount,rw /dev/sda1
allowed?  In that case, don't look through /etc/fstab if the path
is a directory.

I haven't provided a patch because I'm not 100% sure what the best
approach would be.
Does anyone have opinions on what mount should or shouldn't do when
remounting and only one path is given?

You could possibly argue that the current behaviour is correct
as /foo is listed as bind mount.  That doesn't stop is being surprising.
Also
   mount /bar /foo -o remount,rw

doesn't pick up the "bind" flag, so the filesystem gets remounted.
So I think there is definitely something wrong.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

* Re: "mount -o remount,rw" sometimes doesn't work as expected.
  2017-05-19  7:14 "mount -o remount,rw" sometimes doesn't work as expected NeilBrown
@ 2017-05-19  9:11 ` Karel Zak
  2017-05-21 22:16   ` NeilBrown
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2017-05-19  9:11 UTC (permalink / raw)
  To: NeilBrown; +Cc: util-linux

On Fri, May 19, 2017 at 05:14:24PM +1000, NeilBrown wrote:
> Suppose /foo and /bar are bind mounts to the same
> filesystem which is currently mounted read-only,
> and suppose the /etc/fstab contains
> 
>   /dev/sda1 /bar ext4 defaults 0 1
>   /bar /foo none bind 0 0
> 
> Now if I want /foo to be writeable I might try:
> 
>   mount /foo -o remount,rw
> 
> and would then be surprised that this doesn't work.
> 
> What is happening is that because only one path has been given, mount
> needs to find the other and goes looking in /etc/fstab.

Frankly remount & fstab sucks.

The problem is that we want to read fstab because remount works as
"replace all old options with new options", so if you don't read
fstab then all options unspecified on command line will be removed.

Let's imagine your command line is "-o remount,ro", but you have also
"noexec" in your fstab. If you don't read fstab then "noexec" will be
removed by the remount operation. This is unexpected by many users.
They want to change only specified options (ro->rw, etc).

It would be possible to read /proc/self/mountinfo on remount (to get
old options), but unfortunately this is not backwardly compatible,
because we have users who call 

    mount /mnt -o remount

(yes, without another options) and they assume that all options will
be reset according to fstab.


*** See man mount: ***

The remount functionality follows the standard way the mount command
works with options from fstab.  This means that mount does not read
fstab (or  mtab)  only  when  both device and dir are specified.

    mount -o remount,rw /dev/foo /dir

After this call all old mount options are replaced and arbitrary stuff
from fstab (or mtab) is ignored, except the loop= option which is
internally generated and maintained by the mount command.

    mount -o remount,rw  /dir

After this call, mount reads fstab and merges these options with the
options from the command line (-o).  If no mountpoint is found in
fstab, then a remount with  unspecified source is allowed.

> If finds an appropriate line and parses out the options.
> Then the mount system call used is
> 
>  mount( "/bar", "/foo", ..., MS_REMOUNT | MS_BIND ,....)
> 
> This changes the per-mountpoint ro flag to rw, but doesn't change the
> filesystem itself.  This can be seen in /proc/self/mountinfo.  There
> are two r[wo] flags, and they are different.
> 
> Had I run:
> 
>   mount /foo -o remount,bind,rw
> 
> I would have expected this.  But as I didn't explicitly ask for "bind",
> it is confusing.
> 
> I think it might be good to ignore "bind" in /etc/fstab when "remount"
> is used.

Another exception :-) I'll think about it, maybe it's not so bad idea
because "remount,bind" is very special.

> However.... when "remount" is used, the "device" is ignored, so there
> isn't a lot of point hunting through /etc/fstab to find it.

Yes, but then you have to specify all options on command line,
unspecified will be removed by remount op.

We have in TODO for years

 - add options to control fstab/mtab mount options usage, something like:

   --options-mode={ignore,append,prepend,replace}      MNT_OMODE_{IGNORE, ...}
   --options-source={fstab,mtab,disable}               MNT_OMODE_{FSTAB,MTAB,NOTAB}
   --options-source-force                              MNT_OMODE_FORCE


all this already supported by libmount, but no exported to the
mount(8) command line.

Maybe I should implement it :)

> If mount is given just one path and the "remount" option, then maybe
> it shouldn't try to find an /etc/fstab entry at all?
> I guess you might want to remount a device??
> Is
>    mount -o remount,rw /dev/sda1
> allowed?  In that case, don't look through /etc/fstab if the path
> is a directory.
> 
> I haven't provided a patch because I'm not 100% sure what the best
> approach would be.
> Does anyone have opinions on what mount should or shouldn't do when
> remounting and only one path is given?
> 
> You could possibly argue that the current behaviour is correct
> as /foo is listed as bind mount.  That doesn't stop is being surprising.
> Also
>    mount /bar /foo -o remount,rw
> 
> doesn't pick up the "bind" flag, so the filesystem gets remounted.
> So I think there is definitely something wrong.

That's question... "remount,bind" is really special and maybe "bind"
from fstab should be really ignored in this case.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

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

* Re: "mount -o remount,rw" sometimes doesn't work as expected.
  2017-05-19  9:11 ` Karel Zak
@ 2017-05-21 22:16   ` NeilBrown
  2017-05-22 11:26     ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: NeilBrown @ 2017-05-21 22:16 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

[-- Attachment #1: Type: text/plain, Size: 5885 bytes --]

On Fri, May 19 2017, Karel Zak wrote:

> On Fri, May 19, 2017 at 05:14:24PM +1000, NeilBrown wrote:
>> Suppose /foo and /bar are bind mounts to the same
>> filesystem which is currently mounted read-only,
>> and suppose the /etc/fstab contains
>> 
>>   /dev/sda1 /bar ext4 defaults 0 1
>>   /bar /foo none bind 0 0
>> 
>> Now if I want /foo to be writeable I might try:
>> 
>>   mount /foo -o remount,rw
>> 
>> and would then be surprised that this doesn't work.
>> 
>> What is happening is that because only one path has been given, mount
>> needs to find the other and goes looking in /etc/fstab.
>
> Frankly remount & fstab sucks.

:-)

>
> The problem is that we want to read fstab because remount works as
> "replace all old options with new options", so if you don't read
> fstab then all options unspecified on command line will be removed.
>
> Let's imagine your command line is "-o remount,ro", but you have also
> "noexec" in your fstab. If you don't read fstab then "noexec" will be
> removed by the remount operation. This is unexpected by many users.
> They want to change only specified options (ro->rw, etc).
>
> It would be possible to read /proc/self/mountinfo on remount (to get
> old options), but unfortunately this is not backwardly compatible,
> because we have users who call 
>
>     mount /mnt -o remount
>
> (yes, without another options) and they assume that all options will
> be reset according to fstab.

I can see how the need to support that usage make it difficult to ignore
fstab.


>
>
> *** See man mount: ***
>
> The remount functionality follows the standard way the mount command
> works with options from fstab.  This means that mount does not read
> fstab (or  mtab)  only  when  both device and dir are specified.
>
>     mount -o remount,rw /dev/foo /dir
>
> After this call all old mount options are replaced and arbitrary stuff
> from fstab (or mtab) is ignored, except the loop= option which is
> internally generated and maintained by the mount command.

oooh... it just occurred to me that for a mount, I can make it
read/write by:

   mount -o remount,rw none /mount/point

providing I know that no other flags are needed.  I don't need to give
the correct device path, do I?


>
>     mount -o remount,rw  /dir
>
> After this call, mount reads fstab and merges these options with the
> options from the command line (-o).  If no mountpoint is found in
> fstab, then a remount with  unspecified source is allowed.

BTW, if fstab contains just

   /bar /foo none bind 0 0


Then both
  mount -o remount,rw /bar
and
  mount -o remount,rw /foo

will try to remount /foo.  This is also surprising....
Normally you would have an entry for "/bar" listed earlier, and that
would be used for "mount -o remount,rw /bar".  So it probably isn't a
real problem.

>
>> If finds an appropriate line and parses out the options.
>> Then the mount system call used is
>> 
>>  mount( "/bar", "/foo", ..., MS_REMOUNT | MS_BIND ,....)
>> 
>> This changes the per-mountpoint ro flag to rw, but doesn't change the
>> filesystem itself.  This can be seen in /proc/self/mountinfo.  There
>> are two r[wo] flags, and they are different.
>> 
>> Had I run:
>> 
>>   mount /foo -o remount,bind,rw
>> 
>> I would have expected this.  But as I didn't explicitly ask for "bind",
>> it is confusing.
>> 
>> I think it might be good to ignore "bind" in /etc/fstab when "remount"
>> is used.
>
> Another exception :-) I'll think about it, maybe it's not so bad idea
> because "remount,bind" is very special.
>
>> However.... when "remount" is used, the "device" is ignored, so there
>> isn't a lot of point hunting through /etc/fstab to find it.
>
> Yes, but then you have to specify all options on command line,
> unspecified will be removed by remount op.
>
> We have in TODO for years
>
>  - add options to control fstab/mtab mount options usage, something like:
>
>    --options-mode={ignore,append,prepend,replace}      MNT_OMODE_{IGNORE, ...}
>    --options-source={fstab,mtab,disable}               MNT_OMODE_{FSTAB,MTAB,NOTAB}
>    --options-source-force                              MNT_OMODE_FORCE
>
>
> all this already supported by libmount, but no exported to the
> mount(8) command line.
>
> Maybe I should implement it :)

Maybe.  I'm currently focused on the default behaviour being a bit
confusing and this wouldn't help with that.
While the functionality that these flags provide seems sensible,
I do wonder if it would actually get used.

>
>> If mount is given just one path and the "remount" option, then maybe
>> it shouldn't try to find an /etc/fstab entry at all?
>> I guess you might want to remount a device??
>> Is
>>    mount -o remount,rw /dev/sda1
>> allowed?  In that case, don't look through /etc/fstab if the path
>> is a directory.
>> 
>> I haven't provided a patch because I'm not 100% sure what the best
>> approach would be.
>> Does anyone have opinions on what mount should or shouldn't do when
>> remounting and only one path is given?
>> 
>> You could possibly argue that the current behaviour is correct
>> as /foo is listed as bind mount.  That doesn't stop is being surprising.
>> Also
>>    mount /bar /foo -o remount,rw
>> 
>> doesn't pick up the "bind" flag, so the filesystem gets remounted.
>> So I think there is definitely something wrong.
>
> That's question... "remount,bind" is really special and maybe "bind"
> from fstab should be really ignored in this case.

I think that is probably the best way forward, at least in the short
term.
I might try to dig into the code and see how easy this would be.

Thanks for your detailed response!

NeilBrown


>
>     Karel
>
> -- 
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

* Re: "mount -o remount,rw" sometimes doesn't work as expected.
  2017-05-21 22:16   ` NeilBrown
@ 2017-05-22 11:26     ` Karel Zak
  2017-05-31  3:05       ` NeilBrown
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2017-05-22 11:26 UTC (permalink / raw)
  To: NeilBrown; +Cc: util-linux

On Mon, May 22, 2017 at 08:16:36AM +1000, NeilBrown wrote:
> oooh... it just occurred to me that for a mount, I can make it
> read/write by:
> 
>    mount -o remount,rw none /mount/point
> 
> providing I know that no other flags are needed.  I don't need to give
> the correct device path, do I?

Yes, "none" is an usable placeholder there.

> > After this call, mount reads fstab and merges these options with the
> > options from the command line (-o).  If no mountpoint is found in
> > fstab, then a remount with  unspecified source is allowed.
> 
> BTW, if fstab contains just
> 
>    /bar /foo none bind 0 0
> 
> 
> Then both
>   mount -o remount,rw /bar
> and
>   mount -o remount,rw /foo
> 
> will try to remount /foo.  This is also surprising....

Yes, mount(8) goal is to accept "whatever" to keep users happy :-)

This your example is well known disadvantage, the solution is to use
--source and --target options (it's strongly recommended for scripts):

   mount -o remount,rw --source /bar
   mount -o remount,rw --target /foo

in this case mount(8) will not try to be smart...

...

> >> You could possibly argue that the current behaviour is correct
> >> as /foo is listed as bind mount.  That doesn't stop is being surprising.
> >> Also
> >>    mount /bar /foo -o remount,rw
> >> 
> >> doesn't pick up the "bind" flag, so the filesystem gets remounted.
> >> So I think there is definitely something wrong.
> >
> > That's question... "remount,bind" is really special and maybe "bind"
> > from fstab should be really ignored in this case.
> 
> I think that is probably the best way forward, at least in the short
> term.
> I might try to dig into the code and see how easy this would be.

It's trivial patch, see below. Seems to work as expected (I use
"findmnt -o TARGET,VFS-OPTIONS,FS-OPTION" to see ro/rw).

    Karel


diff --git a/libmount/src/context.c b/libmount/src/context.c
index 38e0363..a574758 100644
--- a/libmount/src/context.c
+++ b/libmount/src/context.c
@@ -2038,7 +2038,7 @@ static int apply_table(struct libmnt_context *cxt, struct libmnt_table *tb,
  */
 int mnt_context_apply_fstab(struct libmnt_context *cxt)
 {
-	int rc = -1, isremount = 0;
+	int rc = -1, isremount = 0, iscmdbind = 0;
 	struct libmnt_table *tab = NULL;
 	const char *src = NULL, *tgt = NULL;
 	unsigned long mflags = 0;
@@ -2063,8 +2063,10 @@ int mnt_context_apply_fstab(struct libmnt_context *cxt)
 		cxt->optsmode &= ~MNT_OMODE_FORCE;
 	}
 
-	if (mnt_context_get_mflags(cxt, &mflags) == 0 && mflags & MS_REMOUNT)
-		isremount = 1;
+	if (mnt_context_get_mflags(cxt, &mflags) == 0) {
+		isremount = !!(mflags & MS_REMOUNT);
+		iscmdbind = !!(mflags & MS_BIND);
+	}
 
 	if (cxt->fs) {
 		src = mnt_fs_get_source(cxt->fs);
@@ -2131,6 +2133,12 @@ int mnt_context_apply_fstab(struct libmnt_context *cxt)
 		 * not found are not so important and may be misinterpreted by
 		 * applications... */
 		rc = -MNT_ERR_NOFSTAB;
+
+
+	} else if (isremount && !iscmdbind) {
+
+		/* remove "bind" from fstab (or no-op if not present) */
+		mnt_optstr_remove_option(&cxt->fs->optstr, "bind");
 	}
 	return rc;
 }



-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

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

* Re: "mount -o remount,rw" sometimes doesn't work as expected.
  2017-05-22 11:26     ` Karel Zak
@ 2017-05-31  3:05       ` NeilBrown
  2017-05-31  9:18         ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: NeilBrown @ 2017-05-31  3:05 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

[-- Attachment #1: Type: text/plain, Size: 3710 bytes --]

On Mon, May 22 2017, Karel Zak wrote:

> On Mon, May 22, 2017 at 08:16:36AM +1000, NeilBrown wrote:
>> oooh... it just occurred to me that for a mount, I can make it
>> read/write by:
>> 
>>    mount -o remount,rw none /mount/point
>> 
>> providing I know that no other flags are needed.  I don't need to give
>> the correct device path, do I?
>
> Yes, "none" is an usable placeholder there.
>
>> > After this call, mount reads fstab and merges these options with the
>> > options from the command line (-o).  If no mountpoint is found in
>> > fstab, then a remount with  unspecified source is allowed.
>> 
>> BTW, if fstab contains just
>> 
>>    /bar /foo none bind 0 0
>> 
>> 
>> Then both
>>   mount -o remount,rw /bar
>> and
>>   mount -o remount,rw /foo
>> 
>> will try to remount /foo.  This is also surprising....
>
> Yes, mount(8) goal is to accept "whatever" to keep users happy :-)
>
> This your example is well known disadvantage, the solution is to use
> --source and --target options (it's strongly recommended for scripts):
>
>    mount -o remount,rw --source /bar
>    mount -o remount,rw --target /foo
>
> in this case mount(8) will not try to be smart...
>
> ...
>
>> >> You could possibly argue that the current behaviour is correct
>> >> as /foo is listed as bind mount.  That doesn't stop is being surprising.
>> >> Also
>> >>    mount /bar /foo -o remount,rw
>> >> 
>> >> doesn't pick up the "bind" flag, so the filesystem gets remounted.
>> >> So I think there is definitely something wrong.
>> >
>> > That's question... "remount,bind" is really special and maybe "bind"
>> > from fstab should be really ignored in this case.
>> 
>> I think that is probably the best way forward, at least in the short
>> term.
>> I might try to dig into the code and see how easy this would be.
>
> It's trivial patch, see below. Seems to work as expected (I use
> "findmnt -o TARGET,VFS-OPTIONS,FS-OPTION" to see ro/rw).
>
>     Karel
>

Sorry for the delay.

Yes, this does look easy, makes sense, and does work as expected for me
too.

Reported-and-tested-by: NeilBrown <neilb@suse.com>

will you be queuing this for the next release?

Thanks,
NeilBrown


>
> diff --git a/libmount/src/context.c b/libmount/src/context.c
> index 38e0363..a574758 100644
> --- a/libmount/src/context.c
> +++ b/libmount/src/context.c
> @@ -2038,7 +2038,7 @@ static int apply_table(struct libmnt_context *cxt, struct libmnt_table *tb,
>   */
>  int mnt_context_apply_fstab(struct libmnt_context *cxt)
>  {
> -	int rc = -1, isremount = 0;
> +	int rc = -1, isremount = 0, iscmdbind = 0;
>  	struct libmnt_table *tab = NULL;
>  	const char *src = NULL, *tgt = NULL;
>  	unsigned long mflags = 0;
> @@ -2063,8 +2063,10 @@ int mnt_context_apply_fstab(struct libmnt_context *cxt)
>  		cxt->optsmode &= ~MNT_OMODE_FORCE;
>  	}
>  
> -	if (mnt_context_get_mflags(cxt, &mflags) == 0 && mflags & MS_REMOUNT)
> -		isremount = 1;
> +	if (mnt_context_get_mflags(cxt, &mflags) == 0) {
> +		isremount = !!(mflags & MS_REMOUNT);
> +		iscmdbind = !!(mflags & MS_BIND);
> +	}
>  
>  	if (cxt->fs) {
>  		src = mnt_fs_get_source(cxt->fs);
> @@ -2131,6 +2133,12 @@ int mnt_context_apply_fstab(struct libmnt_context *cxt)
>  		 * not found are not so important and may be misinterpreted by
>  		 * applications... */
>  		rc = -MNT_ERR_NOFSTAB;
> +
> +
> +	} else if (isremount && !iscmdbind) {
> +
> +		/* remove "bind" from fstab (or no-op if not present) */
> +		mnt_optstr_remove_option(&cxt->fs->optstr, "bind");
>  	}
>  	return rc;
>  }
>
>
>
> -- 
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

* Re: "mount -o remount,rw" sometimes doesn't work as expected.
  2017-05-31  3:05       ` NeilBrown
@ 2017-05-31  9:18         ` Karel Zak
  2017-05-31 22:55           ` NeilBrown
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2017-05-31  9:18 UTC (permalink / raw)
  To: NeilBrown; +Cc: util-linux

On Wed, May 31, 2017 at 01:05:26PM +1000, NeilBrown wrote:
> Sorry for the delay.
> 
> Yes, this does look easy, makes sense, and does work as expected for me
> too.
> 
> Reported-and-tested-by: NeilBrown <neilb@suse.com>
> 
> will you be queuing this for the next release?

IMHO it's too late for v2.30. I'd like to wait for v2.31 (September)
as it is not so urgent and common issue :-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

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

* Re: "mount -o remount,rw" sometimes doesn't work as expected.
  2017-05-31  9:18         ` Karel Zak
@ 2017-05-31 22:55           ` NeilBrown
  0 siblings, 0 replies; 7+ messages in thread
From: NeilBrown @ 2017-05-31 22:55 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

On Wed, May 31 2017, Karel Zak wrote:

> On Wed, May 31, 2017 at 01:05:26PM +1000, NeilBrown wrote:
>> Sorry for the delay.
>> 
>> Yes, this does look easy, makes sense, and does work as expected for me
>> too.
>> 
>> Reported-and-tested-by: NeilBrown <neilb@suse.com>
>> 
>> will you be queuing this for the next release?
>
> IMHO it's too late for v2.30. I'd like to wait for v2.31 (September)
> as it is not so urgent and common issue :-)

I'm quite happy with September.  As long as I know it is on some sort of
track, I can forget about it.

Thanks!
NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

end of thread, other threads:[~2017-05-31 22:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-19  7:14 "mount -o remount,rw" sometimes doesn't work as expected NeilBrown
2017-05-19  9:11 ` Karel Zak
2017-05-21 22:16   ` NeilBrown
2017-05-22 11:26     ` Karel Zak
2017-05-31  3:05       ` NeilBrown
2017-05-31  9:18         ` Karel Zak
2017-05-31 22:55           ` NeilBrown

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.