Re: [PATCH v2 05/11] ovl: lookup redirect by file handle

[Amir Goldstein <amir73il@gmail.com>]

On Fri, Apr 28, 2017 at 10:55 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Fri, Apr 28, 2017 at 9:25 AM, Amir Goldstein <amir73il@gmail.com> wrote:
>> With all recent conclusions, do you see a reason to keep origin root fh?
>
> No, I'm quite happy if we don't :)
>
>> For snapshots I need just one thing -
>> Verify that origin.fh matches the lower of merge dir that was found by path.
>> The verification is very cheap (only encode the found dentry), so we may
>> do it in any configuration, just don't know how to act upon it.
>>
>> What to do in case verification fails may need configuration option.
>> For snapshots I need a 'strict' policy meaning that "stale lower" equals
>> "implicit opaque", but that will not do the right thing for copied layers case.
>>
>> The way I have it now in my snapshot patches is overload on the redirect_dir
>> mount option and add a value redirect_dir=fh. The build time and module
>> options are still boolean, but -o redirect_dir=fh sets config->redirect_dir=true
>> and config->redirect_fh=true.
>> config->redirect_fh can later be set to false if the prerequisite (samefs etc)
>> don't apply.
>>
>> I may need to separate the general ofs->redirect_fh capabiltiy from the mount
>> configuration (i.e. config->redirect_dir_fh or make
>> config->redirect_dir an enum).
>>
>> I could also add more policy options for redirect_dir, i.e.:
>> off - pre v4.10 compat
>> on - v4.10 compat (path only)
>> path - same as on, just to explicitly mention for when knowingly copying layers
>> fh - snapshot case, fh must be verified
>> auto - (the default?) best effort w.r.t lower dir renames -
>> lookup by path, verify fh, if fails try to lookup by fh, if fails use
>> path result anyway.
>>
>> I realize you prefer the "minimum configuration" policy, but I'm afraid we
>> are at a cross road of letting the user decide. No?
>
> Is this only about dirs though?

Well, we could add configuration options to decide if and how to follow
and verify fh for non-dir, but:

1. We agreed that trying to follow fh for non-dir is a no-loose situation
    for !redirect and hot cache case and a probable win for redirect with
    cold case
2. For snapshots the behavior should be the same -
    use the lower ino if you can find and hold lower inode
    use upper ino otherwise (i.e. snapshots are not binding the inode
    numbers forever)

So I see no reason, going forward, to provide a user configuration
for the lookup of non-dir.

>
> For now I'd just add a "verify_lower" option defaulting to off and not
> expand "redirect_dir".  That should take care of the snapshot case,
> right?
>

Right.

Thanks,
Amir.