[PATCH] mkfs: fix blkid probe API violations causing weird output

[PATCH] mkfs: fix blkid probe API violations causing weird output

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

The blkid_do_fullprobe function in libblkid 2.38.1 will try to read the
last 512 bytes off the end of a block device.  If the block device has a
2k LBA size, that read will fail.  blkid_do_fullprobe passes the -EIO
back to the caller (mkfs) even though the API documentation says it
only returns 1, 0, or -1.

Change the "cannot detect existing fs" logic to look for any negative
number.  Otherwise, you get unhelpful output like this:

$ mkfs.xfs -l size=32m -b size=4096 /dev/loop3
mkfs.xfs: Use the -f option to force overwrite.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 libxfs/topology.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libxfs/topology.c b/libxfs/topology.c
index 8c6affb4c4e436..96ee74b61b30f5 100644
--- a/libxfs/topology.c
+++ b/libxfs/topology.c
@@ -205,7 +205,8 @@ check_overwrite(
 out:
 	if (pr)
 		blkid_free_probe(pr);
-	if (ret == -1)
+	/* libblkid 2.38.1 lies and can return -EIO */
+	if (ret < 0)
 		fprintf(stderr,
 			_("%s: probe of %s failed, cannot detect "
 			  "existing filesystem.\n"), progname, device);

Re: [PATCH] mkfs: fix blkid probe API violations causing weird output

[Christoph Hellwig]

On Tue, Apr 15, 2025 at 06:28:37PM -0700, Darrick J. Wong wrote:
> +	/* libblkid 2.38.1 lies and can return -EIO */

Can you expand this comment be less terse using the wording in the
commit message?  Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Did you also report the bug to util-linux so that it gets fixed there?

Re: [PATCH] mkfs: fix blkid probe API violations causing weird output

[Darrick J. Wong]

On Tue, Apr 15, 2025 at 09:35:46PM -0700, Christoph Hellwig wrote:
> On Tue, Apr 15, 2025 at 06:28:37PM -0700, Darrick J. Wong wrote:
> > +	/* libblkid 2.38.1 lies and can return -EIO */
> 
> Can you expand this comment be less terse using the wording in the
> commit message?  Otherwise looks good:

Ok.

> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Did you also report the bug to util-linux so that it gets fixed there?

I'm not even sure how to categorize it -- the API docs all say things
like this:

 * Returns: 0 on success, 1 if nothing is detected or -1 on case of error.
 */
int blkid_do_fullprobe(blkid_probe pr)

But then you look at the probe functions that it calls:

static int probe_xfs(blkid_probe pr, const struct blkid_idmag *mag)
{
	struct xfs_super_block *xs;

	xs = blkid_probe_get_sb(pr, mag, struct xfs_super_block);
	if (!xs)
		return errno ? -errno : 1;

or:

static int probe_apfs(blkid_probe pr, const struct blkid_idmag *mag)
{
	struct apfs_super_block *sb;

	sb = blkid_probe_get_sb(pr, mag, struct apfs_super_block);
	if (!sb)
		return errno ? -errno : BLKID_PROBE_NONE;

So I guess it's just ... super broken?

--D

Re: [PATCH] mkfs: fix blkid probe API violations causing weird output

[Christoph Hellwig]

On Tue, Apr 15, 2025 at 09:50:44PM -0700, Darrick J. Wong wrote:
> static int probe_apfs(blkid_probe pr, const struct blkid_idmag *mag)
> {
> 	struct apfs_super_block *sb;
> 
> 	sb = blkid_probe_get_sb(pr, mag, struct apfs_super_block);
> 	if (!sb)
> 		return errno ? -errno : BLKID_PROBE_NONE;
> 
> So I guess it's just ... super broken?

Heh.  Please just send this report to the util-linux list.