[PATCH 0/3] implement zone-aware probing/wiping for zoned btrfs

[PATCH 0/3] implement zone-aware probing/wiping for zoned btrfs

[Naohiro Aota]

This series implements probing and wiping of the superblock of zoned btrfs.

Zoned btrfs is merged with this series:
https://lore.kernel.org/linux-btrfs/20210222160049.GR1993@twin.jikos.cz/T/

And, superblock locations are finalized with this patch:
https://lore.kernel.org/linux-btrfs/BL0PR04MB651442E6ACBF48342BD00FEBE7719@BL0PR04MB6514.namprd04.prod.outlook.com/T/

A zoned block device consists of a number of zones. Zones are either
conventional and accepting random writes or sequential and requiring that
writes be issued in LBA order from each zone write pointer position.

Superblock (and its copies) is the only data structure in btrfs with a
fixed location on a device. Since we cannot overwrite in a sequential write
required zone, we cannot place superblock in the zone.

Thus, zoned btrfs use superblock log writing to update superblock on
sequential write required zones. It uses two zones as a circular buffer to
write updated superblocks. Once the first zone is filled up, start writing
into the second buffer. When both zones are filled up and before start
writing to the first zone again, it reset the first zone.

This series first implements zone based detection of the magic location.
Then, it adds magics for zoned btrfs and implements a probing function to
detect the latest superblock. Finally, this series also implements
zone-aware wiping by zone resetting.


Naohiro Aota (3):
  blkid: implement zone-aware probing
  blkid: add magic and probing for zoned btrfs
  blkid: support zone reset for wipefs

 configure.ac                     |   1 +
 libblkid/src/blkidP.h            |   5 +
 libblkid/src/probe.c             |  91 ++++++++++++++--
 libblkid/src/superblocks/btrfs.c | 178 ++++++++++++++++++++++++++++++-
 4 files changed, 266 insertions(+), 9 deletions(-)

-- 
2.31.1

[PATCH 1/3] blkid: implement zone-aware probing

[Naohiro Aota]

This patch makes libblkid zone-aware. It can probe the magic located at
some offset from the beginning of some specific zone of a device.

Ths patch introduces some new fields to struct blkid_idmag. They indicate
the magic location is placed related to a zone, and the offset in the zone.

