diff mbox series

[v2,3/3] blkid: support zone reset for wipefs

Message ID 20210414013339.2936229-4-naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series implement zone-aware probing/wiping for zoned btrfs | expand

Commit Message

Naohiro Aota April 14, 2021, 1:33 a.m. UTC
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 | 79 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 6 deletions(-)

Comments

Damien Le Moal April 14, 2021, 9:13 a.m. UTC | #1
On 2021/04/14 10:33, 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 | 79 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 73 insertions(+), 6 deletions(-)
> 
> diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
> index 9d180aab5242..23c3621627d4 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"
> @@ -1228,6 +1229,48 @@ int blkid_do_probe(blkid_probe pr)
>  	return rc;
>  }
>  
> +#ifdef HAVE_LINUX_BLKZONED_H
> +static int is_conventional(blkid_probe pr, uint64_t offset)
> +{
> +	struct blk_zone_report *rep = NULL;
> +	size_t rep_size;
> +	int ret;
> +	uint64_t zone_mask;
> +
> +	if (!pr->zone_size)
> +		return 1;
> +
> +	rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone);
> +	rep = calloc(1, rep_size);
> +	if (!rep)
> +		return -1;
> +
> +	zone_mask = ~(pr->zone_size - 1);
> +	rep->sector = (offset & zone_mask) >> 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;
> +}
> +#else
> +static inline int is_conventional(blkid_probe pr __attribute__((__unused__)),
> +				  uint64_t offset __attribute__((__unused__)))
> +{
> +	return 1;
> +}
> +#endif
> +
>  /**
>   * blkid_do_wipe:
>   * @pr: prober
> @@ -1267,6 +1310,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;
> @@ -1302,6 +1346,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"));
> @@ -1309,13 +1358,31 @@ 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 {
> +#ifdef HAVE_LINUX_BLKZONED_H
> +			uint64_t zone_mask = ~(pr->zone_size - 1);
> +			struct blk_zone_range range = {
> +				.sector = (offset & zone_mask) >> 9,
> +				.nr_sectors = pr->zone_size >> 9,
> +			};
> +
> +			rc = ioctl(fd, BLKRESETZONE, &range);
> +			if (rc < 0)
> +				return -1;
> +#else
> +			/* Should not reach here */
> +			assert(0);
> +#endif
> +		}
> +
>  		pr->flags &= ~BLKID_FL_MODIF_BUFF;	/* be paranoid */
>  
>  		return blkid_probe_step_back(pr);
> 

Looks OK to me.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
Johannes Thumshirn April 14, 2021, 9:50 a.m. UTC | #2
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Karel Zak April 14, 2021, 1:57 p.m. UTC | #3
On Wed, Apr 14, 2021 at 10:33:39AM +0900, Naohiro Aota wrote:
> +static int is_conventional(blkid_probe pr, uint64_t offset)
> +{
> +	struct blk_zone_report *rep = NULL;
> +	size_t rep_size;
> +	int ret;
> +	uint64_t zone_mask;
> +
> +	if (!pr->zone_size)
> +		return 1;
> +
> +	rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone);
> +	rep = calloc(1, rep_size);
> +	if (!rep)
> +		return -1;
> +
> +	zone_mask = ~(pr->zone_size - 1);
> +	rep->sector = (offset & zone_mask) >> 9;
> +	rep->nr_zones = 1;
> +	ret = ioctl(blkid_probe_get_fd(pr), BLKREPORTZONE, rep);
> +	if (ret) {
> +		free(rep);
> +		return -1;
> +	}

  ret = blkdev_get_zonereport()

:-)

>  /**
>   * blkid_do_wipe:
>   * @pr: prober
> @@ -1267,6 +1310,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
>  	const char *off = NULL;
>  	size_t len = 0;
>  	uint64_t offset, magoff;
> +	bool conventional;

BTW, nowhere in libblkid we use "bool". It would be probably better to include
<stdbool.h> to blkidP.h.

  Karel
David Sterba April 16, 2021, 4:04 p.m. UTC | #4
On Wed, Apr 14, 2021 at 03:57:42PM +0200, Karel Zak wrote:
> On Wed, Apr 14, 2021 at 10:33:39AM +0900, Naohiro Aota wrote:
> >  /**
> >   * blkid_do_wipe:
> >   * @pr: prober
> > @@ -1267,6 +1310,7 @@ int blkid_do_wipe(blkid_probe pr, int dryrun)
> >  	const char *off = NULL;
> >  	size_t len = 0;
> >  	uint64_t offset, magoff;
> > +	bool conventional;
> 
> BTW, nowhere in libblkid we use "bool". It would be probably better to include
> <stdbool.h> to blkidP.h.

Pulling a whole new header just for one local variable that can be int
seems too much.
diff mbox series

Patch

diff --git a/libblkid/src/probe.c b/libblkid/src/probe.c
index 9d180aab5242..23c3621627d4 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"
@@ -1228,6 +1229,48 @@  int blkid_do_probe(blkid_probe pr)
 	return rc;
 }
 
+#ifdef HAVE_LINUX_BLKZONED_H
+static int is_conventional(blkid_probe pr, uint64_t offset)
+{
+	struct blk_zone_report *rep = NULL;
+	size_t rep_size;
+	int ret;
+	uint64_t zone_mask;
+
+	if (!pr->zone_size)
+		return 1;
+
+	rep_size = sizeof(struct blk_zone_report) + sizeof(struct blk_zone);
+	rep = calloc(1, rep_size);
+	if (!rep)
+		return -1;
+
+	zone_mask = ~(pr->zone_size - 1);
+	rep->sector = (offset & zone_mask) >> 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;
+}
+#else
+static inline int is_conventional(blkid_probe pr __attribute__((__unused__)),
+				  uint64_t offset __attribute__((__unused__)))
+{
+	return 1;
+}
+#endif
+
 /**
  * blkid_do_wipe:
  * @pr: prober
@@ -1267,6 +1310,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;
@@ -1302,6 +1346,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"));
@@ -1309,13 +1358,31 @@  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 {
+#ifdef HAVE_LINUX_BLKZONED_H
+			uint64_t zone_mask = ~(pr->zone_size - 1);
+			struct blk_zone_range range = {
+				.sector = (offset & zone_mask) >> 9,
+				.nr_sectors = pr->zone_size >> 9,
+			};
+
+			rc = ioctl(fd, BLKRESETZONE, &range);
+			if (rc < 0)
+				return -1;
+#else
+			/* Should not reach here */
+			assert(0);
+#endif
+		}
+
 		pr->flags &= ~BLKID_FL_MODIF_BUFF;	/* be paranoid */
 
 		return blkid_probe_step_back(pr);