Re: "r/o bind mounts" patch in 2.6.26-rc6 changes (breaks?) error behaviour

Re: "r/o bind mounts" patch in 2.6.26-rc6 changes (breaks?) error behaviour

[Dave Hansen]

On Fri, 2008-06-20 at 12:16 +0200, Jens Rottmann wrote:
> 
> I noticed the following change in the error behaviour when attempting
> to delete nonexisting files/dirs from R/O-mounted directories:
> 
> Linux 2.6.25.7:
> rm /RO_mounted_path/nonexisting_file
> ==> "No such file or directory"
> 
> Linux 2.6.26-rc6-git6:
> rm /RO_mounted_path/nonexisting_file
> ==> "Read-only file system"

We looked at this way back in the beginning when I started to work on
the patches.  It was decided that the error codes weren't horribly
important.  We did have a case a bit ago where it really mattered, and
Al fixed that up.

I know this changed behavior a bit, but userspace can surely work around
it with read-only operations (stat) when it is really important to get
the error to the user just right.

> @@ -2371,8 +2373,10 @@ static long do_unlinkat(int dfd, const c
>                 if (inode)
>                         atomic_inc(&inode->i_count);
>                 error = mnt_want_write(nd.path.mnt);
> -               if (error)
> +               if (error) {
> +                       error = vfs_unlink(nd.path.dentry->d_inode, dentry);
>                         goto exit2;
> +               }
>                 error = vfs_unlink(nd.path.dentry->d_inode, dentry);
>                 mnt_drop_write(nd.path.mnt);

On a r/w fs where this is being performed on a r/o bind mount, this will
let you unlink files.  I think that's bad.  :)

-- Dave

Re: "r/o bind mounts" patch in 2.6.26-rc6 changes (breaks?) error behaviour

[Dave Hansen]

On Fri, 2008-06-20 at 07:17 -0700, Dave Hansen wrote:
> On Fri, 2008-06-20 at 12:16 +0200, Jens Rottmann wrote:
> > 
> > I noticed the following change in the error behaviour when attempting
> > to delete nonexisting files/dirs from R/O-mounted directories:
> > 
> > Linux 2.6.25.7:
> > rm /RO_mounted_path/nonexisting_file
> > ==> "No such file or directory"
> > 
> > Linux 2.6.26-rc6-git6:
> > rm /RO_mounted_path/nonexisting_file
> > ==> "Read-only file system"
> 
> We looked at this way back in the beginning when I started to work on
> the patches.  It was decided that the error codes weren't horribly
> important.  We did have a case a bit ago where it really mattered, and
> Al fixed that up.
> 
> I know this changed behavior a bit, but userspace can surely work around
> it with read-only operations (stat) when it is really important to get
> the error to the user just right.

Also, there are some nice benefits to the new way of doing it.  To
figure out if there is a file in that location also requires a path
walk.  Now, that isn't the most expensive thing in the world, but it is
a heck of a lot more expensive than just doing a read-only look at a
variable in the mount (in mnt_want_write()).

Don't you *want* warnings when an 'rm -f' runs into a r/o filesystem? ;)

-- Dave

Re: "r/o bind mounts" patch in 2.6.26-rc6 changes (breaks?) error behaviour

[Jens Rottmann]

Dave Hansen wrote:
> ... requires a path walk ...

... which is spared only in the error path, the normal path still does the path walk anyway.

> Don't you *want* warnings when an 'rm -f' runs into a r/o filesystem? ;)

Well, not really. The "rm -f"s are in my startup scripts, supposed to delete stale lock files and lots of stuff that's usually not there unless something went wrong the last time. But most of the time, I leave the whole system mounted R/O, except some tmpfs, dead useful for kernel/hardware hacking/testing.
But now I get masses of error messages on bootup - guess I have to litter the scripts with "if [ -e ... ]; then"s or just live with it.  :-|

Anyway, thanks for solving this mystery for me.  :-)

Jens

Re: "r/o bind mounts" patch in 2.6.26-rc6 changes (breaks?) error behaviour

[Pavel Machek]

On Fri 2008-06-20 07:17:46, Dave Hansen wrote:
> On Fri, 2008-06-20 at 12:16 +0200, Jens Rottmann wrote:
> > 
> > I noticed the following change in the error behaviour when attempting
> > to delete nonexisting files/dirs from R/O-mounted directories:
> > 
> > Linux 2.6.25.7:
> > rm /RO_mounted_path/nonexisting_file
> > ==> "No such file or directory"
> > 
> > Linux 2.6.26-rc6-git6:
> > rm /RO_mounted_path/nonexisting_file
> > ==> "Read-only file system"
> 
> We looked at this way back in the beginning when I started to work on
> the patches.  It was decided that the error codes weren't horribly
> important.  We did have a case a bit ago where it really mattered, and
> Al fixed that up.
> 
> I know this changed behavior a bit, but userspace can surely work around
> it with read-only operations (stat) when it is really important to get
> the error to the user just right.

I'd say userspace should not have to work around this.

Interface change, in stable series, w/o deprecation period, new
interface is worse than the old one...

						Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html