* Re: [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command.
2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command Benoît Canet
@ 2012-08-16 3:13 ` Wenchao Xia
2012-08-22 12:06 ` Benoît Canet
2012-08-16 17:17 ` Luiz Capitulino
2012-08-17 9:36 ` Kevin Wolf
2 siblings, 1 reply; 7+ messages in thread
From: Wenchao Xia @ 2012-08-16 3:13 UTC (permalink / raw)
To: Benoît Canet
Cc: aliguori, stefanha, qemu-devel, lcapitulino, pbonzini, eblake,
Benoît Canet
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 <benoit@irqsave.net>
> ---
> 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 <getopt.h>
> #include <stdio.h>
>
> #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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command.
2012-08-16 3:13 ` Wenchao Xia
@ 2012-08-22 12:06 ` Benoît Canet
0 siblings, 0 replies; 7+ messages in thread
From: Benoît Canet @ 2012-08-22 12:06 UTC (permalink / raw)
To: Wenchao Xia
Cc: aliguori, Benoît Canet, stefanha, qemu-devel, lcapitulino,
pbonzini, eblake
Le Thursday 16 Aug 2012 à 11:13:05 (+0800), Wenchao Xia a écrit :
> 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);
> }
I will post a patch looking like this.
Benoît
>
> 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 <benoit@irqsave.net>
> >---
> > 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 <getopt.h>
> > #include <stdio.h>
> >
> > #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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command.
2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command Benoît Canet
2012-08-16 3:13 ` Wenchao Xia
@ 2012-08-16 17:17 ` Luiz Capitulino
2012-08-17 9:36 ` Kevin Wolf
2 siblings, 0 replies; 7+ messages in thread
From: Luiz Capitulino @ 2012-08-16 17:17 UTC (permalink / raw)
To: Benoît Canet
Cc: aliguori, stefanha, qemu-devel, pbonzini, eblake, xiawenc,
Benoît Canet
On Wed, 15 Aug 2012 20:48:43 +0200
"Benoît Canet" <benoit.canet@gmail.com> wrote:
> 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 <benoit@irqsave.net>
> ---
> 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 <getopt.h>
> #include <stdio.h>
>
> #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",
Missing "m:", causes -m option not to work.
Otherwise looks good.
> + 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command.
2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command Benoît Canet
2012-08-16 3:13 ` Wenchao Xia
2012-08-16 17:17 ` Luiz Capitulino
@ 2012-08-17 9:36 ` Kevin Wolf
2 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2012-08-17 9:36 UTC (permalink / raw)
To: Benoît Canet
Cc: aliguori, stefanha, qemu-devel, lcapitulino, pbonzini, eblake,
xiawenc, Benoît Canet
Am 15.08.2012 20:48, schrieb Benoît Canet:
> 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.
Which is not a problem. A machine doesn't care about shortcut options,
always specifying the long option is fine.
However, I wouldn't call that option --format (which would intuitively
relate to the image format) nor --machine (--machine=human looks really
weird), but --output=(json|human)
> #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"
You should mention both options, not only the non-default one.
> "\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;
Aligning the = to the same column wouldn't hurt.
> +
> + /* XXX: waiting for the qapi to support GSList */
Is this even planned? I would agree with qemu-queue.h structures
available from QAPI, but not GSList. Please change or remove the comment.
> + 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);
Extra semicolon,
The existence of this macro doesn't look quite right at the first sight,
but let me check how it's used.
> +
> 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;
> + }
Check against the valid values and exit with an error if it's neither
"json" nor "human" (or "text" or whatever you want to call it).
> +
> 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;
> }
I'm not sure if mixing both modes this way is a great idea. Maybe it's
better to have separate functions for either filling the ImageInfo
struct or human output, that are called only in the appropriate mode.
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread