[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V4 2/2] qemu-img: Add json output option to the
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH V4 2/2] qemu-img: Add json output option to the info command. |
Date: |
Wed, 22 Aug 2012 08:26:46 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0 |
On 08/22/2012 06:45 AM, Benoît Canet wrote:
> This option --output=[human|json] make qemu-img info output on
> human or JSON representation at the choice of the user.
>
> @@ -1083,7 +1088,6 @@ out:
> return 0;
> }
>
> -
> static void dump_snapshots(BlockDriverState *bs)
> {
Spurious whitespace change.
> +static int img_info(int argc, char **argv)
> +{
> + int c;
> + bool human = false, json = false;
> + const char *filename, *fmt, *output;
> + BlockDriverState *bs;
> + ImageInfo *info;
>
> fmt = NULL;
> + output = NULL;
> for(;;) {
> - c = getopt(argc, argv, "f:h");
> + int option_index = 0;
> + static struct option long_options[] = {
> + {"help", no_argument, 0, 'h'},
> + {"format", required_argument, 0, 'f'},
> + {"output", required_argument, 0, 'm'},
I would define a constant, such as 'enum {OPTION_FORMAT=256};', so that
you don't risk future confusion...
> + {0, 0, 0, 0}
> + };
> + c = getopt_long(argc, argv, "f:h",
...if we later do add an 'm' short option.
> + long_options, &option_index);
> if (c == -1) {
> break;
> }
> @@ -1128,6 +1299,9 @@ static int img_info(int argc, char **argv)
> case 'f':
> fmt = optarg;
> break;
> + case 'm':
Given the above, this would reuse your new named value.
> @@ -1135,52 +1309,42 @@ static int img_info(int argc, char **argv)
> }
> filename = argv[optind++];
>
> + if (output && !strncmp(output, "json", strlen("json"))) {
Why strncmp? It ignores trailing garbage (as in --output=jsonoops).
Stick to strcmp.
> + json = true;
> + } else if (output && !strncmp(output, "human", strlen("human"))) {
And again.
> + human = true;
> + } else {
> + fprintf(stderr,
> + "Error: --output must be used with human or json as
> argument.\n");
> + return 1;
> + }
If we get here, and --output=... was not given, then both human and json
are false. That's a problem, since...
> +
> + if (human) {
> + dump_human_image_info(info);
> + dump_snapshots(bs);
> }
> - bdrv_get_backing_filename(bs, backing_filename,
> sizeof(backing_filename));
> - if (backing_filename[0] != '\0') {
> - bdrv_get_full_backing_filename(bs, backing_filename2,
> - sizeof(backing_filename2));
> - printf("backing file: %s", backing_filename);
> - if (strcmp(backing_filename, backing_filename2) != 0) {
> - printf(" (actual path: %s)", backing_filename2);
> - }
> - putchar('\n');
> +
> + if (json) {
> + collect_snapshots(bs, info);
> + dump_json_image_info(info);
> }
> - dump_snapshots(bs);
> +
> + qapi_free_ImageInfo(info);
...you will end up with no output. You want to default to human output
for back-compat to older usage.
--
Eric Blake address@hidden +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature