[Qemu-devel] [PATCH v3 00/15] qemu-img map: Allow driver to return file of the allocated block

[Qemu-devel] [PATCH v3 00/15] qemu-img map: Allow driver to return file of the allocated block

[Fam Zheng]

v3: Add Eric's rev-by in patches 6, 7, 13, 14.
    12: New, split out from the previous 13.
    12->13: Refactor "entry_mergable" from imp_map().
        Don't mess the merge conditions. [Paolo]
        Address Eric's comments:
        - Check has_foo before using foo.
        - Remove blank line between comments and definition in schema.
        - Use PRId64 instead of %ld.
        - Merge short lines.

v2: Add Eric's rev-by in patches 2, 4, 5.
    01: Refering -> referring in commit message. [Eric]
        Recurse to "file" for sensible "zero" flag. [Paolo]
    12: New. Make MapEntry a QAPI struct. [Paolo, Markus]

I stumbled upon this when looking at external bitmap formats.

Current "qemu-img map" command only displays filename if the data is allocated
in bs (bs->file) itself, or in the backing chain. Otherwise, it displays an
unfriendly error message:

    $ qemu-img create -f vmdk -o subformat=monolithicFlat /tmp/test.vmdk 1G

    $ qemu-img map /tmp/test.vmdk 
    Offset          Length          Mapped to       File
    qemu-img: File contains external, encrypted or compressed clusters.

This can be improved. This series extends the .bdrv_co_get_block_status
callback, to let block driver return the BDS of file; then updates all driver
to implement it; and lastly, it changes qemu-img to use this information in
"map" command:

    $ qemu-img map /tmp/test.vmdk 
    Offset          Length          Mapped to       File
    0               0x40000000      0               /tmp/test-flat.vmdk

    $ qemu-img map --output json /tmp/test.vmdk 
    [{"length": 1073741824, "start": 0, "zero": false, "offset": 0, "depth": 0,
      "file": "/tmp/test-flat.vmdk", "data": true}
    ]


Fam Zheng (15):
  block: Add "file" output parameter to block status query functions
  qcow: Assign bs->file->bs to file in qcow_co_get_block_status
  qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status
  raw: Assign bs to file in raw_co_get_block_status
  iscsi: Assign bs to file in iscsi_co_get_block_status
  parallels: Assign bs->file->bs to file in
    parallels_co_get_block_status
  qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status
  sheepdog: Assign bs to file in sd_co_get_block_status
  vdi: Assign bs->file->bs to file in vdi_co_get_block_status
  vpc: Assign bs->file->bs to file in vpc_co_get_block_status
  vmdk: Return extent's file in bdrv_get_block_status
  qemu-img: In "map", use the returned "file" from bdrv_get_block_status
  qemu-img: Make MapEntry a QAPI struct
  qemu-img: Use QAPI visitor to generate JSON
  iotests: Add "qemu-img map" test for VMDK extents

 block/io.c                 |  42 ++++++++++++-------
 block/iscsi.c              |   9 +++-
 block/mirror.c             |   3 +-
 block/parallels.c          |   3 +-
 block/qcow.c               |   3 +-
 block/qcow2.c              |   3 +-
 block/qed.c                |   6 ++-
 block/raw-posix.c          |   4 +-
 block/raw_bsd.c            |   4 +-
 block/sheepdog.c           |   5 ++-
 block/vdi.c                |   3 +-
 block/vmdk.c               |  13 +++---
 block/vpc.c                |   4 +-
 block/vvfat.c              |   2 +-
 include/block/block.h      |   6 ++-
 include/block/block_int.h  |   3 +-
 qapi/block-core.json       |  27 ++++++++++++
 qemu-img.c                 | 102 +++++++++++++++++++++++++++++----------------
 tests/qemu-iotests/059     |  10 +++++
 tests/qemu-iotests/059.out |  38 +++++++++++++++++
 tests/qemu-iotests/122.out |  96 +++++++++++++++++++++++-------------------
 21 files changed, 270 insertions(+), 116 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

[Fam Zheng]

The added parameter can be used to return the BDS pointer which the
valid offset is referring to. It's value should be ignored unless
BDRV_BLOCK_OFFSET_VALID in ret is set.

Until block drivers fill in the right value, let's clear it explicitly
right before calling .bdrv_get_block_status.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/io.c                | 42 ++++++++++++++++++++++++++++--------------
 block/iscsi.c             |  6 ++++--
 block/mirror.c            |  3 ++-
 block/parallels.c         |  2 +-
 block/qcow.c              |  2 +-
 block/qcow2.c             |  2 +-
 block/qed.c               |  3 ++-
 block/raw-posix.c         |  3 ++-
 block/raw_bsd.c           |  3 ++-
 block/sheepdog.c          |  2 +-
 block/vdi.c               |  2 +-
 block/vmdk.c              |  2 +-
 block/vpc.c               |  2 +-
 block/vvfat.c             |  2 +-
 include/block/block.h     |  6 ++++--
 include/block/block_int.h |  3 ++-
 qemu-img.c                |  7 +++++--
 17 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/block/io.c b/block/io.c
index adc1eab..71930ed 100644
--- a/block/io.c
+++ b/block/io.c
@@ -656,6 +656,7 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num,
 int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
 {
     int64_t target_sectors, ret, nb_sectors, sector_num = 0;
+    BlockDriverState *file;
     int n;
 
     target_sectors = bdrv_nb_sectors(bs);
@@ -668,7 +669,7 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
         if (nb_sectors <= 0) {
             return 0;
         }
-        ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n);
+        ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n, &file);
         if (ret < 0) {
             error_report("error getting block status at sector %" PRId64 ": %s",
                          sector_num, strerror(-ret));
@@ -1455,6 +1456,7 @@ int bdrv_flush_all(void)
 typedef struct BdrvCoGetBlockStatusData {
     BlockDriverState *bs;
     BlockDriverState *base;
+    BlockDriverState **file;
     int64_t sector_num;
     int nb_sectors;
     int *pnum;
@@ -1479,7 +1481,8 @@ typedef struct BdrvCoGetBlockStatusData {
  */
 static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
                                                      int64_t sector_num,
-                                                     int nb_sectors, int *pnum)
+                                                     int nb_sectors, int *pnum,
+                                                     BlockDriverState **file)
 {
     int64_t total_sectors;
     int64_t n;
@@ -1509,16 +1512,19 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
         return ret;
     }
 
-    ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
+    *file = NULL;
+    ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum,
+                                            file);
     if (ret < 0) {
         *pnum = 0;
+        *file = NULL;
         return ret;
     }
 
     if (ret & BDRV_BLOCK_RAW) {
         assert(ret & BDRV_BLOCK_OFFSET_VALID);
         return bdrv_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS,
-                                     *pnum, pnum);
+                                     *pnum, pnum, file);
     }
 
     if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
@@ -1535,13 +1541,14 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
         }
     }
 
