qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info
@ 2013-10-01 12:31 Max Reitz
  2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 1/6] qapi: Add ImageInfoSpecific type Max Reitz
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz

qemu-img info provides only pretty general information about an image.
For any image format, there might be specific options which cannot be
represented in a universal way; for instance, qcow2 provides the
compatibility and lazy_refcount options whose values are certainly
interesting but currently cannot be output by qemu-img info.

Therefore, this series adds a new ImageInfoSpecific union type to
ImageInfo which may be used by block drivers as a template for new types
dedicated to the specific information they can provide, as well as a
function bdrv_get_specific_info for retrieving this information. It also
adds support to qemu-img info and qemu-io -c info to print the content
of these specific structures.

v6:
 - implemented Eric's remarks:
   - corrected description of ImageInfo in qapi-schema.json (patch 1)
   - added root indentation in bdrv_image_info_specific_dump (patch 3);
     Although this is a very minor change, I dropped Eric's Reviewed-by
     because of it, since I'm not sure myself whether the place I've
     added the indentation is actually the right one (maybe the caller
     of bdrv_image_info_specific_dump should control the indentation
     instead of hardcoding it into the function).
     Furthermore, this (his) suggestion was not part of his review for
     this patch, but for patch 6.
   - changed patch 5 from using some non-portable expressions to fully
     bash-specific ones
   - use $() instead of `` and $PWD instead of `pwd` in patch 6 and
     indent format specific information in reference output (according
     to change in patch 3)
 - rebased on Kevin's block branch (changed some line numbers and the
   diff environment in patch 5)

v5:
 - In consideration of Stefan's argument about bdrv_round_to_cluster
   being called in the I/O path, removed ImageInfoSpecific from
   BlockDriverInfo and introduced a new function bdrv_get_specific_info
   instead; this also removes the need for bdrv_put_info.
   Resulting changes in regard to the single patches:
    - patch 2: completely different
    - patch 3: qemu-io-cmds.c:info_f now needs to call
      bdrv_get_specific_info in order to receive the new information
    - patch 4: moved the creation of ImageInfoSpecific from qcow2_get_info
      (which now remains unmodified) to the new qcow2_get_specific_info
      function; additionally, register this function as
      bdrv_qcow2.bdrv_get_specific_info
 - rebased on Kevin's block branch, renamed test from 064 to 065 (to
   avoid conflict with his blockdev-add series; affects patch 6)
 - patches 1 and 5 remain unmodified

v4:
 - changed dirty "grep -A 42" for grepping all lines until EOF in test
   064 to cleaner "sed -n '//,$p'" (patch 6)
 - rebased on Kevin's block branch (affects line numbers in patches 2, 3
   and 4 as well as the group file change in patch 6)

v3:
 - implemented Fam's remarks:
   - bdrv_get_info already initializes all fields to NULL, no need to do
     this manually (patch 2)
   - implemented bdrv_put_info as a wrapper to
     qapi_free_ImageInfoSpecific, though this may change with further
     extensions to BlockDriverInfo (patch 2)
   - changed one occurence of puts("foo") to printf("foo\n") in order to
     be consistent with the surrounding code (patch 3)
   - other patches (1, 4, 5, 6) remain unmodified

v2:
 - following Eric's recommendation: changed the representation of the
   format specific information from an uninterpreted blobbed string to a
   union of format specific types

Max Reitz (6):
  qapi: Add ImageInfoSpecific type
  block: Add bdrv_get_specific_info
  block/qapi: Human-readable ImageInfoSpecific dump
  qcow2: Add support for ImageInfoSpecific
  qemu-iotests: Discard specific info in _img_info
  qemu-iotests: Additional info from qemu-img info

 block.c                      |   9 ++++
 block/qapi.c                 | 124 +++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c                |  19 +++++++
 include/block/block.h        |   1 +
 include/block/block_int.h    |   1 +
 include/block/qapi.h         |   2 +
 qapi-schema.json             |  34 +++++++++++-
 qemu-io-cmds.c               |   9 ++++
 tests/qemu-iotests/065       |  72 +++++++++++++++++++++++++
 tests/qemu-iotests/065.out   |  22 ++++++++
 tests/qemu-iotests/common.rc |  20 ++++++-
 tests/qemu-iotests/group     |   1 +
 12 files changed, 312 insertions(+), 2 deletions(-)
 create mode 100755 tests/qemu-iotests/065
 create mode 100644 tests/qemu-iotests/065.out

