On 1/13/20 4:30 AM, Andrey Shinkevich wrote:
- c = getopt_long(argc, argv, ":hf:r:T:qU",
+ c = getopt_long(argc, argv, ":hf:r:T:qU:M",
We are already inconsistent, but I tend to add options in alphabetical
order, both here...
If I merely move 'M' forward like ':hf:M:r:T:qU', will it be OK?
If you don't mind writing a pre-requisite patch that sorts the existing
options, then the patch adding your option in sorted order is easy. But
that's asking you to do extra work, which I'm not going to insist on, so
I can also live with your patch being in any order as it is no worse
than existing code and anyone that wants to do a cleanup patch to sort
things has roughly the same level of effort whether or not your patch
without sorting lands in the meantime.
+ if ((fix & BDRV_DUMP_META) && output_format != OFORMAT_JSON) {
+ error_report("Metadata output in JSON format only");
+ return 1;
Why this restriction?
This is to remind a user that '-M' can be effective with the option
'--output=json' only. Do you think that a comment in the qemu-img.texi
would be enough and the restriction should be omitted here?
Rather, why can't we come up with some sort of sane human output, so
that we don't have to limit the flag to just --output=json?
+++ b/qemu-img.texi
@@ -230,7 +230,7 @@ specified as well.
For write tests, by default a buffer filled with zeros is written.
This can be
overridden with a pattern byte specified by @var{pattern}.
-@item check [--object @var{objectdef}] [--image-opts] [-q] [-f
@var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T
@var{src_cache}] [-U] @var{filename}
+@item check [--object @var{objectdef}] [--image-opts] [-M] [-q] [-f
@var{fmt}] [--output=@var{ofmt}] [-r [leaks | all]] [-T
@var{src_cache}] [-U] @var{filename}
This mentions that -M is valid, but has no further documentation on what
-M means. Without that, it's anyone's guess.
Thank you Eric, I really missed to supply a comment for the new option
here and am going to put it below. Should I mention that option in
qapi/block-core.json file also with this patch of the series?
Mentioning that the qapi type exists to facilitate a qemu-img option
might not hurt. But more important is that the qemu-img documentation
mentions what -M does; that documentation can point to the qapi docs for
how the output will be structured when --output=json is in effect.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
