On Fri, Jun 16, 2017 at 6:55 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote:
> Hi All:
>
> I found another problem about impure upper dir which have left
> whiteout files. If an "impure && not merged" upper dir have
> left whiteout files(last mount created), ovl cannot clear this
> dir when we removing it, which may lead to rmdir fail or temp
> file left in workdir.
>
> 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
Can you please post an xfstest with that reproducer.
A good starting point is test overlay/012
>
> 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 impure flag on parent dir in ovl_remove_and_whiteout()
> to indicate the dir is impure, and then check and clean it when
> removing this dir.
>
> Signed-off-by: zhangyi (F) <yi.zhang@xxxxxxxxxx>
Apart of the one comment below, you may add
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/overlayfs/dir.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index a63a716..9535e2d 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -336,7 +336,8 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry)
> * 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_dentry_is_impure(dentry)))
> ret = ovl_clear_empty(dentry, &list);
>
> out_free:
> @@ -633,6 +634,11 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
> goto out;
> }
>
> + /* Mark parent "impure" because it may now contain non-pure upper */
> + err = ovl_set_impure(dentry->d_parent, upperdir);
> + if (err)
> + goto out_dput;
> +
This should be done before ovl_whiteout().
You rather have false positive impure than the other way around.
Also please fix comment to "... may now contain whiteouts".
> err = ovl_lock_rename_workdir(workdir, upperdir);
> if (err)
> goto out_dput;
> @@ -690,8 +696,9 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
> struct dentry *opaquedir = NULL;
> int err;
>
> - /* Redirect dir can be !ovl_lower_positive && OVL_TYPE_MERGE */
> - if (is_dir && ovl_dentry_get_redirect(dentry)) {
> + /* Redirect/Impure dir can be !ovl_lower_positive && not clean */
> + if (is_dir && (ovl_dentry_get_redirect(dentry) ||
> + ovl_dentry_is_impure(dentry))) {
> opaquedir = ovl_check_empty_and_clear(dentry);
> err = PTR_ERR(opaquedir);
> if (IS_ERR(opaquedir))
> --
> 1.8.3.1
>
--
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
