Re: [Qemu-devel] [PATCH V3 2/2] qemu-img: Add json output option to the info command.

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

    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