From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4Btk-0000VO-BV for qemu-devel@nongnu.org; Wed, 22 Aug 2012 10:27:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4Btd-0001FE-C2 for qemu-devel@nongnu.org; Wed, 22 Aug 2012 10:27:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55564) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4Btd-0001F0-3s for qemu-devel@nongnu.org; Wed, 22 Aug 2012 10:27:01 -0400 Message-ID: <5034EC26.2070604@redhat.com> Date: Wed, 22 Aug 2012 08:26:46 -0600 From: Eric Blake MIME-Version: 1.0 References: <1345639535-8822-1-git-send-email-benoit@irqsave.net> <1345639535-8822-3-git-send-email-benoit@irqsave.net> In-Reply-To: <1345639535-8822-3-git-send-email-benoit@irqsave.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig66DEFEB4329518A1A464DBD4" Subject: Re: [Qemu-devel] [PATCH V4 2/2] qemu-img: Add json output option to the info command. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= Cc: aliguori@us.ibm.com, stefanha@linux.vnet.ibm.com, qemu-devel@nongnu.org, pbonzini@redhat.com, xiawenc@linux.vnet.ibm.com, =?UTF-8?B?QmVub8OudCBDYW5ldA==?= This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig66DEFEB4329518A1A464DBD4 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08/22/2012 06:45 AM, Beno=C3=AEt Canet wrote: > This option --output=3D[human|json] make qemu-img info output on > human or JSON representation at the choice of the user. >=20 > @@ -1083,7 +1088,6 @@ out: > return 0; > } > =20 > - > static void dump_snapshots(BlockDriverState *bs) > { Spurious whitespace change. > +static int img_info(int argc, char **argv) > +{ > + int c; > + bool human =3D false, json =3D false; > + const char *filename, *fmt, *output; > + BlockDriverState *bs; > + ImageInfo *info; > =20 > fmt =3D NULL; > + output =3D NULL; > for(;;) { > - c =3D getopt(argc, argv, "f:h"); > + int option_index =3D 0; > + static struct option long_options[] =3D { > + {"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=3D256};', so that= you don't risk future confusion... > + {0, 0, 0, 0} > + }; > + c =3D getopt_long(argc, argv, "f:h", =2E..if we later do add an 'm' short option. > + long_options, &option_index); > if (c =3D=3D -1) { > break; > } > @@ -1128,6 +1299,9 @@ static int img_info(int argc, char **argv) > case 'f': > fmt =3D 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 =3D argv[optind++]; > =20 > + if (output && !strncmp(output, "json", strlen("json"))) { Why strncmp? It ignores trailing garbage (as in --output=3Djsonoops). Stick to strcmp. > + json =3D true; > + } else if (output && !strncmp(output, "human", strlen("human"))) {= And again. > + human =3D true; > + } else { > + fprintf(stderr, > + "Error: --output must be used with human or json as argume= nt.\n"); > + return 1; > + } If we get here, and --output=3D... was not given, then both human and jso= n 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_fil= ename)); > - if (backing_filename[0] !=3D '\0') { > - bdrv_get_full_backing_filename(bs, backing_filename2, > - sizeof(backing_filename2)); > - printf("backing file: %s", backing_filename); > - if (strcmp(backing_filename, backing_filename2) !=3D 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); =2E..you will end up with no output. You want to default to human output= for back-compat to older usage. --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig66DEFEB4329518A1A464DBD4 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBCAAGBQJQNOwnAAoJEKeha0olJ0Nqz6YH/iuLW6Z+v/UJxPcnpBJUAvDS LyqUNVZGBlpnmVt9Pq+eQfddcHWmvrVliRv5tCQIjC2b2moD2wJ25zAOJnLFS221 HW0bTlEZcBaRonsmHJsNPc/S977y88BlXrWnc3PkqsHfdrLxOiGvd/LVRQsEf02r 3I2DpT6fTovkz+qdakIyRQmG25TabSXzbOCCT0A8uhntZMbS2n2H/ImGpB+Q23le DSxFlYkcx7jkBCoghCD+ZJML78+Ihut3WyhcN6FtdrCVOhe+7ghXIlP+cyMe8E1x J4xwaClFGfdTk3qCpm/IIB/vpk06IKe6aOOI6gOUNWCfnO1GxwDyCBbOxAKsAGc= =oO8M -----END PGP SIGNATURE----- --------------enig66DEFEB4329518A1A464DBD4--