-    if (bs->file &&
+    if (*file && *file != bs &&
         (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
         (ret & BDRV_BLOCK_OFFSET_VALID)) {
+        BlockDriverState *file2;
         int file_pnum;
 
-        ret2 = bdrv_co_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS,
-                                        *pnum, &file_pnum);
+        ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS,
+                                        *pnum, &file_pnum, &file2);
         if (ret2 >= 0) {
             /* Ignore errors.  This is just providing extra information, it
              * is useful but not necessary.
@@ -1566,14 +1573,15 @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs,
         BlockDriverState *base,
         int64_t sector_num,
         int nb_sectors,
-        int *pnum)
+        int *pnum,
+        BlockDriverState **file)
 {
     BlockDriverState *p;
     int64_t ret = 0;
 
     assert(bs != base);
     for (p = bs; p != base; p = backing_bs(p)) {
-        ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum);
+        ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file);
         if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
             break;
         }
@@ -1592,7 +1600,8 @@ static void coroutine_fn bdrv_get_block_status_above_co_entry(void *opaque)
     data->ret = bdrv_co_get_block_status_above(data->bs, data->base,
                                                data->sector_num,
                                                data->nb_sectors,
-                                               data->pnum);
+                                               data->pnum,
+                                               data->file);
     data->done = true;
 }
 
@@ -1604,12 +1613,14 @@ static void coroutine_fn bdrv_get_block_status_above_co_entry(void *opaque)
 int64_t bdrv_get_block_status_above(BlockDriverState *bs,
                                     BlockDriverState *base,
                                     int64_t sector_num,
-                                    int nb_sectors, int *pnum)
+                                    int nb_sectors, int *pnum,
+                                    BlockDriverState **file)
 {
     Coroutine *co;
     BdrvCoGetBlockStatusData data = {
         .bs = bs,
         .base = base,
+        .file = file,
         .sector_num = sector_num,
         .nb_sectors = nb_sectors,
         .pnum = pnum,
@@ -1633,16 +1644,19 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
 
 int64_t bdrv_get_block_status(BlockDriverState *bs,
                               int64_t sector_num,
-                              int nb_sectors, int *pnum)
+                              int nb_sectors, int *pnum,
+                              BlockDriverState **file)
 {
     return bdrv_get_block_status_above(bs, backing_bs(bs),
-                                       sector_num, nb_sectors, pnum);
+                                       sector_num, nb_sectors, pnum, file);
 }
 
 int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
                                    int nb_sectors, int *pnum)
 {
-    int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
+    BlockDriverState *file;
+    int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum,
+                                        &file);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/iscsi.c b/block/iscsi.c
index bd1f1bf..2d1e230 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -532,7 +532,8 @@ static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun,
 
 static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
                                                   int64_t sector_num,
-                                                  int nb_sectors, int *pnum)
+                                                  int nb_sectors, int *pnum,
+                                                  BlockDriverState **file)
 {
     IscsiLun *iscsilun = bs->opaque;
     struct scsi_get_lba_status *lbas = NULL;
@@ -650,7 +651,8 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
         !iscsi_allocationmap_is_allocated(iscsilun, sector_num, nb_sectors)) {
         int64_t ret;
         int pnum;
-        ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum);
+        BlockDriverState *file;
+        ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum, &file);
         if (ret < 0) {
             return ret;
         }
diff --git a/block/mirror.c b/block/mirror.c
index 52c9abf..c7a26b7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -166,6 +166,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     MirrorOp *op;
     int pnum;
     int64_t ret;
+    BlockDriverState *file;
 
     s->sector_num = hbitmap_iter_next(&s->hbi);
     if (s->sector_num < 0) {
@@ -302,7 +303,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
     trace_mirror_one_iteration(s, sector_num, nb_sectors);
 
     ret = bdrv_get_block_status_above(source, NULL, sector_num,
-                                      nb_sectors, &pnum);
+                                      nb_sectors, &pnum, &file);
     if (ret < 0 || pnum < nb_sectors ||
             (ret & BDRV_BLOCK_DATA && !(ret & BDRV_BLOCK_ZERO))) {
         bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors,
diff --git a/block/parallels.c b/block/parallels.c
index f689fde..d1146f1 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -260,7 +260,7 @@ static coroutine_fn int parallels_co_flush_to_os(BlockDriverState *bs)
 
 
 static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *pnum)
+        int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
 {
     BDRVParallelsState *s = bs->opaque;
     int64_t offset;
diff --git a/block/qcow.c b/block/qcow.c
index 635085e..558f443 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -488,7 +488,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 }
 
 static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *pnum)
+        int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
 {
     BDRVQcowState *s = bs->opaque;
     int index_in_cluster, n;
diff --git a/block/qcow2.c b/block/qcow2.c
index 88f56c8..836888c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1283,7 +1283,7 @@ static void qcow2_reopen_abort(BDRVReopenState *state)
 }
 
 static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *pnum)
+        int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
 {
     BDRVQcow2State *s = bs->opaque;
     uint64_t cluster_offset;
diff --git a/block/qed.c b/block/qed.c
index 9b88895..a6bbd8b 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -724,7 +724,8 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
 
 static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
                                                  int64_t sector_num,
-                                                 int nb_sectors, int *pnum)
+                                                 int nb_sectors, int *pnum,
+                                                 BlockDriverState **file)
 {
     BDRVQEDState *s = bs->opaque;
     size_t len = (size_t)nb_sectors * BDRV_SECTOR_SIZE;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index aec9ec6..2cd7d68 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1830,7 +1830,8 @@ static int find_allocation(BlockDriverState *bs, off_t start,
  */
 static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
                                                     int64_t sector_num,
-                                                    int nb_sectors, int *pnum)
+                                                    int nb_sectors, int *pnum,
+                                                    BlockDriverState **file)
 {
     off_t start, data = 0, hole = 0;
     int64_t total_size;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 915d6fd..7d23c08 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -114,7 +114,8 @@ fail:
 
 static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
                                             int64_t sector_num,
-                                            int nb_sectors, int *pnum)
+                                            int nb_sectors, int *pnum,
+                                            BlockDriverState **file)
 {
     *pnum = nb_sectors;
     return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
diff --git a/block/sheepdog.c b/block/sheepdog.c
index d80e4ed..0f6789e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2709,7 +2709,7 @@ retry:
 
 static coroutine_fn int64_t
 sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
-                       int *pnum)
+                       int *pnum, BlockDriverState **file)
 {
     BDRVSheepdogState *s = bs->opaque;
     SheepdogInode *inode = &s->inode;
diff --git a/block/vdi.c b/block/vdi.c
index 17f435f..2199fd3 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -526,7 +526,7 @@ static int vdi_reopen_prepare(BDRVReopenState *state,
 }
 
 static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *pnum)
+        int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
 {
     /* TODO: Check for too large sector_num (in bdrv_is_allocated or here). */
     BDRVVdiState *s = (BDRVVdiState *)bs->opaque;
diff --git a/block/vmdk.c b/block/vmdk.c
index 6f819e4..f5a56fd 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1248,7 +1248,7 @@ static inline uint64_t vmdk_find_index_in_cluster(VmdkExtent *extent,
 }
 
 static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *pnum)
+        int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
 {
     BDRVVmdkState *s = bs->opaque;
     int64_t index_in_cluster, n, ret;
diff --git a/block/vpc.c b/block/vpc.c
index 299d373..912f5d0 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -578,7 +578,7 @@ static coroutine_fn int vpc_co_write(BlockDriverState *bs, int64_t sector_num,
 }
 
 static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *pnum)
