All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V3 0/2] Add JSON output to qemu-img info
@ 2012-08-15 18:48 Benoît Canet
  2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 1/2] qapi: Add SnapshotInfo and ImageInfo Benoît Canet
  2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command Benoît Canet
  0 siblings, 2 replies; 7+ messages in thread
From: Benoît Canet @ 2012-08-15 18:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, lcapitulino, pbonzini, eblake, xiawenc,
	Benoît Canet

This patchset add a JSON output mode to the qemu-img info command.
It's a rewrite from scratch of the original patchset by Wenchao Xia
following Anthony Liguori advices on JSON formating.

To use it simply add --machine=json to the command line.

Benoît Canet (3):
  qapi: Add SnapshotInfo.
  qapi: Add ImageInfo.
  qemu-img: Add json output option to the info command.

in v2:

eblake: make some field optionals
        squash the two qapi patchs together
        fix a typo on vm_clock_nsec

bcanet: fix a potential memory leak

in v3: 

lcapitulino: 
       remove unneeded test
       put '\n' at the end of json in printf statement
       drop the uneeded head pointer in collect_snapshots

Benoît Canet (2):
  qapi: Add SnapshotInfo and ImageInfo.
  qemu-img: Add json output option to the info command.

 Makefile         |    3 +-
 qapi-schema.json |   60 ++++++++++++++++++++++++
 qemu-img.c       |  133 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 184 insertions(+), 12 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH V3 1/2] qapi: Add SnapshotInfo and ImageInfo.
  2012-08-15 18:48 [Qemu-devel] [PATCH V3 0/2] Add JSON output to qemu-img info Benoît Canet
@ 2012-08-15 18:48 ` Benoît Canet
  2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command Benoît Canet
  1 sibling, 0 replies; 7+ messages in thread
From: Benoît Canet @ 2012-08-15 18:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, lcapitulino, pbonzini, eblake, xiawenc,
	Benoît Canet

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 qapi-schema.json |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index a92adb1..fbf61e7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -126,6 +126,66 @@
             'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
 
 ##
+# @SnapshotInfo
+#
+# @id: unique snapshot id
+#
+# @name: user choosen name
+#
+# @vm-state-size: size of the VM state
+#
+# @date-sec: UTC date of the snapshot
+#
+# @date-nsec: date in nano seconds
+#
+# @vm-clock-nsec: VM clock relative to boot in nano seconds
+#
+# Since: 1.2
+#
+##
+
+{ 'type': 'SnapshotInfo',
+  'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
+            'date-sec': 'int', 'date-nsec': 'int',
+            'vm-clock-nsec': 'int' } }
+
+##
+# @ImageInfo:
+#
+# Information about a QEMU image file
+#
+# @filename: name of the image file
+#
+# @format: format of the image file
+#
+# @actual-size: actual size on disk in bytes of the image
+#
+# @virtual-size: maximum capacity in bytes of the image
+#
+# @dirty-flag: #optional true if image is not cleanly closed
+#
+# @cluster-size: #optional size of a cluster in bytes
+#
+# @encrypted: #optional true if the image is encrypted
+#
+# @backing-filename: #optional name of the backing file
+#
+# @backing-filename-format: #optional the format of the backing file
+#
+# @snapshots: #optional list of VM snapshots
+#
+# Since: 1.2
+#
+##
+
+{ 'type': 'ImageInfo',
+  'data': {'filename': 'str', 'format': 'str', '*dirty-flag': 'bool',
+           'actual-size': 'int', 'virtual-size': 'int',
+           '*cluster-size': 'int', '*encrypted': 'bool',
+           '*backing-filename': 'str', '*backing-filename-format': 'str',
+           '*snapshots': ['SnapshotInfo'] } }
+
+##
 # @StatusInfo:
 #
 # Information about VCPU run state
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [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 0/2] Add JSON output to qemu-img info Benoît Canet
  2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 1/2] qapi: Add SnapshotInfo and ImageInfo Benoît Canet
@ 2012-08-15 18:48 ` Benoît Canet
  2012-08-16  3:13   ` Wenchao Xia
                     ` (2 more replies)
  1 sibling, 3 replies; 7+ messages in thread
From: Benoît Canet @ 2012-08-15 18:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: aliguori, stefanha, lcapitulino, pbonzini, eblake, xiawenc,
	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.

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
-- 
1.7.9.5

^ permalink raw reply related	[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-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-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

* 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

end of thread, other threads:[~2012-08-22 12:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-15 18:48 [Qemu-devel] [PATCH V3 0/2] Add JSON output to qemu-img info Benoît Canet
2012-08-15 18:48 ` [Qemu-devel] [PATCH V3 1/2] qapi: Add SnapshotInfo and ImageInfo Benoît Canet
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.