overlayfs v8 problems with whiteout links

overlayfs v8 problems with whiteout links

[Jordi Pujol]

Hello,

Now working on a persistent Live that uses overlayfs v8 and
tmpfs xattrs, all works near well but sometimes whiteout fails,

I think maybe xattrs create and delete is not synchronized with the operations 
create and delete for whiteout links.

these are commands from a Debian package build, 
showing a command "rm -rf" that cannot remove the files because a whiteout link 
was not removed successfully.

dpkg-deb: building package `nvidia-kernel-2.6.38-4.jpp.1-lnet-amd64' in 
`/usr/src/nvidia-kernel-2.6.38-4.jpp.1-lnet-
amd64_270.41.06-1.lnet1+2.6.38-16_amd64.deb'.
dh_prep
rm: cannot remove `debian/nvidia-kernel-2.6.38-4.jpp.1-lnet-amd64/DEBIAN': 
Directory not empty
dh_prep: rm -rf debian/nvidia-kernel-2.6.38-4.jpp.1-lnet-amd64/ returned exit 
code 1

the directory listing shows the whiteout, but it must be hidden,

# ls -R1l nvidia-kernel-2.6.38-4.jpp.1-lnet-amd64
nvidia-kernel-2.6.38-4.jpp.1-lnet-amd64:
total 4
drwxr-xr-x 2 root root 4096 2011-04-22 10:19 DEBIAN

nvidia-kernel-2.6.38-4.jpp.1-lnet-amd64/DEBIAN:
total 0
lrwxrwxrwx 1 root root 18 2011-04-22 10:19 control -> (overlay-whiteout)

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

Re: overlayfs v8 problems with whiteout links

[Jordi Pujol]

A Divendres 22 Abril 2011 10:43:43, Jordi Pujol va escriure:
> Now working on a persistent Live that uses overlayfs v8 and
> tmpfs xattrs, all works near well but sometimes whiteout fails,
> 
Hello,

I am trying to reproduce this, repeat the same process and analyze the 
situation of evarlayfs at every step,

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

Re: overlayfs v8 problems with whiteout links

[Jordi Pujol]

A Dissabte 23 Abril 2011 14:43:39, Jordi Pujol va escriure:
> I am trying to reproduce this, repeat the same process and analyze the
> situation of evarlayfs at every step,

this problem is reproducible, occurs a lot of times,
here are some logs for various processes that gave this error:

to the log  I have added directory listings for upperdir (/lnet/cow) and 
rootdir ( / )
, listed previously to the error and after it also,

http://livenet.selfip.com/ftp/debian/overlayfs/nvidia-module-build.log
http://livenet.selfip.com/ftp/debian/overlayfs/nvidia-packages-build.log
http://livenet.selfip.com/ftp/debian/overlayfs/nvidia-packages-build-1.log

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

Re: overlayfs v8 problems with whiteout links

[Jordi Pujol]

[-- Attachment #1: Type: Text/Plain, Size: 421 bytes --]

Hello,

I have solved this checking if the lower dentry exists when calling 
ovl_whiteout in ovl_do_remove and ovl_rename,
also added in ovl_whiteout a check for existance of lower dentry and a debug 
message, as it was already planned as a TODO task in the source code.

by now it works, even the debug message not appears,

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

[-- Attachment #2: dir.c-lower-dentry.patch --]
[-- Type: text/x-patch, Size: 1217 bytes --]

--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -33,7 +33,14 @@ static int ovl_whiteout(struct dentry *u
 	const struct cred *old_cred;
 	struct cred *override_cred;
 
-	/* FIXME: recheck lower dentry to see if whiteout is really needed */
+	err = 0;
+	/* recheck lower dentry to see if whiteout is really needed */
+	/* dont consider this an error */
+	if (!ovl_dentry_lower(dentry)) {
+		printk(KERN_DEBUG "overlayfs: DEBUG - no lower dentry to whiteout '%s'\n",
+		       dentry->d_name.name);
+		return 0;
+	}
 
 	err = -ENOMEM;
 	override_cred = prepare_creds();
@@ -376,7 +383,8 @@ static int ovl_do_remove(struct dentry *
 		ovl_dentry_version_inc(dentry->d_parent);
 	}
 
-	if (type != OVL_PATH_UPPER || ovl_dentry_is_opaque(dentry))
+	if ((type != OVL_PATH_UPPER || ovl_dentry_is_opaque(dentry))
+	&& ovl_dentry_lower(dentry))
 		err = ovl_whiteout(upperdir, dentry);
 
 	/*
@@ -554,7 +562,8 @@ static int ovl_rename(struct inode *oldd
 		goto out_dput;
 	}
 
-	if (old_type != OVL_PATH_UPPER || old_opaque)
+	if ((old_type != OVL_PATH_UPPER || old_opaque)
+	&& ovl_dentry_lower(old))
 		err = ovl_whiteout(old_upperdir, old);
 	if (is_dir && old_opaque && !new_opaque)
 		ovl_remove_opaque(olddentry);

Re: overlayfs v8 problems with whiteout links

[Miklos Szeredi]

Jordi Pujol <jordipujolp@gmail.com> writes:

> A Dissabte 23 Abril 2011 14:43:39, Jordi Pujol va escriure:
>> I am trying to reproduce this, repeat the same process and analyze the
>> situation of evarlayfs at every step,
>
> this problem is reproducible, occurs a lot of times,
> here are some logs for various processes that gave this error:
>
> to the log  I have added directory listings for upperdir (/lnet/cow) and 
> rootdir ( / )
> , listed previously to the error and after it also,
>
> http://livenet.selfip.com/ftp/debian/overlayfs/nvidia-module-build.log
> http://livenet.selfip.com/ftp/debian/overlayfs/nvidia-packages-build.log
> http://livenet.selfip.com/ftp/debian/overlayfs/nvidia-packages-build-1.log

Can you please list the relevant lower directory as well?

Thanks,
Miklos

Re: overlayfs v8 problems with whiteout links

[Miklos Szeredi]

Jordi Pujol <jordipujolp@gmail.com> writes:

> Hello,
>
> I have solved this checking if the lower dentry exists when calling 
> ovl_whiteout in ovl_do_remove and ovl_rename,
> also added in ovl_whiteout a check for existance of lower dentry and a debug 
> message, as it was already planned as a TODO task in the source code.
>
> by now it works, even the debug message not appears,
>
> Thanks,
>
> Jordi Pujol
>
> Live never ending Tale
> GNU/Linux Live forever!
> http://livenet.selfip.com
>
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -33,7 +33,14 @@ static int ovl_whiteout(struct dentry *u
>  	const struct cred *old_cred;
>  	struct cred *override_cred;
>  
> -	/* FIXME: recheck lower dentry to see if whiteout is really needed */
> +	err = 0;
> +	/* recheck lower dentry to see if whiteout is really needed */
> +	/* dont consider this an error */
> +	if (!ovl_dentry_lower(dentry)) {
> +		printk(KERN_DEBUG "overlayfs: DEBUG - no lower dentry to whiteout '%s'\n",
> +		       dentry->d_name.name);
> +		return 0;
> +	}

Okay, thanks for the patch. 

It doesn't look like the correct fix, however.  Even though
ovl_dentry_lower() returns NULL, the lower file should normally exist.

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.

Thanks,
Miklos

Re: overlayfs v8 problems with whiteout links

[Jordi Pujol]

[-- Attachment #1: Type: Text/Plain, Size: 864 bytes --]

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,

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

[-- Attachment #2: 09-bug-remove-lower-dentry.patch --]
[-- Type: text/x-patch, Size: 366 bytes --]

--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -376,7 +383,8 @@ static int ovl_do_remove(struct dentry *
 		ovl_dentry_version_inc(dentry->d_parent);
 	}
 
-	if (type != OVL_PATH_UPPER || ovl_dentry_is_opaque(dentry))
+	if ((type != OVL_PATH_UPPER || ovl_dentry_is_opaque(dentry))
+	&& ovl_dentry_lower(dentry))
 		err = ovl_whiteout(upperdir, dentry);
 
 	/*

Re: overlayfs v8 problems with whiteout links

[Jordi Pujol]

A Dimarts 26 Abril 2011 17:34:48, Miklos Szeredi va escriure:
> Can you please list the relevant lower directory as well?
the lower directory arrives to /usr/src
lower /usr/src is empty

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

Re: overlayfs v8 problems with whiteout links

[Miklos Szeredi]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

Jordi Pujol <jordipujolp@gmail.com> 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ovl-fix-remove-after-rename.patch --]
[-- Type: text/x-patch, Size: 1941 bytes --]

commit 999204befc34af97bea495dad0ad1d8dfc5a0fe1
Author: Miklos Szeredi <mszeredi@suse.cz>
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@gmail.com>
    Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

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);

Re: overlayfs v8 problems with whiteout links

[Jordi Pujol]

A Dimecres 27 Abril 2011 13:41:19, Miklos Szeredi va escriure:
> I think I found the real cause of the problem, see attached patch.  Can
> you please test?

OK, I will, now it is starting to compile,

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

Re: overlayfs v8 problems with whiteout links

[Jordi Pujol]

A Dimecres 27 Abril 2011 15:30:35, Jordi Pujol va escriure:
> A Dimecres 27 Abril 2011 13:41:19, Miklos Szeredi va escriure:
> > I think I found the real cause of the problem, see attached patch.  Can
> > you please test?
> 
> OK, I will, now it is starting to compile,
> 

Success, it works,

I have tried it building huge packages in /usr/src directory,
Actual custom kernel Debian source is:

http://livenet.selfip.com/ftp/debian/linux-2.6/linux-2.6.38-4.jpp.6-
lnet_2.6.38-21.tar.bz2

Thanks,

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

Re: overlayfs v8 problems with whiteout links

[Miklos Szeredi]

Jordi Pujol <jordipujolp@gmail.com> writes:

> A Dimecres 27 Abril 2011 15:30:35, Jordi Pujol va escriure:
>> A Dimecres 27 Abril 2011 13:41:19, Miklos Szeredi va escriure:
>> > I think I found the real cause of the problem, see attached patch.  Can
>> > you please test?
>> 
>> OK, I will, now it is starting to compile,
>> 
>
> Success, it works,
>
> I have tried it building huge packages in /usr/src directory,
> Actual custom kernel Debian source is:
>
> http://livenet.selfip.com/ftp/debian/linux-2.6/linux-2.6.38-4.jpp.6-
> lnet_2.6.38-21.tar.bz2

Thanks for testing!

Miklos