-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v6 1/6] qapi: Add ImageInfoSpecific type
  2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
@ 2013-10-01 12:31 ` Max Reitz
  2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 2/6] block: Add bdrv_get_specific_info Max Reitz
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz

Add a new type ImageInfoSpecific as a union for image format specific
information in ImageInfo.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qapi-schema.json | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 145eca8..a472d7d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -210,6 +210,18 @@
             'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
 
 ##
+# @ImageInfoSpecific:
+#
+# A discriminated record of image format specific information structures.
+#
+# Since: 1.7
+##
+
+{ 'union': 'ImageInfoSpecific',
+  'data': {
+  } }
+
+##
 # @ImageInfo:
 #
 # Information about a QEMU image file
@@ -238,6 +250,9 @@
 #
 # @backing-image: #optional info of the backing image (since 1.6)
 #
+# @format-specific: #optional structure supplying additional format-specific
+# information (since 1.7)
+#
 # Since: 1.3
 #
 ##
@@ -248,7 +263,8 @@
            '*cluster-size': 'int', '*encrypted': 'bool',
            '*backing-filename': 'str', '*full-backing-filename': 'str',
            '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
-           '*backing-image': 'ImageInfo' } }
+           '*backing-image': 'ImageInfo',
+           '*format-specific': 'ImageInfoSpecific' } }
 
 ##
 # @ImageCheck:
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v6 2/6] block: Add bdrv_get_specific_info
  2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
  2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 1/6] qapi: Add ImageInfoSpecific type Max Reitz
@ 2013-10-01 12:31 ` Max Reitz
  2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 3/6] block/qapi: Human-readable ImageInfoSpecific dump Max Reitz
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz

Add a function for retrieving an ImageInfoSpecific object from a block
driver.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block.c                   | 9 +++++++++
 block/qapi.c              | 3 +++
 include/block/block.h     | 1 +
 include/block/block_int.h | 1 +
 4 files changed, 14 insertions(+)

diff --git a/block.c b/block.c
index 93e113a..bb5a19f 100644
--- a/block.c
+++ b/block.c
@@ -3322,6 +3322,15 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return drv->bdrv_get_info(bs, bdi);
 }
 
+ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs)
+{
+    BlockDriver *drv = bs->drv;
+    if (drv && drv->bdrv_get_specific_info) {
+        return drv->bdrv_get_specific_info(bs);
+    }
+    return NULL;
+}
+
 int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
                       int64_t pos, int size)
 {
diff --git a/block/qapi.c b/block/qapi.c
index 782051c..ab1dd24 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -134,6 +134,9 @@ void bdrv_query_image_info(BlockDriverState *bs,
         info->dirty_flag = bdi.is_dirty;
         info->has_dirty_flag = true;
     }
+    info->format_specific     = bdrv_get_specific_info(bs);
+    info->has_format_specific = info->format_specific != NULL;
+
     backing_filename = bs->backing_file;
     if (backing_filename[0] != '\0') {
         info->backing_filename = g_strdup(backing_filename);
diff --git a/include/block/block.h b/include/block/block.h
index f808550..e265124 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -335,6 +335,7 @@ int bdrv_get_flags(BlockDriverState *bs);
 int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
                           const uint8_t *buf, int nb_sectors);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
+ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
 void bdrv_round_to_clusters(BlockDriverState *bs,
                             int64_t sector_num, int nb_sectors,
                             int64_t *cluster_sector_num,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 211087a..17b26b2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -168,6 +168,7 @@ struct BlockDriver {
     int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
                                   const char *snapshot_name);
     int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
+    ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
 
     int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
                              int64_t pos);
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v6 3/6] block/qapi: Human-readable ImageInfoSpecific dump
  2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
  2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 1/6] qapi: Add ImageInfoSpecific type Max Reitz
  2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 2/6] block: Add bdrv_get_specific_info Max Reitz
@ 2013-10-01 12:31 ` Max Reitz
  2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 4/6] qcow2: Add support for ImageInfoSpecific Max Reitz
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz

Add a function for generically dumping the ImageInfoSpecific information
in a human-readable format to block/qapi.c.

Use this function in bdrv_image_info_dump and qemu-io-cmds.c:info_f to
allow qemu-img info resp. qemu-io -c info to print that format specific
information.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qapi.c         | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/qapi.h |   2 +
 qemu-io-cmds.c       |   9 ++++
 3 files changed, 132 insertions(+)

diff --git a/block/qapi.c b/block/qapi.c
index ab1dd24..5880b3e 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -25,6 +25,9 @@
 #include "block/qapi.h"
 #include "block/block_int.h"
 #include "qmp-commands.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp/types.h"
 
 /*
  * Returns 0 on success, with *p_list either set to describe snapshot
@@ -426,6 +429,119 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
     }
 }
 
+static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
+                       QDict *dict);
+static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
+                       QList *list);
+
+static void dump_qobject(fprintf_function func_fprintf, void *f,
+                         int comp_indent, QObject *obj)
+{
+    switch (qobject_type(obj)) {
+        case QTYPE_QINT: {
+            QInt *value = qobject_to_qint(obj);
+            func_fprintf(f, "%" PRId64, qint_get_int(value));
+            break;
+        }
+        case QTYPE_QSTRING: {
+            QString *value = qobject_to_qstring(obj);
+            func_fprintf(f, "%s", qstring_get_str(value));
+            break;
+        }
+        case QTYPE_QDICT: {
+            QDict *value = qobject_to_qdict(obj);
+            dump_qdict(func_fprintf, f, comp_indent, value);
+            break;
+        }
+        case QTYPE_QLIST: {
+            QList *value = qobject_to_qlist(obj);
+            dump_qlist(func_fprintf, f, comp_indent, value);
+            break;
+        }
+        case QTYPE_QFLOAT: {
+            QFloat *value = qobject_to_qfloat(obj);
+            func_fprintf(f, "%g", qfloat_get_double(value));
+            break;
+        }
+        case QTYPE_QBOOL: {
+            QBool *value = qobject_to_qbool(obj);
+            func_fprintf(f, "%s", qbool_get_int(value) ? "true" : "false");
+            break;
+        }
+        case QTYPE_QERROR: {
+            QString *value = qerror_human((QError *)obj);
+            func_fprintf(f, "%s", qstring_get_str(value));
+            break;
+        }
+        case QTYPE_NONE:
+            break;
+        case QTYPE_MAX:
+        default:
+            abort();
+    }
+}
+
+static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
+                       QList *list)
+{
+    const QListEntry *entry;
+    int i = 0;
+
+    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
+        qtype_code type = qobject_type(entry->value);
+        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+        const char *format = composite ? "%*s[%i]:\n" : "%*s[%i]: ";
+
+        func_fprintf(f, format, indentation * 4, "", i);
+        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
+        if (!composite) {
+            func_fprintf(f, "\n");
+        }
+    }
+}
+
+static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
+                       QDict *dict)
+{
+    const QDictEntry *entry;
+
+    for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
+        qtype_code type = qobject_type(entry->value);
+        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+        const char *format = composite ? "%*s%s:\n" : "%*s%s: ";
+        char key[strlen(entry->key) + 1];
+        int i;
+
+        /* replace dashes with spaces in key (variable) names */
+        for (i = 0; entry->key[i]; i++) {
+            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
+        }
+        key[i] = 0;
+
+        func_fprintf(f, format, indentation * 4, "", key);
+        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
+        if (!composite) {
+            func_fprintf(f, "\n");
+        }
+    }
+}
+
+void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
+                                   ImageInfoSpecific *info_spec)
+{
+    Error *local_err = NULL;
+    QmpOutputVisitor *ov = qmp_output_visitor_new();
+    QObject *obj, *data;
+
+    visit_type_ImageInfoSpecific(qmp_output_get_visitor(ov), &info_spec, NULL,
+                                 &local_err);
+    obj = qmp_output_get_qobject(ov);
+    assert(qobject_type(obj) == QTYPE_QDICT);
+    data = qdict_get(qobject_to_qdict(obj), "data");
+    dump_qobject(func_fprintf, f, 1, data);
+    qmp_output_visitor_cleanup(ov);
+}
+
 void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
                           ImageInfo *info)
 {
@@ -496,4 +612,9 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
             func_fprintf(f, "\n");
         }
     }
+
+    if (info->has_format_specific) {
+        func_fprintf(f, "Format specific information:\n");
+        bdrv_image_info_specific_dump(func_fprintf, f, info->format_specific);
+    }
 }
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 0496cc9..9518ee4 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -42,6 +42,8 @@ BlockStats *bdrv_query_stats(const BlockDriverState *bs);
 
 void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
                         QEMUSnapshotInfo *sn);
+void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
+                                   ImageInfoSpecific *info_spec);
 void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
                           ImageInfo *info);
 #endif
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 8565d49..667f4e4 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -10,6 +10,7 @@
 
 #include "qemu-io.h"
 #include "block/block_int.h"
+#include "block/qapi.h"
 #include "qemu/main-loop.h"
 
 #define CMD_NOFILE_OK   0x01
@@ -1678,6 +1679,7 @@ static const cmdinfo_t length_cmd = {
 static int info_f(BlockDriverState *bs, int argc, char **argv)
 {
     BlockDriverInfo bdi;
+    ImageInfoSpecific *spec_info;
     char s1[64], s2[64];
     int ret;
 
@@ -1699,6 +1701,13 @@ static int info_f(BlockDriverState *bs, int argc, char **argv)
     printf("cluster size: %s\n", s1);
     printf("vm state offset: %s\n", s2);
 
+    spec_info = bdrv_get_specific_info(bs);
+    if (spec_info) {
+        printf("Format specific information:\n");
+        bdrv_image_info_specific_dump(fprintf, stdout, spec_info);
+        qapi_free_ImageInfoSpecific(spec_info);
+    }
+
     return 0;
 }
 
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v6 4/6] qcow2: Add support for ImageInfoSpecific
  2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
                   ` (2 preceding siblings ...)
  2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 3/6] block/qapi: Human-readable ImageInfoSpecific dump Max Reitz