Also, this commit introduces `zone_size` to struct blkid_struct_probe. It
stores the size of zones of a device.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 configure.ac          |  1 +
 libblkid/src/blkidP.h |  5 +++++
 libblkid/src/probe.c  | 26 ++++++++++++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index bebb4085425a..52b164e834db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -302,6 +302,7 @@ AC_CHECK_HEADERS([ \
 	lastlog.h \
 	libutil.h \
 	linux/btrfs.h \
+	linux/blkzoned.h \
 	linux/capability.h \
 	linux/cdrom.h \
 	linux/falloc.h \
diff --git a/libblkid/src/blkidP.h b/libblkid/src/blkidP.h
index a3fe6748a969..e3a160aa97c0 100644
--- a/libblkid/src/blkidP.h
+++ b/libblkid/src/blkidP.h
@@ -150,6 +150,10 @@ struct blkid_idmag
 	const char	*hoff;		/* hint which contains byte offset to kboff */
 	long		kboff;		/* kilobyte offset of superblock */
 	unsigned int	sboff;		/* byte offset within superblock */
+
+	int		is_zoned;	/* indicate magic location is calcluated based on zone position  */
+	long		zonenum;	/* zone number which has superblock */
+	long		kboff_inzone;	/* kilobyte offset of superblock in a zone */
 };
 
 /*
@@ -206,6 +210,7 @@ struct blkid_struct_probe
 	dev_t			disk_devno;	/* devno of the whole-disk or 0 */
 	unsigned int		blkssz;		/* sector size (BLKSSZGET ioctl) */
 	mode_t			mode;		/* struct stat.sb_mode */
+	uint64_t		zone_size;	/* zone size (BLKGETZONESZ ioctl) */
 
 	int			flags;		/* private library flags */
 	int			prob_flags;	/* always zeroized by blkid_do_*() */
diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
index a47a8720d4ac..102766e57aa0 100644
--- a/libblkid/src/probe.c
+++ b/libblkid/src/probe.c
@@ -94,6 +94,9 @@
 #ifdef HAVE_LINUX_CDROM_H
 #include <linux/cdrom.h>
 #endif
+#ifdef HAVE_LINUX_BLKZONED_H
+#include <linux/blkzoned.h>
+#endif
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
@@ -871,6 +874,7 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
 #ifdef CDROM_GET_CAPABILITY
 	long last_written = 0;
 #endif
+	uint32_t zone_size_sector;
 
 	blkid_reset_probe(pr);
 	blkid_probe_reset_buffers(pr);
@@ -897,6 +901,7 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
 	pr->wipe_off = 0;
 	pr->wipe_size = 0;
 	pr->wipe_chain = NULL;
+	pr->zone_size = 0;
 
 	if (fd < 0)
 		return 1;
@@ -996,6 +1001,11 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
 #endif
 	free(dm_uuid);
 
+# ifdef HAVE_LINUX_BLKZONED_H
+	if (S_ISBLK(sb.st_mode) && !ioctl(pr->fd, BLKGETZONESZ, &zone_size_sector))
+		pr->zone_size = zone_size_sector << 9;
+# endif
+
 	DBG(LOWPROBE, ul_debug("ready for low-probing, offset=%"PRIu64", size=%"PRIu64"",
 				pr->off, pr->size));
 	DBG(LOWPROBE, ul_debug("whole-disk: %s, regfile: %s",
@@ -1064,12 +1074,24 @@ int blkid_probe_get_idmag(blkid_probe pr, const struct blkid_idinfo *id,
 	/* try to detect by magic string */
 	while(mag && mag->magic) {
 		unsigned char *buf;
+		uint64_t kboff;
 		uint64_t hint_offset;
 
 		if (!mag->hoff || blkid_probe_get_hint(pr, mag->hoff, &hint_offset) < 0)
 			hint_offset = 0;
 
-		off = hint_offset + ((mag->kboff + (mag->sboff >> 10)) << 10);
+		/* If the magic is for zoned device, skip non-zoned device */
+		if (mag->is_zoned && !pr->zone_size) {
+			mag++;
+			continue;
+		}
+
+		if (!mag->is_zoned)
+			kboff = mag->kboff;
+		else
+			kboff = ((mag->zonenum * pr->zone_size) >> 10) + mag->kboff_inzone;
+
+		off = hint_offset + ((kboff + (mag->sboff >> 10)) << 10);
 		buf = blkid_probe_get_buffer(pr, off, 1024);
 
 		if (!buf && errno)
@@ -1079,7 +1101,7 @@ int blkid_probe_get_idmag(blkid_probe pr, const struct blkid_idinfo *id,
 				buf + (mag->sboff & 0x3ff), mag->len)) {
 
 			DBG(LOWPROBE, ul_debug("\tmagic sboff=%u, kboff=%ld",
-				mag->sboff, mag->kboff));
+				mag->sboff, kboff));
 			if (offset)
 				*offset = off + (mag->sboff & 0x3ff);
 			if (res)
-- 
2.31.1

[PATCH 2/3] blkid: add magic and probing for zoned btrfs

[Naohiro Aota]

This commit adds zone-aware magics and probing functions for zoned btrfs.

Superblock (and its copies) is the only data structure in btrfs with a
fixed location on a device. Since we cannot overwrite in a sequential write
required zone, we cannot place superblock in the zone.

Thus, zoned btrfs use superblock log writing to update superblock on
sequential write required zones. It uses two zones as a circular buffer to
write updated superblocks. Once the first zone is filled up, start writing
into the second buffer. When both zones are filled up and before start
writing to the first zone again, it reset the first zone.

We can determine the position of the latest superblock by reading write
pointer information from a device. One corner case is when both zones are
full. For this situation, we read out the last superblock of each zone and
compare them to determine which zone is older.

The magics can detect a superblock magic ("_BHRfs_M") at the beginning of
zone #0 or zone #1 to see if it is zoned btrfs. When both zones are filled
up, zoned btrfs reset the first zone to write a new superblock. If btrfs
crash at the moment, we do not see a superblock at zone #0. Thus, we need
to check not only zone #0 but also zone #1.

It also supports temporary magic ("!BHRfS_M") in zone #0. The mkfs.btrfs
first writes the temporary superblock to the zone during the mkfs process.
It will survive there until the zones are filled up and reset. So, we also
need to detect this temporary magic.

Finally, this commit extends probe_btrfs() to load the latest superblock
determined by the write pointers.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 libblkid/src/superblocks/btrfs.c | 178 ++++++++++++++++++++++++++++++-
 1 file changed, 177 insertions(+), 1 deletion(-)

diff --git a/libblkid/src/superblocks/btrfs.c b/libblkid/src/superblocks/btrfs.c
index f0fde700d896..bce4055054fb 100644
--- a/libblkid/src/superblocks/btrfs.c
+++ b/libblkid/src/superblocks/btrfs.c
@@ -9,6 +9,12 @@
 #include <unistd.h>
 #include <string.h>
 #include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+
+#ifdef HAVE_LINUX_BLKZONED_H
+#include <linux/blkzoned.h>
+#endif
 
 #include "superblocks.h"
 
@@ -59,11 +65,169 @@ struct btrfs_super_block {
 	uint8_t label[256];
 } __attribute__ ((__packed__));
 
+#define BTRFS_SUPER_INFO_SIZE 4096
+
+/* Number of superblock log zones */
+#define BTRFS_NR_SB_LOG_ZONES 2
+
+/* Introduce some macros and types to unify the code with kernel side */
+#define SECTOR_SHIFT 9
+
+#define ASSERT(x) assert(x)
+
+typedef uint64_t u64;
+typedef uint64_t sector_t;
+typedef uint8_t u8;
+
+static int sb_write_pointer(int fd, struct blk_zone *zones, u64 *wp_ret)
+{
+	bool empty[BTRFS_NR_SB_LOG_ZONES];
+	bool full[BTRFS_NR_SB_LOG_ZONES];
+	sector_t sector;
+
+	ASSERT(zones[0].type != BLK_ZONE_TYPE_CONVENTIONAL &&
+	       zones[1].type != BLK_ZONE_TYPE_CONVENTIONAL);
+
+	empty[0] = zones[0].cond == BLK_ZONE_COND_EMPTY;
+	empty[1] = zones[1].cond == BLK_ZONE_COND_EMPTY;
+	full[0] = zones[0].cond == BLK_ZONE_COND_FULL;
+	full[1] = zones[1].cond == BLK_ZONE_COND_FULL;
+
+	/*
+	 * Possible states of log buffer zones
+	 *
+	 *           Empty[0]  In use[0]  Full[0]
+	 * Empty[1]         *          x        0
+	 * In use[1]        0          x        0
+	 * Full[1]          1          1        C
+	 *
+	 * Log position:
+	 *   *: Special case, no superblock is written
+	 *   0: Use write pointer of zones[0]
+	 *   1: Use write pointer of zones[1]
+	 *   C: Compare super blcoks from zones[0] and zones[1], use the latest
+	 *      one determined by generation
+	 *   x: Invalid state
+	 */
+
+	if (empty[0] && empty[1]) {
+		/* Special case to distinguish no superblock to read */
+		*wp_ret = zones[0].start << SECTOR_SHIFT;
+		return -ENOENT;
+	} else if (full[0] && full[1]) {
+		/* Compare two super blocks */
+		u8 buf[BTRFS_NR_SB_LOG_ZONES][BTRFS_SUPER_INFO_SIZE];
+		struct btrfs_super_block *super[BTRFS_NR_SB_LOG_ZONES];
+		int i;
+		int ret;
+
+		for (i = 0; i < BTRFS_NR_SB_LOG_ZONES; i++) {
+			u64 bytenr;
+
+			bytenr = ((zones[i].start + zones[i].len)
+				   << SECTOR_SHIFT) - BTRFS_SUPER_INFO_SIZE;
+
+			ret = pread64(fd, buf[i], BTRFS_SUPER_INFO_SIZE,
+				      bytenr);
+			if (ret != BTRFS_SUPER_INFO_SIZE)
+				return -EIO;
+			super[i] = (struct btrfs_super_block *)&buf[i];
+		}
+
+		if (super[0]->generation > super[1]->generation)
+			sector = zones[1].start;
+		else
+			sector = zones[0].start;
+	} else if (!full[0] && (empty[1] || full[1])) {
+		sector = zones[0].wp;
+	} else if (full[0]) {
+		sector = zones[1].wp;
+	} else {
+		return -EUCLEAN;
+	}
+	*wp_ret = sector << SECTOR_SHIFT;
+	return 0;
+}
+
+static int sb_log_offset(blkid_probe pr, uint64_t *bytenr_ret)
+{
+	uint32_t zone_num = 0;
+	uint32_t zone_size_sector;
+	struct blk_zone_report *rep;
+	struct blk_zone *zones;
+	size_t rep_size;
+	int ret;
+	uint64_t wp;
+
+	zone_size_sector = pr->zone_size >> SECTOR_SHIFT;
+
+	rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone) * 2;
+	rep = malloc(rep_size);
+	if (!rep)
+		return -errno;
+
+	memset(rep, 0, rep_size);
+	rep->sector = zone_num * zone_size_sector;
+	rep->nr_zones = 2;
+
+	ret = ioctl(pr->fd, BLKREPORTZONE, rep);
+	if (ret) {
+		ret = -errno;
+		goto out;
+	}
+	if (rep->nr_zones != 2) {
+		ret = 1;
+		goto out;
+	}
+
+	zones = (struct blk_zone *)(rep + 1);
+
+	if (zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL) {
+		*bytenr_ret = zones[0].start << SECTOR_SHIFT;
+		ret = 0;
+		goto out;
+	} else if (zones[1].type == BLK_ZONE_TYPE_CONVENTIONAL) {
+		*bytenr_ret = zones[1].start << SECTOR_SHIFT;
+		ret = 0;
+		goto out;
+	}
+
+	ret = sb_write_pointer(pr->fd, zones, &wp);
+	if (ret != -ENOENT && ret) {
+		ret = 1;
+		goto out;
+	}
+	if (ret != -ENOENT) {
+		if (wp == zones[0].start << SECTOR_SHIFT)
+			wp = (zones[1].start + zones[1].len) << SECTOR_SHIFT;
+		wp -= BTRFS_SUPER_INFO_SIZE;
+	}
+	*bytenr_ret = wp;
+
+	ret = 0;
+out:
+	free(rep);
+
+	return ret;
+}
+
 static int probe_btrfs(blkid_probe pr, const struct blkid_idmag *mag)
 {
 	struct btrfs_super_block *bfs;
+	int ret;
 
-	bfs = blkid_probe_get_sb(pr, mag, struct btrfs_super_block);
+	if (pr->zone_size) {
+		uint64_t offset = 0;
+
+		ret = sb_log_offset(pr, &offset);
+		if (ret)
+			return ret;
+		bfs = (struct btrfs_super_block *)
+			blkid_probe_get_buffer(pr, offset,
+					       sizeof(struct btrfs_super_block));
+	} else {
+		bfs = blkid_probe_get_sb(pr, mag, struct btrfs_super_block);
+	}
 	if (!bfs)
 		return errno ? -errno : 1;
 
@@ -88,6 +252,18 @@ const struct blkid_idinfo btrfs_idinfo =
 	.magics		=
 	{
 	  { .magic = "_BHRfS_M", .len = 8, .sboff = 0x40, .kboff = 64 },
+	  /* For zoned btrfs */
+	  { .magic = "_BHRfS_M", .len = 8, .sboff = 0x40,
+	    .is_zoned = 1, .zonenum = 0, .kboff_inzone = 0 },
+	  { .magic = "_BHRfS_M", .len = 8, .sboff = 0x40,
+	    .is_zoned = 1, .zonenum = 1, .kboff_inzone = 0 },
+	  /*
+	   * For zoned btrfs, we also need to detect a temporary superblock
+	   * at zone #0. Mkfs.btrfs creates it in the initialize process.
+	   * It persits until both zones are filled up then reset.
+	   */
+	  { .magic = "!BHRfS_M", .len = 8, .sboff = 0x40,
+	    .is_zoned = 1, .zonenum = 0, .kboff_inzone = 0 },
 	  { NULL }
 	}
 };
-- 
2.31.1

[PATCH 3/3] blkid: support zone reset for wipefs

[Naohiro Aota]

We cannot overwrite superblock magic in a sequential required zone. So,
wipefs cannot work as it is. Instead, this commit implements the wiping by
zone resetting.

Zone resetting must be done only for a sequential write zone. This is
checked by is_conventional().

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 libblkid/src/probe.c | 65 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
index 102766e57aa0..7454a14bdfe6 100644
--- a/libblkid/src/probe.c
+++ b/libblkid/src/probe.c
@@ -107,6 +107,7 @@
 #include <stdint.h>
 #include <stdarg.h>
 #include <limits.h>
+#include <stdbool.h>
 
 #include "blkidP.h"
 #include "all-io.h"
@@ -1225,6 +1226,40 @@ int blkid_do_probe(blkid_probe pr)
 	return rc;
 }
 
+static int is_conventional(blkid_probe pr, uint64_t offset)
+{
+	struct blk_zone_report *rep = NULL;
+	size_t rep_size;
+	bool conventional;
+	int ret;
+
+	if (!pr->zone_size)
+		return 1;
+
+	rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone);
+	rep = malloc(rep_size);
+	if (!rep)
+		return -1;
+
+	memset(rep, 0, rep_size);
+	rep->sector = (offset & pr->zone_size) >> 9;
+	rep->nr_zones = 1;
+	ret = ioctl(blkid_probe_get_fd(pr), BLKREPORTZONE, rep);
+	if (ret) {
+		free(rep);
+		return -1;
+	}
+
+	if (rep->zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL)
+		ret = 1;
+	else
+		ret = 0;
+
+	free(rep);
+
+	return ret;
+}
+
 /**
  * blkid_do_wipe:
  * @pr: prober
@@ -1264,6 +1299,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
 	const char *off = NULL;
 	size_t len = 0;
 	uint64_t offset, magoff;
+	bool conventional;
 	char buf[BUFSIZ];
 	int fd, rc = 0;
 	struct blkid_chain *chn;
@@ -1299,6 +1335,11 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
 	if (len > sizeof(buf))
 		len = sizeof(buf);
 
+	rc = is_conventional(pr, offset);
+	if (rc < 0)
+		return rc;
+	conventional = rc == 1;
+
 	DBG(LOWPROBE, ul_debug(
 	    "do_wipe [offset=0x%"PRIx64" (%"PRIu64"), len=%zu, chain=%s, idx=%d, dryrun=%s]\n",
 	    offset, offset, len, chn->driver->name, chn->idx, dryrun ? "yes" : "not"));
@@ -1306,13 +1347,25 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
 	if (lseek(fd, offset, SEEK_SET) == (off_t) -1)
 		return -1;
 
-	memset(buf, 0, len);
-
 	if (!dryrun && len) {
-		/* wipen on device */
-		if (write_all(fd, buf, len))
-			return -1;
-		fsync(fd);
+		if (conventional) {
+			memset(buf, 0, len);
+
+			/* wipen on device */
+			if (write_all(fd, buf, len))
+				return -1;
+			fsync(fd);
+		} else {
+			struct blk_zone_range range = {
+			    (offset & pr->zone_size) >> 9,
+			    pr->zone_size >> 9,
+			};
+
+			rc = ioctl(fd, BLKRESETZONE, &range);
+			if (rc < 0)
+				return -1;
+		}
+
 		pr->flags &= ~BLKID_FL_MODIF_BUFF;	/* be paranoid */
 
 		return blkid_probe_step_back(pr);
