[Qemu-devel] [PATCH v4 0/4] Fix conversion from ISO to VMDK streamOptimized

[Qemu-devel] [PATCH v4 0/4] Fix conversion from ISO to VMDK streamOptimized

[Fam Zheng]

Previouly, convert from ISO to VMDK with subformat=streamOptimized fails:

    $ ./qemu-img convert -O vmdk -o subformat=streamOptimized foo.iso bar.vmdk
    VMDK: can't write to allocated cluster for streamOptimized
    qemu-img: error while writing sector 64: Input/output error

Because current code in qemu-img.c uses the normal convert loop, rather than
the compressed == true loop. In VMDK streamOptimized, writes must be in cluster
granularity, because overlapped write to an allocated cluster is -EIO.

This series adds is_compressed into BlockDriverInfo, and uses compressed
convertion loop if the target block driver sets this field to true.

It also implements .bdrv_get_info and .bdrv_write_compressed in VMDK driver to
fit into this framework.

Adds a test case to cover the code path in question: source image cluster size
is smaller.

v4: Rename .is_compressed -> .needs_compressed_writes. (Stefan)
    has -> have in commit log in patch 3.

v3: Finally revisit and address Stefan's review comments on v2 from 2 months
    ago. Sorry for being so slow on this one.
    The general approach is the same, changes include:

    - Split originial "[PATCH v2 5/5] mirror: Check for bdrv_get_info result" and
      send to list separately, because it's irrelevant with this series.
    - Don't report cluster_size in bdrv_get_info if it's a flat extent, no need
      to change type of BlockDriverInfo.cluster_size to 64 bits. So drop
      "[PATCH v2 3/5] block: Change BlockDriverInfo.cluster_size to 64 bits".
    - Add qemu-iotests case as [PATCH 4/4].


Fam Zheng (4):
  qemu-img: Convert by cluster size if target is compressed
  vmdk: Implement .bdrv_write_compressed
  vmdk: Implement .bdrv_get_info()
  qemu-iotests: Test converting to streamOptimized from small cluster
    size

 block/vmdk.c               | 35 +++++++++++++++++++++++++++++++++++
 include/block/block.h      |  4 ++++
 qemu-img.c                 |  1 +
 tests/qemu-iotests/059     |  7 +++++++
 tests/qemu-iotests/059.out |  8 ++++++++
 5 files changed, 55 insertions(+)

-- 
1.9.2

[Qemu-devel] [PATCH v4 1/4] qemu-img: Convert by cluster size if target is compressed

[Fam Zheng]

If target block driver forces compression, qemu-img convert needs to
write by cluster size as well as "-c" option.

Particularly, this applies for converting to VMDK streamOptimized
format.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/block/block.h | 4 ++++
 qemu-img.c            | 1 +
 2 files changed, 5 insertions(+)

diff --git a/include/block/block.h b/include/block/block.h
index 467fb2b..27d8598 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -34,6 +34,10 @@ typedef struct BlockDriverInfo {
      * opened with BDRV_O_UNMAP flag for this to work.
      */
     bool can_write_zeroes_with_unmap;
+    /*
+     * True if this block driver only supports compressed writes
+     */
+    bool needs_compressed_writes;
 } BlockDriverInfo;
 
 typedef struct BlockFragInfo {
diff --git a/qemu-img.c b/qemu-img.c
index 96f4463..ed4471b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1480,6 +1480,7 @@ static int img_convert(int argc, char **argv)
             goto out;
         }
     } else {
+        compress = compress || bdi.needs_compressed_writes;
         cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
     }
 
-- 
1.9.2

[Qemu-devel] [PATCH v4 2/4] vmdk: Implement .bdrv_write_compressed

[Fam Zheng]

Add a wrapper function to support "compressed" path in qemu-img convert.
Only support streamOptimized subformat case for now (num_extents == 1
and extent compression is true).

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

diff --git a/block/vmdk.c b/block/vmdk.c
index d7e48e0..a7783f7 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1510,6 +1510,19 @@ static coroutine_fn int vmdk_co_write(BlockDriverState *bs, int64_t sector_num,
     return ret;
 }
 
+static int vmdk_write_compressed(BlockDriverState *bs,
+                                 int64_t sector_num,
+                                 const uint8_t *buf,
+                                 int nb_sectors)
+{
+    BDRVVmdkState *s = bs->opaque;
+    if (s->num_extents == 1 && s->extents[0].compressed) {
+        return vmdk_write(bs, sector_num, buf, nb_sectors, false, false);
+    } else {
+        return -ENOTSUP;
+    }
+}
+
 static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs,
                                              int64_t sector_num,
                                              int nb_sectors,
@@ -2123,6 +2136,7 @@ static BlockDriver bdrv_vmdk = {
     .bdrv_reopen_prepare          = vmdk_reopen_prepare,
     .bdrv_read                    = vmdk_co_read,
     .bdrv_write                   = vmdk_co_write,
+    .bdrv_write_compressed        = vmdk_write_compressed,
     .bdrv_co_write_zeroes         = vmdk_co_write_zeroes,
     .bdrv_close                   = vmdk_close,
     .bdrv_create                  = vmdk_create,
-- 
1.9.2

[Qemu-devel] [PATCH v4 3/4] vmdk: Implement .bdrv_get_info()

[Fam Zheng]

This will return cluster_size and needs_compressed_writes to caller, if all the
extents have the same value (or there's only one extent). Otherwise return
-ENOTSUP.

cluster_size is only reported for sparse formats.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c               | 21 +++++++++++++++++++++
 tests/qemu-iotests/059.out |  1 +
 2 files changed, 22 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index a7783f7..ab18ece 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2090,6 +2090,26 @@ static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs)
     return spec_info;
 }
 
