On 01/21/2013 07:25 AM, Federico Simoncelli wrote: > This option --output=[human|json] make qemu-img check output an human
s/make/makes/; s/an human/a human/ > or JSON representation at the choice of the user. > > Signed-off-by: Federico Simoncelli <fsimo...@redhat.com> > --- > qapi-schema.json | 46 +++++++++++ > qemu-img-cmds.hx | 4 +- > qemu-img.c | 232 +++++++++++++++++++++++++++++++++++++++-------------- > qemu-img.texi | 5 +- > 4 files changed, 221 insertions(+), 66 deletions(-) > > +++ b/qemu-img.c > @@ -42,6 +42,16 @@ typedef struct img_cmd_t { > int (*handler)(int argc, char **argv); > } img_cmd_t; > > +enum { > + OPTION_OUTPUT = 256, > + OPTION_BACKING_CHAIN = 257, > +}; > + > +typedef enum OutputFormat { > + OFORMAT_JSON, > + OFORMAT_HUMAN, > +} OutputFormat; I think it may be worth splitting this into two patches; one to refactor the existing code (hoist definitions, create dump_human_image_check) with no semantic change, and then a second to introduce the new json handling. But I'm not the maintainer, so I won't insist on it if others think it is okay as-is. > +static void dump_human_image_check(ImageCheck *check) > +{ > + if (!(check->corruptions || check->leaks || check->check_errors)) { > + printf("No errors were found on the image.\n"); Pre-existing, but I think 'in the image' sounds nicer than 'on the image'. > @@ -424,77 +538,81 @@ static int img_check(int argc, char **argv) > } > filename = argv[optind++]; > > + if (output && !strcmp(output, "json")) { > + output_format = OFORMAT_JSON; > + } else if (output && !strcmp(output, "human")) { > + output_format = OFORMAT_HUMAN; > + } else if (output) { > + error_report("--output must be used with human or json as > argument."); > + return 1; > + } Is this chunk of code something that should be reused via a helper function, rather than copied and pasted among different subcommands? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature