From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1qWh-0006DU-Vt for qemu-devel@nongnu.org; Wed, 15 Aug 2012 23:13:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T1qWf-0002LF-T7 for qemu-devel@nongnu.org; Wed, 15 Aug 2012 23:13:39 -0400 Received: from e28smtp05.in.ibm.com ([122.248.162.5]:33588) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T1qWe-0002L8-VK for qemu-devel@nongnu.org; Wed, 15 Aug 2012 23:13:37 -0400 Received: from /spool/local by e28smtp05.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 16 Aug 2012 08:43:34 +0530 Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7G3DV6U38273118 for ; Thu, 16 Aug 2012 08:43:31 +0530 Received: from d28av01.in.ibm.com (loopback [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7G8h5xr010813 for ; Thu, 16 Aug 2012 14:13:05 +0530 Message-ID: <502C6541.60308@linux.vnet.ibm.com> Date: Thu, 16 Aug 2012 11:13:05 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1345056523-10249-1-git-send-email-benoit@irqsave.net> <1345056523-10249-3-git-send-email-benoit@irqsave.net> In-Reply-To: <1345056523-10249-3-git-send-email-benoit@irqsave.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V3 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, lcapitulino@redhat.com, pbonzini@redhat.com, eblake@redhat.com, =?UTF-8?B?QmVub8OudCBDYW5ldA==?= I was busy in coding libqblock so forgot to move forward on qemu json info patches, thanks for your advance. one suggestion: how about folder all human printf into function: dump_human_image_info(image_info), and at the end, make the final output functions as mirror: if (human) { dump_human_image_info(image_info); } else { dump_json_image_info(image_info); } With this, information collecting and output generating were separated, it will be easy to do more modification if we want. for eg, if we want to get the string generated and write it to another place or pipe, we would only need to modify dump_human_image_info() > This additionnal --machine=json option make qemu-img info output on > stdout a JSON formated representation of the image informations. > > --machine=json was choosen instead of --format=json because the > info command already have a -f parameter. > > example: > { > "backing-filename-format": "raw", > "snapshots": [ > { > "vm-clock-nsec": 4647590161, > "name": "truc", > "date-sec": 1345034924, > "date-nsec": 870405000, > "id": "1", > "vm-state-size": 80805256 > }, > { > "vm-clock-nsec": 7065282574, > "name": "onx", > "date-sec": 1345034927, > "date-nsec": 914633000, > "id": "2", > "vm-state-size": 75927370 > }, > { > "vm-clock-nsec": 10108695046, > "name": "list", > "date-sec": 1345034939, > "date-nsec": 39119000, > "id": "3", > "vm-state-size": 75890567 > } > ], > "virtual-size": 1073741824, > "filename": "./snapshot.img", > "cluster-size": 65536, > "format": "qcow2", > "actual-size": 242614272, > "backing-filename": "truc5.raw", > "dirty-flag": false > } > > Signed-off-by: Benoit Canet > --- > Makefile | 3 +- > qemu-img.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----- > 2 files changed, 124 insertions(+), 12 deletions(-) > > diff --git a/Makefile b/Makefile > index ab82ef3..9ba064b 100644 > --- a/Makefile > +++ b/Makefile > @@ -160,7 +160,8 @@ tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o qemu-timer.o \ > iohandler.o cutils.o iov.o async.o > tools-obj-$(CONFIG_POSIX) += compatfd.o > > -qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) > +qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) $(qapi-obj-y) \ > + qapi-visit.o qapi-types.o > qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y) > qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) > > diff --git a/qemu-img.c b/qemu-img.c > index 80cfb9b..5e20f5d 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -21,12 +21,16 @@ > * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > * THE SOFTWARE. > */ > +#include "qapi-visit.h" > +#include "qapi/qmp-output-visitor.h" > +#include "qjson.h" > #include "qemu-common.h" > #include "qemu-option.h" > #include "qemu-error.h" > #include "osdep.h" > #include "sysemu.h" > #include "block_int.h" > +#include > #include > > #ifdef _WIN32 > @@ -84,6 +88,7 @@ static void help(void) > " '-p' show progress of command (only certain commands)\n" > " '-S' indicates the consecutive number of bytes that must contain only zeros\n" > " for qemu-img to create a sparse image during conversion\n" > + " '-m' or '--machine' takes the format in which the output must be done (json)\n" > "\n" > "Parameters to check subcommand:\n" > " '-r' tries to repair any inconsistencies that are found during the check.\n" > @@ -1102,21 +1107,86 @@ static void dump_snapshots(BlockDriverState *bs) > g_free(sn_tab); > } > > +static void collect_snapshots(BlockDriverState *bs , ImageInfo *image_info) > +{ > + int i, sn_count; > + QEMUSnapshotInfo *sn_tab = NULL; > + SnapshotInfoList *sn_info_list, *cur_item = NULL; > + sn_count = bdrv_snapshot_list(bs, &sn_tab); > + > + for (i = 0; i < sn_count; i++) { > + image_info->has_snapshots = true; > + sn_info_list = g_new0(SnapshotInfoList, 1); > + > + sn_info_list->value = g_new0(SnapshotInfo, 1); > + sn_info_list->value->id = g_strdup(sn_tab[i].id_str); > + sn_info_list->value->name = g_strdup(sn_tab[i].name); > + sn_info_list->value->vm_state_size = sn_tab[i].vm_state_size; > + sn_info_list->value->date_sec = sn_tab[i].date_sec; > + sn_info_list->value->date_nsec = sn_tab[i].date_nsec; > + sn_info_list->value->vm_clock_nsec = sn_tab[i].vm_clock_nsec; > + > + /* XXX: waiting for the qapi to support GSList */ > + if (!cur_item) { > + image_info->snapshots = cur_item = sn_info_list; > + } else { > + cur_item->next = sn_info_list; > + cur_item = sn_info_list; > + } > + > + } > + > + g_free(sn_tab); > +} > + > +static void dump_json_image_info(ImageInfo *image_info) > +{ > + Error *errp = NULL; > + QString *str; > + QmpOutputVisitor *ov = qmp_output_visitor_new(); > + QObject *obj; > + visit_type_ImageInfo(qmp_output_get_visitor(ov), > + &image_info, NULL, &errp); > + obj = qmp_output_get_qobject(ov); > + str = qobject_to_json_pretty(obj); > + assert(str != NULL); > + printf("%s\n", qstring_get_str(str)); > + qobject_decref(obj); > + qmp_output_visitor_cleanup(ov); > + QDECREF(str); > +} > + > +#define PRINTH(human, args...) do { \ > + if (human) { \ > + printf(args); \ > + } } while (0); > + > static int img_info(int argc, char **argv) > { > int c; > - const char *filename, *fmt; > - BlockDriverState *bs; > + bool human = true; > + const char *filename, *fmt, *machine; > + BlockDriverState *bs, *backing_bs = NULL; > char size_buf[128], dsize_buf[128]; > uint64_t total_sectors; > int64_t allocated_size; > char backing_filename[1024]; > char backing_filename2[1024]; > BlockDriverInfo bdi; > + ImageInfo *image_info; > > fmt = NULL; > + machine = 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'}, > + {"machine", required_argument, 0, 'm'}, > + {0, 0, 0, 0} > + }; > + c = getopt_long(argc, argv, "f:h", > + long_options, &option_index); > if (c == -1) { > break; > } > @@ -1128,6 +1198,9 @@ static int img_info(int argc, char **argv) > case 'f': > fmt = optarg; > break; > + case 'm': > + machine = optarg; > + break; > } > } > if (optind >= argc) { > @@ -1135,8 +1208,14 @@ static int img_info(int argc, char **argv) > } > filename = argv[optind++]; > > + image_info = g_new0(ImageInfo, 1); > + if (machine && !strncmp(machine, "json", strlen("json"))) { > + human = false; > + } > + > bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING); > if (!bs) { > + g_free(image_info); > return 1; > } > bdrv_get_geometry(bs, &total_sectors); > @@ -1148,39 +1227,71 @@ static int img_info(int argc, char **argv) > get_human_readable_size(dsize_buf, sizeof(dsize_buf), > allocated_size); > } > - printf("image: %s\n" > + PRINTH(human, "image: %s\n" > "file format: %s\n" > "virtual size: %s (%" PRId64 " bytes)\n" > "disk size: %s\n", > filename, bdrv_get_format_name(bs), size_buf, > (total_sectors * 512), > dsize_buf); > + image_info->filename = g_strdup(filename); > + image_info->format = g_strdup(bdrv_get_format_name(bs)); > + image_info->virtual_size = total_sectors * 512; > + image_info->actual_size = allocated_size >= 0 ? allocated_size : 0; > if (bdrv_is_encrypted(bs)) { > - printf("encrypted: yes\n"); > + PRINTH(human, "encrypted: yes\n"); > + image_info->encrypted = true; > + image_info->has_encrypted = true; > } > if (bdrv_get_info(bs, &bdi) >= 0) { > + image_info->has_cluster_size = true; > + image_info->has_dirty_flag = true; > if (bdi.cluster_size != 0) { > - printf("cluster_size: %d\n", bdi.cluster_size); > + PRINTH(human, "cluster_size: %d\n", bdi.cluster_size); > + image_info->cluster_size = bdi.cluster_size; > } > if (bdi.is_dirty) { > - printf("cleanly shut down: no\n"); > + PRINTH(human, "cleanly shut down: no\n"); > + image_info->dirty_flag = true; > + } else { > + image_info->dirty_flag = false; > } > } > 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); > + backing_bs = bdrv_new_open(backing_filename2, fmt, > + BDRV_O_FLAGS | BDRV_O_NO_BACKING); > + image_info->backing_filename = g_strdup(backing_filename); > + image_info->backing_filename_format = > + g_strdup(bdrv_get_format_name(backing_bs)); > + bdrv_delete(backing_bs); > + PRINTH(human, "backing file: %s", backing_filename); > if (strcmp(backing_filename, backing_filename2) != 0) { > - printf(" (actual path: %s)", backing_filename2); > + PRINTH(human, " (actual path: %s)", backing_filename2); > + } > + if (human) { > + putchar('\n'); > } > - putchar('\n'); > + image_info->has_backing_filename = true; > + image_info->has_backing_filename_format = true; > } > - dump_snapshots(bs); > + > + if (human) { > + dump_snapshots(bs); > + } else { > + collect_snapshots(bs, image_info); > + dump_json_image_info(image_info); > + } > + > + qapi_free_ImageInfo(image_info); > bdrv_delete(bs); > return 0; > } > > +#undef PRINTH > + > #define SNAPSHOT_LIST 1 > #define SNAPSHOT_CREATE 2 > #define SNAPSHOT_APPLY 3 > -- Best Regards Wenchao Xia