On Sat, Jun 24, 2017 at 8:57 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote:
> If an "impure && not merged" upper dir have left whiteouts
> (last mount created), ovl cannot clear this dir when we
> removing it, which may lead to rmdir fail or temp file left
> in workdir.
>
> Simple reproducer:
> mkdir lower upper work merge
> mkdir -p lower/dir
> touch lower/dir/a
> mount -t overlay overlay -olowerdir=lower,upperdir=upper,\
> workdir=work merge
> rm merge/dir/a
> umount merge
> rm -rf lower/*
> touch lower/dir (*)
> mount -t overlay overlay -olowerdir=lower,upperdir=upper,\
> workdir=work merge
> rm -rf merge/dir
>
> Syslog dump:
> overlayfs: cleanup of 'work/#7' failed (-39)
>
> (*): if we are not creat this file, the result is different:
> rm: cannot remove "dir/": Directory not empty
>
> This patch add explicitly check of OVL_XATTR_ORIGIN, use
> (merge || origin) to indicate the dir may have whiteouts.
> Finally, check and clean up the dir when deleting it.
>
> Signed-off-by: zhangyi (F) <yi.zhang@xxxxxxxxxx>
>
> ---
> V3:
> - use origin instead of impure flag
> - add check in ovl_rename()
>
> V2:
> - fix comment in ovl_remove_and_whiteout()
> - post reproducer to xfstest
>
> ---
Sorry, I keep finding more stuff...
> {
> @@ -331,12 +339,14 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry)
> * replace it with an exact replica of itself.
> *
> * If no upperdentry then skip clearing whiteouts.
> + * Origined directory may have whiteouts, should clean up.
> *
> * Can race with copy-up, since we don't hold the upperdir mutex.
> * Doesn't matter, since copy-up can't create a non-empty directory
> * from an empty one.
> */
> - if (OVL_TYPE_UPPER(type) && OVL_TYPE_MERGE(type))
> + if (OVL_TYPE_UPPER(type) && (OVL_TYPE_MERGE(type) ||
> + ovl_is_origin(dentry)))
> ret = ovl_clear_empty(dentry, &list);
>
We may have been a little stupid here.
It does not matter if MERGE/ORIGIN/whatever.
We already checked for whiteout existence in ovl_check_empty_dir(),
so actually ovl_clear_empty() is needed IFF
!OVL_TYPE_UPPER(type) && !list_empty(&list).
Actually, list can be non-empty due to whiteouts in lower dir
and then ovl_cleanup_whiteouts() will report errors when trying to
cleanup those lower whiteouts (if I am reading the code correctly).
Please test case of whiteouts in both lower and upper dirs of a merge
dir. rmdir may succeed, but with errors.
To fix this properly, need another hunk of my readdir patch (that's
actually a hunk I stole off Miklos' old readdir patch) that stores the
p->idx field in the list/cache.
Then, ovl_check_empty_dir() can return a list of elements with
(p->is_whiteout && p->idx == 0) and ovl_cleanup_whiteouts() can
can be called IFF !list_empty(&list) to clean them.
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
