On Wed, Jan 24, 2018 at 1:04 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Wed, Jan 24, 2018 at 12:34 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>> On Fri, Jan 19, 2018 at 9:10 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>>> On Fri, Jan 19, 2018 at 1:07 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>>>> On Fri, Jan 19, 2018 at 12:39 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>>>>> On Fri, Jan 19, 2018 at 02:23:35AM +0200, Amir Goldstein wrote:
>>>>>> > How is this for an option?
>>>>> [...]
>>>>>> > +struct dentry *d_obtain_alias_fsdata(struct inode *inode, void **fsdata)
>>>>>> > +{
>>>>>> > + return __d_obtain_alias(inode, 1, fsdata);
>>>>>> > }
>>>>>> > EXPORT_SYMBOL(d_obtain_alias);
>>>>>
>>>>> It would work, but I like this interface better:
>>>>>
>>>>> +extern struct dentry * d_alloc_anon(struct super_block *);
>>>>> +extern struct dentry * d_instantiate_anon(struct dentry *, struct inode *);
>>>>>
>>>>
>>>> OK. Thanks for the patch!
>>>>
>>>
>>> Added your dcache patch to the series and reworked my patches
>>> to use the new helpers.
>>>
>>> Tested result is pushed to:
>>> https://github.com/amir73il/linux/commits/ovl-nfs-export-v3
>>>
>>> Prep patches changes since v2:
>>> - Rebased over fix patch "hash all directory inodes for fsnotify"
>>> - Rename mount/config option from "verify" to "nfs_export"
>>> - Force r/o mount when index dir creation fails
>>> - Allow enabling "nfs_export" for non-upper mount
>>> - Require "redirect_dir=nofollow" for non-upper mount
>>> - Rename dir index entries xattr from ".origin" to ".upper"
>>> - Re-factor ovl_{get|set|verify}_origin() helpers
>>> - Simplify test for temp index name (starts with #)
>>> - Abandon ovl_dentry_is_renamed() test for lower st_ino
>>> - Document overhead on mount with full index
>>> - Document change of behavior when verifying lower origin
>>> - Added patch to make room in ovl_entry struct
>>>
>>> NFS export changes since v2:
>>> - Fix exportfs ops for r/o overlay with no upperdir
>>> - Document reason for copy up directory on encode
>>> - Take care of racing with rename while connecting dir
>>> - Explain the reasons for choosing the 'connected' dir approach
>>> - Do not add dentry without ovl_entry to dcache
>>>
>>> Optimizations TODO:
>>> - Copy up on encode only when lower ancestor is below middle layer redirect
>>> - Hash inode by fh to avoid origin decode of whiteout fh
>>>
>>> As far as I know, the series is now functionally correct and all comments
>>> so far addressed. The remaining optimizations will be done on top of this
>>> series.
>>
>> Pushed to overlayfs-next with one fix (do not warn about falling back
>> to nfs_export=off if nfs_export is already off).
>
> Good fix.
> That warning was a late addition to V3 after allowing nfs_export without
> index for on-upper r/o mount.
>
>>
>> That spurious warning makes me wonder: how much of the option matrix is tested?
>>
>
> Good question... (adding Eryu in CC)
>
> I know Eryu tested with default kernel configuration, because one
> of my tests found a bug with NFS export and redirect_dir=off.
> (I fixed the specific test to require redirect_dir)
>
> I have posted xfstests for overlay NFS export support with samefs
> and non-samefs configuration and for non-samefs, there are two
> lower layers not on the same fs. I also have posted a test for non-upper
> overlay with two lower layers on samefs and non-samefs.
>
> One of the overlay xfstests explicitly turns off index with index=off,
> (overlay/036), so we have coverage for falling back to nfs_export=off
> when NFS export is enabled by default (as it usually is in my setup).
>
> There is an overlay xfstest with multiple lower layers and no-upper
> and that test shows the warning about falling back to nfs_export=off
> due to redirect_dir=nofollow requirement (with default kernel configuration).
>
> I did not specifically test underlying fs with no xattr support and no
> file handle support, though the latter was already tested for index
> with squashfs (no uuid) and the nfs_export=off code is piggy backed
> in the same place as index=off.
>
Clarification: it may be implied when I wrote that "we have test coverage..."
that there are automatic tests to verify falling back to nfs_export=off.
This is not the case. I was only listing "falling back" cases whose warning
I regularly see in my routine overlay/quick xfstest as expected.
Thanks,
Amir.
