[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v3 2/5] qemu-io: add zoned block device operations.
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC v3 2/5] qemu-io: add zoned block device operations. |
Date: |
Tue, 28 Jun 2022 08:56:51 +0100 |
On Mon, Jun 27, 2022 at 08:19:14AM +0800, Sam Li wrote:
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 2f0d8ac25a..3f2592b9f5 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1706,6 +1706,122 @@ static const cmdinfo_t flush_cmd = {
> .oneline = "flush all in-core file state to disk",
> };
>
> +static int zone_report_f(BlockBackend *blk, int argc, char **argv)
> +{
> + int ret;
> + int64_t offset, len, nr_zones;
> + int i = 0;
> +
> + ++optind;
> + offset = cvtnum(argv[optind]);
> + ++optind;
> + len = cvtnum(argv[optind]);
> + ++optind;
> + nr_zones = cvtnum(argv[optind]);
> +
> + g_autofree BlockZoneDescriptor *zones = g_new(BlockZoneDescriptor,
> nr_zones);
> + ret = blk_zone_report(blk, offset, len, &nr_zones, zones);
> + while (i < nr_zones) {
Does blk_zone_report() set nr_zones to 0 on failure or do we need to
check if (ret < 0) here?
> + fprintf(stdout, "start: 0x%lx, len 0x%lx, cap 0x%lx, wptr 0x%lx, "
The rest of the source file uses printf() instead of fprintf(stdout,
...). That's usually preferred because it's shorter.
> + "zcond:%u, [type: %u]\n",
Please use PRIx64 instead of lx format specifiers for portability. On
32-bit hosts lx is 32-bit, not 64-bit. You can grep QEMU's code for
examples of PRIx64.
> + zones[i].start, zones[i].length, zones[i].cap, zones[i].wp,
> + zones[i].cond, zones[i].type);
> + ++i;
> + }
A for loop is more idiomatic:
for (int i = 0; i < nr_zones; i++) {
...
}
> + return ret;
> +}
> +
> +static const cmdinfo_t zone_report_cmd = {
> + .name = "zone_report",
> + .altname = "f",
> + .cfunc = zone_report_f,
> + .argmin = 3,
> + .argmax = 3,
> + .args = "offset [offset..] len [len..] number [num..]",
The arguments are "offset len number". This command does not accept
optional offset/len/num arguments.
> + .oneline = "report a number of zones",
Maybe "report zone information".
> +};
> +
> +static int zone_open_f(BlockBackend *blk, int argc, char **argv)
> +{
> + int64_t offset, len;
> + ++optind;
> + offset = cvtnum(argv[optind]);
> + ++optind;
> + len = cvtnum(argv[optind]);
> + return blk_zone_mgmt(blk, zone_open, offset, len);
Where is the error reported? When I look at read_f() I see:
if (ret < 0) {
printf("read failed: %s\n", strerror(-ret));
I think something similar is needed because qemu-io.c does not print an
error message for us. The same is true for the other commands defined in
this patch.
> +}
> +
> +static const cmdinfo_t zone_open_cmd = {
> + .name = "zone_open",
> + .altname = "f",
> + .cfunc = zone_open_f,
> + .argmin = 2,
> + .argmax = 2,
> + .args = "offset [offset..] len [len..]",
There are no optional offset/len args. The same is true for the other
commands below.
signature.asc
Description: PGP signature
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., (continued)
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., Sam Li, 2022/06/28
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., Damien Le Moal, 2022/06/28
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., Sam Li, 2022/06/28
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., Damien Le Moal, 2022/06/28
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., Sam Li, 2022/06/28
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., Damien Le Moal, 2022/06/28
- Re: [RFC v3 1/5] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls., Sam Li, 2022/06/28
[RFC v3 2/5] qemu-io: add zoned block device operations., Sam Li, 2022/06/26
[RFC v3 3/5] file-posix: introduce get_sysfs_long_val for zoned device information., Sam Li, 2022/06/26
[RFC v3 4/5] file-posix: introduce get_sysfs_str_val for device zoned model., Sam Li, 2022/06/26