From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: Hans Holmberg <Hans.Holmberg@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:45:25 +0000 [thread overview]
Message-ID: <CO2PR04MB23435FDC9F9E3689CB663CB3E7F10@CO2PR04MB2343.namprd04.prod.outlook.com> (raw)
In-Reply-To: 20200324090622.GA5373@gsv
On 2020/03/24 18:06, Hans Holmberg wrote:
> 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>
Oops. I copy pasted and did not notice your typo :)
>
>
>
>> 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"),
OK. Will send a v2.
>
> Otherwise:
>
> Reviewed-by: Hans Holmberg <Hans.Holmberg@wdc.com>
>
>> close(fd);
>> --
>> 2.25.1
>>
--
Damien Le Moal
Western Digital Research
prev parent reply other threads:[~2020-03-24 9:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 5:30 [PATCH] blkzone: add open/close/finish commands Damien Le Moal
2020-03-24 9:06 ` Hans Holmberg
2020-03-24 9:45 ` Damien Le Moal [this message]
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=CO2PR04MB23435FDC9F9E3689CB663CB3E7F10@CO2PR04MB2343.namprd04.prod.outlook.com \
--to=damien.lemoal@wdc.com \
--cc=Aravind.Ramesh@wdc.com \
--cc=Hans.Holmberg@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).