Stefan Hajnoczi <stefa...@redhat.com> 于2022年6月28日周二 16:20写道: > > 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?
I'll check if (ret<0) in zone_report and other commands in this patch as well. > > > + 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. Noted. It is necessary. > > > + 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. Thanks for reviewing! Sam