Bug#909378: sbuild: sbuild-createchroot must not consider “lost+found” for non empty directory
Johannes Schauer writes:
> Hi Daniel,
Hello.
[...]
> as I thought more about this problem I now found a better solution that makes
> both of us happy. Consider this diff:
>
> -opendir(my $dh, $target) or die "Can't opendir($target): $!\n";
> -# Attempt reading the directory thrice. If the third time succeeds, then
> it
> -# has more entries than just "." and ".." and must thus not be empty.
> -readdir $dh;
> -readdir $dh;
> -die "$target is not empty" if (readdir $dh);
> +# check if the directory is empty or contains nothing more than an
> +# empty lost+found directory. The latter exists on freshly created
> +# ext3 and ext4 partitions.
> +# rationale for requiring an empty directory:
> https://bugs.debian.org/833525
> +opendir(my $dh, $target) or die "Can't opendir($target): $!";
> +while (my $entry = readdir $dh) {
> + # skip the "." and ".." entries
> + next if $entry eq ".";
> + next if $entry eq "..";
> + # if the entry is a directory named "lost+found" then skip it
> + # if it's empty
> + if ($entry eq "lost+found" and -d "$target/$entry") {
> + opendir(my $dh2, "$target/$entry");
> + # Attempt reading the directory thrice. If the third time
> + # succeeds, then it has more entries than just "." and ".."
> + # and must thus not be empty.
> + readdir $dh2;
> + readdir $dh2;
> + # rationale for requiring an empty directory:
> + # https://bugs.debian.org/833525
> + if (readdir $dh2) {
> + die "$target contains a non-empty lost+found directory";
> + }
> + closedir($dh2);
> + } else {
> + die "$target is not empty";
> + }
> +}
> +closedir($dh);
>
> The new version now check if either the target directory is empty, or it has
> only an empty lost+found directory in it. It will abort in any other case.
> This
> means that:
>
> - we still get all the safety and make sure nothing of value will
> accidentally
>get deleted
>
> - you don't need to manually rmdir a fresh lost+found directory on a new
>partition
>
> What do you think?
It's perfect for me.
I already took care of the rmdir in my LVM based provisioning script but
I'm sure it will be less confusing for new users.
Regards.
--
Daniel Dehennin
Récupérer ma clef GPG: gpg --recv-keys 0xCC1E9E5B7A6FE2DF
Fingerprint: 3E69 014E 5C23 50E8 9ED6 2AAD CC1E 9E5B 7A6F E2DF
Bug#909378: Re: Bug#909378: sbuild: sbuild-createchroot must not consider “lost+found” for non empty directory
Hi Daniel,
On Sun, 23 Sep 2018 13:21:38 +0200 Johannes Schauer wrote:
> Quoting Daniel Dehennin (2018-09-22 19:52:55)
> > I'm trying to setup an LVM schroot but I got the following error:
> >
> > #+begin_src
> > /srv/chroot/sid-amd64-sbuild is not empty at /usr/bin/sbuild-createchroot
> > line 279, line 13.
> > #+end_src
> >
> > The problem is that sbuild-createchroot check for 3 directory entries
> > without
> > taking âlost+foundâ as an exception.
>
> the reason why sbuild requires an empty target directory, are debootstrap bugs
> like these that make you loose everything in case a wrong code path is taken:
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833525
>
> Now imagine that you actually had some previously recovered files in
> lost+found. If you are unlucky, those would then be deleted by a faulty
> debootstrap.
>
> Why don't you just rmdir the directory? Then you also made sure that there is
> indeed nothing inside that you still need. fsck will re-create it in case it
> needs it.
as I thought more about this problem I now found a better solution that makes
both of us happy. Consider this diff:
-opendir(my $dh, $target) or die "Can't opendir($target): $!\n";
-# Attempt reading the directory thrice. If the third time succeeds, then it
-# has more entries than just "." and ".." and must thus not be empty.
-readdir $dh;
-readdir $dh;
-die "$target is not empty" if (readdir $dh);
+# check if the directory is empty or contains nothing more than an
+# empty lost+found directory. The latter exists on freshly created
+# ext3 and ext4 partitions.
+# rationale for requiring an empty directory:
https://bugs.debian.org/833525
+opendir(my $dh, $target) or die "Can't opendir($target): $!";
+while (my $entry = readdir $dh) {
+ # skip the "." and ".." entries
+ next if $entry eq ".";
+ next if $entry eq "..";
+ # if the entry is a directory named "lost+found" then skip it
+ # if it's empty
+ if ($entry eq "lost+found" and -d "$target/$entry") {
+ opendir(my $dh2, "$target/$entry");
+ # Attempt reading the directory thrice. If the third time
+ # succeeds, then it has more entries than just "." and ".."
+ # and must thus not be empty.
+ readdir $dh2;
+ readdir $dh2;
+ # rationale for requiring an empty directory:
+ # https://bugs.debian.org/833525
+ if (readdir $dh2) {
+ die "$target contains a non-empty lost+found directory";
+ }
+ closedir($dh2);
+ } else {
+ die "$target is not empty";
+ }
+}
+closedir($dh);
The new version now check if either the target directory is empty, or it has
only an empty lost+found directory in it. It will abort in any other case. This
means that:
- we still get all the safety and make sure nothing of value will accidentally
get deleted
- you don't need to manually rmdir a fresh lost+found directory on a new
partition
What do you think?
Thanks!
cheers, josch
signature.asc
Description: signature
Bug#909378: sbuild: sbuild-createchroot must not consider “lost+found” for non empty directory
Johannes Schauer writes: > Hi, > > Quoting Daniel Dehennin (2018-09-22 19:52:55) >> I'm trying to setup an LVM schroot but I got the following error: >> >> #+begin_src >> /srv/chroot/sid-amd64-sbuild is not empty at /usr/bin/sbuild-createchroot >> line 279, line 13. >> #+end_src >> >> The problem is that sbuild-createchroot check for 3 directory entries without >> taking “lost+found” as an exception. > > the reason why sbuild requires an empty target directory, are debootstrap bugs > like these that make you loose everything in case a wrong code path is taken: > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833525 > > Now imagine that you actually had some previously recovered files in > lost+found. If you are unlucky, those would then be deleted by a faulty > debootstrap. > > Why don't you just rmdir the directory? Then you also made sure that there is > indeed nothing inside that you still need. fsck will re-create it in case it > needs it. Thanks for the explanation, I already add a rmdir of “lost+found” as a workaround. You can considere this report closed. Have a good day. -- Daniel Dehennin Récupérer ma clef GPG: gpg --recv-keys 0xCC1E9E5B7A6FE2DF Fingerprint: 3E69 014E 5C23 50E8 9ED6 2AAD CC1E 9E5B 7A6F E2DF signature.asc Description: PGP signature
Bug#909378: sbuild: sbuild-createchroot must not consider “lost+found” for non empty directory
Hi, Quoting Daniel Dehennin (2018-09-22 19:52:55) > I'm trying to setup an LVM schroot but I got the following error: > > #+begin_src > /srv/chroot/sid-amd64-sbuild is not empty at /usr/bin/sbuild-createchroot > line 279, line 13. > #+end_src > > The problem is that sbuild-createchroot check for 3 directory entries without > taking “lost+found” as an exception. the reason why sbuild requires an empty target directory, are debootstrap bugs like these that make you loose everything in case a wrong code path is taken: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833525 Now imagine that you actually had some previously recovered files in lost+found. If you are unlucky, those would then be deleted by a faulty debootstrap. Why don't you just rmdir the directory? Then you also made sure that there is indeed nothing inside that you still need. fsck will re-create it in case it needs it. Thanks! cheers, josch signature.asc Description: signature
