Jordi Pujol <jordipujolp@xxxxxxxxx> writes:
> A Dimarts 26 Abril 2011 17:50:54, Miklos Szeredi va escriure:
>> It doesn't look like the correct fix, however. Even though
>> ovl_dentry_lower() returns NULL, the lower file should normally exist.
> it solves the problem, but we should investigate why,
>
>>
>> Please look a the ovl_do_lookup() logic: if oe->opaque is set to true,
>> then oe->lowerdentry will be set to NULL, even when the lower dentry
>> does actually exist.
>>
>> So your patch might cover up the problem that you've been observing
>> (because it won't create the whiteouts in some cases), but it will
>> introduce another bug.
>>
> Agree, the working line in this patch is checking for ovl_dentry_lower in
> ovl_do_remove, the other lines are part of the test; should not be used.
>
> another patch is attached,
I think I found the real cause of the problem, see attached patch. Can
you please test?
Thanks,
Miklos
commit 999204befc34af97bea495dad0ad1d8dfc5a0fe1
Author: Miklos Szeredi <mszeredi@xxxxxxx>
Date: Wed Apr 27 13:23:27 2011 +0200
ovl: fix remove after rename
If the destination of the rename didn't exist on either the upper or
the lower tree then the renamed object was erronously set to opaque.
This caused the object to be replaced by a whiteout on unlink/rmdir.
This in turn could result in the whiteout "showing through", since
whiteouts are not handled in directories existing on the upper layer
only, causing failure to remove an empty directory, for example.
Reported-by: Jordi Pujol <jordipujolp@xxxxxxxxx>
Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxx>
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 81f5fb9..c4ea1d3 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -461,6 +461,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
{
int err;
enum ovl_path_type old_type;
+ enum ovl_path_type new_type;
struct dentry *old_upperdir;
struct dentry *new_upperdir;
struct dentry *olddentry;
@@ -476,8 +477,6 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
return -EXDEV;
if (new->d_inode) {
- enum ovl_path_type new_type;
-
new_type = ovl_path_type(new);
if (new_type == OVL_PATH_LOWER && old_type == OVL_PATH_LOWER) {
@@ -497,6 +496,8 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
if (err)
return err;
}
+ } else {
+ new_type = OVL_PATH_UPPER;
}
err = ovl_copy_up(old);
@@ -534,8 +535,7 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
goto out_dput;
old_opaque = ovl_dentry_is_opaque(old);
- new_opaque = ovl_dentry_is_opaque(new) ||
- ovl_path_type(new) != OVL_PATH_UPPER;
+ new_opaque = ovl_dentry_is_opaque(new) || new_type != OVL_PATH_UPPER;
if (is_dir && !old_opaque && new_opaque) {
err = ovl_set_opaque(olddentry);
