Util-Linux Archive on lore.kernel.org
 help / color / Atom feed
From: Hans Holmberg <Hans.Holmberg@wdc.com>
To: Damien Le Moal <Damien.LeMoal@wdc.com>
Cc: "util-linux@vger.kernel.org" <util-linux@vger.kernel.org>,
	Karel Zak <kzak@redhat.com>,
	Benno Schulenberg <bensberg@justemail.net>,
	Aravind Ramesh <Aravind.Ramesh@wdc.com>
Subject: Re: [PATCH] blkzone: add open/close/finish commands
Date: Tue, 24 Mar 2020 09:06:26 +0000
Message-ID: <20200324090622.GA5373@gsv> (raw)
In-Reply-To: <20200324053018.18878-1-damien.lemoal@wdc.com>

On Tue, Mar 24, 2020 at 02:30:18PM +0900, Damien Le Moal wrote:
> 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>
> Tested-by: Hans Holmnerg <Hans.Holmberg@wdc.com>

I should be old enough to spell my name right by now.

Tested-by: Hans Holmberg <Hans.Holmberg@wdc.com>



> Signed-off-by: Damien Le Moal <damien.lemoal@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..b862aa67d 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: successfully %s zones in range from %" PRIu64 ", to %" PRIu64),
>  			ctl->devname,
> +			ctl->command->name,
>  			ctl->offset,
>  			ctl->offset + zlen);

Nice, but the verbose output looks wierd:

root@qemu:~/repos/util-linux# ./blkzone  close -v -o 0 -c 1 /dev/nvme0n1 
/dev/nvme0n1: successfully close zones in range from 0, to 524288root@qemu:~/repos/util-linux#

Something like this instead?

		printf(_("%s: successfull %s of zones in range from %" PRIu64 ", to %" PRIu64 "\n"),

Otherwise:

Reviewed-by: Hans Holmberg <Hans.Holmberg@wdc.com>

>  	close(fd);
> -- 
> 2.25.1
> 

  reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24  5:30 Damien Le Moal
2020-03-24  9:06 ` Hans Holmberg [this message]
2020-03-24  9:45   ` Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200324090622.GA5373@gsv \
    --to=hans.holmberg@wdc.com \
    --cc=Aravind.Ramesh@wdc.com \
    --cc=Damien.LeMoal@wdc.com \
    --cc=bensberg@justemail.net \
    --cc=kzak@redhat.com \
    --cc=util-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Util-Linux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/util-linux/0 util-linux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 util-linux util-linux/ https://lore.kernel.org/util-linux \
		util-linux@vger.kernel.org
	public-inbox-index util-linux

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.util-linux


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git