@ 2013-10-01 12:32 ` Max Reitz
  2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 5/6] qemu-iotests: Discard specific info in _img_info Max Reitz
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz

Add a new ImageInfoSpecificQCow2 type as a subtype of ImageInfoSpecific.
This contains the compatibility level as a string and an optional
lazy_refcounts boolean (optional means mandatory for compat >= 1.1 and
not available for compat == 0.10).

Also, add qcow2_get_specific_info, which returns this information.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2.c    | 19 +++++++++++++++++++
 qapi-schema.json | 16 ++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 4a9888c..396650d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1810,6 +1810,24 @@ static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return 0;
 }
 
+static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+    ImageInfoSpecific *spec_info = g_new0(ImageInfoSpecific, 1);
+
+    spec_info->kind = IMAGE_INFO_SPECIFIC_KIND_QCOW2;
+    spec_info->qcow2 = g_new0(ImageInfoSpecificQCow2, 1);
+    if (s->qcow_version == 2) {
+        spec_info->qcow2->compat = g_strdup("0.10");
+    } else if (s->qcow_version == 3) {
+        spec_info->qcow2->compat = g_strdup("1.1");
+        spec_info->qcow2->lazy_refcounts = s->use_lazy_refcounts;
+        spec_info->qcow2->has_lazy_refcounts = true;
+    }
+
+    return spec_info;
+}
+
 #if 0
 static void dump_refcounts(BlockDriverState *bs)
 {
@@ -2130,6 +2148,7 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_snapshot_list     = qcow2_snapshot_list,
     .bdrv_snapshot_load_tmp     = qcow2_snapshot_load_tmp,
     .bdrv_get_info      = qcow2_get_info,
+    .bdrv_get_specific_info = qcow2_get_specific_info,
 
     .bdrv_save_vmstate    = qcow2_save_vmstate,
     .bdrv_load_vmstate    = qcow2_load_vmstate,
diff --git a/qapi-schema.json b/qapi-schema.json
index a472d7d..17ea1c3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -210,6 +210,21 @@
             'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
 
 ##
+# @ImageInfoSpecificQCow2:
+#
+# @compat: compatibility level
+#
+# @lazy-refcounts: #optional on or off; only valid for compat >= 1.1
+#
+# Since: 1.7
+##
+{ 'type': 'ImageInfoSpecificQCow2',
+  'data': {
+      'compat': 'str',
+      '*lazy-refcounts': 'bool'
+  } }
+
+##
 # @ImageInfoSpecific:
 #
 # A discriminated record of image format specific information structures.
@@ -219,6 +234,7 @@
 
 { 'union': 'ImageInfoSpecific',
   'data': {
+      'qcow2': 'ImageInfoSpecificQCow2'
   } }
 
 ##
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v6 5/6] qemu-iotests: Discard specific info in _img_info
  2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
                   ` (3 preceding siblings ...)
  2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 4/6] qcow2: Add support for ImageInfoSpecific Max Reitz
