Re: [PATCH v2] ovl: return error on mount if metacopy cannot be enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 31, 2018 at 4:17 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>
> On Wed, Oct 31, 2018 at 04:05:02PM +0200, Amir Goldstein wrote:
> > On Wed, Oct 31, 2018 at 3:47 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> > ...
> > >
> > > So "strict" will change behavior. That is where we think configuration
> > > is not right/suboptimal, we will fail mount?
> > >
> > > I feel little odd about enabling "strict" implicitly just because
> > > "metacopy" has been passed in. To me, for all new mount options,
> > > "strict" should be default implementation (and does not require
> > > strict to be on as such).
> > >
> > > For old options, users are already happy with what they are seeing
> > > as of now. Those who want strict behavior, they should pass in
> > > "strict=on" and then behavior of old knobs will change without
> > > breaking backward compatibility.
> > >
> >
> > We need to think of users and documentation and real life use cases.
> > We need to think of overlayfs code maintenance and overlayfs developers.
> > We need to come up with a compromise that is the best of all worlds.
> >
> > Maintaining different behavior per feature complicates the code and
> > IMO brings no real value to users.
> >
> > If you think there is a concrete real world problem with metacopy=on
> > implying strict=on, please present the case.
> >
> > Are you interested in making metacopy=on work on sub-optimal upper
> > file systems? Which one?
> > Are you interested in making index=on,metacopy=on fallback to
> > index=off,metacopy=on on underlying fs without file handle support?
> > Why?
> > What is the real life use case for which you wish to preserve those
> > behaviors that are mostly there because we made mistakes in the
> > past?
>
> I don't have a good example. But, following is my thought process that
> why I think turning on strict with metacopy=on is not very good.
>
> - Turning on strict on, can make a working configuration fail due to
>   unrelated reasons. Say d_type is not supported and user is fine with
>   that and wants metacopy=on. Now that will not work and it will be
>   confusing to understand that what metacopy has to do with d_type.
>

It is not in the best of our (overlayfs developers) interest to allow users
to shoot their own feet. We allow that when we have no choice (backward
compat), but we really have no reason to do that with new features.
In fact, new features is our *only* opportunity to deprecate old code
that is sub-optimal and only exists because of backward compat reasons.

To use a very concrete example, index=on enables the exclusive upperdir
workdir checks. Not because index=on needs exclusive upperdir/workdir
more than index=off, but just because we can. And now docker cannot deal
with exclusive upperdir/workdir because docker has mount leak bugs, so
docker sets index=off to workaround its own bugs.

If ever docker would want to provide sane hardlink behavior to its users,
docker will have to fix its mount leaks bugs first. We keep looking backwards,
but general direction should be forward!

> - Are you 100% convinced that all the users who are not complaining
>   about current behavior, will not want that behavior for old knobs
>   with metacopy=on. If you decouple strict from metacopy, then it
>   allows you to support these users if somebody comes back.
>

If users really want metacopy with no d_type, they can do that with
metacopy=on,strict=off. You see metacopy does not enforce strict=on
it just implies strict=on, just like it implies redirect_dir=on.

Thanks,
Amir.



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux