[PATCH] bcachefs-tools: fix broken libblkid superblock wipe

[PATCH] bcachefs-tools: fix broken libblkid superblock wipe

[Brian Foster]

When playing around with comparing some fstests results on different
filesystems, I noticed that a 'bcachefs format' of a previously
btrfs-formatted device still continued to mount as btrfs. The reason
for this is that the blkid wipe invoked via open_for_format() is not
working correctly. blkid_do_wipe() depends on the "SBMAGIC[_OFFSET]"
values to do any work, and the associated superblock magic flag is
not enabled on the probe.

Set the probe flags to explicitly enable the values the bcachefs
code depends on in the probe. This includes the type, label and
superblock magic information.

There are also a couple quirks in the libblkid code that might be
worth noting. One is that the superblock enablement and flag setting
functions appear hardcoded to return zero, so we just combine the
error checks. Second, while blkid_do_wipe() can return an error, it
actually doesn't in the scenario being addressed here because it
doesn't seem to distinguish between the values being absent because
nothing was found by the probe or because the values weren't enabled
in the first place. Regardless, add an error check here in the event
the wipe does explicitly fail for some unexpected reason.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 tools-util.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools-util.c b/tools-util.c
index 923a666..7d4a3c1 100644
--- a/tools-util.c
+++ b/tools-util.c
@@ -232,7 +232,10 @@ int open_for_format(struct dev_opts *dev, bool force)
 		die("blkid error 1");
 	if (blkid_probe_set_device(pr, dev->bdev->bd_buffered_fd, 0, 0))
 		die("blkid error 2");
-	if (blkid_probe_enable_partitions(pr, true))
+	if (blkid_probe_enable_partitions(pr, true) ||
+	    blkid_probe_enable_superblocks(pr, true) ||
+	    blkid_probe_set_superblocks_flags(pr,
+			BLKID_SUBLKS_LABEL|BLKID_SUBLKS_TYPE|BLKID_SUBLKS_MAGIC))
 		die("blkid error 3");
 	if (blkid_do_fullprobe(pr) < 0)
 		die("blkid error 4");
@@ -250,8 +253,10 @@ int open_for_format(struct dev_opts *dev, bool force)
 		fputs("Proceed anyway?", stdout);
 		if (!ask_yn())
 			exit(EXIT_FAILURE);
-		while (blkid_do_probe(pr) == 0)
-			blkid_do_wipe(pr, 0);
+		while (blkid_do_probe(pr) == 0) {
+			if (blkid_do_wipe(pr, 0))
+				die("Failed to wipe preexisting metadata.");
+		}
 	}
 
 	blkid_free_probe(pr);
-- 
2.41.0

Re: [PATCH] bcachefs-tools: fix broken libblkid superblock wipe

[Kent Overstreet]

On Fri, Nov 17, 2023 at 08:12:58AM -0500, Brian Foster wrote:
> When playing around with comparing some fstests results on different
> filesystems, I noticed that a 'bcachefs format' of a previously
> btrfs-formatted device still continued to mount as btrfs. The reason
> for this is that the blkid wipe invoked via open_for_format() is not
> working correctly. blkid_do_wipe() depends on the "SBMAGIC[_OFFSET]"
> values to do any work, and the associated superblock magic flag is
> not enabled on the probe.
> 
> Set the probe flags to explicitly enable the values the bcachefs
> code depends on in the probe. This includes the type, label and
> superblock magic information.
> 
> There are also a couple quirks in the libblkid code that might be
> worth noting. One is that the superblock enablement and flag setting
> functions appear hardcoded to return zero, so we just combine the
> error checks. Second, while blkid_do_wipe() can return an error, it
> actually doesn't in the scenario being addressed here because it
> doesn't seem to distinguish between the values being absent because
> nothing was found by the probe or because the values weren't enabled
> in the first place. Regardless, add an error check here in the event
> the wipe does explicitly fail for some unexpected reason.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Nice! Other people have reported this, glad it's fixed :)