@ 2013-10-01 12:32 ` Max Reitz
  2013-10-01 12:47   ` Max Reitz
  2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 6/6] qemu-iotests: Additional info from qemu-img info Max Reitz
  2013-10-01 12:50 ` [Qemu-devel] [PATCH v6 0/6] Provide additional info through " Eric Blake
  6 siblings, 1 reply; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz

In _img_info, filter out additional information specific to the image
format provided by qemu-img info, since tests designed for multiple
image formats would produce different outputs for every image format
otherwise.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.rc | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 1b22db0..73b7dde 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -197,12 +197,30 @@ _check_test_img()
 
 _img_info()
 {
+    discard=0
+    regex_json_spec_start='^ *"format-specific": \{'
     $QEMU_IMG info "$@" "$TEST_IMG" 2>&1 | \
         sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
             -e "s#$TEST_DIR#TEST_DIR#g" \
             -e "s#$IMGFMT#IMGFMT#g" \
             -e "/^disk size:/ D" \
-            -e "/actual-size/ D"
+            -e "/actual-size/ D" | \
+        while IFS='' read line; do
+            if [[ $line == "Format specific information:" ]]; then
+                discard=1
+            elif [[ $line =~ $regex_json_spec_start ]]; then
+                discard=2
+                regex_json_spec_end="^${line%%[^ ]*}\\}, *$"
+            fi
+            if [[ $discard == 0 ]]; then
+                echo "$line"
+            elif [[ $discard == 1 && ! $line ]]; then
+                echo
+                discard=0
+            elif [[ $discard == 2 && $line =~ $regex_json_spec_end ]]; then
+                discard=0
+            fi
+        done
 }
 
 _get_pids_by_name()
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v6 6/6] qemu-iotests: Additional info from qemu-img info
  2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
                   ` (4 preceding siblings ...)
  2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 5/6] qemu-iotests: Discard specific info in _img_info Max Reitz
@ 2013-10-01 12:32 ` Max Reitz
  2013-10-01 12:50 ` [Qemu-devel] [PATCH v6 0/6] Provide additional info through " Eric Blake
  6 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kevin Wolf, Fam Zheng, Stefan Hajnoczi, Max Reitz

Add a test for the additional information now provided by qemu-img info
when used on qcow2 images. This test only covers the human-readable
output, not the JSON dump.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/065     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/065.out | 22 ++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 95 insertions(+)
 create mode 100755 tests/qemu-iotests/065
 create mode 100644 tests/qemu-iotests/065.out

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
new file mode 100755
index 0000000..908af87
--- /dev/null
+++ b/tests/qemu-iotests/065
@@ -0,0 +1,72 @@
+#!/bin/bash
+#
+# Test for additional information emitted by qemu-img info on qcow2
+# images
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+IMG_SIZE=64M
+
+echo
+echo "=== Testing qcow2 image with -o compat=0.10 ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img $IMG_SIZE
+# don't use _img_info, since that function will filter out the
+# additional information we're about to test for
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=off" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+echo
+echo "=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img $IMG_SIZE
+$QEMU_IMG info "$TEST_IMG" | sed -n '/^Format specific information:$/,$p'
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/065.out b/tests/qemu-iotests/065.out
new file mode 100644
index 0000000..471631e
--- /dev/null
+++ b/tests/qemu-iotests/065.out
@@ -0,0 +1,22 @@
+QA output created by 065
+
+=== Testing qcow2 image with -o compat=0.10 ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+    compat: 0.10
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=off ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+
+=== Testing qcow2 image with -o compat=1.1,lazy_refcounts=on ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Format specific information:
+    compat: 1.1
+    lazy refcounts: true
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1ad02e5..f1a68b0 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -69,3 +69,4 @@
 061 rw auto
 062 rw auto
 063 rw auto
+065 rw auto
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PATCH v6 5/6] qemu-iotests: Discard specific info in _img_info
  2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 5/6] qemu-iotests: Discard specific info in _img_info Max Reitz
