[PATCH v2] blkzone: add open/close/finish commands

[PATCH v2] blkzone: add open/close/finish commands

[Damien Le Moal]

From: Aravind Ramesh <Aravind.Ramesh@wdc.com>

Introduce blkzone open, close and finish commands, issuing BLKOPENZONE,
BLKCLOSEZONE and BLKFINISHZONE ioctl commands to open, close or finish
a range of zones of a zoned block device.

Since these three commands are similar to the existing zone reset
command, the existing zone reset command implementation is changed into
the generic blkzone_action() internal handler function for processing
all zone actions.

The BLKOPENZONE, BLKCLOSEZONE and BLKFINISHZONE ioctl commands codes are
defined in linux/blkzoned.h starting with kernel version 5.5. To ensure
that the blkzone utility compiles even with older blkzoned.h kernel
header versions, these ioctl commands are internally defined if the
blkzoned.h header definition is not present. Execution of these commands
on kernels older than 5.5 will result in a -ENOTTY error (missing
ioctl).

Signed-off-by: Aravind Ramesh <Aravind.Ramesh@wdc.com>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Tested-by: Hans Holmberg <Hans.Holmberg@wdc.com>
Reviewed-by: Hans Holmberg <Hans.Holmberg@wdc.com>
---
 sys-utils/blkzone.8 | 27 +++++++++++++++--
 sys-utils/blkzone.c | 74 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 91 insertions(+), 10 deletions(-)

diff --git a/sys-utils/blkzone.8 b/sys-utils/blkzone.8
index 83d3dd7ce..f50e3f5df 100644
--- a/sys-utils/blkzone.8
+++ b/sys-utils/blkzone.8
@@ -55,10 +55,31 @@ x?:Reserved conditions (should not be reported)
 The command \fBblkzone reset\fP is used to reset one or more zones. Unlike
 .BR sg_reset_wp (8),
 this command operates from the block layer and can reset a range of zones.
+
+.SS open
+The command \fBblkzone open\fP is used to explicitly open one or more zones.
+Unlike
+.BR sg_zone (8),
+open action, this command operates from the block layer and can open a range
+of zones.
+
+.SS close
+The command \fBblkzone close\fP is used to close one or more zones. Unlike
+.BR sg_zone (8),
+close action, this command operates from the block layer and can close a range
+of zones.
+
+.SS finish
+The command \fBblkzone finish\fP is used to finish (transition to full condition)
+one or more zones. Unlike
+.BR sg_zone (8),
+finish action, this command operates from the block layer and can finish a range
+of zones.
+
 .PP
-By default, the command will operate from the zone at device
-sector 0 and reset all zones. Options may be used to modify this behavior
-as well as specify the operation to be performed on the zone, as explained below.
+By default, the reset, open, close and finish commands will operate from the zone
+at device sector 0 and operate on all zones. Options may be used to modify this
+behavior as explained below.
 
 .SH OPTIONS
 The
diff --git a/sys-utils/blkzone.c b/sys-utils/blkzone.c
index 715f03fef..b6ddaf8ab 100644
--- a/sys-utils/blkzone.c
+++ b/sys-utils/blkzone.c
@@ -44,10 +44,23 @@
 #include "sysfs.h"
 #include "optutils.h"
 
+/*
+ * These ioctls are defined in linux/blkzoned.h starting with kernel 5.5.
+ */
+#ifndef BLKOPENZONE
+#define BLKOPENZONE	_IOW(0x12, 134, struct blk_zone_range)
+#endif
+#ifndef BLKCLOSEZONE
+#define BLKCLOSEZONE	_IOW(0x12, 135, struct blk_zone_range)
+#endif
+#ifndef BLKFINISHZONE
+#define BLKFINISHZONE	_IOW(0x12, 136, struct blk_zone_range)
+#endif
+
 struct blkzone_control;
 
 static int blkzone_report(struct blkzone_control *ctl);
-static int blkzone_reset(struct blkzone_control *ctl);
+static int blkzone_action(struct blkzone_control *ctl);
 
 struct blkzone_command {
 	const char *name;
@@ -71,9 +84,28 @@ struct blkzone_control {
 
 static const struct blkzone_command commands[] = {
 	{ "report",	blkzone_report, N_("Report zone information about the given device") },
-	{ "reset",	blkzone_reset,  N_("Reset a range of zones.") }
+	{ "reset",	blkzone_action, N_("Reset a range of zones.") },
+	{ "open",	blkzone_action, N_("Open a range of zones.") },
+	{ "close",	blkzone_action, N_("Close a range of zones.") },
+	{ "finish",	blkzone_action, N_("Set a range of zones to Full.") }
+};
+
+/*
+ * The action values must match the command index in the command array.
+ */
+enum blkzone_action {
+	BLK_ZONE_NO_ACTION = 0,
+	BLK_ZONE_RESET,
+	BLK_ZONE_OPEN,
+	BLK_ZONE_CLOSE,
+	BLK_ZONE_FINISH,
 };
 
+static enum blkzone_action command_action(const struct blkzone_command *command)
+{
+	return command - &commands[0];
+}
+
 static const struct blkzone_command *name_to_command(const char *name)
 {
 	size_t i;
@@ -246,15 +278,41 @@ static int blkzone_report(struct blkzone_control *ctl)
 }
 
 /*
- * blkzone reset
+ * blkzone reset, open, close, and finish.
  */
-static int blkzone_reset(struct blkzone_control *ctl)
+
+static int blkzone_action(struct blkzone_control *ctl)
 {
 	struct blk_zone_range za = { .sector = 0 };
 	unsigned long zonesize;
+	unsigned long ioctl_cmd;
+	const char *ioctl_name;
 	uint64_t zlen;
 	int fd;
 
+	switch (command_action(ctl->command)) {
+	case BLK_ZONE_RESET:
+		ioctl_cmd = BLKRESETZONE;
+		ioctl_name = "BLKRESETZONE";
+		break;
+	case BLK_ZONE_OPEN:
+		ioctl_cmd = BLKOPENZONE;
+		ioctl_name = "BLKOPENZONE";
+		break;
+	case BLK_ZONE_CLOSE:
+		ioctl_cmd = BLKCLOSEZONE;
+		ioctl_name = "BLKCLOSEZONE";
+		break;
+	case BLK_ZONE_FINISH:
+		ioctl_cmd = BLKFINISHZONE;
+		ioctl_name = "BLKFINISHZONE";
+		break;
+	case BLK_ZONE_NO_ACTION:
+		/* fallthrough */
+	default:
+		errx(EXIT_FAILURE, _("Invalid zone action"));
+	}
+
 	zonesize = blkdev_chunk_sectors(ctl->devname);
 	if (!zonesize)
 		errx(EXIT_FAILURE, _("%s: unable to determine zone size"), ctl->devname);
@@ -288,11 +346,13 @@ static int blkzone_reset(struct blkzone_control *ctl)
 	za.sector = ctl->offset;
 	za.nr_sectors = zlen;
 
-	if (ioctl(fd, BLKRESETZONE, &za) == -1)
-		err(EXIT_FAILURE, _("%s: BLKRESETZONE ioctl failed"), ctl->devname);
+	if (ioctl(fd, ioctl_cmd, &za) == -1)
+		err(EXIT_FAILURE, _("%s: %s ioctl failed"),
+		    ctl->devname, ioctl_name);
 	else if (ctl->verbose)
-		printf(_("%s: successfully reset in range from %" PRIu64 ", to %" PRIu64),
+		printf(_("%s: successfull %s of zones in range from %" PRIu64 ", to %" PRIu64),
 			ctl->devname,
+			ctl->command->name,
 			ctl->offset,
 			ctl->offset + zlen);
 	close(fd);
-- 
2.25.1

Re: [PATCH v2] blkzone: add open/close/finish commands

[Karel Zak]

On Tue, Mar 24, 2020 at 07:02:17PM +0900, Damien Le Moal wrote:
>  sys-utils/blkzone.8 | 27 +++++++++++++++--
>  sys-utils/blkzone.c | 74 ++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 91 insertions(+), 10 deletions(-)

Applied, thanks.

>  static const struct blkzone_command commands[] = {
>  	{ "report",	blkzone_report, N_("Report zone information about the given device") },
> -	{ "reset",	blkzone_reset,  N_("Reset a range of zones.") }
> +	{ "reset",	blkzone_action, N_("Reset a range of zones.") },
> +	{ "open",	blkzone_action, N_("Open a range of zones.") },
> +	{ "close",	blkzone_action, N_("Close a range of zones.") },
> +	{ "finish",	blkzone_action, N_("Set a range of zones to Full.") }
> +};
> +
> +/*
> + * The action values must match the command index in the command array.
> + */
> +enum blkzone_action {
> +	BLK_ZONE_NO_ACTION = 0,
> +	BLK_ZONE_RESET,
> +	BLK_ZONE_OPEN,
> +	BLK_ZONE_CLOSE,
> +	BLK_ZONE_FINISH,
>  };

If you add ioctl_cmd and ioctl_name to the struct blkzone_command,
and you define commands[] as:

 commands[] = {
    { 
        .name = "report",
        .handler = blkzone_report,
        .help = N_("Report zone information about the given device") 
    },{
        .name = "reset",
        .handler = blkzone_action,
        .help = N_("Reset a range of zones."),
        .ioctl_cmd = BLKRESETZONE,
        .ioctl_name = "BLKRESETZONE"
    },
    ...
 };

than you do not need this switch() and command_action()

>  
> +	switch (command_action(ctl->command)) {
> +	case BLK_ZONE_RESET:
> +		ioctl_cmd = BLKRESETZONE;
> +		ioctl_name = "BLKRESETZONE";
> +		break;
> +	case BLK_ZONE_OPEN:
> +		ioctl_cmd = BLKOPENZONE;
> +		ioctl_name = "BLKOPENZONE";
> +		break;
> +	case BLK_ZONE_CLOSE:
> +		ioctl_cmd = BLKCLOSEZONE;
> +		ioctl_name = "BLKCLOSEZONE";
> +		break;
> +	case BLK_ZONE_FINISH:
> +		ioctl_cmd = BLKFINISHZONE;
> +		ioctl_name = "BLKFINISHZONE";
> +		break;
> +	case BLK_ZONE_NO_ACTION:
> +		/* fallthrough */
> +	default:
> +		errx(EXIT_FAILURE, _("Invalid zone action"));
  
but you can use ctl->command.ioclt_cmd  etc.

Just idea :-)

 Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v2] blkzone: add open/close/finish commands

[Damien Le Moal]

On 2020/03/25 20:36, Karel Zak wrote:
> On Tue, Mar 24, 2020 at 07:02:17PM +0900, Damien Le Moal wrote:
>>  sys-utils/blkzone.8 | 27 +++++++++++++++--
>>  sys-utils/blkzone.c | 74 ++++++++++++++++++++++++++++++++++++++++-----
>>  2 files changed, 91 insertions(+), 10 deletions(-)
> 
> Applied, thanks.
> 
>>  static const struct blkzone_command commands[] = {
>>  	{ "report",	blkzone_report, N_("Report zone information about the given device") },
>> -	{ "reset",	blkzone_reset,  N_("Reset a range of zones.") }
>> +	{ "reset",	blkzone_action, N_("Reset a range of zones.") },
>> +	{ "open",	blkzone_action, N_("Open a range of zones.") },
>> +	{ "close",	blkzone_action, N_("Close a range of zones.") },
>> +	{ "finish",	blkzone_action, N_("Set a range of zones to Full.") }
>> +};
>> +
>> +/*
>> + * The action values must match the command index in the command array.
>> + */
>> +enum blkzone_action {
>> +	BLK_ZONE_NO_ACTION = 0,
>> +	BLK_ZONE_RESET,
>> +	BLK_ZONE_OPEN,
>> +	BLK_ZONE_CLOSE,
>> +	BLK_ZONE_FINISH,
>>  };
> 
> If you add ioctl_cmd and ioctl_name to the struct blkzone_command,
> and you define commands[] as:
> 
>  commands[] = {
>     { 
>         .name = "report",
>         .handler = blkzone_report,
>         .help = N_("Report zone information about the given device") 
>     },{
>         .name = "reset",
>         .handler = blkzone_action,
>         .help = N_("Reset a range of zones."),
>         .ioctl_cmd = BLKRESETZONE,
>         .ioctl_name = "BLKRESETZONE"
>     },
>     ...
>  };
> 
> than you do not need this switch() and command_action()
> 
>>  
>> +	switch (command_action(ctl->command)) {
>> +	case BLK_ZONE_RESET:
>> +		ioctl_cmd = BLKRESETZONE;
>> +		ioctl_name = "BLKRESETZONE";
>> +		break;
>> +	case BLK_ZONE_OPEN:
>> +		ioctl_cmd = BLKOPENZONE;
>> +		ioctl_name = "BLKOPENZONE";
>> +		break;
>> +	case BLK_ZONE_CLOSE:
>> +		ioctl_cmd = BLKCLOSEZONE;
>> +		ioctl_name = "BLKCLOSEZONE";
>> +		break;
>> +	case BLK_ZONE_FINISH:
>> +		ioctl_cmd = BLKFINISHZONE;
>> +		ioctl_name = "BLKFINISHZONE";
>> +		break;
>> +	case BLK_ZONE_NO_ACTION:
>> +		/* fallthrough */
>> +	default:
>> +		errx(EXIT_FAILURE, _("Invalid zone action"));
>   
> but you can use ctl->command.ioclt_cmd  etc.
> 
> Just idea :-)

OK. Sending a v3 !

> 
>  Karel
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] blkzone: add open/close/finish commands

[Damien Le Moal]

On 2020/03/25 20:36, Karel Zak wrote:
> On Tue, Mar 24, 2020 at 07:02:17PM +0900, Damien Le Moal wrote:
>>  sys-utils/blkzone.8 | 27 +++++++++++++++--
>>  sys-utils/blkzone.c | 74 ++++++++++++++++++++++++++++++++++++++++-----
>>  2 files changed, 91 insertions(+), 10 deletions(-)
> 
> Applied, thanks.
> 
>>  static const struct blkzone_command commands[] = {
>>  	{ "report",	blkzone_report, N_("Report zone information about the given device") },
>> -	{ "reset",	blkzone_reset,  N_("Reset a range of zones.") }
>> +	{ "reset",	blkzone_action, N_("Reset a range of zones.") },
>> +	{ "open",	blkzone_action, N_("Open a range of zones.") },
>> +	{ "close",	blkzone_action, N_("Close a range of zones.") },
>> +	{ "finish",	blkzone_action, N_("Set a range of zones to Full.") }
>> +};
>> +
>> +/*
>> + * The action values must match the command index in the command array.
>> + */
>> +enum blkzone_action {
>> +	BLK_ZONE_NO_ACTION = 0,
>> +	BLK_ZONE_RESET,
>> +	BLK_ZONE_OPEN,
>> +	BLK_ZONE_CLOSE,
>> +	BLK_ZONE_FINISH,
>>  };
> 
> If you add ioctl_cmd and ioctl_name to the struct blkzone_command,
> and you define commands[] as:
> 
>  commands[] = {
>     { 
>         .name = "report",
>         .handler = blkzone_report,
>         .help = N_("Report zone information about the given device") 
>     },{
>         .name = "reset",
>         .handler = blkzone_action,
>         .help = N_("Reset a range of zones."),
>         .ioctl_cmd = BLKRESETZONE,
>         .ioctl_name = "BLKRESETZONE"
>     },
>     ...
>  };
> 
> than you do not need this switch() and command_action()
> 
>>  
>> +	switch (command_action(ctl->command)) {
>> +	case BLK_ZONE_RESET:
>> +		ioctl_cmd = BLKRESETZONE;
>> +		ioctl_name = "BLKRESETZONE";
>> +		break;
>> +	case BLK_ZONE_OPEN:
>> +		ioctl_cmd = BLKOPENZONE;
>> +		ioctl_name = "BLKOPENZONE";
>> +		break;
>> +	case BLK_ZONE_CLOSE:
>> +		ioctl_cmd = BLKCLOSEZONE;
>> +		ioctl_name = "BLKCLOSEZONE";
>> +		break;
>> +	case BLK_ZONE_FINISH:
>> +		ioctl_cmd = BLKFINISHZONE;
>> +		ioctl_name = "BLKFINISHZONE";
>> +		break;
>> +	case BLK_ZONE_NO_ACTION:
>> +		/* fallthrough */
>> +	default:
>> +		errx(EXIT_FAILURE, _("Invalid zone action"));
>   
> but you can use ctl->command.ioclt_cmd  etc.
> 
> Just idea :-)

I just sent a V3 of the full patch with this cleanup.
Did you prefer an incremental change ?
(I did not see that you said you applied v2 above... Thanks !)


> 
>  Karel
> 


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] blkzone: add open/close/finish commands

[Karel Zak]

On Wed, Mar 25, 2020 at 03:12:04PM +0000, Damien Le Moal wrote:
> I just sent a V3 of the full patch with this cleanup.
> Did you prefer an incremental change ?
> (I did not see that you said you applied v2 above... Thanks !)

No problem, I'll generate the diff locally. Thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com