* [Qemu-devel] [PATCH] let qemu-img info genereate json output
@ 2012-07-27 10:20 Wenchao Xia
2012-07-27 10:33 ` Daniel P. Berrange
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Wenchao Xia @ 2012-07-27 10:20 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, stefanha, Wenchao Xia
This patch would add option -j in qemu-img info command, which
would generate json output in stdout.
Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
qemu-img.c | 306 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 264 insertions(+), 42 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 80cfb9b..a514c17 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -33,6 +33,9 @@
#include <windows.h>
#endif
+#include "qint.h"
+#include "qjson.h"
+
typedef struct img_cmd_t {
const char *name;
int (*handler)(int argc, char **argv);
@@ -84,6 +87,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"
+ " '-j' try get json output, which would be in stdout, only valid in info command\n"
"\n"
"Parameters to check subcommand:\n"
" '-r' tries to repair any inconsistencies that are found during the check.\n"
@@ -1102,21 +1106,210 @@ static void dump_snapshots(BlockDriverState *bs)
g_free(sn_tab);
}
+/* string must be allocated on heap */
+struct img_infos {
+ char *filename;
+ char *fmt;
+ uint64_t total_sectors;
+ int64_t allocated_size;
+ int32 enc_flag;
+ BlockDriverInfo *bdi;
+ char *backing_filename;
+ char *backing_filename_full;
+ QEMUSnapshotInfo *sn_tab;
+ int nb_sns;
+};
+
+static void img_infos_init(struct img_infos *pinfo)
+{
+ memset(pinfo, 0, sizeof(struct img_infos));
+}
+
+#define TRY_FREE(p) { \
+ if ((p) != NULL) { \
+ g_free((p)); \
+ (p) = NULL; \
+ } \
+}
+static void img_infos_uninit(struct img_infos *pinfo)
+{
+ TRY_FREE(pinfo->filename);
+ TRY_FREE(pinfo->fmt);
+ TRY_FREE(pinfo->bdi);
+ TRY_FREE(pinfo->backing_filename);
+ TRY_FREE(pinfo->backing_filename_full);
+ TRY_FREE(pinfo->sn_tab);
+}
+
+static void snapshot_info_to_list(QList *qlist, QEMUSnapshotInfo *sn)
+{
+ char buf[128];
+ QInt *qint;
+ QString *qstr;
+ QDict *qdic = qdict_new();
+
+ qstr = qstring_from_str(sn->id_str);
+ qdict_put_obj(qdic, "id", QOBJECT(qstr));
+ qstr = qstring_from_str(sn->name);
+ qdict_put_obj(qdic, "name", QOBJECT(qstr));
+
+ snprintf(buf, sizeof(buf), "%ld", sn->vm_state_size);
+ qstr = qstring_from_str(buf);
+ qdict_put_obj(qdic, "vm_state_size", QOBJECT(qstr));
+
+ qint = qint_from_int(sn->date_sec);
+ qdict_put_obj(qdic, "date_sec", QOBJECT(qint));
+ qint = qint_from_int(sn->date_nsec);
+ qdict_put_obj(qdic, "date_nsec", QOBJECT(qint));
+ snprintf(buf, sizeof(buf), "%ld", sn->vm_clock_nsec);
+ qstr = qstring_from_str(buf);
+ qdict_put_obj(qdic, "vm_clock_nsec", QOBJECT(qstr));
+ qlist_append(qlist, qdic);
+}
+
+static void img_infos_print_json(struct img_infos *pinfo, int ret)
+{
+ char buf[128];
+ QInt *qint;
+ QDict *qdic, *qdic_all;
+ QString *qstr;
+ QList *qlist;
+ int i;
+
+ qdic_all = qdict_new();
+
+ qint = qint_from_int(ret);
+ qdict_put_obj(qdic_all, "return", QOBJECT(qint));
+ if (ret != 0) {
+ goto print;
+ }
+
+ qdic = qdict_new();
+ if (pinfo->filename != NULL) {
+ qstr = qstring_from_str(pinfo->filename);
+ qdict_put_obj(qdic, "filename", QOBJECT(qstr));
+ }
+ if (pinfo->fmt != NULL) {
+ qstr = qstring_from_str(pinfo->fmt);
+ qdict_put_obj(qdic, "fmt", QOBJECT(qstr));
+ }
+ snprintf(buf, sizeof(buf), "%ld", pinfo->total_sectors * 512);
+ qstr = qstring_from_str(buf);
+ qdict_put_obj(qdic, "virtual_size", QOBJECT(qstr));
+ snprintf(buf, sizeof(buf), "%ld", pinfo->allocated_size);
+ qstr = qstring_from_str(buf);
+ qdict_put_obj(qdic, "actual_size", QOBJECT(qstr));
+ qint = qint_from_int(pinfo->enc_flag);
+ qdict_put_obj(qdic, "encrypted", QOBJECT(qint));
+
+ if (pinfo->bdi != NULL) {
+ qint = qint_from_int(pinfo->bdi->cluster_size);
+ qdict_put_obj(qdic, "cluster_size", QOBJECT(qint));
+ qint = qint_from_int(pinfo->bdi->is_dirty);
+ qdict_put_obj(qdic, "dirty_flag", QOBJECT(qint));
+ }
+
+ if (pinfo->backing_filename != NULL) {
+ qstr = qstring_from_str(pinfo->backing_filename);
+ qdict_put_obj(qdic, "backing_filename", QOBJECT(qstr));
+ if ((pinfo->backing_filename != NULL) &&
+ (0 != strcmp(pinfo->backing_filename,
+ pinfo->backing_filename_full))
+ ) {
+ qstr = qstring_from_str(pinfo->backing_filename_full);
+ qdict_put_obj(qdic, "backing_filename_full", QOBJECT(qstr));
+ }
+ }
+
+ qlist = qlist_new();
+ i = 0;
+ while (i < pinfo->nb_sns) {
+ snapshot_info_to_list(qlist, &(pinfo->sn_tab[i]));
+ i++;
+ }
+ qdict_put_obj(qdic, "snapshot_list", QOBJECT(qlist));
+
+ qdict_put_obj(qdic_all, "information", QOBJECT(qdic));
+
+ print:
+ qstr = qobject_to_json_pretty(QOBJECT(qdic_all));
+ printf("%s\n", qstring_get_str(qstr));
+ qobject_decref(QOBJECT(qstr));
+
+ qobject_decref(QOBJECT(qdic_all));
+}
+
+static void img_infos_print(struct img_infos *pinfo)
+{
+ char size_buf[128], dsize_buf[128], buf[256];
+ int i;
+ QEMUSnapshotInfo *sn;
+
+ get_human_readable_size(size_buf, sizeof(size_buf),
+ pinfo->total_sectors * 512);
+ if (pinfo->allocated_size < 0) {
+ snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
+ } else {
+ get_human_readable_size(dsize_buf, sizeof(dsize_buf),
+ pinfo->allocated_size);
+ }
+
+ printf("image: %s\n"
+ "file format: %s\n"
+ "virtual size: %s (%" PRId64 " bytes)\n"
+ "disk size: %s\n",
+ pinfo->filename, pinfo->fmt, size_buf,
+ (pinfo->total_sectors * 512),
+ dsize_buf);
+
+ if (pinfo->enc_flag > 0) {
+ printf("encrypted: yes\n");
+ }
+
+ if (pinfo->bdi != NULL) {
+ if (pinfo->bdi->cluster_size != 0) {
+ printf("cluster_size: %d\n", pinfo->bdi->cluster_size);
+ }
+ if (pinfo->bdi->is_dirty) {
+ printf("cleanly shut down: no\n");
+ }
+ }
+
+ if (pinfo->backing_filename != NULL) {
+ printf("backing file: %s", pinfo->backing_filename);
+ if ((pinfo->backing_filename != NULL) &&
+ (0 != strcmp(pinfo->backing_filename,
+ pinfo->backing_filename_full))
+ ) {
+ printf(" (actual path: %s)", pinfo->backing_filename_full);
+ }
+ putchar('\n');
+ }
+
+ if (pinfo->nb_sns <= 0) {
+ return;
+ }
+
+ printf("Snapshot list:\n");
+ printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+ for (i = 0; i < pinfo->nb_sns; i++) {
+ sn = &(pinfo->sn_tab[i]);
+ printf("%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+ }
+}
+
+#define FILENAME_LEN_MAX (1024)
static int img_info(int argc, char **argv)
{
int c;
const char *filename, *fmt;
BlockDriverState *bs;
- 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;
+ int json_flag = 0, ret = 0;
+ struct img_infos my_img_infos;
fmt = NULL;
for(;;) {
- c = getopt(argc, argv, "f:h");
+ c = getopt(argc, argv, "f:h:j");
if (c == -1) {
break;
}
@@ -1128,6 +1321,9 @@ static int img_info(int argc, char **argv)
case 'f':
fmt = optarg;
break;
+ case 'j':
+ json_flag = 1;
+ break;
}
}
if (optind >= argc) {
@@ -1135,50 +1331,76 @@ static int img_info(int argc, char **argv)
}
filename = argv[optind++];
+ img_infos_init(&my_img_infos);
+ my_img_infos.filename = strdup(filename);
+
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
if (!bs) {
- return 1;
+ ret = 1;
+ goto output;
}
- bdrv_get_geometry(bs, &total_sectors);
- get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
- allocated_size = bdrv_get_allocated_file_size(bs);
- if (allocated_size < 0) {
- snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
- } else {
- get_human_readable_size(dsize_buf, sizeof(dsize_buf),
- allocated_size);
+
+ my_img_infos.fmt = strdup(bdrv_get_format_name(bs));
+
+ bdrv_get_geometry(bs, &(my_img_infos.total_sectors));
+ my_img_infos.allocated_size = bdrv_get_allocated_file_size(bs);
+
+ my_img_infos.enc_flag = bdrv_is_encrypted(bs);
+ my_img_infos.bdi = g_malloc(sizeof(BlockDriverInfo));
+ if (my_img_infos.bdi == NULL) {
+ ret = -1;
+ goto output;
}
- printf("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);
- if (bdrv_is_encrypted(bs)) {
- printf("encrypted: yes\n");
+ if (bdrv_get_info(bs, my_img_infos.bdi) < 0) {
+ g_free(my_img_infos.bdi);
+ my_img_infos.bdi = NULL;
}
- if (bdrv_get_info(bs, &bdi) >= 0) {
- if (bdi.cluster_size != 0) {
- printf("cluster_size: %d\n", bdi.cluster_size);
+
+ my_img_infos.backing_filename = g_malloc(FILENAME_LEN_MAX);
+ if (my_img_infos.backing_filename == NULL) {
+ ret = -1;
+ goto output;
+ }
+ memset(my_img_infos.backing_filename, 0, FILENAME_LEN_MAX);
+ bdrv_get_backing_filename(bs, my_img_infos.backing_filename,
+ FILENAME_LEN_MAX);
+ if (my_img_infos.backing_filename[0] != '\0') {
+ my_img_infos.backing_filename_full = g_malloc(FILENAME_LEN_MAX);
+ if (my_img_infos.backing_filename_full == NULL) {
+ ret = -1;
+ goto output;
}
- if (bdi.is_dirty) {
- printf("cleanly shut down: no\n");
+ memset(my_img_infos.backing_filename_full, 0, FILENAME_LEN_MAX);
+ bdrv_get_full_backing_filename(bs, my_img_infos.backing_filename_full,
+ FILENAME_LEN_MAX);
+ if (my_img_infos.backing_filename_full[0] == '\0') {
+ g_free(my_img_infos.backing_filename_full);
+ my_img_infos.backing_filename_full = NULL;
}
+ } else {
+ g_free(my_img_infos.backing_filename);
+ my_img_infos.backing_filename = NULL;
}
- 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);
- if (strcmp(backing_filename, backing_filename2) != 0) {
- printf(" (actual path: %s)", backing_filename2);
- }
- putchar('\n');
+
+ my_img_infos.nb_sns = bdrv_snapshot_list(bs, &(my_img_infos.sn_tab));
+
+ output:
+ if ((ret != 0) && (json_flag == 0)) {
+ goto clean;
}
- dump_snapshots(bs);
- bdrv_delete(bs);
- return 0;
+
+ if (json_flag > 0) {
+ img_infos_print_json(&my_img_infos, ret);
+ } else {
+ img_infos_print(&my_img_infos);
+ }
+
+ clean:
+ if (bs != NULL) {
+ bdrv_delete(bs);
+ }
+ img_infos_uninit(&my_img_infos);
+ return ret;
}
#define SNAPSHOT_LIST 1
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-07-27 10:20 [Qemu-devel] [PATCH] let qemu-img info genereate json output Wenchao Xia
@ 2012-07-27 10:33 ` Daniel P. Berrange
2012-07-27 10:49 ` Paolo Bonzini
2012-07-27 13:50 ` Anthony Liguori
2012-07-30 14:34 ` Stefan Hajnoczi
2 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrange @ 2012-07-27 10:33 UTC (permalink / raw)
To: Wenchao Xia; +Cc: pbonzini, aliguori, qemu-devel, stefanha
On Fri, Jul 27, 2012 at 06:20:48PM +0800, Wenchao Xia wrote:
> This patch would add option -j in qemu-img info command, which
> would generate json output in stdout.
I like this idea in general, because currently apps (oVirt, OpenStack, etc)
rely on parsing the human format, which is just as evil as libvirt relying
on -help format.
It would be helpful if you actually included the JSON output in your
commit message. For the benefit of other reviews, it generates the
following:
#qemu-img info -j /var/lib/libvirt/images/bar.qcow2
{
"information": {
"actual_size": "139264",
"fmt": "qcow2",
"virtual_size": "10485760",
"filename": "/var/lib/libvirt/images/bar.qcow2",
"cluster_size": 65536,
"encrypted": 0,
"snapshot_list": [
],
"dirty_flag": 0,
"backing_filename": "/dev/sda1"
},
"return": 0
}
IIUC,the 'return' element here is just duplicating the qemu-img
exit status. I think this is rather dubious, and would rather
just see the stuff in the 'information' sub-block be output
directly. It also seems to forget to mention the backing
file format.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-07-27 10:33 ` Daniel P. Berrange
@ 2012-07-27 10:49 ` Paolo Bonzini
2012-07-27 10:52 ` Daniel P. Berrange
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2012-07-27 10:49 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: aliguori, Wenchao Xia, stefanha, qemu-devel
Il 27/07/2012 12:33, Daniel P. Berrange ha scritto:
> #qemu-img info -j /var/lib/libvirt/images/bar.qcow2
> {
> "information": {
> "actual_size": "139264",
> "fmt": "qcow2",
> "virtual_size": "10485760",
> "filename": "/var/lib/libvirt/images/bar.qcow2",
> "cluster_size": 65536,
> "encrypted": 0,
> "snapshot_list": [
> ],
> "dirty_flag": 0,
> "backing_filename": "/dev/sda1"
> },
> "return": 0
> }
>
> IIUC,the 'return' element here is just duplicating the qemu-img
> exit status. I think this is rather dubious, and would rather
> just see the stuff in the 'information' sub-block be output
> directly. It also seems to forget to mention the backing
> file format.
I wonder if we could add this also to the QEMU monitor ("info
block-image foo"), so that the code would be shared between qemu-img.c
and QEMU.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-07-27 10:49 ` Paolo Bonzini
@ 2012-07-27 10:52 ` Daniel P. Berrange
2012-07-27 13:52 ` Anthony Liguori
0 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Berrange @ 2012-07-27 10:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: aliguori, Wenchao Xia, stefanha, qemu-devel
On Fri, Jul 27, 2012 at 12:49:53PM +0200, Paolo Bonzini wrote:
> Il 27/07/2012 12:33, Daniel P. Berrange ha scritto:
> > #qemu-img info -j /var/lib/libvirt/images/bar.qcow2
> > {
> > "information": {
> > "actual_size": "139264",
> > "fmt": "qcow2",
> > "virtual_size": "10485760",
> > "filename": "/var/lib/libvirt/images/bar.qcow2",
> > "cluster_size": 65536,
> > "encrypted": 0,
> > "snapshot_list": [
> > ],
> > "dirty_flag": 0,
> > "backing_filename": "/dev/sda1"
> > },
> > "return": 0
> > }
> >
> > IIUC,the 'return' element here is just duplicating the qemu-img
> > exit status. I think this is rather dubious, and would rather
> > just see the stuff in the 'information' sub-block be output
> > directly. It also seems to forget to mention the backing
> > file format.
>
> I wonder if we could add this also to the QEMU monitor ("info
> block-image foo"), so that the code would be shared between qemu-img.c
> and QEMU.
It would certainly make sense to have the code & data format shared
between the two, so apps don't need to have 2 different JSON parsers
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-07-27 10:20 [Qemu-devel] [PATCH] let qemu-img info genereate json output Wenchao Xia
2012-07-27 10:33 ` Daniel P. Berrange
@ 2012-07-27 13:50 ` Anthony Liguori
2012-08-15 8:49 ` Benoît Canet
2012-07-30 14:34 ` Stefan Hajnoczi
2 siblings, 1 reply; 9+ messages in thread
From: Anthony Liguori @ 2012-07-27 13:50 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: pbonzini, stefanha
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> This patch would add option -j in qemu-img info command, which
> would generate json output in stdout.
This is a great idea.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> qemu-img.c | 306 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 264 insertions(+), 42 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 80cfb9b..a514c17 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -33,6 +33,9 @@
> #include <windows.h>
> #endif
>
> +#include "qint.h"
> +#include "qjson.h"
> +
> typedef struct img_cmd_t {
> const char *name;
> int (*handler)(int argc, char **argv);
> @@ -84,6 +87,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"
> + " '-j' try get json output, which would be in stdout,
> only valid in info command\n"
I think an --format=json option would be a bit more extensible and
better matches what most tools are doing these days.
> "\n"
> "Parameters to check subcommand:\n"
> " '-r' tries to repair any inconsistencies that are found during the check.\n"
> @@ -1102,21 +1106,210 @@ static void dump_snapshots(BlockDriverState *bs)
> g_free(sn_tab);
> }
>
> +/* string must be allocated on heap */
> +struct img_infos {
CodingStyle
> + char *filename;
> + char *fmt;
> + uint64_t total_sectors;
> + int64_t allocated_size;
> + int32 enc_flag;
> + BlockDriverInfo *bdi;
> + char *backing_filename;
> + char *backing_filename_full;
> + QEMUSnapshotInfo *sn_tab;
> + int nb_sns;
> +};
> +
> +static void img_infos_init(struct img_infos *pinfo)
> +{
> + memset(pinfo, 0, sizeof(struct img_infos));
> +}
> +
> +#define TRY_FREE(p) { \
> + if ((p) != NULL) { \
> + g_free((p)); \
> + (p) = NULL; \
> + } \
> +}
> +static void img_infos_uninit(struct img_infos *pinfo)
> +{
> + TRY_FREE(pinfo->filename);
> + TRY_FREE(pinfo->fmt);
> + TRY_FREE(pinfo->bdi);
> + TRY_FREE(pinfo->backing_filename);
> + TRY_FREE(pinfo->backing_filename_full);
> + TRY_FREE(pinfo->sn_tab);
> +}
> +
> +static void snapshot_info_to_list(QList *qlist, QEMUSnapshotInfo *sn)
> +{
> + char buf[128];
> + QInt *qint;
> + QString *qstr;
> + QDict *qdic = qdict_new();
> +
> + qstr = qstring_from_str(sn->id_str);
> + qdict_put_obj(qdic, "id", QOBJECT(qstr));
> + qstr = qstring_from_str(sn->name);
> + qdict_put_obj(qdic, "name", QOBJECT(qstr));
> +
> + snprintf(buf, sizeof(buf), "%ld", sn->vm_state_size);
> + qstr = qstring_from_str(buf);
> + qdict_put_obj(qdic, "vm_state_size", QOBJECT(qstr));
> +
> + qint = qint_from_int(sn->date_sec);
> + qdict_put_obj(qdic, "date_sec", QOBJECT(qint));
> + qint = qint_from_int(sn->date_nsec);
> + qdict_put_obj(qdic, "date_nsec", QOBJECT(qint));
> + snprintf(buf, sizeof(buf), "%ld", sn->vm_clock_nsec);
> + qstr = qstring_from_str(buf);
> + qdict_put_obj(qdic, "vm_clock_nsec", QOBJECT(qstr));
> + qlist_append(qlist, qdic);
> +}
No need to open code all of this. Just describe the output as a type in
qapi-schema.json. Then you can just #include "qapi-visit.h", then call
visit_type_ImageInfo passing in a QMPOutputVisitor.
You can then pass the QObject to the json code to pretty print it.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-07-27 10:52 ` Daniel P. Berrange
@ 2012-07-27 13:52 ` Anthony Liguori
0 siblings, 0 replies; 9+ messages in thread
From: Anthony Liguori @ 2012-07-27 13:52 UTC (permalink / raw)
To: Daniel P. Berrange, Paolo Bonzini; +Cc: Wenchao Xia, stefanha, qemu-devel
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Fri, Jul 27, 2012 at 12:49:53PM +0200, Paolo Bonzini wrote:
>> Il 27/07/2012 12:33, Daniel P. Berrange ha scritto:
>> > #qemu-img info -j /var/lib/libvirt/images/bar.qcow2
>> > {
>> > "information": {
>> > "actual_size": "139264",
>> > "fmt": "qcow2",
>> > "virtual_size": "10485760",
>> > "filename": "/var/lib/libvirt/images/bar.qcow2",
>> > "cluster_size": 65536,
>> > "encrypted": 0,
>> > "snapshot_list": [
>> > ],
>> > "dirty_flag": 0,
>> > "backing_filename": "/dev/sda1"
>> > },
>> > "return": 0
>> > }
>> >
>> > IIUC,the 'return' element here is just duplicating the qemu-img
>> > exit status. I think this is rather dubious, and would rather
>> > just see the stuff in the 'information' sub-block be output
>> > directly. It also seems to forget to mention the backing
>> > file format.
>>
>> I wonder if we could add this also to the QEMU monitor ("info
>> block-image foo"), so that the code would be shared between qemu-img.c
>> and QEMU.
>
> It would certainly make sense to have the code & data format shared
> between the two, so apps don't need to have 2 different JSON parsers
We should definitely describe the output in qapi-schema.json. Whether
we also expose a QMP command--I'm not really convinced.
It seems a bit unusual to me to want to get this info from an arbitrary
image file from within QEMU. I'm not sure I see the use-case.
I can definitely see it become exposed as part of query-block though as
an image-info parameter.
Regards,
Anthony Liguori
>
>
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org :|
> |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-07-27 10:20 [Qemu-devel] [PATCH] let qemu-img info genereate json output Wenchao Xia
2012-07-27 10:33 ` Daniel P. Berrange
2012-07-27 13:50 ` Anthony Liguori
@ 2012-07-30 14:34 ` Stefan Hajnoczi
2 siblings, 0 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2012-07-30 14:34 UTC (permalink / raw)
To: Wenchao Xia; +Cc: pbonzini, aliguori, qemu-devel
On Fri, Jul 27, 2012 at 06:20:48PM +0800, Wenchao Xia wrote:
> This patch would add option -j in qemu-img info command, which
> would generate json output in stdout.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
> qemu-img.c | 306 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 264 insertions(+), 42 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 80cfb9b..a514c17 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -33,6 +33,9 @@
> #include <windows.h>
> #endif
>
> +#include "qint.h"
> +#include "qjson.h"
> +
> typedef struct img_cmd_t {
> const char *name;
> int (*handler)(int argc, char **argv);
> @@ -84,6 +87,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"
> + " '-j' try get json output, which would be in stdout, only valid in info command\n"
"try" makes it sound like this command can fail in some special way. Instead, I suggest:
" '--format=json' show output in machine-parsable JSON (only for info command)"
> "\n"
> "Parameters to check subcommand:\n"
> " '-r' tries to repair any inconsistencies that are found during the check.\n"
> @@ -1102,21 +1106,210 @@ static void dump_snapshots(BlockDriverState *bs)
> g_free(sn_tab);
> }
>
> +/* string must be allocated on heap */
> +struct img_infos {
> + char *filename;
> + char *fmt;
> + uint64_t total_sectors;
> + int64_t allocated_size;
> + int32 enc_flag;
> + BlockDriverInfo *bdi;
> + char *backing_filename;
> + char *backing_filename_full;
> + QEMUSnapshotInfo *sn_tab;
> + int nb_sns;
> +};
> +
> +static void img_infos_init(struct img_infos *pinfo)
> +{
> + memset(pinfo, 0, sizeof(struct img_infos));
> +}
> +
> +#define TRY_FREE(p) { \
> + if ((p) != NULL) { \
> + g_free((p)); \
> + (p) = NULL; \
> + } \
> +}
This is not necessary because free(NULL) is a nop.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-07-27 13:50 ` Anthony Liguori
@ 2012-08-15 8:49 ` Benoît Canet
2012-08-15 13:17 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: Benoît Canet @ 2012-08-15 8:49 UTC (permalink / raw)
To: Anthony Liguori; +Cc: pbonzini, Wenchao Xia, stefanha, qemu-devel
Le Friday 27 Jul 2012 à 08:50:43 (-0500), Anthony Liguori a écrit :
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
>
> > This patch would add option -j in qemu-img info command, which
> > would generate json output in stdout.
>
> This is a great idea.
>
> >
> > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> > ---
> > qemu-img.c | 306 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
> > 1 files changed, 264 insertions(+), 42 deletions(-)
> >
> > diff --git a/qemu-img.c b/qemu-img.c
> > index 80cfb9b..a514c17 100644
> > --- a/qemu-img.c
> > +++ b/qemu-img.c
> > @@ -33,6 +33,9 @@
> > #include <windows.h>
> > #endif
> >
> > +#include "qint.h"
> > +#include "qjson.h"
> > +
> > typedef struct img_cmd_t {
> > const char *name;
> > int (*handler)(int argc, char **argv);
> > @@ -84,6 +87,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"
> > + " '-j' try get json output, which would be in stdout,
> > only valid in info command\n"
>
> I think an --format=json option would be a bit more extensible and
> better matches what most tools are doing these days.
The qemu-img info subcommand already use the "-f" short option.
What alternative could be use instead of --format=json ?
Benoît
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output
2012-08-15 8:49 ` Benoît Canet
@ 2012-08-15 13:17 ` Eric Blake
0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2012-08-15 13:17 UTC (permalink / raw)
To: Benoît Canet
Cc: qemu-devel, pbonzini, Anthony Liguori, Wenchao Xia, stefanha
[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]
On 08/15/2012 02:49 AM, Benoît Canet wrote:
>> I think an --format=json option would be a bit more extensible and
>> better matches what most tools are doing these days.
>
> The qemu-img info subcommand already use the "-f" short option.
> What alternative could be use instead of --format=json ?
You are right that the mnemonic 'format' collides with -f and -F
(rebase), and 'output' collides with -o and -O (convert). Maybe we
could use the mnemonic '--layout=json', since '-L' appears to be
available? Or maybe '--machine=json' with -m, to indicate that the
output is machine-parseable (and where other machine-parseable layouts
like xml might be added in the future)?
Also, there's no rule that says that the short option must match the
mnemonic of the long option; we could always go with the short option of
'-j' even if the long option is spelled '--format', even if it means a
theoretical addition of '--format=xml' would map to the odd-looking '-j
xml'.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-08-15 13:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-27 10:20 [Qemu-devel] [PATCH] let qemu-img info genereate json output Wenchao Xia
2012-07-27 10:33 ` Daniel P. Berrange
2012-07-27 10:49 ` Paolo Bonzini
2012-07-27 10:52 ` Daniel P. Berrange
2012-07-27 13:52 ` Anthony Liguori
2012-07-27 13:50 ` Anthony Liguori
2012-08-15 8:49 ` Benoît Canet
2012-08-15 13:17 ` Eric Blake
2012-07-30 14:34 ` Stefan Hajnoczi
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.