+        int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file)
 {
     BDRVVPCState *s = bs->opaque;
     VHDFooter *footer = (VHDFooter*) s->footer_buf;
diff --git a/block/vvfat.c b/block/vvfat.c
index b184eca..b6b410a 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2884,7 +2884,7 @@ static coroutine_fn int vvfat_co_write(BlockDriverState *bs, int64_t sector_num,
 }
 
 static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs,
-	int64_t sector_num, int nb_sectors, int* n)
+	int64_t sector_num, int nb_sectors, int *n, BlockDriverState **file)
 {
     BDRVVVFATState* s = bs->opaque;
     *n = s->sector_count - sector_num;
diff --git a/include/block/block.h b/include/block/block.h
index 73edb1a..60e7ae6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -383,11 +383,13 @@ int bdrv_has_zero_init(BlockDriverState *bs);
 bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs);
 bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
 int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
-                              int nb_sectors, int *pnum);
+                              int nb_sectors, int *pnum,
+                              BlockDriverState **file);
 int64_t bdrv_get_block_status_above(BlockDriverState *bs,
                                     BlockDriverState *base,
                                     int64_t sector_num,
-                                    int nb_sectors, int *pnum);
+                                    int nb_sectors, int *pnum,
+                                    BlockDriverState **file);
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
                       int *pnum);
 int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 4012e36..d890abb 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -165,7 +165,8 @@ struct BlockDriver {
     int coroutine_fn (*bdrv_co_discard)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors);
     int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors, int *pnum);
+        int64_t sector_num, int nb_sectors, int *pnum,
+        BlockDriverState **file);
 
     /*
      * Invalidate any cached meta-data.
diff --git a/qemu-img.c b/qemu-img.c
index 033011c..7954242 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1259,9 +1259,10 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
     n = MIN(s->total_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS);
 
     if (s->sector_next_status <= sector_num) {
+        BlockDriverState *file;
         ret = bdrv_get_block_status(blk_bs(s->src[s->src_cur]),
                                     sector_num - s->src_cur_offset,
-                                    n, &n);
+                                    n, &n, &file);
         if (ret < 0) {
             return ret;
         }
@@ -2189,6 +2190,7 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
 {
     int64_t ret;
     int depth;
+    BlockDriverState *file;
 
     /* As an optimization, we could cache the current range of unallocated
      * clusters in each file of the chain, and avoid querying the same
@@ -2197,7 +2199,8 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
 
     depth = 0;
     for (;;) {
-        ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &nb_sectors);
+        ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &nb_sectors,
+                                    &file);
         if (ret < 0) {
             return ret;
         }
-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

[Stefan Hajnoczi]

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

On Thu, Nov 26, 2015 at 01:05:21PM +0800, Fam Zheng wrote:
> @@ -1535,13 +1541,14 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
>          }
>      }
>  
> -    if (bs->file &&
> +    if (*file && *file != bs &&
>          (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
>          (ret & BDRV_BLOCK_OFFSET_VALID)) {

What is the purpose of this change?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

[Fam Zheng]

On Mon, 11/30 16:38, Stefan Hajnoczi wrote:
> On Thu, Nov 26, 2015 at 01:05:21PM +0800, Fam Zheng wrote:
> > @@ -1535,13 +1541,14 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
> >          }
> >      }
> >  
> > -    if (bs->file &&
> > +    if (*file && *file != bs &&
> >          (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> >          (ret & BDRV_BLOCK_OFFSET_VALID)) {
> 
> What is the purpose of this change?

The code here is to sensibly detect "zero" by going into the "file" which the
offset is valid for. Now the "file" is no longer always "bs->file", so we use
the returned "file" pointer instead.

Fam

Re: [Qemu-devel] [PATCH v3 01/15] block: Add "file" output parameter to block status query functions

[Stefan Hajnoczi]

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

On Mon, Nov 30, 2015 at 05:09:49PM +0800, Fam Zheng wrote:
> On Mon, 11/30 16:38, Stefan Hajnoczi wrote:
> > On Thu, Nov 26, 2015 at 01:05:21PM +0800, Fam Zheng wrote:
> > > @@ -1535,13 +1541,14 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
> > >          }
> > >      }
> > >  
> > > -    if (bs->file &&
> > > +    if (*file && *file != bs &&
> > >          (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
> > >          (ret & BDRV_BLOCK_OFFSET_VALID)) {
> > 
> > What is the purpose of this change?
> 
> The code here is to sensibly detect "zero" by going into the "file" which the
> offset is valid for. Now the "file" is no longer always "bs->file", so we use
> the returned "file" pointer instead.

Thanks for explaining!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v3 02/15] qcow: Assign bs->file->bs to file in qcow_co_get_block_status

[Fam Zheng]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/qcow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow.c b/block/qcow.c
index 558f443..b59383f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -509,6 +509,7 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs,
         return BDRV_BLOCK_DATA;
     }
     cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+    *file = bs->file->bs;
     return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | cluster_offset;
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 03/15] qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/qcow2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 836888c..7634c42 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1302,6 +1302,7 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
         !s->cipher) {
         index_in_cluster = sector_num & (s->cluster_sectors - 1);
         cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS);
+        *file = bs->file->bs;
         status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset;
     }
     if (ret == QCOW2_CLUSTER_ZERO) {
-- 
2.4.3

[Qemu-devel] [PATCH v3 04/15] raw: Assign bs to file in raw_co_get_block_status

[Fam Zheng]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/raw-posix.c | 1 +
 block/raw_bsd.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 2cd7d68..9988aa4 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1873,6 +1873,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
         *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE);
         ret = BDRV_BLOCK_ZERO;
     }
+    *file = bs;
     return ret | BDRV_BLOCK_OFFSET_VALID | start;
 }
 
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 7d23c08..2d4c896 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -118,6 +118,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
                                             BlockDriverState **file)
 {
     *pnum = nb_sectors;
+    *file = bs;
     return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
            (sector_num << BDRV_SECTOR_BITS);
 }
-- 
2.4.3

[Qemu-devel] [PATCH v3 05/15] iscsi: Assign bs to file in iscsi_co_get_block_status

[Fam Zheng]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index 2d1e230..8c7f1b3 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -625,6 +625,9 @@ out:
     if (iTask.task != NULL) {
         scsi_free_scsi_task(iTask.task);
     }
+    if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID) {
+        *file = bs;
+    }
     return ret;
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 06/15] parallels: Assign bs->file->bs to file in parallels_co_get_block_status

[Fam Zheng]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/parallels.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/parallels.c b/block/parallels.c
index d1146f1..6552f32 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -273,6 +273,7 @@ static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs,
         return 0;
     }
 
+    *file = bs->file->bs;
     return (offset << BDRV_SECTOR_BITS) |
         BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
 }
-- 
2.4.3

[Qemu-devel] [PATCH v3 07/15] qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status

[Fam Zheng]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/qed.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/qed.c b/block/qed.c
index a6bbd8b..03af9c1 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -692,6 +692,7 @@ typedef struct {
     uint64_t pos;
     int64_t status;
     int *pnum;
+    BlockDriverState **file;
 } QEDIsAllocatedCB;
 
 static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t len)
@@ -703,6 +704,7 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l
     case QED_CLUSTER_FOUND:
         offset |= qed_offset_into_cluster(s, cb->pos);
         cb->status = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
+        *cb->file = cb->bs->file->bs;
         break;
     case QED_CLUSTER_ZERO:
         cb->status = BDRV_BLOCK_ZERO;
@@ -734,6 +736,7 @@ static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs,
         .pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE,
         .status = BDRV_BLOCK_OFFSET_MASK,
         .pnum = pnum,
+        .file = file,
     };
     QEDRequest request = { .l2_table = NULL };
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 08/15] sheepdog: Assign bs to file in sd_co_get_block_status

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/sheepdog.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 0f6789e..d5e7ff8 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2740,6 +2740,9 @@ sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     if (*pnum > nb_sectors) {
         *pnum = nb_sectors;
     }
+    if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID) {
+        *file = bs;
+    }
     return ret;
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 09/15] vdi: Assign bs->file->bs to file in vdi_co_get_block_status

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vdi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/vdi.c b/block/vdi.c
index 2199fd3..6b1a57b 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -550,6 +550,7 @@ static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs,
     offset = s->header.offset_data +
                               (uint64_t)bmap_entry * s->block_size +
                               sector_in_block * SECTOR_SIZE;
+    *file = bs->file->bs;
     return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset;
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 10/15] vpc: Assign bs->file->bs to file in vpc_co_get_block_status

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vpc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/vpc.c b/block/vpc.c
index 912f5d0..412ff41 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -588,6 +588,7 @@ static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs,
 
     if (be32_to_cpu(footer->type) == VHD_FIXED) {
         *pnum = nb_sectors;
+        *file = bs->file->bs;
         return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
                (sector_num << BDRV_SECTOR_BITS);
     }
@@ -609,6 +610,7 @@ static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs,
         /* *pnum can't be greater than one block for allocated
          * sectors since there is always a bitmap in between. */
         if (allocated) {
+            *file = bs->file->bs;
             return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
         }
         if (nb_sectors == 0) {
-- 
2.4.3

[Qemu-devel] [PATCH v3 11/15] vmdk: Return extent's file in bdrv_get_block_status

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index f5a56fd..b60a5af 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1265,6 +1265,7 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
                              0, 0);
     qemu_co_mutex_unlock(&s->lock);
 
+    index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
     switch (ret) {
     case VMDK_ERROR:
         ret = -EIO;
@@ -1276,15 +1277,13 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
         ret = BDRV_BLOCK_ZERO;
         break;
     case VMDK_OK:
-        ret = BDRV_BLOCK_DATA;
-        if (extent->file == bs->file && !extent->compressed) {
-            ret |= BDRV_BLOCK_OFFSET_VALID | offset;
-        }
-
+        ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID;
+        ret |= (offset + (index_in_cluster << BDRV_SECTOR_BITS))
+                & BDRV_BLOCK_OFFSET_MASK;
+        *file = extent->file->bs;
         break;
     }
 
-    index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num);
     n = extent->cluster_sectors - index_in_cluster;
     if (n > nb_sectors) {
         n = nb_sectors;
-- 
2.4.3

[Qemu-devel] [PATCH v3 12/15] qemu-img: In "map", use the returned "file" from bdrv_get_block_status

[Fam Zheng]

Now all drivers should return a correct "file", we can make use of it,
even with the recursion into backing chain above.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 7954242..a7fa794 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2222,7 +2222,7 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
     e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK;
     e->offset = ret & BDRV_BLOCK_OFFSET_MASK;
     e->depth = depth;
-    e->bs = bs;
+    e->bs = file;
     return 0;
 }
 
-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 12/15] qemu-img: In "map", use the returned "file" from bdrv_get_block_status

[Eric Blake]

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

On 11/25/2015 10:05 PM, Fam Zheng wrote:
> Now all drivers should return a correct "file", we can make use of it,
> even with the recursion into backing chain above.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 7954242..a7fa794 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -2222,7 +2222,7 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
>      e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK;
>      e->offset = ret & BDRV_BLOCK_OFFSET_MASK;
>      e->depth = depth;
> -    e->bs = bs;
> +    e->bs = file;
>      return 0;
>  }
>  
> 

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

[Qemu-devel] [PATCH v3 13/15] qemu-img: Make MapEntry a QAPI struct

[Fam Zheng]

The "flags" bit mask is expanded to two booleans, "data" and "zero";
"bs" is replaced with "filename" string.

Refactor the merge conditions in img_map() into entry_mergable().

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qapi/block-core.json | 27 ++++++++++++++++++++
 qemu-img.c           | 71 +++++++++++++++++++++++++++++++---------------------
 2 files changed, 69 insertions(+), 29 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index a07b13f..ee2294f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -186,6 +186,33 @@
            '*fragmented-clusters': 'int', '*compressed-clusters': 'int' } }
 
 ##
+# @MapEntry:
+#
+# Mapping information from a virtual block range to a host file range
+#
+# @start: the start byte of the mapped virtual range
+#
+# @length: the number of bytes of the mapped virtual range
+#
+# @data: whether the mapped range has data
+#
+# @zero: whether the virtual blocks are zeroed
+#
+# @depth: the depth of the mapping
+#
+# @offset: #optional the offset in file that the virtual sectors are mapped to
+#
+# @filename: #optional filename that is referred to by @offset
+#
+# Since: 2.6
+#
+##
+{ 'struct': 'MapEntry',
+  'data': {'start': 'int', 'length': 'int', 'data': 'bool',
+           'zero': 'bool', 'depth': 'int', '*offset': 'int',
+           '*filename': 'str' } }
+
+##
 # @BlockdevCacheInfo
 #
 # Cache mode information for a block device
diff --git a/qemu-img.c b/qemu-img.c
index a7fa794..1d19c86 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2133,47 +2133,37 @@ static int img_info(int argc, char **argv)
     return 0;
 }
 
-
-typedef struct MapEntry {
-    int flags;
-    int depth;
-    int64_t start;
-    int64_t length;
-    int64_t offset;
-    BlockDriverState *bs;
-} MapEntry;
-
 static void dump_map_entry(OutputFormat output_format, MapEntry *e,
                            MapEntry *next)
 {
     switch (output_format) {
     case OFORMAT_HUMAN:
-        if ((e->flags & BDRV_BLOCK_DATA) &&
-            !(e->flags & BDRV_BLOCK_OFFSET_VALID)) {
+        if (e->data && !e->has_offset) {
             error_report("File contains external, encrypted or compressed clusters.");
             exit(1);
         }
-        if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == BDRV_BLOCK_DATA) {
+        if (e->data && !e->zero) {
             printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n",
-                   e->start, e->length, e->offset, e->bs->filename);
+                   e->start, e->length,
+                   e->has_offset ? e->offset : 0,
+                   e->has_filename ? e->filename : "");
         }
         /* This format ignores the distinction between 0, ZERO and ZERO|DATA.
          * Modify the flags here to allow more coalescing.
          */
-        if (next &&
-            (next->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) != BDRV_BLOCK_DATA) {
-            next->flags &= ~BDRV_BLOCK_DATA;
-            next->flags |= BDRV_BLOCK_ZERO;
+        if (next && (!next->data || next->zero)) {
+            next->data = false;
+            next->zero = true;
         }
         break;
     case OFORMAT_JSON:
-        printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64", \"depth\": %d,"
-               " \"zero\": %s, \"data\": %s",
+        printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64","
+               " \"depth\": %"PRId64", \"zero\": %s, \"data\": %s",
                (e->start == 0 ? "[" : ",\n"),
                e->start, e->length, e->depth,
-               (e->flags & BDRV_BLOCK_ZERO) ? "true" : "false",
-               (e->flags & BDRV_BLOCK_DATA) ? "true" : "false");
-        if (e->flags & BDRV_BLOCK_OFFSET_VALID) {
+               e->zero ? "true" : "false",
+               e->data ? "true" : "false");
+        if (e->has_offset) {
             printf(", \"offset\": %"PRId64"", e->offset);
         }
         putchar('}');
@@ -2219,13 +2209,39 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num,
 
     e->start = sector_num * BDRV_SECTOR_SIZE;
     e->length = nb_sectors * BDRV_SECTOR_SIZE;
-    e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK;
+    e->data = !!(ret & BDRV_BLOCK_DATA);
+    e->zero = !!(ret & BDRV_BLOCK_ZERO);
     e->offset = ret & BDRV_BLOCK_OFFSET_MASK;
+    e->has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID);
     e->depth = depth;
-    e->bs = file;
+    if (file && e->has_offset) {
+        e->has_filename = true;
+        e->filename = file->filename;
+    }
     return 0;
 }
 
+static inline bool entry_mergable(const MapEntry *curr, const MapEntry *next)
+{
+    if (curr->length == 0) {
+        return false;
+    }
+    if (curr->zero != next->zero ||
+        curr->data != next->data ||
+        curr->depth != next->depth ||
+        curr->has_filename != next->has_filename ||
+        curr->has_offset != next->has_offset) {
+        return false;
+    }
+    if (curr->has_filename && strcmp(curr->filename, next->filename)) {
+        return false;
+    }
+    if (curr->has_offset && curr->offset + curr->length != next->offset) {
+        return false;
+    }
+    return true;
+}
+
 static int img_map(int argc, char **argv)
 {
     int c;
@@ -2307,10 +2323,7 @@ static int img_map(int argc, char **argv)
             goto out;
         }
 
-        if (curr.length != 0 && curr.flags == next.flags &&
-            curr.depth == next.depth &&
-            ((curr.flags & BDRV_BLOCK_OFFSET_VALID) == 0 ||
-             curr.offset + curr.length == next.offset)) {
+        if (entry_mergable(&curr, &next)) {
             curr.length += next.length;
             continue;
         }
-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 13/15] qemu-img: Make MapEntry a QAPI struct

[Eric Blake]

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

On 11/25/2015 10:05 PM, Fam Zheng wrote:
> The "flags" bit mask is expanded to two booleans, "data" and "zero";
> "bs" is replaced with "filename" string.
> 
> Refactor the merge conditions in img_map() into entry_mergable().

s/mergable/mergeable/ here and in the patch body

> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  qapi/block-core.json | 27 ++++++++++++++++++++
>  qemu-img.c           | 71 +++++++++++++++++++++++++++++++---------------------
>  2 files changed, 69 insertions(+), 29 deletions(-)
> 

With the spelling fix,
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: 604 bytes --]

[Qemu-devel] [PATCH v3 14/15] qemu-img: Use QAPI visitor to generate JSON

[Fam Zheng]

A visible improvement is that "filename" is now included in the output
if it's valid.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qemu-img.c                 | 34 ++++++++++------
 tests/qemu-iotests/122.out | 96 ++++++++++++++++++++++++++--------------------
 2 files changed, 77 insertions(+), 53 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 1d19c86..fccddc0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -25,6 +25,8 @@
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qint.h"
+#include "qapi/qmp/qbool.h"
 #include "qemu-common.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
@@ -2136,10 +2138,14 @@ static int img_info(int argc, char **argv)
 static void dump_map_entry(OutputFormat output_format, MapEntry *e,
                            MapEntry *next)
 {
+    QString *str;
+    QObject *obj;
+    QmpOutputVisitor *ov;
+
     switch (output_format) {
     case OFORMAT_HUMAN:
         if (e->data && !e->has_offset) {
-            error_report("File contains external, encrypted or compressed clusters.");
+            error_report("File contains encrypted or compressed clusters.");
             exit(1);
         }
         if (e->data && !e->zero) {
@@ -2157,20 +2163,26 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e,
         }
         break;
     case OFORMAT_JSON:
-        printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64","
-               " \"depth\": %"PRId64", \"zero\": %s, \"data\": %s",
-               (e->start == 0 ? "[" : ",\n"),
-               e->start, e->length, e->depth,
-               e->zero ? "true" : "false",
-               e->data ? "true" : "false");
-        if (e->has_offset) {
-            printf(", \"offset\": %"PRId64"", e->offset);
-        }
-        putchar('}');
+        ov = qmp_output_visitor_new();
+        visit_type_MapEntry(qmp_output_get_visitor(ov),
+                            &e, NULL, &error_abort);
+        obj = qmp_output_get_qobject(ov);
+        str = qobject_to_json(obj);
+        assert(str != NULL);
 
+        if (e->start == 0) {
+            printf("[");
+        } else {
+            printf(",");
+        }
+        printf("%s\n", qstring_get_str(str));
         if (!next) {
             printf("]\n");
         }
+
+        qobject_decref(obj);
+        qmp_output_visitor_cleanup(ov);
+        QDECREF(str);
         break;
     }
 }
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index 0068e96..760759e 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -49,12 +49,13 @@ read 65536/65536 bytes at offset 4194304
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 65536/65536 bytes at offset 8388608
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": true},
-{ "start": 65536, "length": 4128768, "depth": 0, "zero": true, "data": false},
-{ "start": 4194304, "length": 65536, "depth": 0, "zero": false, "data": true},
-{ "start": 4259840, "length": 4128768, "depth": 0, "zero": true, "data": false},
-{ "start": 8388608, "length": 65536, "depth": 0, "zero": false, "data": true},
-{ "start": 8454144, "length": 4128768, "depth": 0, "zero": true, "data": false}]
+[{"length": 65536, "start": 0, "zero": false, "depth": 0, "data": true}
+,{"length": 4128768, "start": 65536, "zero": true, "depth": 0, "data": false}
+,{"length": 65536, "start": 4194304, "zero": false, "depth": 0, "data": true}
+,{"length": 4128768, "start": 4259840, "zero": true, "depth": 0, "data": false}
+,{"length": 65536, "start": 8388608, "zero": false, "depth": 0, "data": true}
+,{"length": 4128768, "start": 8454144, "zero": true, "depth": 0, "data": false}
+]
 read 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 65536/65536 bytes at offset 4194304
@@ -76,12 +77,13 @@ wrote 1024/1024 bytes at offset 1046528
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 1024/1024 bytes at offset 0
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": true},
-{ "start": 65536, "length": 65536, "depth": 0, "zero": true, "data": false},
-{ "start": 131072, "length": 196608, "depth": 0, "zero": false, "data": true},
-{ "start": 327680, "length": 655360, "depth": 0, "zero": true, "data": false},
-{ "start": 983040, "length": 65536, "depth": 0, "zero": false, "data": true},
-{ "start": 1048576, "length": 1046528, "depth": 0, "zero": true, "data": false}]
+[{"length": 65536, "start": 0, "zero": false, "depth": 0, "data": true}
+,{"length": 65536, "start": 65536, "zero": true, "depth": 0, "data": false}
+,{"length": 196608, "start": 131072, "zero": false, "depth": 0, "data": true}
+,{"length": 655360, "start": 327680, "zero": true, "depth": 0, "data": false}
+,{"length": 65536, "start": 983040, "zero": false, "depth": 0, "data": true}
+,{"length": 1046528, "start": 1048576, "zero": true, "depth": 0, "data": false}
+]
 read 16384/16384 bytes at offset 0
 16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 16384/16384 bytes at offset 16384
@@ -112,16 +114,18 @@ read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true, "offset": 327680},
-{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data": false}]
+[{"length": 6291456, "start": 0, "zero": false, "offset": 327680, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+,{"length": 60817408, "start": 6291456, "zero": true, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": false}
+]
 
 convert -c -S 0:
 read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true},
-{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data": false}]
+[{"length": 6291456, "start": 0, "zero": false, "depth": 0, "data": true}
+,{"length": 60817408, "start": 6291456, "zero": true, "depth": 0, "data": false}
+]
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
 wrote 33554432/33554432 bytes at offset 0
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -136,7 +140,8 @@ read 30408704/30408704 bytes at offset 3145728
 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 33554432/33554432 bytes at offset 33554432
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, "offset": 327680}]
+[{"length": 67108864, "start": 0, "zero": false, "offset": 327680, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+]
 
 convert -c -S 0 with source backing file:
 read 3145728/3145728 bytes at offset 0
@@ -145,7 +150,8 @@ read 30408704/30408704 bytes at offset 3145728
 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 33554432/33554432 bytes at offset 33554432
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true}]
+[{"length": 67108864, "start": 0, "zero": false, "depth": 0, "data": true}
+]
 
 convert -S 0 -B ...
 read 3145728/3145728 bytes at offset 0
@@ -154,7 +160,8 @@ read 30408704/30408704 bytes at offset 3145728
 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 33554432/33554432 bytes at offset 33554432
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, "offset": 327680}]
+[{"length": 67108864, "start": 0, "zero": false, "offset": 327680, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+]
 
 convert -c -S 0 -B ...
 read 3145728/3145728 bytes at offset 0
@@ -163,7 +170,8 @@ read 30408704/30408704 bytes at offset 3145728
 29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 33554432/33554432 bytes at offset 33554432
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true}]
+[{"length": 67108864, "start": 0, "zero": false, "depth": 0, "data": true}
+]
 
 === Non-zero -S ===
 