@ 2013-10-01 12:47   ` Max Reitz
  0 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2013-10-01 12:47 UTC (permalink / raw)
  To: Max Reitz; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi

On 2013-10-01 14:32, Max Reitz wrote:
> In _img_info, filter out additional information specific to the image
> format provided by qemu-img info, since tests designed for multiple
> image formats would produce different outputs for every image format
> otherwise.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
Ah, I just noticed I forgot to include a sample of what is to be 
stripped away, so this will have to do for now:

All existing tests using qemu-img info rely on it not emitting image 
format specific information. Therefore, this data has to be stripped away.

In a human-readable dump, that information will always be last for each 
"image information block" (multiple blocks are emitted when looking at 
the backing file chain). Every block is seperated by an empty line. 
Therefore, in this case, everything starting with the line "Format 
specific information:" up to that empty line (or EOF, if it's the last 
block) has to be stripped.

The JSON dump will always emit "pretty" JSON data. Therefore, the ending 
brace of every object will be indented as much as the object itself is 
(and every line in between has more indentation). Thus, in this case, we 
have to strip all the data beginning with ' *"format-specific": {' until 
a line ' *},' appears which is indented just as much as the former was.

Hm, now that I think of it… Actually, the ending line is rather ' *},?', 
since the comma is "optional", depending on whether it is the last 
object or not. Well, I think this calls for v7, then. I'll include this 
description in patch v7 5/6.

Max

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

* Re: [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info
  2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
                   ` (5 preceding siblings ...)
  2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 6/6] qemu-iotests: Additional info from qemu-img info Max Reitz
@ 2013-10-01 12:50 ` Eric Blake
  6 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2013-10-01 12:50 UTC (permalink / raw)
  To: Max Reitz; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi

[-- Attachment #1: Type: text/plain, Size: 2273 bytes --]

On 10/01/2013 06:31 AM, Max Reitz wrote:
> qemu-img info provides only pretty general information about an image.
> For any image format, there might be specific options which cannot be
> represented in a universal way; for instance, qcow2 provides the
> compatibility and lazy_refcount options whose values are certainly
> interesting but currently cannot be output by qemu-img info.
> 
> Therefore, this series adds a new ImageInfoSpecific union type to
> ImageInfo which may be used by block drivers as a template for new types
> dedicated to the specific information they can provide, as well as a
> function bdrv_get_specific_info for retrieving this information. It also
> adds support to qemu-img info and qemu-io -c info to print the content
> of these specific structures.
> 
> v6:
>  - implemented Eric's remarks:
>    - corrected description of ImageInfo in qapi-schema.json (patch 1)
>    - added root indentation in bdrv_image_info_specific_dump (patch 3);
>      Although this is a very minor change, I dropped Eric's Reviewed-by
>      because of it, since I'm not sure myself whether the place I've
>      added the indentation is actually the right one (maybe the caller
>      of bdrv_image_info_specific_dump should control the indentation
>      instead of hardcoding it into the function).
>      Furthermore, this (his) suggestion was not part of his review for
>      this patch, but for patch 6.

I think what you did was fine (if we ever have a reason to reuse
bdrv_image_info_specific_dump in a new location with different
indentation, we could rework it to add an indentation parameter at that
time).  And I like how the output now looks in the updated 6/6 patch.

>    - changed patch 5 from using some non-portable expressions to fully
>      bash-specific ones
>    - use $() instead of `` and $PWD instead of `pwd` in patch 6 and
>      indent format specific information in reference output (according
>      to change in patch 3)
>  - rebased on Kevin's block branch (changed some line numbers and the
>    diff environment in patch 5)

Series: Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 621 bytes --]

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

end of thread, other threads:[~2013-10-01 13:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-01 12:31 [Qemu-devel] [PATCH v6 0/6] Provide additional info through qemu-img info Max Reitz
2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 1/6] qapi: Add ImageInfoSpecific type Max Reitz
2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 2/6] block: Add bdrv_get_specific_info Max Reitz
2013-10-01 12:31 ` [Qemu-devel] [PATCH v6 3/6] block/qapi: Human-readable ImageInfoSpecific dump Max Reitz
2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 4/6] qcow2: Add support for ImageInfoSpecific Max Reitz
2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 5/6] qemu-iotests: Discard specific info in _img_info Max Reitz
2013-10-01 12:47   ` Max Reitz
2013-10-01 12:32 ` [Qemu-devel] [PATCH v6 6/6] qemu-iotests: Additional info from qemu-img info Max Reitz
2013-10-01 12:50 ` [Qemu-devel] [PATCH v6 0/6] Provide additional info through " Eric Blake

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).