Re: [PATCH v3 6/6] fsck.overlay: add impure xattr check

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

 



On Fri, Dec 29, 2017 at 4:21 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote:
> On 2017/12/28 21:18, Amir Goldstein Wrote:
>> On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote:
>>> An impure directory is a directory with "trusted.overlay.impure" xattr
>>> valued 'y', which indicate that this directory may contain copy-uped
>>> targets from lower layers. In oredr to prevent 'd_ino' change while
>>> copy-up (it create a new inode in upper layer) in getdents(2), impure
>>> xattr will be set in the parent directory, letting overlay filesystem
>>> check and get d_ino from lower origin target to ensure consistent d_ino.
>>>
>>> There are three situations of setting impure xattr:
>>> 1) Copyup lower target in a directory.
>>> 2) Link an origined target (already copy-uped, have origin xattr) into
>>>    a directory.
>>> 3) Rename an origined target (include merged subdirectories) into a
>>>    new directory.
>>>
>>> So, if a direcotry which contains several origined targets or redirect
>>> directories, the impure xattr should be set. If not, fix this xattr.
>>>
>>> Signed-off-by: zhangyi (F) <yi.zhang@xxxxxxxxxx>
>>> ---
[...]
>>>                 case FTS_DEFAULT:
>>>                         /* Check whiteouts */
>>> -                       err = __check_entry(sctx, sop->whiteout);
>>> -                       ret = (err) ? err : ret;
>>> +                       ret = scan_check_entry(sop->whiteout, sctx);
>>> +                       if (ret)
>>> +                               goto out;
>>> +
>>> +                       /* Check origin */
>>> +                       ret = scan_check_entry(sop->origin, sctx);
>>> +                       if (ret)
>>> +                               goto out;
>>
>> All the re-factoring of scan helpers and this additional origin check
>> do not seem relevant to this change's subject (impure dir check).
>>
> Current origin check function only count origin targets in a directory
> (this function can used for future check). Impure dir check use this
> counts to distinguish this directory is impure or not, so this origin
> check is necessary in this patch.
>

I see. Anyway, the cleaner the patch is, the easier it is to review it.
A re-factoring patch that does not change behavior and declares this
in commit message is easy to verify in review and a logic change
patch that declares the logical change in commit message is easy to
review. Mixing them both in a single patch without being able to declare
that this is only a logical change not that this is only re-factoring makes
reviewing harder.

This is generally speaking. This patch was easy enough to review
anyway, but other reviewers may not feel the same way.

Cheers,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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