[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V7 2/2] qemu-img: Add json output option to the
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH V7 2/2] qemu-img: Add json output option to the info command. |
Date: |
Wed, 05 Sep 2012 11:41:06 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
Am 05.09.2012 11:25, schrieb Benoît Canet:
> This option --output=[human|json] make qemu-img info output on
> human or JSON representation at the choice of the user.
>
> example:
> {
> "snapshots": [
> {
> "vm-clock-nsec": 637102488,
> "name": "vm-20120821145509",
> "date-sec": 1345553709,
> "date-nsec": 220289000,
> "vm-clock-sec": 20,
> "id": "1",
> "vm-state-size": 96522745
> },
> {
> "vm-clock-nsec": 28210866,
> "name": "vm-20120821154059",
> "date-sec": 1345556459,
> "date-nsec": 171392000,
> "vm-clock-sec": 46,
> "id": "2",
> "vm-state-size": 101208714
> }
> ],
> "virtual-size": 1073741824,
> "filename": "snap.qcow2",
> "cluster-size": 65536,
> "format": "qcow2",
> "actual-size": 985587712,
> "dirty-flag": false
> }
>
> Signed-off-by: Benoit Canet <address@hidden>
Looks much better, but I still have a few comments:
> + bdrv_get_backing_filename(bs, backing_filename,
> sizeof(backing_filename));
> + if (backing_filename[0] != '\0') {
> + info->backing_filename = g_strdup(backing_filename);
> + info->has_backing_filename = true;
> + bdrv_get_full_backing_filename(bs, backing_filename2,
> + sizeof(backing_filename2));
> +
> + if (strcmp(backing_filename, backing_filename2) != 0) {
> + info->full_backing_filename =
> + g_strdup(backing_filename2);
> + info->has_full_backing_filename = true;
> + }
> +
> + info->backing_filename_format = bs->backing_format;
> + info->has_backing_filename_format = true;
I believe we should check bs->backing_format[0] first. If it's empty,
don't include the format.
> + }
> +}
> +
> +static void dump_human_image_info(ImageInfo *info)
> +{
> + char size_buf[128], dsize_buf[128];
> + if (!info->has_actual_size) {
> + snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
> + } else {
> + get_human_readable_size(dsize_buf, sizeof(dsize_buf),
> + info->actual_size);
> + }
> + get_human_readable_size(size_buf, sizeof(size_buf), info->virtual_size);
> + printf("image: %s\n"
> + "file format: %s\n"
> + "virtual size: %s (%" PRId64 " bytes)\n"
> + "disk size: %s\n",
> + info->filename, info->format, size_buf,
> + info->virtual_size,
> + dsize_buf);
> +
> + if (info->has_encrypted && info->encrypted) {
> + printf("encrypted: yes\n");
> + }
> +
> + if (info->has_cluster_size) {
> + printf("cluster_size: %" PRId64 "\n", info->cluster_size);
> + }
> +
> + if (info->has_dirty_flag && info->dirty_flag) {
> + printf("cleanly shut down: no\n");
> + }
> +
> + if (info->has_backing_filename) {
> + printf("backing file: %s", info->backing_filename);
> + if (info->has_full_backing_filename) {
> + printf(" (actual path: %s)", info->full_backing_filename);
> + }
Maybe add the backing file format here if it's available?
> @@ -1135,48 +1291,36 @@ static int img_info(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;
> + }
> +
> + info = g_new0(ImageInfo, 1);
> bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
> if (!bs) {
> + g_free(info);
> return 1;
> }
If you move thee g_new0 below, you don't have to free here.
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 6b42e35..75a7c8b 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -129,12 +129,13 @@ created as a copy on write image of the specified base
> image; the
> @var{backing_file} should have the same content as the input's base image,
> however the path, image format, etc may differ.
>
> address@hidden info [-f @var{fmt}] @var{filename}
> address@hidden info [-f @var{fmt}] address@hidden @var{filename}
>
> Give information about the disk image @var{filename}. Use it in
> particular to know the size reserved on disk which can be different
> from the displayed size. If VM snapshots are stored in the disk image,
> -they are displayed too.
> +they are displayed too. The command can output in the format @var{ofmt}
> +which is either human or json.
"address@hidden or @code{json}" is better, I think.
Kevin