-- 
2.31.1

Re: [PATCH 1/3] blkid: implement zone-aware probing

[Damien Le Moal]

On 2021/04/14 7:11, Naohiro Aota wrote:
> This patch makes libblkid zone-aware. It can probe the magic located at
> some offset from the beginning of some specific zone of a device.
> 
> Ths patch introduces some new fields to struct blkid_idmag. They indicate
> the magic location is placed related to a zone, and the offset in the zone.
> 
> Also, this commit introduces `zone_size` to struct blkid_struct_probe. It
> stores the size of zones of a device.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  configure.ac          |  1 +
>  libblkid/src/blkidP.h |  5 +++++
>  libblkid/src/probe.c  | 26 ++++++++++++++++++++++++--
>  3 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index bebb4085425a..52b164e834db 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -302,6 +302,7 @@ AC_CHECK_HEADERS([ \
>  	lastlog.h \
>  	libutil.h \
>  	linux/btrfs.h \
> +	linux/blkzoned.h \
>  	linux/capability.h \
>  	linux/cdrom.h \
>  	linux/falloc.h \
> diff --git a/libblkid/src/blkidP.h b/libblkid/src/blkidP.h
> index a3fe6748a969..e3a160aa97c0 100644
> --- a/libblkid/src/blkidP.h
> +++ b/libblkid/src/blkidP.h
> @@ -150,6 +150,10 @@ struct blkid_idmag
>  	const char	*hoff;		/* hint which contains byte offset to kboff */
>  	long		kboff;		/* kilobyte offset of superblock */
>  	unsigned int	sboff;		/* byte offset within superblock */
> +
> +	int		is_zoned;	/* indicate magic location is calcluated based on zone position  */
> +	long		zonenum;	/* zone number which has superblock */
> +	long		kboff_inzone;	/* kilobyte offset of superblock in a zone */
>  };
>  
>  /*
> @@ -206,6 +210,7 @@ struct blkid_struct_probe
>  	dev_t			disk_devno;	/* devno of the whole-disk or 0 */
>  	unsigned int		blkssz;		/* sector size (BLKSSZGET ioctl) */
>  	mode_t			mode;		/* struct stat.sb_mode */
> +	uint64_t		zone_size;	/* zone size (BLKGETZONESZ ioctl) */
>  
>  	int			flags;		/* private library flags */
>  	int			prob_flags;	/* always zeroized by blkid_do_*() */
> diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
> index a47a8720d4ac..102766e57aa0 100644
> --- a/libblkid/src/probe.c
> +++ b/libblkid/src/probe.c
> @@ -94,6 +94,9 @@
>  #ifdef HAVE_LINUX_CDROM_H
>  #include <linux/cdrom.h>
>  #endif
> +#ifdef HAVE_LINUX_BLKZONED_H
> +#include <linux/blkzoned.h>
> +#endif
>  #ifdef HAVE_SYS_STAT_H
>  #include <sys/stat.h>
>  #endif
> @@ -871,6 +874,7 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
>  #ifdef CDROM_GET_CAPABILITY
>  	long last_written = 0;
>  #endif
> +	uint32_t zone_size_sector;

Move this declaration under the # ifdef HAVE_LINUX_BLKZONED_H where it is used
below ?

>  
>  	blkid_reset_probe(pr);
>  	blkid_probe_reset_buffers(pr);
> @@ -897,6 +901,7 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
>  	pr->wipe_off = 0;
>  	pr->wipe_size = 0;
>  	pr->wipe_chain = NULL;
> +	pr->zone_size = 0;
>  
>  	if (fd < 0)
>  		return 1;
> @@ -996,6 +1001,11 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
>  #endif
>  	free(dm_uuid);
>  
> +# ifdef HAVE_LINUX_BLKZONED_H
> +	if (S_ISBLK(sb.st_mode) && !ioctl(pr->fd, BLKGETZONESZ, &zone_size_sector))
> +		pr->zone_size = zone_size_sector << 9;
> +# endif

So something like:

# ifdef HAVE_LINUX_BLKZONED_H
	if (S_ISBLK(sb.st_mode)) {
		 uint32_t zone_size_sector;

		if (!ioctl(pr->fd, BLKGETZONESZ, &zone_size_sector))
			pr->zone_size = zone_size_sector << 9;
	}
# endif

Otherwise, you probably will get a compiler warning as zone_size_sector will be
unused if HAVE_LINUX_BLKZONED_H is false.

> +
>  	DBG(LOWPROBE, ul_debug("ready for low-probing, offset=%"PRIu64", size=%"PRIu64"",
>  				pr->off, pr->size));
>  	DBG(LOWPROBE, ul_debug("whole-disk: %s, regfile: %s",
> @@ -1064,12 +1074,24 @@ int blkid_probe_get_idmag(blkid_probe pr, const struct blkid_idinfo *id,
>  	/* try to detect by magic string */
>  	while(mag && mag->magic) {
>  		unsigned char *buf;
> +		uint64_t kboff;
>  		uint64_t hint_offset;
>  
>  		if (!mag->hoff || blkid_probe_get_hint(pr, mag->hoff, &hint_offset) < 0)
>  			hint_offset = 0;
>  
> -		off = hint_offset + ((mag->kboff + (mag->sboff >> 10)) << 10);
> +		/* If the magic is for zoned device, skip non-zoned device */
> +		if (mag->is_zoned && !pr->zone_size) {
> +			mag++;
> +			continue;
> +		}
> +
> +		if (!mag->is_zoned)
> +			kboff = mag->kboff;
> +		else
> +			kboff = ((mag->zonenum * pr->zone_size) >> 10) + mag->kboff_inzone;
> +
> +		off = hint_offset + ((kboff + (mag->sboff >> 10)) << 10);
>  		buf = blkid_probe_get_buffer(pr, off, 1024);
>  
>  		if (!buf && errno)
> @@ -1079,7 +1101,7 @@ int blkid_probe_get_idmag(blkid_probe pr, const struct blkid_idinfo *id,
>  				buf + (mag->sboff & 0x3ff), mag->len)) {
>  
>  			DBG(LOWPROBE, ul_debug("\tmagic sboff=%u, kboff=%ld",
> -				mag->sboff, mag->kboff));
> +				mag->sboff, kboff));
>  			if (offset)
>  				*offset = off + (mag->sboff & 0x3ff);
>  			if (res)
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 3/3] blkid: support zone reset for wipefs

[Damien Le Moal]

On 2021/04/14 7:11, Naohiro Aota wrote:
> We cannot overwrite superblock magic in a sequential required zone. So,
> wipefs cannot work as it is. Instead, this commit implements the wiping by
> zone resetting.
> 
> Zone resetting must be done only for a sequential write zone. This is
> checked by is_conventional().
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  libblkid/src/probe.c | 65 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 59 insertions(+), 6 deletions(-)
> 
> diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
> index 102766e57aa0..7454a14bdfe6 100644
> --- a/libblkid/src/probe.c
> +++ b/libblkid/src/probe.c
> @@ -107,6 +107,7 @@
>  #include <stdint.h>
>  #include <stdarg.h>
>  #include <limits.h>
> +#include <stdbool.h>
>  
>  #include "blkidP.h"
>  #include "all-io.h"
> @@ -1225,6 +1226,40 @@ int blkid_do_probe(blkid_probe pr)
>  	return rc;
>  }
>  
> +static int is_conventional(blkid_probe pr, uint64_t offset)
> +{
> +	struct blk_zone_report *rep = NULL;
> +	size_t rep_size;
> +	bool conventional;
> +	int ret;
> +
> +	if (!pr->zone_size)
> +		return 1;
> +
> +	rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone);
> +	rep = malloc(rep_size);
> +	if (!rep)
> +		return -1;
> +
> +	memset(rep, 0, rep_size);

Use calloc() to get the zeroing done at the same time as allocation, may be ?

> +	rep->sector = (offset & pr->zone_size) >> 9;

Why the "& pr->zone_size" ? This seems wrong. To get the zone info of the zone
containing offset, you can just have:

	rep->sector = offset >> 9;

And if you want to align to the zone start, then:

	rep->sector = (offset & (~(pr->zone_size - 1))) >> 9;

> +	rep->nr_zones = 1;
> +	ret = ioctl(blkid_probe_get_fd(pr), BLKREPORTZONE, rep);
> +	if (ret) {
> +		free(rep);
> +		return -1;
> +	}
> +
> +	if (rep->zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL)
> +		ret = 1;
> +	else
> +		ret = 0;
> +
> +	free(rep);
> +
> +	return ret;
> +}
> +
>  /**
>   * blkid_do_wipe:
>   * @pr: prober
> @@ -1264,6 +1299,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
>  	const char *off = NULL;
>  	size_t len = 0;
>  	uint64_t offset, magoff;
> +	bool conventional;
>  	char buf[BUFSIZ];
>  	int fd, rc = 0;
>  	struct blkid_chain *chn;
> @@ -1299,6 +1335,11 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
>  	if (len > sizeof(buf))
>  		len = sizeof(buf);
>  
> +	rc = is_conventional(pr, offset);
> +	if (rc < 0)
> +		return rc;
> +	conventional = rc == 1;
> +
>  	DBG(LOWPROBE, ul_debug(
>  	    "do_wipe [offset=0x%"PRIx64" (%"PRIu64"), len=%zu, chain=%s, idx=%d, dryrun=%s]\n",
>  	    offset, offset, len, chn->driver->name, chn->idx, dryrun ? "yes" : "not"));
> @@ -1306,13 +1347,25 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
>  	if (lseek(fd, offset, SEEK_SET) == (off_t) -1)
>  		return -1;
>  
> -	memset(buf, 0, len);
> -
>  	if (!dryrun && len) {
> -		/* wipen on device */
> -		if (write_all(fd, buf, len))
> -			return -1;
> -		fsync(fd);
> +		if (conventional) {
> +			memset(buf, 0, len);
> +
> +			/* wipen on device */
> +			if (write_all(fd, buf, len))
> +				return -1;
> +			fsync(fd);
> +		} else {
> +			struct blk_zone_range range = {
> +			    (offset & pr->zone_size) >> 9,
> +			    pr->zone_size >> 9,
> +			};

Please add the field names for clarity and fix the sector position (it looks
very wrong):

			struct blk_zone_range range = {
			    .sector = (offset & (~(pr->zone_size - 1))) >> 9,
			    .nr_sectors = pr->zone_size >> 9,
			};

> +
> +			rc = ioctl(fd, BLKRESETZONE, &range);
> +			if (rc < 0)
> +				return -1;
> +		}
> +
>  		pr->flags &= ~BLKID_FL_MODIF_BUFF;	/* be paranoid */
>  
>  		return blkid_probe_step_back(pr);
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/3] blkid: implement zone-aware probing

[Naohiro Aota]

On Tue, Apr 13, 2021 at 10:38:37PM +0000, Damien Le Moal wrote:
> On 2021/04/14 7:11, Naohiro Aota wrote:
> > This patch makes libblkid zone-aware. It can probe the magic located at
> > some offset from the beginning of some specific zone of a device.
> > 
> > Ths patch introduces some new fields to struct blkid_idmag. They indicate
> > the magic location is placed related to a zone, and the offset in the zone.
> > 
> > Also, this commit introduces `zone_size` to struct blkid_struct_probe. It
> > stores the size of zones of a device.
> > 
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> > ---
> >  configure.ac          |  1 +
> >  libblkid/src/blkidP.h |  5 +++++
> >  libblkid/src/probe.c  | 26 ++++++++++++++++++++++++--
> >  3 files changed, 30 insertions(+), 2 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index bebb4085425a..52b164e834db 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -302,6 +302,7 @@ AC_CHECK_HEADERS([ \
> >  	lastlog.h \
> >  	libutil.h \
> >  	linux/btrfs.h \
> > +	linux/blkzoned.h \
> >  	linux/capability.h \
> >  	linux/cdrom.h \
> >  	linux/falloc.h \
> > diff --git a/libblkid/src/blkidP.h b/libblkid/src/blkidP.h
> > index a3fe6748a969..e3a160aa97c0 100644
> > --- a/libblkid/src/blkidP.h
> > +++ b/libblkid/src/blkidP.h
> > @@ -150,6 +150,10 @@ struct blkid_idmag
> >  	const char	*hoff;		/* hint which contains byte offset to kboff */
> >  	long		kboff;		/* kilobyte offset of superblock */
> >  	unsigned int	sboff;		/* byte offset within superblock */
> > +
> > +	int		is_zoned;	/* indicate magic location is calcluated based on zone position  */
> > +	long		zonenum;	/* zone number which has superblock */
> > +	long		kboff_inzone;	/* kilobyte offset of superblock in a zone */
> >  };
> >  
> >  /*
> > @@ -206,6 +210,7 @@ struct blkid_struct_probe
> >  	dev_t			disk_devno;	/* devno of the whole-disk or 0 */
> >  	unsigned int		blkssz;		/* sector size (BLKSSZGET ioctl) */
> >  	mode_t			mode;		/* struct stat.sb_mode */
> > +	uint64_t		zone_size;	/* zone size (BLKGETZONESZ ioctl) */
> >  
> >  	int			flags;		/* private library flags */
> >  	int			prob_flags;	/* always zeroized by blkid_do_*() */
> > diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
> > index a47a8720d4ac..102766e57aa0 100644
> > --- a/libblkid/src/probe.c
> > +++ b/libblkid/src/probe.c
> > @@ -94,6 +94,9 @@
> >  #ifdef HAVE_LINUX_CDROM_H
> >  #include <linux/cdrom.h>
> >  #endif
> > +#ifdef HAVE_LINUX_BLKZONED_H
> > +#include <linux/blkzoned.h>
> > +#endif
> >  #ifdef HAVE_SYS_STAT_H
> >  #include <sys/stat.h>
> >  #endif
> > @@ -871,6 +874,7 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
> >  #ifdef CDROM_GET_CAPABILITY
> >  	long last_written = 0;
> >  #endif
> > +	uint32_t zone_size_sector;
> 
> Move this declaration under the # ifdef HAVE_LINUX_BLKZONED_H where it is used
> below ?
> 
> >  
> >  	blkid_reset_probe(pr);
> >  	blkid_probe_reset_buffers(pr);
> > @@ -897,6 +901,7 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
> >  	pr->wipe_off = 0;
> >  	pr->wipe_size = 0;
> >  	pr->wipe_chain = NULL;
> > +	pr->zone_size = 0;
> >  
> >  	if (fd < 0)
> >  		return 1;
> > @@ -996,6 +1001,11 @@ int blkid_probe_set_device(blkid_probe pr, int fd,
> >  #endif
> >  	free(dm_uuid);
> >  
> > +# ifdef HAVE_LINUX_BLKZONED_H
> > +	if (S_ISBLK(sb.st_mode) && !ioctl(pr->fd, BLKGETZONESZ, &zone_size_sector))
> > +		pr->zone_size = zone_size_sector << 9;
> > +# endif
> 
> So something like:
> 
> # ifdef HAVE_LINUX_BLKZONED_H
> 	if (S_ISBLK(sb.st_mode)) {
> 		 uint32_t zone_size_sector;
> 
> 		if (!ioctl(pr->fd, BLKGETZONESZ, &zone_size_sector))
> 			pr->zone_size = zone_size_sector << 9;
> 	}
> # endif
> 
> Otherwise, you probably will get a compiler warning as zone_size_sector will be
> unused if HAVE_LINUX_BLKZONED_H is false.

My bad. I should have undef the HAVE_LINUX_BLKZONED_H and compile
it. I'll rewrite and send a new series.

Thanks,

> > +
> >  	DBG(LOWPROBE, ul_debug("ready for low-probing, offset=%"PRIu64", size=%"PRIu64"",
> >  				pr->off, pr->size));
> >  	DBG(LOWPROBE, ul_debug("whole-disk: %s, regfile: %s",
> > @@ -1064,12 +1074,24 @@ int blkid_probe_get_idmag(blkid_probe pr, const struct blkid_idinfo *id,
> >  	/* try to detect by magic string */
> >  	while(mag && mag->magic) {
> >  		unsigned char *buf;
> > +		uint64_t kboff;
> >  		uint64_t hint_offset;
> >  
> >  		if (!mag->hoff || blkid_probe_get_hint(pr, mag->hoff, &hint_offset) < 0)
> >  			hint_offset = 0;
> >  
> > -		off = hint_offset + ((mag->kboff + (mag->sboff >> 10)) << 10);
> > +		/* If the magic is for zoned device, skip non-zoned device */
> > +		if (mag->is_zoned && !pr->zone_size) {
> > +			mag++;
> > +			continue;
> > +		}
> > +
> > +		if (!mag->is_zoned)
> > +			kboff = mag->kboff;
> > +		else
> > +			kboff = ((mag->zonenum * pr->zone_size) >> 10) + mag->kboff_inzone;
> > +
> > +		off = hint_offset + ((kboff + (mag->sboff >> 10)) << 10);
> >  		buf = blkid_probe_get_buffer(pr, off, 1024);
> >  
> >  		if (!buf && errno)
> > @@ -1079,7 +1101,7 @@ int blkid_probe_get_idmag(blkid_probe pr, const struct blkid_idinfo *id,
> >  				buf + (mag->sboff & 0x3ff), mag->len)) {
> >  
> >  			DBG(LOWPROBE, ul_debug("\tmagic sboff=%u, kboff=%ld",
> > -				mag->sboff, mag->kboff));
> > +				mag->sboff, kboff));
> >  			if (offset)
> >  				*offset = off + (mag->sboff & 0x3ff);
> >  			if (res)
> > 
> 
> 
> -- 
> Damien Le Moal
> Western Digital Research

Re: [PATCH 3/3] blkid: support zone reset for wipefs

[Naohiro Aota]

On Tue, Apr 13, 2021 at 10:47:00PM +0000, Damien Le Moal wrote:
> On 2021/04/14 7:11, Naohiro Aota wrote:
> > We cannot overwrite superblock magic in a sequential required zone. So,
> > wipefs cannot work as it is. Instead, this commit implements the wiping by
> > zone resetting.
> > 
> > Zone resetting must be done only for a sequential write zone. This is
> > checked by is_conventional().
> > 
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> > ---
> >  libblkid/src/probe.c | 65 ++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 59 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
> > index 102766e57aa0..7454a14bdfe6 100644
> > --- a/libblkid/src/probe.c
> > +++ b/libblkid/src/probe.c
> > @@ -107,6 +107,7 @@
> >  #include <stdint.h>
> >  #include <stdarg.h>
> >  #include <limits.h>
> > +#include <stdbool.h>
> >  
> >  #include "blkidP.h"
> >  #include "all-io.h"
> > @@ -1225,6 +1226,40 @@ int blkid_do_probe(blkid_probe pr)
> >  	return rc;
> >  }
> >  
> > +static int is_conventional(blkid_probe pr, uint64_t offset)
> > +{
> > +	struct blk_zone_report *rep = NULL;
> > +	size_t rep_size;
> > +	bool conventional;
> > +	int ret;
> > +
> > +	if (!pr->zone_size)
> > +		return 1;
> > +
> > +	rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone);
> > +	rep = malloc(rep_size);
> > +	if (!rep)
> > +		return -1;
> > +
> > +	memset(rep, 0, rep_size);
> 
> Use calloc() to get the zeroing done at the same time as allocation, may be ?

Will do.

> > +	rep->sector = (offset & pr->zone_size) >> 9;
> 
> Why the "& pr->zone_size" ? This seems wrong. To get the zone info of the zone
> containing offset, you can just have:
> 
> 	rep->sector = offset >> 9;
> 
> And if you want to align to the zone start, then:
> 
> 	rep->sector = (offset & (~(pr->zone_size - 1))) >> 9;

Ouch. I was silly and missed this bug because it was only doing reset
on zone #0 and #1. I'll introduce "zone_mask" and do the correct
calculation.

> > +	rep->nr_zones = 1;
> > +	ret = ioctl(blkid_probe_get_fd(pr), BLKREPORTZONE, rep);
> > +	if (ret) {
> > +		free(rep);
> > +		return -1;
> > +	}
> > +
> > +	if (rep->zones[0].type == BLK_ZONE_TYPE_CONVENTIONAL)
> > +		ret = 1;
> > +	else
> > +		ret = 0;
> > +
> > +	free(rep);
> > +
> > +	return ret;
> > +}
> > +
> >  /**
> >   * blkid_do_wipe:
> >   * @pr: prober
> > @@ -1264,6 +1299,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
> >  	const char *off = NULL;
> >  	size_t len = 0;
> >  	uint64_t offset, magoff;
> > +	bool conventional;
> >  	char buf[BUFSIZ];
> >  	int fd, rc = 0;
> >  	struct blkid_chain *chn;
> > @@ -1299,6 +1335,11 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
> >  	if (len > sizeof(buf))
> >  		len = sizeof(buf);
> >  
> > +	rc = is_conventional(pr, offset);
> > +	if (rc < 0)
> > +		return rc;
> > +	conventional = rc == 1;
> > +
> >  	DBG(LOWPROBE, ul_debug(
> >  	    "do_wipe [offset=0x%"PRIx64" (%"PRIu64"), len=%zu, chain=%s, idx=%d, dryrun=%s]\n",
> >  	    offset, offset, len, chn->driver->name, chn->idx, dryrun ? "yes" : "not"));
> > @@ -1306,13 +1347,25 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
> >  	if (lseek(fd, offset, SEEK_SET) == (off_t) -1)
> >  		return -1;
> >  
> > -	memset(buf, 0, len);
> > -
> >  	if (!dryrun && len) {
> > -		/* wipen on device */
> > -		if (write_all(fd, buf, len))
> > -			return -1;
> > -		fsync(fd);
> > +		if (conventional) {
> > +			memset(buf, 0, len);
> > +
> > +			/* wipen on device */
> > +			if (write_all(fd, buf, len))
> > +				return -1;
> > +			fsync(fd);
> > +		} else {
> > +			struct blk_zone_range range = {
> > +			    (offset & pr->zone_size) >> 9,
> > +			    pr->zone_size >> 9,
> > +			};
> 
> Please add the field names for clarity and fix the sector position (it looks
> very wrong):
> 
> 			struct blk_zone_range range = {
> 			    .sector = (offset & (~(pr->zone_size - 1))) >> 9,
> 			    .nr_sectors = pr->zone_size >> 9,
> 			};

Will do the same.

> > +
> > +			rc = ioctl(fd, BLKRESETZONE, &range);
> > +			if (rc < 0)
> > +				return -1;
> > +		}
> > +
> >  		pr->flags &= ~BLKID_FL_MODIF_BUFF;	/* be paranoid */
> >  
> >  		return blkid_probe_step_back(pr);
> > 
> 
> 
> -- 
> Damien Le Moal
> Western Digital Research