On Mon, Mar 8, 2021 at 5:23 PM Kevin Locke <kevin@xxxxxxxxxxxxxxx> wrote:
>
> Add "xino" to the list of features which cause undefined behavior for
> offline changes to the lower tree in the "Changes to underlying
> filesystems" section of the documentation to make users aware of
> potential issues if the lower tree is modified and xino was enabled.
>
> This omission was noticed by Amir Goldstein, who mentioned that xino is
> one of the "forbidden" features for making offline changes to the lower
> tree and that it wasn't currently documented.
>
Hi Kevin,
Thanks for following up on this.
I see my original comment did not make it to the list, because the message
was formatted incorrectly.
My full comment was:
"...
My feeling is that we need to adjust the fix and not treat the case
of xino=auto as "user opted-in for xino" thus disabling origin inode
decode for lower without uuid.
Also the documentation does not mention the xino feature as one
of the "forbidden" features for this use case.
..."
So your Documentation fix may represent the current code, but
I think we should fix the code instead.
When looking again, I actually don't see a reason to include "xino"
in this check at all (not xino=on nor xino=auto):
if (!ofs->config.index && !ofs->config.metacopy && !ofs->config.xino &&
uuid_is_null(uuid))
return false;
The reason that "index" and "metacopy" are in this check is because
they *need* to follow the lower inode of a non-dir upper in order to
operate correctly. The same is not true for "xino".
Moreover, "xino" will happily be enabled also when lower fs does not
support file handles at all. It will operate sub-optimally, but it will live up
to the promise to provide a unified inode namespace and uniform st_dev.
> Signed-off-by: Kevin Locke <kevin@xxxxxxxxxxxxxxx>
> ---
> Documentation/filesystems/overlayfs.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 78240e29b0bb..52d47bed9ef8 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -476,9 +476,9 @@ a crash or deadlock.
>
> Offline changes, when the overlay is not mounted, are allowed to the
> upper tree. Offline changes to the lower tree are only allowed if the
> -"metadata only copy up", "inode index", and "redirect_dir" features
> -have not been used. If the lower tree is modified and any of these
> -features has been used, the behavior of the overlay is undefined,
> +"metadata only copy up", "inode index", "redirect_dir", and "xino"
> +features have not been used. If the lower tree is modified and any of
> +these features has been used, the behavior of the overlay is undefined,
> though it will not result in a crash or deadlock.
Note that "redirect_dir" is not one of the "forbidden" features.
Thanks,
Amir.