@@ -178,32 +186,36 @@ wrote 1024/1024 bytes at offset 17408
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 convert -S 4k
-[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": 8192},
-{ "start": 1024, "length": 7168, "depth": 0, "zero": true, "data": false},
-{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": 9216},
-{ "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": 10240},
-{ "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]
+[{"length": 1024, "start": 0, "zero": false, "offset": 8192, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+,{"length": 7168, "start": 1024, "zero": true, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": false}
+,{"length": 1024, "start": 8192, "zero": false, "offset": 9216, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+,{"length": 8192, "start": 9216, "zero": true, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": false}
+,{"length": 1024, "start": 17408, "zero": false, "offset": 10240, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+,{"length": 67090432, "start": 18432, "zero": true, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": false}
+]
 
 convert -c -S 4k
-[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true},
-{ "start": 1024, "length": 7168, "depth": 0, "zero": true, "data": false},
-{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true},
-{ "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true},
-{ "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]
+[{"length": 1024, "start": 0, "zero": false, "depth": 0, "data": true}
+,{"length": 7168, "start": 1024, "zero": true, "depth": 0, "data": false}
+,{"length": 1024, "start": 8192, "zero": false, "depth": 0, "data": true}
+,{"length": 8192, "start": 9216, "zero": true, "depth": 0, "data": false}
+,{"length": 1024, "start": 17408, "zero": false, "depth": 0, "data": true}
+,{"length": 67090432, "start": 18432, "zero": true, "depth": 0, "data": false}
+]
 
 convert -S 8k
-[{ "start": 0, "length": 9216, "depth": 0, "zero": false, "data": true, "offset": 8192},
-{ "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true, "offset": 17408},
-{ "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]
+[{"length": 9216, "start": 0, "zero": false, "offset": 8192, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+,{"length": 8192, "start": 9216, "zero": true, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": false}
+,{"length": 1024, "start": 17408, "zero": false, "offset": 17408, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": true}
+,{"length": 67090432, "start": 18432, "zero": true, "filename": "TEST_DIR/t.IMGFMT.orig", "depth": 0, "data": false}
+]
 
 convert -c -S 8k
-[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true},
-{ "start": 1024, "length": 7168, "depth": 0, "zero": true, "data": false},
-{ "start": 8192, "length": 1024, "depth": 0, "zero": false, "data": true},
-{ "start": 9216, "length": 8192, "depth": 0, "zero": true, "data": false},
-{ "start": 17408, "length": 1024, "depth": 0, "zero": false, "data": true},
-{ "start": 18432, "length": 67090432, "depth": 0, "zero": true, "data": false}]
+[{"length": 1024, "start": 0, "zero": false, "depth": 0, "data": true}
+,{"length": 7168, "start": 1024, "zero": true, "depth": 0, "data": false}
+,{"length": 1024, "start": 8192, "zero": false, "depth": 0, "data": true}
+,{"length": 8192, "start": 9216, "zero": true, "depth": 0, "data": false}
+,{"length": 1024, "start": 17408, "zero": false, "depth": 0, "data": true}
+,{"length": 67090432, "start": 18432, "zero": true, "depth": 0, "data": false}
+]
 *** done
-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 14/15] qemu-img: Use QAPI visitor to generate JSON

[Eric Blake]

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

On 11/25/2015 10:05 PM, Fam Zheng wrote:
> A visible improvement is that "filename" is now included in the output
> if it's valid.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  qemu-img.c                 | 34 ++++++++++------
>  tests/qemu-iotests/122.out | 96 ++++++++++++++++++++++++++--------------------
>  2 files changed, 77 insertions(+), 53 deletions(-)
> 
> @@ -2157,20 +2163,26 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e,
>          }
>          break;
>      case OFORMAT_JSON:
> -        printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64","

The space after '{' here is interesting below [1]

> -               " \"depth\": %"PRId64", \"zero\": %s, \"data\": %s",
> -               (e->start == 0 ? "[" : ",\n"),
> -               e->start, e->length, e->depth,
> -               e->zero ? "true" : "false",
> -               e->data ? "true" : "false");
> -        if (e->has_offset) {
> -            printf(", \"offset\": %"PRId64"", e->offset);
> -        }
> -        putchar('}');
> +        ov = qmp_output_visitor_new();
> +        visit_type_MapEntry(qmp_output_get_visitor(ov),
> +                            &e, NULL, &error_abort);
> +        obj = qmp_output_get_qobject(ov);
> +        str = qobject_to_json(obj);
> +        assert(str != NULL);

For what it's worth, I have a patch series on my local tree that adds
json_output_visitor_new(), and which would not only bypass the wasted
QObject by doing a multi-step qapi=>QObject=>JSON, but it also has the
benefit of outputting JSON that is in qapi order (under your control)
rather than QDict hash order (deterministic and still valid JSON, but
hard to predict and not always the nicest results for human readers).

> +++ b/tests/qemu-iotests/122.out
> @@ -49,12 +49,13 @@ read 65536/65536 bytes at offset 4194304
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  read 65536/65536 bytes at offset 8388608
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -[{ "start": 0, "length": 65536, "depth": 0, "zero": false, "data": true},
> -{ "start": 65536, "length": 4128768, "depth": 0, "zero": true, "data": false},
> -{ "start": 4194304, "length": 65536, "depth": 0, "zero": false, "data": true},
> -{ "start": 4259840, "length": 4128768, "depth": 0, "zero": true, "data": false},
> -{ "start": 8388608, "length": 65536, "depth": 0, "zero": false, "data": true},
> -{ "start": 8454144, "length": 4128768, "depth": 0, "zero": true, "data": false}]
> +[{"length": 65536, "start": 0, "zero": false, "depth": 0, "data": true}

Thus, my series conflicts with your patch, and one of us will have to
rebase on the other.  But with my series, we'd have yet more churn here,
looking like:

> {"start": 65536, "length": 4128768, "data": true, "zero": true, "depth": 0},

unless we change patch 13/15 to lay out the qapi struct in the same
order as the existing output.  But some churn is inevitable; both the
QObject formatter and my pending patches output a leading "{KEY", rather
than "{ KEY" (back to point [1] above), regardless of which key gets
displayed first.

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

[Qemu-devel] [PATCH v3 15/15] iotests: Add "qemu-img map" test for VMDK extents

[Fam Zheng]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/059     | 10 ++++++++++
 tests/qemu-iotests/059.out | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 0ded0c3..261d8b0 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -133,6 +133,16 @@ $QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io
 
 echo
+echo "=== Testing qemu-img map on extents ==="
+for fmt in twoGbMaxExtentSparse twoGbMaxExtentFlat; do
+    IMGOPTS="subformat=$fmt" _make_test_img 31G
+    $QEMU_IO -c "write 65024 1k" "$TEST_IMG" | _filter_qemu_io
+    $QEMU_IO -c "write 2147483136 1k" "$TEST_IMG" | _filter_qemu_io
+    $QEMU_IO -c "write 5G 1k" "$TEST_IMG" | _filter_qemu_io
+    $QEMU_IMG map "$TEST_IMG" | _filter_testdir
+done
+
+echo
 echo "=== Testing afl image with a very large capacity ==="
 _use_sample_img afl9.vmdk.bz2
 _img_info
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 00057fe..54eb530 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2337,6 +2337,44 @@ e1000003f0:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
 read 1024/1024 bytes at offset 966367641600
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+=== Testing qemu-img map on extents ===
+Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=33285996544 subformat=twoGbMaxExtentSparse
+wrote 1024/1024 bytes at offset 65024
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 2147483136
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 5368709120
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset          Length          Mapped to       File
+0               0x20000         0x50000         TEST_DIR/iotest-version3-s001.vmdk
+0x7fff0000      0x10000         0x70000         TEST_DIR/iotest-version3-s001.vmdk
+0x80000000      0x10000         0x50000         TEST_DIR/iotest-version3-s002.vmdk
+0x140000000     0x10000         0x50000         TEST_DIR/iotest-version3-s003.vmdk
+Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=33285996544 subformat=twoGbMaxExtentFlat
+wrote 1024/1024 bytes at offset 65024
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 2147483136
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1024/1024 bytes at offset 5368709120
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset          Length          Mapped to       File
+0               0x80000000      0               TEST_DIR/iotest-version3-f001.vmdk
+0x80000000      0x80000000      0               TEST_DIR/iotest-version3-f002.vmdk
+0x100000000     0x80000000      0               TEST_DIR/iotest-version3-f003.vmdk
+0x180000000     0x80000000      0               TEST_DIR/iotest-version3-f004.vmdk
+0x200000000     0x80000000      0               TEST_DIR/iotest-version3-f005.vmdk
+0x280000000     0x80000000      0               TEST_DIR/iotest-version3-f006.vmdk
+0x300000000     0x80000000      0               TEST_DIR/iotest-version3-f007.vmdk
+0x380000000     0x80000000      0               TEST_DIR/iotest-version3-f008.vmdk
+0x400000000     0x80000000      0               TEST_DIR/iotest-version3-f009.vmdk
+0x480000000     0x80000000      0               TEST_DIR/iotest-version3-f010.vmdk
+0x500000000     0x80000000      0               TEST_DIR/iotest-version3-f011.vmdk
+0x580000000     0x80000000      0               TEST_DIR/iotest-version3-f012.vmdk
+0x600000000     0x80000000      0               TEST_DIR/iotest-version3-f013.vmdk
+0x680000000     0x80000000      0               TEST_DIR/iotest-version3-f014.vmdk
+0x700000000     0x80000000      0               TEST_DIR/iotest-version3-f015.vmdk
+0x780000000     0x40000000      0               TEST_DIR/iotest-version3-f016.vmdk
+
 === Testing afl image with a very large capacity ===
 qemu-img: Can't get size of device 'image': File too large
 *** done
-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 00/15] qemu-img map: Allow driver to return file of the allocated block

[Stefan Hajnoczi]

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

On Thu, Nov 26, 2015 at 01:05:20PM +0800, Fam Zheng wrote:
> v3: Add Eric's rev-by in patches 6, 7, 13, 14.
>     12: New, split out from the previous 13.
>     12->13: Refactor "entry_mergable" from imp_map().
>         Don't mess the merge conditions. [Paolo]
>         Address Eric's comments:
>         - Check has_foo before using foo.
>         - Remove blank line between comments and definition in schema.
>         - Use PRId64 instead of %ld.
>         - Merge short lines.
> 
> v2: Add Eric's rev-by in patches 2, 4, 5.
>     01: Refering -> referring in commit message. [Eric]
>         Recurse to "file" for sensible "zero" flag. [Paolo]
>     12: New. Make MapEntry a QAPI struct. [Paolo, Markus]
> 
> I stumbled upon this when looking at external bitmap formats.
> 
> Current "qemu-img map" command only displays filename if the data is allocated
> in bs (bs->file) itself, or in the backing chain. Otherwise, it displays an
> unfriendly error message:
> 
>     $ qemu-img create -f vmdk -o subformat=monolithicFlat /tmp/test.vmdk 1G
> 
>     $ qemu-img map /tmp/test.vmdk 
>     Offset          Length          Mapped to       File
>     qemu-img: File contains external, encrypted or compressed clusters.
> 
> This can be improved. This series extends the .bdrv_co_get_block_status
> callback, to let block driver return the BDS of file; then updates all driver
> to implement it; and lastly, it changes qemu-img to use this information in
> "map" command:
> 
>     $ qemu-img map /tmp/test.vmdk 
>     Offset          Length          Mapped to       File
>     0               0x40000000      0               /tmp/test-flat.vmdk
> 
>     $ qemu-img map --output json /tmp/test.vmdk 
>     [{"length": 1073741824, "start": 0, "zero": false, "offset": 0, "depth": 0,
>       "file": "/tmp/test-flat.vmdk", "data": true}
>     ]
> 
> 
> Fam Zheng (15):
>   block: Add "file" output parameter to block status query functions
>   qcow: Assign bs->file->bs to file in qcow_co_get_block_status
>   qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status
>   raw: Assign bs to file in raw_co_get_block_status
>   iscsi: Assign bs to file in iscsi_co_get_block_status
>   parallels: Assign bs->file->bs to file in
>     parallels_co_get_block_status
>   qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status
>   sheepdog: Assign bs to file in sd_co_get_block_status
>   vdi: Assign bs->file->bs to file in vdi_co_get_block_status
>   vpc: Assign bs->file->bs to file in vpc_co_get_block_status
>   vmdk: Return extent's file in bdrv_get_block_status
>   qemu-img: In "map", use the returned "file" from bdrv_get_block_status
>   qemu-img: Make MapEntry a QAPI struct
>   qemu-img: Use QAPI visitor to generate JSON
>   iotests: Add "qemu-img map" test for VMDK extents
> 
>  block/io.c                 |  42 ++++++++++++-------
>  block/iscsi.c              |   9 +++-
>  block/mirror.c             |   3 +-
>  block/parallels.c          |   3 +-
>  block/qcow.c               |   3 +-
>  block/qcow2.c              |   3 +-
>  block/qed.c                |   6 ++-
>  block/raw-posix.c          |   4 +-
>  block/raw_bsd.c            |   4 +-
>  block/sheepdog.c           |   5 ++-
>  block/vdi.c                |   3 +-
>  block/vmdk.c               |  13 +++---
>  block/vpc.c                |   4 +-
>  block/vvfat.c              |   2 +-
>  include/block/block.h      |   6 ++-
>  include/block/block_int.h  |   3 +-
>  qapi/block-core.json       |  27 ++++++++++++
>  qemu-img.c                 | 102 +++++++++++++++++++++++++++++----------------
>  tests/qemu-iotests/059     |  10 +++++
>  tests/qemu-iotests/059.out |  38 +++++++++++++++++
>  tests/qemu-iotests/122.out |  96 +++++++++++++++++++++++-------------------
>  21 files changed, 270 insertions(+), 116 deletions(-)

Looks good overall but I posted a question on one patch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH v3 00/15] qemu-img map: Allow driver to return file of the allocated block

[Stefan Hajnoczi]

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

On Thu, Nov 26, 2015 at 01:05:20PM +0800, Fam Zheng wrote:
> v3: Add Eric's rev-by in patches 6, 7, 13, 14.
>     12: New, split out from the previous 13.
>     12->13: Refactor "entry_mergable" from imp_map().
>         Don't mess the merge conditions. [Paolo]
>         Address Eric's comments:
>         - Check has_foo before using foo.
>         - Remove blank line between comments and definition in schema.
>         - Use PRId64 instead of %ld.
>         - Merge short lines.
> 
> v2: Add Eric's rev-by in patches 2, 4, 5.
>     01: Refering -> referring in commit message. [Eric]
>         Recurse to "file" for sensible "zero" flag. [Paolo]
>     12: New. Make MapEntry a QAPI struct. [Paolo, Markus]
> 
> I stumbled upon this when looking at external bitmap formats.
> 
> Current "qemu-img map" command only displays filename if the data is allocated
> in bs (bs->file) itself, or in the backing chain. Otherwise, it displays an
> unfriendly error message:
> 
>     $ qemu-img create -f vmdk -o subformat=monolithicFlat /tmp/test.vmdk 1G
> 
>     $ qemu-img map /tmp/test.vmdk 
>     Offset          Length          Mapped to       File
>     qemu-img: File contains external, encrypted or compressed clusters.
> 
> This can be improved. This series extends the .bdrv_co_get_block_status
> callback, to let block driver return the BDS of file; then updates all driver
> to implement it; and lastly, it changes qemu-img to use this information in
> "map" command:
> 
>     $ qemu-img map /tmp/test.vmdk 
>     Offset          Length          Mapped to       File
>     0               0x40000000      0               /tmp/test-flat.vmdk
> 
>     $ qemu-img map --output json /tmp/test.vmdk 
>     [{"length": 1073741824, "start": 0, "zero": false, "offset": 0, "depth": 0,
>       "file": "/tmp/test-flat.vmdk", "data": true}
>     ]
> 
> 
> Fam Zheng (15):
>   block: Add "file" output parameter to block status query functions
>   qcow: Assign bs->file->bs to file in qcow_co_get_block_status
>   qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status
>   raw: Assign bs to file in raw_co_get_block_status
>   iscsi: Assign bs to file in iscsi_co_get_block_status
>   parallels: Assign bs->file->bs to file in
>     parallels_co_get_block_status
>   qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status
>   sheepdog: Assign bs to file in sd_co_get_block_status
>   vdi: Assign bs->file->bs to file in vdi_co_get_block_status
>   vpc: Assign bs->file->bs to file in vpc_co_get_block_status
>   vmdk: Return extent's file in bdrv_get_block_status
>   qemu-img: In "map", use the returned "file" from bdrv_get_block_status
>   qemu-img: Make MapEntry a QAPI struct
>   qemu-img: Use QAPI visitor to generate JSON
>   iotests: Add "qemu-img map" test for VMDK extents
> 
>  block/io.c                 |  42 ++++++++++++-------
>  block/iscsi.c              |   9 +++-
>  block/mirror.c             |   3 +-
>  block/parallels.c          |   3 +-
>  block/qcow.c               |   3 +-
>  block/qcow2.c              |   3 +-
>  block/qed.c                |   6 ++-
>  block/raw-posix.c          |   4 +-
>  block/raw_bsd.c            |   4 +-
>  block/sheepdog.c           |   5 ++-
>  block/vdi.c                |   3 +-
>  block/vmdk.c               |  13 +++---
>  block/vpc.c                |   4 +-
>  block/vvfat.c              |   2 +-
>  include/block/block.h      |   6 ++-
>  include/block/block_int.h  |   3 +-
>  qapi/block-core.json       |  27 ++++++++++++
>  qemu-img.c                 | 102 +++++++++++++++++++++++++++++----------------
>  tests/qemu-iotests/059     |  10 +++++
>  tests/qemu-iotests/059.out |  38 +++++++++++++++++
>  tests/qemu-iotests/122.out |  96 +++++++++++++++++++++++-------------------
>  21 files changed, 270 insertions(+), 116 deletions(-)
> 
> -- 
> 2.4.3
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]