On Tue, Oct 24, 2017 at 12:59 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> On Thu, Oct 12, 2017 at 6:03 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>> Commit fbaf94ee3cd5 ("ovl: don't set origin on broken lower hardlink")
>> attempt to avoid the condition of non-indexed upper inode with lower
>> hardlink as origin. If this condition is found, lookup returns EIO.
>>
>> The protection of commit mentioned above does not cover the case of lower
>> that is not a hardlink when it is copied up (with either index=off/on)
>> and then lower is hardlinked while overlay is offline.
>>
>> Changes to lower layer while overlayfs is offline should not result in
>> unexpected behavior, so a permanent EIO error after creating a link in
>> lower layer should not be considered as correct behavior.
>>
>> This fix replaces EIO error with a warning in cases where upper has
>> origin but no index is found, or index is found that does not match upper
>> inode. In those cases, lookup will not fail and the returned overlay
>> inode will be hashed by upper inode instead of by lower origin inode.
>>
>> Fixes: 359f392ca53e ("ovl: lookup index entry for copy up origin")
>> Cc: <stable@xxxxxxxxxxxxxxx> # v4.13
>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>> ---
>>
>> Miklos,
>>
>> Following a discussion with Vivek about metacopy feature and the option
>> of setting ORIGIN for non-indexed lower hardlinks on copy up, I came to
>> a conclusion that the current EIO behavior is not quite tollerant to lower
>> changes as one would hope and that it should be fixed in stable kernels.
>
> Okay, but I started wondering if we really should be writing warnings
> to the kernel log if this situation is considered normal.
>
> Is it worth warning about these?
>
No I guess it is not worth it for plain index=on case, but it may be worth
it for upcoming index=all, which implies a complete index.
Although we can defer the warning to file handle encode time when that
matters.
BTW, I just sent a patch to remove the backward compatibility disclaimer
about OVERLAY_FS_INDEX from Kconfig following the EIO change.
Maybe I was being too brave?...
Let me know if you want me to send a new patch for EIO behavior
or you will sort it out yourself (remember there is also the ENOENT case).
Amir.