On Wed, Oct 26, 2016 at 11:34 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> Before introducing redirect_dir feature, the condition
> !ovl_lower_positive(dentry) for a directory, implied that
> it is a pure upper directory, which may be removed if empty.
>
> Now that directory can be redirect, it is possible that
> upper does not cover any lower (i.e. !ovl_lower_positive(dentry)),
> but the directory is a merge (with redirected path) and maybe
> non empty.
>
> Check for this case in ovl_remove_upper().
>
> This change fixes the following test case from rename-pop-dir.py
> of unionmount-testsuite:
>
> """Remove dir and rename old name"""
> d = ctx.non_empty_dir()
> d2 = ctx.no_dir()
>
> ctx.rmdir(d, err=ENOTEMPTY)
> ctx.rename(d, d2)
> ctx.rmdir(d, err=ENOENT)
> ctx.rmdir(d2, err=ENOTEMPTY)
>
> ./run --ov rename-pop-dir
> /mnt/a/no_dir103: Expected error (Directory not empty) was not produced
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/overlayfs/dir.c | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 065e021..d750ed9 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -672,9 +672,18 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
> {
> struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent);
> struct inode *dir = upperdir->d_inode;
> - struct dentry *upper;
> + struct dentry *upper = NULL;
> + struct dentry *opaquedir = NULL;
> int err;
>
> + /* Redirect dir can be !ovl_lower_positive && OVL_TYPE_MERGE */
> + if (is_dir && ovl_dentry_is_redirect(dentry)) {
> + opaquedir = ovl_check_empty_and_clear(dentry);
> + err = PTR_ERR(opaquedir);
> + if (IS_ERR(opaquedir))
> + return err;
> + }
> +
> inode_lock_nested(dir, I_MUTEX_PARENT);
> upper = lookup_one_len(dentry->d_name.name, upperdir,
> dentry->d_name.len);
> @@ -683,14 +692,15 @@ static int ovl_remove_upper(struct dentry *dentry, bool is_dir)
> goto out_unlock;
>
> err = -ESTALE;
> - if (upper == ovl_dentry_upper(dentry)) {
> - if (is_dir)
> - err = vfs_rmdir(dir, upper);
> - else
> - err = vfs_unlink(dir, upper, NULL);
> - ovl_dentry_version_inc(dentry->d_parent);
> - }
> - dput(upper);
> + if ((opaquedir && upper != opaquedir) ||
> + upper != ovl_dentry_upper(dentry))
This will always error out for the opaquedir case. So it needs to be:
if ((opaquedir && upper != opaquedir) ||
(!opaquedir && upper != ovl_dentry_upper(dentry)))
Pushed to "redirect" branch with this fix.
Thanks,
Miklos
--
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