+static int vmdk_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+    int i;
+    BDRVVmdkState *s = bs->opaque;
+    assert(s->num_extents);
+    bdi->needs_compressed_writes = s->extents[0].compressed;
+    if (!s->extents[0].flat) {
+        bdi->cluster_size = s->extents[0].cluster_sectors << BDRV_SECTOR_BITS;
+    }
+    /* See if we have multiple extents but they have different cases */
+    for (i = 1; i < s->num_extents; i++) {
+        if (bdi->needs_compressed_writes != s->extents[i].compressed ||
+            (bdi->cluster_size && bdi->cluster_size !=
+                s->extents[i].cluster_sectors << BDRV_SECTOR_BITS)) {
+            return -ENOTSUP;
+        }
+    }
+    return 0;
+}
+
 static QEMUOptionParameter vmdk_create_options[] = {
     {
         .name = BLOCK_OPT_SIZE,
@@ -2146,6 +2166,7 @@ static BlockDriver bdrv_vmdk = {
     .bdrv_has_zero_init           = vmdk_has_zero_init,
     .bdrv_get_specific_info       = vmdk_get_specific_info,
     .bdrv_refresh_limits          = vmdk_refresh_limits,
+    .bdrv_get_info                = vmdk_get_info,
 
     .create_options               = vmdk_create_options,
 };
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 3371c86..14c0957 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2050,6 +2050,7 @@ qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at least
 image: TEST_DIR/iotest-version3.IMGFMT
 file format: IMGFMT
 virtual size: 1.0G (1073741824 bytes)
+cluster_size: 65536
 
 === Testing 4TB monolithicFlat creation and IO ===
 Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104
-- 
1.9.2

[Qemu-devel] [PATCH v4 4/4] qemu-iotests: Test converting to streamOptimized from small cluster size

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/059     | 7 +++++++
 tests/qemu-iotests/059.out | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index ca5aa16..26a2fd3 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -104,6 +104,13 @@ truncate -s 10M $TEST_IMG
 _img_info
 
 echo
+echo "=== Converting to streamOptimized from image with small cluster size==="
+TEST_IMG="$TEST_IMG.qcow2" IMGFMT=qcow2 IMGOPTS="cluster_size=4096" _make_test_img 1G
+$QEMU_IO -c "write -P 0xa 0 512" "$TEST_IMG.qcow2" | _filter_qemu_io
+$QEMU_IO -c "write -P 0xb 10240 512" "$TEST_IMG.qcow2" | _filter_qemu_io
+$QEMU_IMG convert -f qcow2 -O vmdk -o subformat=streamOptimized "$TEST_IMG.qcow2" "$TEST_IMG" 2>&1
+
+echo
 echo "=== Testing version 3 ==="
 _use_sample_img iotest-version3.vmdk.bz2
 _img_info
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 14c0957..eba0ded 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2046,6 +2046,13 @@ RW 12582912 VMFS "dummy.IMGFMT" 1
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at least 13172736 bytes
 
+=== Converting to streamOptimized from image with small cluster size===
+Formatting 'TEST_DIR/t.vmdk.IMGFMT', fmt=IMGFMT size=1073741824
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 10240
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
 === Testing version 3 ===
 image: TEST_DIR/iotest-version3.IMGFMT
 file format: IMGFMT
-- 
1.9.2

Re: [Qemu-devel] [PATCH v4 0/4] Fix conversion from ISO to VMDK streamOptimized

[Stefan Hajnoczi]

On Tue, May 06, 2014 at 09:08:42PM +0800, Fam Zheng wrote:
> Previouly, convert from ISO to VMDK with subformat=streamOptimized fails:
> 
>     $ ./qemu-img convert -O vmdk -o subformat=streamOptimized foo.iso bar.vmdk
>     VMDK: can't write to allocated cluster for streamOptimized
>     qemu-img: error while writing sector 64: Input/output error
> 
> Because current code in qemu-img.c uses the normal convert loop, rather than
> the compressed == true loop. In VMDK streamOptimized, writes must be in cluster
> granularity, because overlapped write to an allocated cluster is -EIO.
> 
> This series adds is_compressed into BlockDriverInfo, and uses compressed
> convertion loop if the target block driver sets this field to true.
> 
> It also implements .bdrv_get_info and .bdrv_write_compressed in VMDK driver to
> fit into this framework.
> 
> Adds a test case to cover the code path in question: source image cluster size
> is smaller.
> 
> v4: Rename .is_compressed -> .needs_compressed_writes. (Stefan)
>     has -> have in commit log in patch 3.
> 
> v3: Finally revisit and address Stefan's review comments on v2 from 2 months
>     ago. Sorry for being so slow on this one.
>     The general approach is the same, changes include:
> 
>     - Split originial "[PATCH v2 5/5] mirror: Check for bdrv_get_info result" and
>       send to list separately, because it's irrelevant with this series.
>     - Don't report cluster_size in bdrv_get_info if it's a flat extent, no need
>       to change type of BlockDriverInfo.cluster_size to 64 bits. So drop
>       "[PATCH v2 3/5] block: Change BlockDriverInfo.cluster_size to 64 bits".
>     - Add qemu-iotests case as [PATCH 4/4].
> 
> 
> Fam Zheng (4):
>   qemu-img: Convert by cluster size if target is compressed
>   vmdk: Implement .bdrv_write_compressed
>   vmdk: Implement .bdrv_get_info()
>   qemu-iotests: Test converting to streamOptimized from small cluster
>     size
> 
>  block/vmdk.c               | 35 +++++++++++++++++++++++++++++++++++
>  include/block/block.h      |  4 ++++
>  qemu-img.c                 |  1 +
>  tests/qemu-iotests/059     |  7 +++++++
>  tests/qemu-iotests/059.out |  8 ++++++++
>  5 files changed, 55 insertions(+)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan