All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 00/15] backup compression
@ 2016-07-04 12:28 Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 01/15] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
                   ` (16 more replies)
  0 siblings, 17 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

The idea is simple - backup is "written-once" data. It is written block
by block and it is large enough. It would be nice to save storage
space and compress it.

These patches add the ability to compress data during backup. This
functionality is implemented by means of adding options to the qmp/hmp
commands(drive-backup, blockdev-backup). The implementation is quite
simple, because the responsibility for data compression imposed on the
format driver.

This patchset is based on the Eric's QAPI changes
  http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>

Changes from v1:
- added unittest for backup compression (12, 13)

Changes from v2:
- implemented a new .bdrv_co_write_compressed interface to replace the
  old .bdrv_write_compressed (2,3,4,5,6)

Changes from v3:
- added the byte-based interfaces:
  blk_pwrite_compressed()/blk_co_pwritev_compressed() (1, 7)
- fix drive/blockdev-backup documentation (10, 11)

Changes form v4:
- added assert that offset and count are aligned (1)
- reuse RwCo and bdrv_co_pwritev() for write compressed (2)
- converted interfaces to byte-based for format drivers (2, 3, 5, 6)
- move an unrelated cleanup in a separate patches (4, 7)
- turn on dirty_bitmaps for the compressed writes (9)
- added simplify drive/blockdev-backup by using the boxed commands (10, 11)
- reworded drive/blockdev-backup documentation about compression (12, 13)
- fix s/bakup/backup/ (14)

Pavel Butsykin (15):
  block: switch blk_write_compressed() to byte-based interface
  block/io: reuse bdrv_co_pwritev() for write compressed
  qcow2: add qcow2_co_pwritev_compressed
  qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
  vmdk: add vmdk_co_pwritev_compressed
  qcow: add qcow_co_pwritev_compressed
  qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
  block: remove BlockDriver.bdrv_write_compressed
  block/io: turn on dirty_bitmaps for the compressed writes
  block: simplify drive-backup
  block: simplify blockdev-backup
  drive-backup: added support for data compression
  blockdev-backup: added support for data compression
  qemu-iotests: test backup compression in 055
  qemu-iotests: add vmdk for test backup compression in 055

 block/backup.c                 |  12 ++-
 block/block-backend.c          |  12 +--
 block/io.c                     |  37 ++++-----
 block/qcow.c                   |  71 +++++++++-------
 block/qcow2.c                  |  83 +++++++++++--------
 block/vmdk.c                   |  55 ++-----------
 blockdev.c                     | 182 ++++++++++++++---------------------------
 hmp-commands.hx                |   8 +-
 hmp.c                          |  32 +++++---
 include/block/block.h          |   5 +-
 include/block/block_int.h      |   5 +-
 include/sysemu/block-backend.h |   4 +-
 qapi/block-core.json           |  18 +++-
 qemu-img.c                     |   8 +-
 qemu-io-cmds.c                 |   2 +-
 qmp-commands.hx                |   9 +-
 tests/qemu-iotests/055         | 118 ++++++++++++++++++++++++++
 tests/qemu-iotests/055.out     |   4 +-
 tests/qemu-iotests/iotests.py  |  10 +--
 19 files changed, 369 insertions(+), 306 deletions(-)

-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 01/15] block: switch blk_write_compressed() to byte-based interface
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 02/15] block/io: reuse bdrv_co_pwritev() for write compressed Denis V. Lunev
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

This is a preparatory patch, which continues the general trend of the
transition to the byte-based interfaces.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c          |  8 ++++----
 block/io.c                     | 11 +++++++----
 include/block/block.h          |  4 ++--
 include/sysemu/block-backend.h |  4 ++--
 qemu-img.c                     |  6 ++++--
 qemu-io-cmds.c                 |  2 +-
 6 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 34500e6..3c1fc50 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1477,15 +1477,15 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                           flags | BDRV_REQ_ZERO_WRITE);
 }
 
-int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
-                         const uint8_t *buf, int nb_sectors)
+int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
+                          int count)
 {
-    int ret = blk_check_request(blk, sector_num, nb_sectors);
+    int ret = blk_check_byte_request(blk, offset, count);
     if (ret < 0) {
         return ret;
     }
 
-    return bdrv_write_compressed(blk_bs(blk), sector_num, buf, nb_sectors);
+    return bdrv_pwrite_compressed(blk_bs(blk), offset, buf, count);
 }
 
 int blk_truncate(BlockBackend *blk, int64_t offset)
diff --git a/block/io.c b/block/io.c
index 7cf3645..22f20b1 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1824,8 +1824,8 @@ int bdrv_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
-int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
-                          const uint8_t *buf, int nb_sectors)
+int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset,
+                           const void *buf, int bytes)
 {
     BlockDriver *drv = bs->drv;
     int ret;
@@ -1836,14 +1836,17 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
     if (!drv->bdrv_write_compressed) {
         return -ENOTSUP;
     }
-    ret = bdrv_check_request(bs, sector_num, nb_sectors);
+    ret = bdrv_check_byte_request(bs, offset, bytes);
     if (ret < 0) {
         return ret;
     }
 
     assert(QLIST_EMPTY(&bs->dirty_bitmaps));
+    assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+    assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 
-    return drv->bdrv_write_compressed(bs, sector_num, buf, nb_sectors);
+    return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf,
+                                      bytes >> BDRV_SECTOR_BITS);
 }
 
 typedef struct BdrvVmstateCo {
diff --git a/include/block/block.h b/include/block/block.h
index 733a8ec..60cd7d1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -401,8 +401,8 @@ const char *bdrv_get_node_name(const BlockDriverState *bs);
 const char *bdrv_get_device_name(const BlockDriverState *bs);
 const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
 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_pwrite_compressed(BlockDriverState *bs, int64_t offset,
+                           const void *buf, int bytes);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
 void bdrv_round_sectors_to_clusters(BlockDriverState *bs,
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index c04af8e..57df069 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -203,8 +203,8 @@ void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
                   BlockCompletionFunc *cb, void *opaque);
 int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
                                       int count, BdrvRequestFlags flags);
-int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
-                         const uint8_t *buf, int nb_sectors);
+int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
+                          int count);
 int blk_truncate(BlockBackend *blk, int64_t offset);
 int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
diff --git a/qemu-img.c b/qemu-img.c
index 0ad6051..5b2b8df 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1579,7 +1579,9 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors,
                     break;
                 }
 
-                ret = blk_write_compressed(s->target, sector_num, buf, n);
+                ret = blk_pwrite_compressed(s->target,
+                                            sector_num << BDRV_SECTOR_BITS,
+                                            buf, n << BDRV_SECTOR_BITS);
                 if (ret < 0) {
                     return ret;
                 }
@@ -1716,7 +1718,7 @@ static int convert_do_copy(ImgConvertState *s)
 
     if (s->compressed) {
         /* signal EOF to align */
-        ret = blk_write_compressed(s->target, 0, NULL, 0);
+        ret = blk_pwrite_compressed(s->target, 0, NULL, 0);
         if (ret < 0) {
             goto fail;
         }
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 09e879f..2ae3b14 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -504,7 +504,7 @@ static int do_write_compressed(BlockBackend *blk, char *buf, int64_t offset,
         return -ERANGE;
     }
 
-    ret = blk_write_compressed(blk, offset >> 9, (uint8_t *)buf, count >> 9);
+    ret = blk_pwrite_compressed(blk, offset, buf, count);
     if (ret < 0) {
         return ret;
     }
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 02/15] block/io: reuse bdrv_co_pwritev() for write compressed
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 01/15] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 03/15] qcow2: add qcow2_co_pwritev_compressed Denis V. Lunev
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

For bdrv_pwrite_compressed() it looks like most of the code creating
coroutine is duplicated in bdrv_prwv_co(). So we can just add a flag
(BDRV_REQ_WRITE_COMPRESSED) and use bdrv_prwv_co() as a generic one.
In the end we get coroutine oriented function for write compressed by using
bdrv_co_pwritev/blk_co_pwritev with BDRV_REQ_WRITE_COMPRESSED flag.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/io.c                | 57 ++++++++++++++++++++++++++++++++++-------------
 include/block/block.h     |  3 ++-
 include/block/block_int.h |  3 +++
 qemu-img.c                |  2 +-
 4 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/block/io.c b/block/io.c
index 22f20b1..49742eb 100644
--- a/block/io.c
+++ b/block/io.c
@@ -902,6 +902,20 @@ emulate_flags:
     return ret;
 }
 
+static int coroutine_fn
+bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
+                               uint64_t bytes, QEMUIOVector *qiov)
+{
+    BlockDriver *drv = bs->drv;
+
+    if (!drv->bdrv_co_pwritev_compressed) {
+        return -ENOTSUP;
+    }
+
+    assert(QLIST_EMPTY(&bs->dirty_bitmaps));
+    return drv->bdrv_co_pwritev_compressed(bs, offset, bytes, qiov);
+}
+
 static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs,
         int64_t offset, unsigned int bytes, QEMUIOVector *qiov)
 {
@@ -1511,9 +1525,14 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
         bytes = ROUND_UP(bytes, align);
     }
 
-    ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
-                               use_local_qiov ? &local_qiov : qiov,
-                               flags);
+    if (flags & BDRV_REQ_WRITE_COMPRESSED) {
+        ret = bdrv_driver_pwritev_compressed(
+                bs, offset, bytes, use_local_qiov ? &local_qiov : qiov);
+    } else {
+        ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
+                                   use_local_qiov ? &local_qiov : qiov,
+                                   flags);
+    }
 
 fail:
 
@@ -1827,26 +1846,32 @@ int bdrv_is_allocated_above(BlockDriverState *top,
 int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset,
                            const void *buf, int bytes)
 {
+    QEMUIOVector qiov;
+    struct iovec iov;
     BlockDriver *drv = bs->drv;
-    int ret;
 
     if (!drv) {
         return -ENOMEDIUM;
     }
-    if (!drv->bdrv_write_compressed) {
-        return -ENOTSUP;
-    }
-    ret = bdrv_check_byte_request(bs, offset, bytes);
-    if (ret < 0) {
-        return ret;
-    }
 
-    assert(QLIST_EMPTY(&bs->dirty_bitmaps));
-    assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-    assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+    if (drv->bdrv_write_compressed) {
+        int ret = bdrv_check_byte_request(bs, offset, bytes);
+        if (ret < 0) {
+            return ret;
+        }
+        assert(QLIST_EMPTY(&bs->dirty_bitmaps));
+        assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+        assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+        return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf,
+                                          bytes >> BDRV_SECTOR_BITS);
+    }
+    iov = (struct iovec) {
+        .iov_base = (void *)buf,
+        .iov_len = bytes,
+    };
+    qemu_iovec_init_external(&qiov, &iov, 1);
 
-    return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf,
-                                      bytes >> BDRV_SECTOR_BITS);
+    return bdrv_prwv_co(bs, offset, &qiov, true, BDRV_REQ_WRITE_COMPRESSED);
 }
 
 typedef struct BdrvVmstateCo {
diff --git a/include/block/block.h b/include/block/block.h
index 60cd7d1..0d01ffd 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -65,9 +65,10 @@ typedef enum {
     BDRV_REQ_MAY_UNMAP          = 0x4,
     BDRV_REQ_NO_SERIALISING     = 0x8,
     BDRV_REQ_FUA                = 0x10,
+    BDRV_REQ_WRITE_COMPRESSED   = 0x20,
 
     /* Mask of valid flags */
-    BDRV_REQ_MASK               = 0x1f,
+    BDRV_REQ_MASK               = 0x3f,
 } BdrvRequestFlags;
 
 typedef struct BlockSizes {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2057156..17a5efe 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -207,6 +207,9 @@ struct BlockDriver {
     int (*bdrv_write_compressed)(BlockDriverState *bs, int64_t sector_num,
                                  const uint8_t *buf, int nb_sectors);
 
+    int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
+        uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
+
     int (*bdrv_snapshot_create)(BlockDriverState *bs,
                                 QEMUSnapshotInfo *sn_info);
     int (*bdrv_snapshot_goto)(BlockDriverState *bs,
diff --git a/qemu-img.c b/qemu-img.c
index 5b2b8df..7bb1cfc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2023,7 +2023,7 @@ static int img_convert(int argc, char **argv)
         const char *preallocation =
             qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
 
-        if (!drv->bdrv_write_compressed) {
+        if (!drv->bdrv_write_compressed && !drv->bdrv_co_pwritev_compressed) {
             error_report("Compression not supported for this file format");
             ret = -1;
             goto out;
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 03/15] qcow2: add qcow2_co_pwritev_compressed
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 01/15] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 02/15] block/io: reuse bdrv_co_pwritev() for write compressed Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Added implementation of the qcow2_co_pwritev_compressed function that
will allow us to safely use compressed writes for the qcow2 from running
VMs.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 79 +++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 23f666d..0da44e0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2534,37 +2534,47 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
 
 /* XXX: put compressed sectors first, then all the cluster aligned
    tables to avoid losing bytes in alignment */
-static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num,
-                                  const uint8_t *buf, int nb_sectors)
+static coroutine_fn int
+qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
+                            uint64_t bytes, QEMUIOVector *qiov)
 {
     BDRVQcow2State *s = bs->opaque;
+    QEMUIOVector hd_qiov;
+    struct iovec iov;
     z_stream strm;
     int ret, out_len;
-    uint8_t *out_buf;
+    uint8_t *buf, *out_buf;
     uint64_t cluster_offset;
 
-    if (nb_sectors == 0) {
+    if (bytes == 0) {
         /* align end of file to a sector boundary to ease reading with
            sector based I/Os */
         cluster_offset = bdrv_getlength(bs->file->bs);
         return bdrv_truncate(bs->file->bs, cluster_offset);
     }
 
-    if (nb_sectors != s->cluster_sectors) {
+    if (bytes != s->cluster_size) {
         ret = -EINVAL;
 
         /* Zero-pad last write if image size is not cluster aligned */
-        if (sector_num + nb_sectors == bs->total_sectors &&
-            nb_sectors < s->cluster_sectors) {
+        if (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS &&
+            bytes < s->cluster_size)
+        {
             uint8_t *pad_buf = qemu_blockalign(bs, s->cluster_size);
             memset(pad_buf, 0, s->cluster_size);
-            memcpy(pad_buf, buf, nb_sectors * BDRV_SECTOR_SIZE);
-            ret = qcow2_write_compressed(bs, sector_num,
-                                         pad_buf, s->cluster_sectors);
+            qemu_iovec_to_buf(qiov, 0, pad_buf, s->cluster_size);
+            iov = (struct iovec) {
+                .iov_base   = pad_buf,
+                .iov_len    = s->cluster_size,
+            };
+            qemu_iovec_init_external(&hd_qiov, &iov, 1);
+            ret = qcow2_co_pwritev_compressed(bs, offset, bytes, &hd_qiov);
             qemu_vfree(pad_buf);
         }
         return ret;
     }
+    buf = qemu_blockalign(bs, s->cluster_size);
+    qemu_iovec_to_buf(qiov, 0, buf, s->cluster_size);
 
     out_buf = g_malloc(s->cluster_size + (s->cluster_size / 1000) + 128);
 
@@ -2595,33 +2605,44 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num,
 
     if (ret != Z_STREAM_END || out_len >= s->cluster_size) {
         /* could not compress: write normal cluster */
-        ret = bdrv_write(bs, sector_num, buf, s->cluster_sectors);
+        ret = qcow2_co_pwritev(bs, offset, bytes, qiov, 0);
         if (ret < 0) {
             goto fail;
         }
-    } else {
-        cluster_offset = qcow2_alloc_compressed_cluster_offset(bs,
-            sector_num << 9, out_len);
-        if (!cluster_offset) {
-            ret = -EIO;
-            goto fail;
-        }
-        cluster_offset &= s->cluster_offset_mask;
+        goto success;
+    }
 
-        ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset, out_len);
-        if (ret < 0) {
-            goto fail;
-        }
+    qemu_co_mutex_lock(&s->lock);
+    cluster_offset =
+        qcow2_alloc_compressed_cluster_offset(bs, offset, out_len);
+    if (!cluster_offset) {
+        qemu_co_mutex_unlock(&s->lock);
+        ret = -EIO;
+        goto fail;
+    }
+    cluster_offset &= s->cluster_offset_mask;
 
-        BLKDBG_EVENT(bs->file, BLKDBG_WRITE_COMPRESSED);
-        ret = bdrv_pwrite(bs->file->bs, cluster_offset, out_buf, out_len);
-        if (ret < 0) {
-            goto fail;
-        }
+    ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset, out_len);
+    qemu_co_mutex_unlock(&s->lock);
+    if (ret < 0) {
+        goto fail;
     }
 
+    iov = (struct iovec) {
+        .iov_base   = out_buf,
+        .iov_len    = out_len,
+    };
+    qemu_iovec_init_external(&hd_qiov, &iov, 1);
+
+    BLKDBG_EVENT(bs->file, BLKDBG_WRITE_COMPRESSED);
+    ret = bdrv_co_pwritev(bs->file->bs, cluster_offset, out_len, &hd_qiov, 0);
+    if (ret < 0) {
+        goto fail;
+    }
+success:
     ret = 0;
 fail:
+    qemu_vfree(buf);
     g_free(out_buf);
     return ret;
 }
@@ -3366,7 +3387,7 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_co_pwrite_zeroes  = qcow2_co_pwrite_zeroes,
     .bdrv_co_discard        = qcow2_co_discard,
     .bdrv_truncate          = qcow2_truncate,
-    .bdrv_write_compressed  = qcow2_write_compressed,
+    .bdrv_co_pwritev_compressed = qcow2_co_pwritev_compressed,
     .bdrv_make_empty        = qcow2_make_empty,
 
     .bdrv_snapshot_create   = qcow2_snapshot_create,
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (2 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 03/15] qcow2: add qcow2_co_pwritev_compressed Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-14 12:35   ` Eric Blake
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 05/15] vmdk: add vmdk_co_pwritev_compressed Denis V. Lunev
                   ` (12 subsequent siblings)
  16 siblings, 2 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Since the function became use a vector instead of a buffer there is no
sense to use a recursive code.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 0da44e0..56f16d7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2553,27 +2553,17 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
         return bdrv_truncate(bs->file->bs, cluster_offset);
     }
 
+    buf = qemu_blockalign(bs, s->cluster_size);
     if (bytes != s->cluster_size) {
-        ret = -EINVAL;
-
-        /* Zero-pad last write if image size is not cluster aligned */
-        if (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS &&
-            bytes < s->cluster_size)
+        if (bytes > s->cluster_size ||
+            offset + bytes != bs->total_sectors << BDRV_SECTOR_BITS)
         {
-            uint8_t *pad_buf = qemu_blockalign(bs, s->cluster_size);
-            memset(pad_buf, 0, s->cluster_size);
-            qemu_iovec_to_buf(qiov, 0, pad_buf, s->cluster_size);
-            iov = (struct iovec) {
-                .iov_base   = pad_buf,
-                .iov_len    = s->cluster_size,
-            };
-            qemu_iovec_init_external(&hd_qiov, &iov, 1);
-            ret = qcow2_co_pwritev_compressed(bs, offset, bytes, &hd_qiov);
-            qemu_vfree(pad_buf);
+            qemu_vfree(buf);
+            return -EINVAL;
         }
-        return ret;
+        /* Zero-pad last write if image size is not cluster aligned */
+        memset(buf + bytes, 0, s->cluster_size - bytes);
     }
-    buf = qemu_blockalign(bs, s->cluster_size);
     qemu_iovec_to_buf(qiov, 0, buf, s->cluster_size);
 
     out_buf = g_malloc(s->cluster_size + (s->cluster_size / 1000) + 128);
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 05/15] vmdk: add vmdk_co_pwritev_compressed
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (3 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 06/15] qcow: add qcow_co_pwritev_compressed Denis V. Lunev
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Added implementation of the vmdk_co_pwritev_compressed function that
will allow us to safely use compressed writes for the vmdk from running
VMs.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 55 +++++--------------------------------------------------
 1 file changed, 5 insertions(+), 50 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2901692..18bebd6 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1649,56 +1649,11 @@ vmdk_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
     return ret;
 }
 
-typedef struct VmdkWriteCompressedCo {
-    BlockDriverState *bs;
-    int64_t sector_num;
-    const uint8_t *buf;
-    int nb_sectors;
-    int ret;
-} VmdkWriteCompressedCo;
-
-static void vmdk_co_write_compressed(void *opaque)
-{
-    VmdkWriteCompressedCo *co = opaque;
-    QEMUIOVector local_qiov;
-    uint64_t offset = co->sector_num * BDRV_SECTOR_SIZE;
-    uint64_t bytes = co->nb_sectors * BDRV_SECTOR_SIZE;
-
-    struct iovec iov = (struct iovec) {
-        .iov_base   = (uint8_t*) co->buf,
-        .iov_len    = bytes,
-    };
-    qemu_iovec_init_external(&local_qiov, &iov, 1);
-
-    co->ret = vmdk_pwritev(co->bs, offset, bytes, &local_qiov, false, false);
-}
-
-static int vmdk_write_compressed(BlockDriverState *bs,
-                                 int64_t sector_num,
-                                 const uint8_t *buf,
-                                 int nb_sectors)
+static int coroutine_fn
+vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
+                           uint64_t bytes, QEMUIOVector *qiov)
 {
-    BDRVVmdkState *s = bs->opaque;
-
-    if (s->num_extents == 1 && s->extents[0].compressed) {
-        Coroutine *co;
-        AioContext *aio_context = bdrv_get_aio_context(bs);
-        VmdkWriteCompressedCo data = {
-            .bs         = bs,
-            .sector_num = sector_num,
-            .buf        = buf,
-            .nb_sectors = nb_sectors,
-            .ret        = -EINPROGRESS,
-        };
-        co = qemu_coroutine_create(vmdk_co_write_compressed);
-        qemu_coroutine_enter(co, &data);
-        while (data.ret == -EINPROGRESS) {
-            aio_poll(aio_context, true);
-        }
-        return data.ret;
-    } else {
-        return -ENOTSUP;
-    }
+    return vmdk_co_pwritev(bs, offset, bytes, qiov, 0);
 }
 
 static int coroutine_fn vmdk_co_pwrite_zeroes(BlockDriverState *bs,
@@ -2397,7 +2352,7 @@ static BlockDriver bdrv_vmdk = {
     .bdrv_reopen_prepare          = vmdk_reopen_prepare,
     .bdrv_co_preadv               = vmdk_co_preadv,
     .bdrv_co_pwritev              = vmdk_co_pwritev,
-    .bdrv_write_compressed        = vmdk_write_compressed,
+    .bdrv_co_pwritev_compressed   = vmdk_co_pwritev_compressed,
     .bdrv_co_pwrite_zeroes        = vmdk_co_pwrite_zeroes,
     .bdrv_close                   = vmdk_close,
     .bdrv_create                  = vmdk_create,
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 06/15] qcow: add qcow_co_pwritev_compressed
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (4 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 05/15] vmdk: add vmdk_co_pwritev_compressed Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 07/15] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion Denis V. Lunev
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Added implementation of the qcow_co_pwritev_compressed function that
will allow us to safely use compressed writes for the qcow from running
VMs.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow.c | 67 ++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 24 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 312af52..aadba67 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -916,30 +916,40 @@ static int qcow_make_empty(BlockDriverState *bs)
 
 /* XXX: put compressed sectors first, then all the cluster aligned
    tables to avoid losing bytes in alignment */
-static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num,
-                                 const uint8_t *buf, int nb_sectors)
+static coroutine_fn int
+qcow_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
+                           uint64_t bytes, QEMUIOVector *qiov)
 {
     BDRVQcowState *s = bs->opaque;
+    QEMUIOVector hd_qiov;
+    struct iovec iov;
     z_stream strm;
     int ret, out_len;
-    uint8_t *out_buf;
+    uint8_t *buf, *out_buf;
     uint64_t cluster_offset;
 
-    if (nb_sectors != s->cluster_sectors) {
+    if (bytes != s->cluster_size) {
         ret = -EINVAL;
 
         /* Zero-pad last write if image size is not cluster aligned */
-        if (sector_num + nb_sectors == bs->total_sectors &&
-            nb_sectors < s->cluster_sectors) {
+        if (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS &&
+            bytes < s->cluster_size)
+        {
             uint8_t *pad_buf = qemu_blockalign(bs, s->cluster_size);
             memset(pad_buf, 0, s->cluster_size);
-            memcpy(pad_buf, buf, nb_sectors * BDRV_SECTOR_SIZE);
-            ret = qcow_write_compressed(bs, sector_num,
-                                        pad_buf, s->cluster_sectors);
+            qemu_iovec_to_buf(qiov, 0, pad_buf, s->cluster_size);
+            iov = (struct iovec) {
+                .iov_base   = pad_buf,
+                .iov_len    = s->cluster_size,
+            };
+            qemu_iovec_init_external(&hd_qiov, &iov, 1);
+            ret = qcow_co_pwritev_compressed(bs, offset, bytes, &hd_qiov);
             qemu_vfree(pad_buf);
         }
         return ret;
     }
+    buf = qemu_blockalign(bs, s->cluster_size);
+    qemu_iovec_to_buf(qiov, 0, buf, qiov->size);
 
     out_buf = g_malloc(s->cluster_size + (s->cluster_size / 1000) + 128);
 
@@ -970,27 +980,36 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num,
 
     if (ret != Z_STREAM_END || out_len >= s->cluster_size) {
         /* could not compress: write normal cluster */
-        ret = bdrv_write(bs, sector_num, buf, s->cluster_sectors);
-        if (ret < 0) {
-            goto fail;
-        }
-    } else {
-        cluster_offset = get_cluster_offset(bs, sector_num << 9, 2,
-                                            out_len, 0, 0);
-        if (cluster_offset == 0) {
-            ret = -EIO;
-            goto fail;
-        }
-
-        cluster_offset &= s->cluster_offset_mask;
-        ret = bdrv_pwrite(bs->file->bs, cluster_offset, out_buf, out_len);
+        ret = qcow_co_writev(bs, offset >> BDRV_SECTOR_BITS,
+                             bytes >> BDRV_SECTOR_BITS, qiov);
         if (ret < 0) {
             goto fail;
         }
+        goto success;
     }
 
+    qemu_co_mutex_lock(&s->lock);
+    cluster_offset = get_cluster_offset(bs, offset, 2, out_len, 0, 0);
+    qemu_co_mutex_unlock(&s->lock);
+    if (cluster_offset == 0) {
+        ret = -EIO;
+        goto fail;
+    }
+    cluster_offset &= s->cluster_offset_mask;
+
+    iov = (struct iovec) {
+        .iov_base   = out_buf,
+        .iov_len    = out_len,
+    };
+    qemu_iovec_init_external(&hd_qiov, &iov, 1);
+    ret = bdrv_co_pwritev(bs->file->bs, cluster_offset, out_len, &hd_qiov, 0);
+    if (ret < 0) {
+        goto fail;
+    }
+success:
     ret = 0;
 fail:
+    qemu_vfree(buf);
     g_free(out_buf);
     return ret;
 }
@@ -1043,7 +1062,7 @@ static BlockDriver bdrv_qcow = {
 
     .bdrv_set_key           = qcow_set_key,
     .bdrv_make_empty        = qcow_make_empty,
-    .bdrv_write_compressed  = qcow_write_compressed,
+    .bdrv_co_pwritev_compressed = qcow_co_pwritev_compressed,
     .bdrv_get_info          = qcow_get_info,
 
     .create_opts            = &qcow_create_opts,
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 07/15] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (5 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 06/15] qcow: add qcow_co_pwritev_compressed Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 08/15] block: remove BlockDriver.bdrv_write_compressed Denis V. Lunev
                   ` (9 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Since the function became use a vector instead of a buffer there is no
sense to use a recursive code.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index aadba67..7bbbb5f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -928,27 +928,17 @@ qcow_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
     uint8_t *buf, *out_buf;
     uint64_t cluster_offset;
 
+    buf = qemu_blockalign(bs, s->cluster_size);
     if (bytes != s->cluster_size) {
-        ret = -EINVAL;
-
-        /* Zero-pad last write if image size is not cluster aligned */
-        if (offset + bytes == bs->total_sectors << BDRV_SECTOR_BITS &&
-            bytes < s->cluster_size)
+        if (bytes > s->cluster_size ||
+            offset + bytes != bs->total_sectors << BDRV_SECTOR_BITS)
         {
-            uint8_t *pad_buf = qemu_blockalign(bs, s->cluster_size);
-            memset(pad_buf, 0, s->cluster_size);
-            qemu_iovec_to_buf(qiov, 0, pad_buf, s->cluster_size);
-            iov = (struct iovec) {
-                .iov_base   = pad_buf,
-                .iov_len    = s->cluster_size,
-            };
-            qemu_iovec_init_external(&hd_qiov, &iov, 1);
-            ret = qcow_co_pwritev_compressed(bs, offset, bytes, &hd_qiov);
-            qemu_vfree(pad_buf);
+            qemu_vfree(buf);
+            return -EINVAL;
         }
-        return ret;
+        /* Zero-pad last write if image size is not cluster aligned */
+        memset(buf + bytes, 0, s->cluster_size - bytes);
     }
-    buf = qemu_blockalign(bs, s->cluster_size);
     qemu_iovec_to_buf(qiov, 0, buf, qiov->size);
 
     out_buf = g_malloc(s->cluster_size + (s->cluster_size / 1000) + 128);
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 08/15] block: remove BlockDriver.bdrv_write_compressed
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (6 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 07/15] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 09/15] block/io: turn on dirty_bitmaps for the compressed writes Denis V. Lunev
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

There are no block drivers left that implement the old
.bdrv_write_compressed interface, so it can be removed. Also now we have
no need to use the bdrv_pwrite_compressed function and we can remove it
entirely.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/block-backend.c     |  8 ++------
 block/io.c                | 31 -------------------------------
 include/block/block.h     |  2 --
 include/block/block_int.h |  3 ---
 qemu-img.c                |  2 +-
 5 files changed, 3 insertions(+), 43 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 3c1fc50..a6c212a 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1480,12 +1480,8 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
                           int count)
 {
-    int ret = blk_check_byte_request(blk, offset, count);
-    if (ret < 0) {
-        return ret;
-    }
-
-    return bdrv_pwrite_compressed(blk_bs(blk), offset, buf, count);
+    return blk_prw(blk, offset, (void *) buf, count, blk_write_entry,
+                   BDRV_REQ_WRITE_COMPRESSED);
 }
 
 int blk_truncate(BlockBackend *blk, int64_t offset)
diff --git a/block/io.c b/block/io.c
index 49742eb..82ba079 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1843,37 +1843,6 @@ int bdrv_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
-int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset,
-                           const void *buf, int bytes)
-{
-    QEMUIOVector qiov;
-    struct iovec iov;
-    BlockDriver *drv = bs->drv;
-
-    if (!drv) {
-        return -ENOMEDIUM;
-    }
-
-    if (drv->bdrv_write_compressed) {
-        int ret = bdrv_check_byte_request(bs, offset, bytes);
-        if (ret < 0) {
-            return ret;
-        }
-        assert(QLIST_EMPTY(&bs->dirty_bitmaps));
-        assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-        assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
-        return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf,
-                                          bytes >> BDRV_SECTOR_BITS);
-    }
-    iov = (struct iovec) {
-        .iov_base = (void *)buf,
-        .iov_len = bytes,
-    };
-    qemu_iovec_init_external(&qiov, &iov, 1);
-
-    return bdrv_prwv_co(bs, offset, &qiov, true, BDRV_REQ_WRITE_COMPRESSED);
-}
-
 typedef struct BdrvVmstateCo {
     BlockDriverState    *bs;
     QEMUIOVector        *qiov;
diff --git a/include/block/block.h b/include/block/block.h
index 0d01ffd..bbc0a7b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -402,8 +402,6 @@ const char *bdrv_get_node_name(const BlockDriverState *bs);
 const char *bdrv_get_device_name(const BlockDriverState *bs);
 const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
 int bdrv_get_flags(BlockDriverState *bs);
-int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset,
-                           const void *buf, int bytes);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
 void bdrv_round_sectors_to_clusters(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 17a5efe..d8048e3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -204,9 +204,6 @@ struct BlockDriver {
     bool has_variable_length;
     int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
 
-    int (*bdrv_write_compressed)(BlockDriverState *bs, int64_t sector_num,
-                                 const uint8_t *buf, int nb_sectors);
-
     int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
 
diff --git a/qemu-img.c b/qemu-img.c
index 7bb1cfc..d90bac6 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2023,7 +2023,7 @@ static int img_convert(int argc, char **argv)
         const char *preallocation =
             qemu_opt_get(opts, BLOCK_OPT_PREALLOC);
 
-        if (!drv->bdrv_write_compressed && !drv->bdrv_co_pwritev_compressed) {
+        if (!drv->bdrv_co_pwritev_compressed) {
             error_report("Compression not supported for this file format");
             ret = -1;
             goto out;
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 09/15] block/io: turn on dirty_bitmaps for the compressed writes
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (7 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 08/15] block: remove BlockDriver.bdrv_write_compressed Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 10/15] block: simplify drive-backup Denis V. Lunev
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Previously was added the assert:

  commit 1755da16e32c15b22a521e8a38539e4b5cf367f3
  Author: Paolo Bonzini <pbonzini@redhat.com>
  Date:   Thu Oct 18 16:49:18 2012 +0200
  block: introduce new dirty bitmap functionality

Now the compressed write is always in coroutine and setting the bits is
done after the write, so that we can return the dirty_bitmaps for the
compressed writes.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/io.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/block/io.c b/block/io.c
index 82ba079..5e4e894 100644
--- a/block/io.c
+++ b/block/io.c
@@ -912,7 +912,6 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
         return -ENOTSUP;
     }
 
-    assert(QLIST_EMPTY(&bs->dirty_bitmaps));
     return drv->bdrv_co_pwritev_compressed(bs, offset, bytes, qiov);
 }
 
@@ -1302,6 +1301,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
     } else if (flags & BDRV_REQ_ZERO_WRITE) {
         bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
         ret = bdrv_co_do_pwrite_zeroes(bs, offset, bytes, flags);
+    } else if (flags & BDRV_REQ_WRITE_COMPRESSED) {
+        ret = bdrv_driver_pwritev_compressed(bs, offset, bytes, qiov);
     } else {
         bdrv_debug_event(bs, BLKDBG_PWRITEV);
         ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, flags);
@@ -1525,14 +1526,9 @@ int coroutine_fn bdrv_co_pwritev(BlockDriverState *bs,
         bytes = ROUND_UP(bytes, align);
     }
 
-    if (flags & BDRV_REQ_WRITE_COMPRESSED) {
-        ret = bdrv_driver_pwritev_compressed(
-                bs, offset, bytes, use_local_qiov ? &local_qiov : qiov);
-    } else {
-        ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
-                                   use_local_qiov ? &local_qiov : qiov,
-                                   flags);
-    }
+    ret = bdrv_aligned_pwritev(bs, &req, offset, bytes,
+                               use_local_qiov ? &local_qiov : qiov,
+                               flags);
 
 fail:
 
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 10/15] block: simplify drive-backup
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (8 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 09/15] block/io: turn on dirty_bitmaps for the compressed writes Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 11/15] block: simplify blockdev-backup Denis V. Lunev
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Now that we can support boxed commands, use it to greatly reduce the
number of parameters (and likelihood of getting out of sync) when
adjusting drive-backup parameters.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 109 +++++++++++++++++----------------------------------
 hmp.c                |  29 ++++++++------
 qapi/block-core.json |   3 +-
 3 files changed, 55 insertions(+), 86 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index b693a99..f002cc0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1835,17 +1835,8 @@ typedef struct DriveBackupState {
     BlockJob *job;
 } DriveBackupState;
 
-static void do_drive_backup(const char *device, const char *target,
-                            bool has_format, const char *format,
-                            enum MirrorSyncMode sync,
-                            bool has_mode, enum NewImageMode mode,
-                            bool has_speed, int64_t speed,
-                            bool has_bitmap, const char *bitmap,
-                            bool has_on_source_error,
-                            BlockdevOnError on_source_error,
-                            bool has_on_target_error,
-                            BlockdevOnError on_target_error,
-                            BlockJobTxn *txn, Error **errp);
+static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
+                            Error **errp);
 
 static void drive_backup_prepare(BlkActionState *common, Error **errp)
 {
@@ -1875,15 +1866,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
     bdrv_drained_begin(blk_bs(blk));
     state->bs = blk_bs(blk);
 
-    do_drive_backup(backup->device, backup->target,
-                    backup->has_format, backup->format,
-                    backup->sync,
-                    backup->has_mode, backup->mode,
-                    backup->has_speed, backup->speed,
-                    backup->has_bitmap, backup->bitmap,
-                    backup->has_on_source_error, backup->on_source_error,
-                    backup->has_on_target_error, backup->on_target_error,
-                    common->block_job_txn, &local_err);
+    do_drive_backup(backup, common->block_job_txn, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -3150,17 +3133,7 @@ out:
     aio_context_release(aio_context);
 }
 
-static void do_drive_backup(const char *device, const char *target,
-                            bool has_format, const char *format,
-                            enum MirrorSyncMode sync,
-                            bool has_mode, enum NewImageMode mode,
-                            bool has_speed, int64_t speed,
-                            bool has_bitmap, const char *bitmap,
-                            bool has_on_source_error,
-                            BlockdevOnError on_source_error,
-                            bool has_on_target_error,
-                            BlockdevOnError on_target_error,
-                            BlockJobTxn *txn, Error **errp)
+static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp)
 {
     BlockBackend *blk;
     BlockDriverState *bs;
@@ -3173,23 +3146,23 @@ static void do_drive_backup(const char *device, const char *target,
     int flags;
     int64_t size;
 
-    if (!has_speed) {
-        speed = 0;
+    if (!backup->has_speed) {
+        backup->speed = 0;
     }
-    if (!has_on_source_error) {
-        on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+    if (!backup->has_on_source_error) {
+        backup->on_source_error = BLOCKDEV_ON_ERROR_REPORT;
     }
-    if (!has_on_target_error) {
-        on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+    if (!backup->has_on_target_error) {
+        backup->on_target_error = BLOCKDEV_ON_ERROR_REPORT;
     }
-    if (!has_mode) {
-        mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+    if (!backup->has_mode) {
+        backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
     }
 
-    blk = blk_by_name(device);
+    blk = blk_by_name(backup->device);
     if (!blk) {
         error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
-                  "Device '%s' not found", device);
+                  "Device '%s' not found", backup->device);
         return;
     }
 
@@ -3199,13 +3172,14 @@ static void do_drive_backup(const char *device, const char *target,
     /* Although backup_run has this check too, we need to use bs->drv below, so
      * do an early check redundantly. */
     if (!blk_is_available(blk)) {
-        error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+        error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, backup->device);
         goto out;
     }
     bs = blk_bs(blk);
 
-    if (!has_format) {
-        format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
+    if (!backup->has_format) {
+        backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
+                         NULL : (char *)bs->drv->format_name;
     }
 
     /* Early check to avoid creating target */
@@ -3217,13 +3191,13 @@ static void do_drive_backup(const char *device, const char *target,
 
     /* See if we have a backing HD we can use to create our new image
      * on top of. */
-    if (sync == MIRROR_SYNC_MODE_TOP) {
+    if (backup->sync == MIRROR_SYNC_MODE_TOP) {
         source = backing_bs(bs);
         if (!source) {
-            sync = MIRROR_SYNC_MODE_FULL;
+            backup->sync = MIRROR_SYNC_MODE_FULL;
         }
     }
-    if (sync == MIRROR_SYNC_MODE_NONE) {
+    if (backup->sync == MIRROR_SYNC_MODE_NONE) {
         source = bs;
     }
 
@@ -3233,14 +3207,14 @@ static void do_drive_backup(const char *device, const char *target,
         goto out;
     }
 
-    if (mode != NEW_IMAGE_MODE_EXISTING) {
-        assert(format);
+    if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
+        assert(backup->format);
         if (source) {
-            bdrv_img_create(target, format, source->filename,
+            bdrv_img_create(backup->target, backup->format, source->filename,
                             source->drv->format_name, NULL,
                             size, flags, &local_err, false);
         } else {
-            bdrv_img_create(target, format, NULL, NULL, NULL,
+            bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL,
                             size, flags, &local_err, false);
         }
     }
@@ -3250,29 +3224,29 @@ static void do_drive_backup(const char *device, const char *target,
         goto out;
     }
 
-    if (format) {
+    if (backup->format) {
         options = qdict_new();
-        qdict_put(options, "driver", qstring_from_str(format));
+        qdict_put(options, "driver", qstring_from_str(backup->format));
     }
 
-    target_bs = bdrv_open(target, NULL, options, flags, errp);
+    target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
     if (!target_bs) {
         goto out;
     }
 
     bdrv_set_aio_context(target_bs, aio_context);
 
-    if (has_bitmap) {
-        bmap = bdrv_find_dirty_bitmap(bs, bitmap);
+    if (backup->has_bitmap) {
+        bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);
         if (!bmap) {
-            error_setg(errp, "Bitmap '%s' could not be found", bitmap);
+            error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
             bdrv_unref(target_bs);
             goto out;
         }
     }
 
-    backup_start(bs, target_bs, speed, sync, bmap,
-                 on_source_error, on_target_error,
+    backup_start(bs, target_bs, backup->speed, backup->sync, bmap,
+                 backup->on_source_error, backup->on_target_error,
                  block_job_cb, bs, txn, &local_err);
     bdrv_unref(target_bs);
     if (local_err != NULL) {
@@ -3284,22 +3258,9 @@ out:
     aio_context_release(aio_context);
 }
 
-void qmp_drive_backup(const char *device, const char *target,
-                      bool has_format, const char *format,
-                      enum MirrorSyncMode sync,
-                      bool has_mode, enum NewImageMode mode,
-                      bool has_speed, int64_t speed,
-                      bool has_bitmap, const char *bitmap,
-                      bool has_on_source_error, BlockdevOnError on_source_error,
-                      bool has_on_target_error, BlockdevOnError on_target_error,
-                      Error **errp)
+void qmp_drive_backup(DriveBackup *arg, Error **errp)
 {
-    return do_drive_backup(device, target, has_format, format, sync,
-                           has_mode, mode, has_speed, speed,
-                           has_bitmap, bitmap,
-                           has_on_source_error, on_source_error,
-                           has_on_target_error, on_target_error,
-                           NULL, errp);
+    return do_drive_backup(arg, NULL, errp);
 }
 
 BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
diff --git a/hmp.c b/hmp.c
index 354c714..818519e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1109,8 +1109,24 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
     const char *format = qdict_get_try_str(qdict, "format");
     bool reuse = qdict_get_try_bool(qdict, "reuse", false);
     bool full = qdict_get_try_bool(qdict, "full", false);
-    enum NewImageMode mode;
     Error *err = NULL;
+    DriveBackup backup = {
+        .device = (char *)device,
+        .target = (char *)filename,
+        .has_format = !!format,
+        .format = (char *)format,
+        .sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
+        .has_mode = true,
+        .mode = reuse ? NEW_IMAGE_MODE_EXISTING : NEW_IMAGE_MODE_ABSOLUTE_PATHS,
+        .has_speed = false,
+        .speed = 0,
+        .has_bitmap = false,
+        .bitmap = NULL,
+        .has_on_source_error = false,
+        .on_source_error = 0,
+        .has_on_target_error = false,
+        .on_target_error = 0,
+    };
 
     if (!filename) {
         error_setg(&err, QERR_MISSING_PARAMETER, "target");
@@ -1118,16 +1134,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    if (reuse) {
-        mode = NEW_IMAGE_MODE_EXISTING;
-    } else {
-        mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
-    }
-
-    qmp_drive_backup(device, filename, !!format, format,
-                     full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
-                     true, mode, false, 0, false, NULL,
-                     false, 0, false, 0, &err);
+    qmp_drive_backup(&backup, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b91b07c..6bd627e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1074,7 +1074,8 @@
 #
 # Since 1.6
 ##
-{ 'command': 'drive-backup', 'data': 'DriveBackup' }
+{ 'command': 'drive-backup', 'box': true,
+  'data': 'DriveBackup' }
 
 ##
 # @blockdev-backup
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 11/15] block: simplify blockdev-backup
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (9 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 10/15] block: simplify drive-backup Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
  2016-07-14 12:37   ` [Qemu-devel] " Eric Blake
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 12/15] drive-backup: added support for data compression Denis V. Lunev
                   ` (5 subsequent siblings)
  16 siblings, 2 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Now that we can support boxed commands, use it to greatly reduce the
number of parameters (and likelihood of getting out of sync) when
adjusting blockdev-backup parameters.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 65 +++++++++++++++-------------------------------------
 qapi/block-core.json |  6 ++++-
 2 files changed, 24 insertions(+), 47 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index f002cc0..8f55590 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1903,14 +1903,8 @@ typedef struct BlockdevBackupState {
     AioContext *aio_context;
 } BlockdevBackupState;
 
-static void do_blockdev_backup(const char *device, const char *target,
-                               enum MirrorSyncMode sync,
-                               bool has_speed, int64_t speed,
-                               bool has_on_source_error,
-                               BlockdevOnError on_source_error,
-                               bool has_on_target_error,
-                               BlockdevOnError on_target_error,
-                               BlockJobTxn *txn, Error **errp);
+static void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
+                               Error **errp);
 
 static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
 {
@@ -1950,12 +1944,7 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
     state->bs = blk_bs(blk);
     bdrv_drained_begin(state->bs);
 
-    do_blockdev_backup(backup->device, backup->target,
-                       backup->sync,
-                       backup->has_speed, backup->speed,
-                       backup->has_on_source_error, backup->on_source_error,
-                       backup->has_on_target_error, backup->on_target_error,
-                       common->block_job_txn, &local_err);
+    do_blockdev_backup(backup, common->block_job_txn, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -3268,14 +3257,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
     return bdrv_named_nodes_list(errp);
 }
 
-void do_blockdev_backup(const char *device, const char *target,
-                         enum MirrorSyncMode sync,
-                         bool has_speed, int64_t speed,
-                         bool has_on_source_error,
-                         BlockdevOnError on_source_error,
-                         bool has_on_target_error,
-                         BlockdevOnError on_target_error,
-                         BlockJobTxn *txn, Error **errp)
+void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
 {
     BlockBackend *blk;
     BlockDriverState *bs;
@@ -3283,19 +3265,19 @@ void do_blockdev_backup(const char *device, const char *target,
     Error *local_err = NULL;
     AioContext *aio_context;
 
-    if (!has_speed) {
-        speed = 0;
+    if (!backup->has_speed) {
+        backup->speed = 0;
     }
-    if (!has_on_source_error) {
-        on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+    if (!backup->has_on_source_error) {
+        backup->on_source_error = BLOCKDEV_ON_ERROR_REPORT;
     }
-    if (!has_on_target_error) {
-        on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+    if (!backup->has_on_target_error) {
+        backup->on_target_error = BLOCKDEV_ON_ERROR_REPORT;
     }
 
-    blk = blk_by_name(device);
+    blk = blk_by_name(backup->device);
     if (!blk) {
-        error_setg(errp, "Device '%s' not found", device);
+        error_setg(errp, "Device '%s' not found", backup->device);
         return;
     }
 
@@ -3303,12 +3285,12 @@ void do_blockdev_backup(const char *device, const char *target,
     aio_context_acquire(aio_context);
 
     if (!blk_is_available(blk)) {
-        error_setg(errp, "Device '%s' has no medium", device);
+        error_setg(errp, "Device '%s' has no medium", backup->device);
         goto out;
     }
     bs = blk_bs(blk);
 
-    target_bs = bdrv_lookup_bs(target, target, errp);
+    target_bs = bdrv_lookup_bs(backup->target, backup->target, errp);
     if (!target_bs) {
         goto out;
     }
@@ -3324,8 +3306,9 @@ void do_blockdev_backup(const char *device, const char *target,
             goto out;
         }
     }
-    backup_start(bs, target_bs, speed, sync, NULL, on_source_error,
-                 on_target_error, block_job_cb, bs, txn, &local_err);
+    backup_start(bs, target_bs, backup->speed, backup->sync, NULL,
+                 backup->on_source_error, backup->on_target_error, block_job_cb,
+                 bs, txn, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
     }
@@ -3333,19 +3316,9 @@ out:
     aio_context_release(aio_context);
 }
 
-void qmp_blockdev_backup(const char *device, const char *target,
-                         enum MirrorSyncMode sync,
-                         bool has_speed, int64_t speed,
-                         bool has_on_source_error,
-                         BlockdevOnError on_source_error,
-                         bool has_on_target_error,
-                         BlockdevOnError on_target_error,
-                         Error **errp)
+void qmp_blockdev_backup(BlockdevBackup *arg, Error **errp)
 {
-    do_blockdev_backup(device, target, sync, has_speed, speed,
-                       has_on_source_error, on_source_error,
-                       has_on_target_error, on_target_error,
-                       NULL, errp);
+    do_blockdev_backup(arg, NULL, errp);
 }
 
 /* Parameter check and block job starting for drive mirroring.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6bd627e..8e0750a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1088,9 +1088,13 @@
 #
 # For the arguments, see the documentation of BlockdevBackup.
 #
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
 # Since 2.3
 ##
-{ 'command': 'blockdev-backup', 'data': 'BlockdevBackup' }
+{ 'command': 'blockdev-backup', 'box': true,
+  'data': 'BlockdevBackup' }
 
 
 ##
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 12/15] drive-backup: added support for data compression
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (10 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 11/15] block: simplify blockdev-backup Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 13/15] blockdev-backup: " Denis V. Lunev
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

The idea is simple - backup is "written-once" data. It is written block
by block and it is large enough. It would be nice to save storage
space and compress it.

The patch adds a flag to the qmp/hmp drive-backup command which enables
block compression. Compression should be implemented in the format driver
to enable this feature.

There are some limitations of the format driver to allow compressed writes.
We can write data only once. Though for backup this is perfectly fine.
These limitations are maintained by the driver and the error will be
reported if we are doing something wrong.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/backup.c            | 12 +++++++++++-
 blockdev.c                |  9 ++++++---
 hmp-commands.hx           |  8 +++++---
 hmp.c                     |  3 +++
 include/block/block_int.h |  1 +
 qapi/block-core.json      |  5 ++++-
 qmp-commands.hx           |  5 ++++-
 7 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 581269b..6adca5e 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -47,6 +47,7 @@ typedef struct BackupBlockJob {
     uint64_t sectors_read;
     unsigned long *done_bitmap;
     int64_t cluster_size;
+    bool compress;
     NotifierWithReturn before_write;
     QLIST_HEAD(, CowRequest) inflight_reqs;
 } BackupBlockJob;
@@ -154,7 +155,8 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
                                        bounce_qiov.size, BDRV_REQ_MAY_UNMAP);
         } else {
             ret = blk_co_pwritev(job->target, start * job->cluster_size,
-                                 bounce_qiov.size, &bounce_qiov, 0);
+                                 bounce_qiov.size, &bounce_qiov,
+                                 job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0);
         }
         if (ret < 0) {
             trace_backup_do_cow_write_fail(job, start, ret);
@@ -477,6 +479,7 @@ static void coroutine_fn backup_run(void *opaque)
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
                   int64_t speed, MirrorSyncMode sync_mode,
                   BdrvDirtyBitmap *sync_bitmap,
+                  bool compress,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockCompletionFunc *cb, void *opaque,
@@ -508,6 +511,12 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
+    if (compress && target->drv->bdrv_co_pwritev_compressed == NULL) {
+        error_setg(errp, "Compression is not supported for this drive %s",
+                   bdrv_get_device_name(target));
+        return;
+    }
+
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
         return;
     }
@@ -555,6 +564,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
     job->sync_mode = sync_mode;
     job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ?
                        sync_bitmap : NULL;
+    job->compress = compress;
 
     /* If there is no backing file on the target, we cannot rely on COW if our
      * backup cluster size is smaller than the target cluster size. Even for
diff --git a/blockdev.c b/blockdev.c
index 8f55590..fa88900 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3147,6 +3147,9 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp)
     if (!backup->has_mode) {
         backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
     }
+    if (!backup->has_compress) {
+        backup->compress = false;
+    }
 
     blk = blk_by_name(backup->device);
     if (!blk) {
@@ -3235,8 +3238,8 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp)
     }
 
     backup_start(bs, target_bs, backup->speed, backup->sync, bmap,
-                 backup->on_source_error, backup->on_target_error,
-                 block_job_cb, bs, txn, &local_err);
+                 backup->compress, backup->on_source_error,
+                 backup->on_target_error, block_job_cb, bs, txn, &local_err);
     bdrv_unref(target_bs);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
@@ -3306,7 +3309,7 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
             goto out;
         }
     }
-    backup_start(bs, target_bs, backup->speed, backup->sync, NULL,
+    backup_start(bs, target_bs, backup->speed, backup->sync, NULL, false,
                  backup->on_source_error, backup->on_target_error, block_job_cb,
                  bs, txn, &local_err);
     if (local_err != NULL) {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 98b4b1a..f91c7ba 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1181,8 +1181,8 @@ ETEXI
 
     {
         .name       = "drive_backup",
-        .args_type  = "reuse:-n,full:-f,device:B,target:s,format:s?",
-        .params     = "[-n] [-f] device target [format]",
+        .args_type  = "reuse:-n,full:-f,compress:-c,device:B,target:s,format:s?",
+        .params     = "[-n] [-f] [-c] device target [format]",
         .help       = "initiates a point-in-time\n\t\t\t"
                       "copy for a device. The device's contents are\n\t\t\t"
                       "copied to the new image file, excluding data that\n\t\t\t"
@@ -1190,7 +1190,9 @@ ETEXI
                       "The -n flag requests QEMU to reuse the image found\n\t\t\t"
                       "in new-image-file, instead of recreating it from scratch.\n\t\t\t"
                       "The -f flag requests QEMU to copy the whole disk,\n\t\t\t"
-                      "so that the result does not need a backing file.\n\t\t\t",
+                      "so that the result does not need a backing file.\n\t\t\t"
+                      "The -c flag requests QEMU to compress backup data\n\t\t\t"
+                      "(if the target format supports it).\n\t\t\t",
         .mhandler.cmd = hmp_drive_backup,
     },
 STEXI
diff --git a/hmp.c b/hmp.c
index 818519e..0de28b3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1109,6 +1109,7 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
     const char *format = qdict_get_try_str(qdict, "format");
     bool reuse = qdict_get_try_bool(qdict, "reuse", false);
     bool full = qdict_get_try_bool(qdict, "full", false);
+    bool compress = qdict_get_try_bool(qdict, "compress", false);
     Error *err = NULL;
     DriveBackup backup = {
         .device = (char *)device,
@@ -1122,6 +1123,8 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
         .speed = 0,
         .has_bitmap = false,
         .bitmap = NULL,
+        .has_compress = !!compress,
+        .compress = compress,
         .has_on_source_error = false,
         .on_source_error = 0,
         .has_on_target_error = false,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d8048e3..d5f063c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -729,6 +729,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
                   int64_t speed, MirrorSyncMode sync_mode,
                   BdrvDirtyBitmap *sync_bitmap,
+                  bool compress,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockCompletionFunc *cb, void *opaque,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8e0750a..0cbad5a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -888,6 +888,9 @@
 #          Must be present if sync is "incremental", must NOT be present
 #          otherwise. (Since 2.4)
 #
+# @compress: #optional true to compress data, if the target format supports it.
+#            (default: false) (since 2.7)
+#
 # @on-source-error: #optional the action to take on an error on the source,
 #                   default 'report'.  'stop' and 'enospc' can only be used
 #                   if the block device supports io-status (see BlockInfo).
@@ -905,7 +908,7 @@
 { 'struct': 'DriveBackup',
   'data': { 'device': 'str', 'target': 'str', '*format': 'str',
             'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
-            '*speed': 'int', '*bitmap': 'str',
+            '*speed': 'int', '*bitmap': 'str', '*compress': 'bool',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 09b1e38..810d0fe 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1213,7 +1213,8 @@ EQMP
     {
         .name       = "drive-backup",
         .args_type  = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
-                      "bitmap:s?,on-source-error:s?,on-target-error:s?",
+                      "bitmap:s?,compress:b?,"
+                      "on-source-error:s?,on-target-error:s?",
         .mhandler.cmd_new = qmp_marshal_drive_backup,
     },
 
@@ -1247,6 +1248,8 @@ Arguments:
 - "mode": whether and how QEMU should create a new image
           (NewImageMode, optional, default 'absolute-paths')
 - "speed": the maximum speed, in bytes per second (json-int, optional)
+- "compress": true to compress data, if the target format supports it.
+              (json-bool, optional, default false)
 - "on-source-error": the action to take on an error on the source, default
                      'report'.  'stop' and 'enospc' can only be used
                      if the block device supports io-status.
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 13/15] blockdev-backup: added support for data compression
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (11 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 12/15] drive-backup: added support for data compression Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 14/15] qemu-iotests: test backup compression in 055 Denis V. Lunev
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

The idea is simple - backup is "written-once" data. It is written block
by block and it is large enough. It would be nice to save storage
space and compress it.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 9 ++++++---
 qapi/block-core.json | 4 ++++
 qmp-commands.hx      | 4 +++-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fa88900..279af57 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3277,6 +3277,9 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
     if (!backup->has_on_target_error) {
         backup->on_target_error = BLOCKDEV_ON_ERROR_REPORT;
     }
+    if (!backup->has_compress) {
+        backup->compress = false;
+    }
 
     blk = blk_by_name(backup->device);
     if (!blk) {
@@ -3309,9 +3312,9 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
             goto out;
         }
     }
-    backup_start(bs, target_bs, backup->speed, backup->sync, NULL, false,
-                 backup->on_source_error, backup->on_target_error, block_job_cb,
-                 bs, txn, &local_err);
+    backup_start(bs, target_bs, backup->speed, backup->sync, NULL,
+                 backup->compress, backup->on_source_error,
+                 backup->on_target_error, block_job_cb, bs, txn, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
     }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0cbad5a..386c7ff 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -926,6 +926,9 @@
 # @speed: #optional the maximum speed, in bytes per second. The default is 0,
 #         for unlimited.
 #
+# @compress: #optional true to compress data, if the target format supports it.
+#            (default: false) (since 2.7)
+#
 # @on-source-error: #optional the action to take on an error on the source,
 #                   default 'report'.  'stop' and 'enospc' can only be used
 #                   if the block device supports io-status (see BlockInfo).
@@ -944,6 +947,7 @@
   'data': { 'device': 'str', 'target': 'str',
             'sync': 'MirrorSyncMode',
             '*speed': 'int',
+            '*compress': 'bool',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 810d0fe..2cf6e1f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1269,7 +1269,7 @@ EQMP
 
     {
         .name       = "blockdev-backup",
-        .args_type  = "sync:s,device:B,target:B,speed:i?,"
+        .args_type  = "sync:s,device:B,target:B,speed:i?,compress:b?,"
                       "on-source-error:s?,on-target-error:s?",
         .mhandler.cmd_new = qmp_marshal_blockdev_backup,
     },
@@ -1291,6 +1291,8 @@ Arguments:
           sectors allocated in the topmost image, or "none" to only replicate
           new I/O (MirrorSyncMode).
 - "speed": the maximum speed, in bytes per second (json-int, optional)
+- "compress": true to compress data, if the target format supports it.
+              (json-bool, optional, default false)
 - "on-source-error": the action to take on an error on the source, default
                      'report'.  'stop' and 'enospc' can only be used
                      if the block device supports io-status.
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 14/15] qemu-iotests: test backup compression in 055
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (12 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 13/15] blockdev-backup: " Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 15/15] qemu-iotests: add vmdk for " Denis V. Lunev
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Added cases to check the backup compression out of qcow2, raw in qcow2
on drive-backup and blockdev-backup.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/055        | 97 +++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/055.out    |  4 +-
 tests/qemu-iotests/iotests.py | 10 ++---
 3 files changed, 104 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index c8e3578..be81a42 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -451,5 +451,102 @@ class TestSingleTransaction(iotests.QMPTestCase):
         self.assert_qmp(result, 'error/class', 'GenericError')
         self.assert_no_active_block_jobs()
 
+
+class TestDriveCompression(iotests.QMPTestCase):
+    image_len = 64 * 1024 * 1024 # MB
+    outfmt = 'qcow2'
+
+    def setUp(self):
+        # Write data to the image so we can compare later
+        qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestDriveCompression.image_len))
+        qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x11 0 64k', test_img)
+        qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x00 64k 128k', test_img)
+        qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x22 162k 32k', test_img)
+        qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img)
+
+        qemu_img('create', '-f', TestDriveCompression.outfmt, blockdev_target_img,
+                 str(TestDriveCompression.image_len))
+        self.vm = iotests.VM().add_drive(test_img).add_drive(blockdev_target_img,
+                                                             format=TestDriveCompression.outfmt)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(blockdev_target_img)
+        try:
+            os.remove(target_img)
+        except OSError:
+            pass
+
+    def do_test_compress_complete(self, cmd, **args):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp(cmd, device='drive0', sync='full', compress=True, **args)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_until_completed()
+
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(test_img, blockdev_target_img,
+                                               iotests.imgfmt, TestDriveCompression.outfmt),
+                        'target image does not match source after backup')
+
+    def test_complete_compress_drive_backup(self):
+        self.do_test_compress_complete('drive-backup', target=blockdev_target_img, mode='existing')
+
+    def test_complete_compress_blockdev_backup(self):
+        self.do_test_compress_complete('blockdev-backup', target='drive1')
+
+    def do_test_compress_cancel(self, cmd, **args):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp(cmd, device='drive0', sync='full', compress=True, **args)
+        self.assert_qmp(result, 'return', {})
+
+        event = self.cancel_and_wait()
+        self.assert_qmp(event, 'data/type', 'backup')
+
+    def test_compress_cancel_drive_backup(self):
+        self.do_test_compress_cancel('drive-backup', target=blockdev_target_img, mode='existing')
+
+    def test_compress_cancel_blockdev_backup(self):
+        self.do_test_compress_cancel('blockdev-backup', target='drive1')
+
+    def do_test_compress_pause(self, cmd, **args):
+        self.assert_no_active_block_jobs()
+
+        self.vm.pause_drive('drive0')
+        result = self.vm.qmp(cmd, device='drive0', sync='full', compress=True, **args)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('block-job-pause', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.vm.resume_drive('drive0')
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        offset = self.dictpath(result, 'return[0]/offset')
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/offset', offset)
+
+        result = self.vm.qmp('block-job-resume', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_until_completed()
+
+        self.vm.shutdown()
+        self.assertTrue(iotests.compare_images(test_img, blockdev_target_img,
+                                               iotests.imgfmt, TestDriveCompression.outfmt),
+                        'target image does not match source after backup')
+
+    def test_compress_pause_drive_backup(self):
+        self.do_test_compress_pause('drive-backup', target=blockdev_target_img, mode='existing')
+
+    def test_compress_pause_blockdev_backup(self):
+        self.do_test_compress_pause('blockdev-backup', target='drive1')
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['raw', 'qcow2'])
diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out
index 42314e9..5ce2f9a 100644
--- a/tests/qemu-iotests/055.out
+++ b/tests/qemu-iotests/055.out
@@ -1,5 +1,5 @@
-........................
+..............................
 ----------------------------------------------------------------------
-Ran 24 tests
+Ran 30 tests
 
 OK
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1687c33..68830a42 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -89,10 +89,10 @@ def qemu_io(*args):
         sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
     return subp.communicate()[0]
 
-def compare_images(img1, img2):
+def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
     '''Return True if two image files are identical'''
-    return qemu_img('compare', '-f', imgfmt,
-                    '-F', imgfmt, img1, img2) == 0
+    return qemu_img('compare', '-f', fmt1,
+                    '-F', fmt2, img1, img2) == 0
 
 def create_image(name, size):
     '''Create a fully-allocated raw image with sector markers'''
@@ -175,14 +175,14 @@ class VM(object):
         self._args.append(opts)
         return self
 
-    def add_drive(self, path, opts='', interface='virtio'):
+    def add_drive(self, path, opts='', interface='virtio', format=imgfmt):
         '''Add a virtio-blk drive to the VM'''
         options = ['if=%s' % interface,
                    'id=drive%d' % self._num_drives]
 
         if path is not None:
             options.append('file=%s' % path)
-            options.append('format=%s' % imgfmt)
+            options.append('format=%s' % format)
             options.append('cache=%s' % cachemode)
 
         if opts:
-- 
2.1.4

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

* [Qemu-devel] [PATCH v5 15/15] qemu-iotests: add vmdk for test backup compression in 055
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (13 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 14/15] qemu-iotests: test backup compression in 055 Denis V. Lunev
@ 2016-07-04 12:28 ` Denis V. Lunev
  2016-07-08 13:13 ` [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
  2016-07-19 21:58 ` Jeff Cody
  16 siblings, 0 replies; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-04 12:28 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: den, Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

The vmdk format has support for compression, it would be fine to add it for
the test backup compression.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/055 | 57 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 18 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index be81a42..cf5a423 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -454,7 +454,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
 
 class TestDriveCompression(iotests.QMPTestCase):
     image_len = 64 * 1024 * 1024 # MB
-    outfmt = 'qcow2'
+    fmt_supports_compression = [{'type': 'qcow2', 'args': ()},
+                                {'type': 'vmdk', 'args': ('-o', 'subformat=streamOptimized')}]
 
     def setUp(self):
         # Write data to the image so we can compare later
@@ -464,12 +465,6 @@ class TestDriveCompression(iotests.QMPTestCase):
         qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x22 162k 32k', test_img)
         qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img)
 
-        qemu_img('create', '-f', TestDriveCompression.outfmt, blockdev_target_img,
-                 str(TestDriveCompression.image_len))
-        self.vm = iotests.VM().add_drive(test_img).add_drive(blockdev_target_img,
-                                                             format=TestDriveCompression.outfmt)
-        self.vm.launch()
-
     def tearDown(self):
         self.vm.shutdown()
         os.remove(test_img)
@@ -479,7 +474,18 @@ class TestDriveCompression(iotests.QMPTestCase):
         except OSError:
             pass
 
-    def do_test_compress_complete(self, cmd, **args):
+    def do_prepare_drives(self, fmt, args):
+        self.vm = iotests.VM().add_drive(test_img)
+
+        qemu_img('create', '-f', fmt, blockdev_target_img,
+                 str(TestDriveCompression.image_len), *args)
+        self.vm.add_drive(blockdev_target_img, format=fmt)
+
+        self.vm.launch()
+
+    def do_test_compress_complete(self, cmd, format, **args):
+        self.do_prepare_drives(format['type'], format['args'])
+
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp(cmd, device='drive0', sync='full', compress=True, **args)
@@ -489,16 +495,21 @@ class TestDriveCompression(iotests.QMPTestCase):
 
         self.vm.shutdown()
         self.assertTrue(iotests.compare_images(test_img, blockdev_target_img,
-                                               iotests.imgfmt, TestDriveCompression.outfmt),
+                                               iotests.imgfmt, format['type']),
                         'target image does not match source after backup')
 
     def test_complete_compress_drive_backup(self):
-        self.do_test_compress_complete('drive-backup', target=blockdev_target_img, mode='existing')
+        for format in TestDriveCompression.fmt_supports_compression:
+            self.do_test_compress_complete('drive-backup', format,
+                                           target=blockdev_target_img, mode='existing')
 
     def test_complete_compress_blockdev_backup(self):
-        self.do_test_compress_complete('blockdev-backup', target='drive1')
+        for format in TestDriveCompression.fmt_supports_compression:
+            self.do_test_compress_complete('blockdev-backup', format, target='drive1')
+
+    def do_test_compress_cancel(self, cmd, format, **args):
+        self.do_prepare_drives(format['type'], format['args'])
 
-    def do_test_compress_cancel(self, cmd, **args):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp(cmd, device='drive0', sync='full', compress=True, **args)
@@ -507,13 +518,20 @@ class TestDriveCompression(iotests.QMPTestCase):
         event = self.cancel_and_wait()
         self.assert_qmp(event, 'data/type', 'backup')
 
+        self.vm.shutdown()
+
     def test_compress_cancel_drive_backup(self):
-        self.do_test_compress_cancel('drive-backup', target=blockdev_target_img, mode='existing')
+        for format in TestDriveCompression.fmt_supports_compression:
+            self.do_test_compress_cancel('drive-backup', format,
+                                         target=blockdev_target_img, mode='existing')
 
     def test_compress_cancel_blockdev_backup(self):
-        self.do_test_compress_cancel('blockdev-backup', target='drive1')
+       for format in TestDriveCompression.fmt_supports_compression:
+            self.do_test_compress_cancel('blockdev-backup', format, target='drive1')
+
+    def do_test_compress_pause(self, cmd, format, **args):
+        self.do_prepare_drives(format['type'], format['args'])
 
-    def do_test_compress_pause(self, cmd, **args):
         self.assert_no_active_block_jobs()
 
         self.vm.pause_drive('drive0')
@@ -539,14 +557,17 @@ class TestDriveCompression(iotests.QMPTestCase):
 
         self.vm.shutdown()
         self.assertTrue(iotests.compare_images(test_img, blockdev_target_img,
-                                               iotests.imgfmt, TestDriveCompression.outfmt),
+                                               iotests.imgfmt, format['type']),
                         'target image does not match source after backup')
 
     def test_compress_pause_drive_backup(self):
-        self.do_test_compress_pause('drive-backup', target=blockdev_target_img, mode='existing')
+        for format in TestDriveCompression.fmt_supports_compression:
+            self.do_test_compress_pause('drive-backup', format,
+                                        target=blockdev_target_img, mode='existing')
 
     def test_compress_pause_blockdev_backup(self):
-        self.do_test_compress_pause('blockdev-backup', target='drive1')
+        for format in TestDriveCompression.fmt_supports_compression:
+            self.do_test_compress_pause('blockdev-backup', format, target='drive1')
 
 if __name__ == '__main__':
     iotests.main(supported_fmts=['raw', 'qcow2'])
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH v5 00/15] backup compression
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (14 preceding siblings ...)
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 15/15] qemu-iotests: add vmdk for " Denis V. Lunev
@ 2016-07-08 13:13 ` Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-19 21:58 ` Jeff Cody
  16 siblings, 1 reply; 32+ messages in thread
From: Denis V. Lunev @ 2016-07-08 13:13 UTC (permalink / raw)
  To: qemu-block, qemu-devel
  Cc: Pavel Butsykin, Jeff Cody, Markus Armbruster, Eric Blake,
	John Snow, Stefan Hajnoczi, Kevin Wolf

On 07/04/2016 03:28 PM, Denis V. Lunev wrote:
> The idea is simple - backup is "written-once" data. It is written block
> by block and it is large enough. It would be nice to save storage
> space and compress it.
>
> These patches add the ability to compress data during backup. This
> functionality is implemented by means of adding options to the qmp/hmp
> commands(drive-backup, blockdev-backup). The implementation is quite
> simple, because the responsibility for data compression imposed on the
> format driver.
>
> This patchset is based on the Eric's QAPI changes
>    http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
>
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
>
> Changes from v1:
> - added unittest for backup compression (12, 13)
>
> Changes from v2:
> - implemented a new .bdrv_co_write_compressed interface to replace the
>    old .bdrv_write_compressed (2,3,4,5,6)
>
> Changes from v3:
> - added the byte-based interfaces:
>    blk_pwrite_compressed()/blk_co_pwritev_compressed() (1, 7)
> - fix drive/blockdev-backup documentation (10, 11)
>
> Changes form v4:
> - added assert that offset and count are aligned (1)
> - reuse RwCo and bdrv_co_pwritev() for write compressed (2)
> - converted interfaces to byte-based for format drivers (2, 3, 5, 6)
> - move an unrelated cleanup in a separate patches (4, 7)
> - turn on dirty_bitmaps for the compressed writes (9)
> - added simplify drive/blockdev-backup by using the boxed commands (10, 11)
> - reworded drive/blockdev-backup documentation about compression (12, 13)
> - fix s/bakup/backup/ (14)
>
> Pavel Butsykin (15):
>    block: switch blk_write_compressed() to byte-based interface
>    block/io: reuse bdrv_co_pwritev() for write compressed
>    qcow2: add qcow2_co_pwritev_compressed
>    qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
>    vmdk: add vmdk_co_pwritev_compressed
>    qcow: add qcow_co_pwritev_compressed
>    qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
>    block: remove BlockDriver.bdrv_write_compressed
>    block/io: turn on dirty_bitmaps for the compressed writes
>    block: simplify drive-backup
>    block: simplify blockdev-backup
>    drive-backup: added support for data compression
>    blockdev-backup: added support for data compression
>    qemu-iotests: test backup compression in 055
>    qemu-iotests: add vmdk for test backup compression in 055
>
>   block/backup.c                 |  12 ++-
>   block/block-backend.c          |  12 +--
>   block/io.c                     |  37 ++++-----
>   block/qcow.c                   |  71 +++++++++-------
>   block/qcow2.c                  |  83 +++++++++++--------
>   block/vmdk.c                   |  55 ++-----------
>   blockdev.c                     | 182 ++++++++++++++---------------------------
>   hmp-commands.hx                |   8 +-
>   hmp.c                          |  32 +++++---
>   include/block/block.h          |   5 +-
>   include/block/block_int.h      |   5 +-
>   include/sysemu/block-backend.h |   4 +-
>   qapi/block-core.json           |  18 +++-
>   qemu-img.c                     |   8 +-
>   qemu-io-cmds.c                 |   2 +-
>   qmp-commands.hx                |   9 +-
>   tests/qemu-iotests/055         | 118 ++++++++++++++++++++++++++
>   tests/qemu-iotests/055.out     |   4 +-
>   tests/qemu-iotests/iotests.py  |  10 +--
>   19 files changed, 369 insertions(+), 306 deletions(-)
>
ping

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

* Re: [Qemu-devel] [PATCH v5 02/15] block/io: reuse bdrv_co_pwritev() for write compressed
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 02/15] block/io: reuse bdrv_co_pwritev() for write compressed Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:13PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> For bdrv_pwrite_compressed() it looks like most of the code creating
> coroutine is duplicated in bdrv_prwv_co(). So we can just add a flag
> (BDRV_REQ_WRITE_COMPRESSED) and use bdrv_prwv_co() as a generic one.
> In the end we get coroutine oriented function for write compressed by using
> bdrv_co_pwritev/blk_co_pwritev with BDRV_REQ_WRITE_COMPRESSED flag.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/io.c                | 57 ++++++++++++++++++++++++++++++++++-------------
>  include/block/block.h     |  3 ++-
>  include/block/block_int.h |  3 +++
>  qemu-img.c                |  2 +-
>  4 files changed, 47 insertions(+), 18 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-14 12:35   ` Eric Blake
  1 sibling, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:15PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Since the function became use a vector instead of a buffer there is no
> sense to use a recursive code.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 07/15] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 07/15] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:18PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Since the function became use a vector instead of a buffer there is no
> sense to use a recursive code.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 08/15] block: remove BlockDriver.bdrv_write_compressed
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 08/15] block: remove BlockDriver.bdrv_write_compressed Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:19PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> There are no block drivers left that implement the old
> .bdrv_write_compressed interface, so it can be removed. Also now we have
> no need to use the bdrv_pwrite_compressed function and we can remove it
> entirely.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/block-backend.c     |  8 ++------
>  block/io.c                | 31 -------------------------------
>  include/block/block.h     |  2 --
>  include/block/block_int.h |  3 ---
>  qemu-img.c                |  2 +-
>  5 files changed, 3 insertions(+), 43 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 09/15] block/io: turn on dirty_bitmaps for the compressed writes
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 09/15] block/io: turn on dirty_bitmaps for the compressed writes Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:20PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Previously was added the assert:
> 
>   commit 1755da16e32c15b22a521e8a38539e4b5cf367f3
>   Author: Paolo Bonzini <pbonzini@redhat.com>
>   Date:   Thu Oct 18 16:49:18 2012 +0200
>   block: introduce new dirty bitmap functionality
> 
> Now the compressed write is always in coroutine and setting the bits is
> done after the write, so that we can return the dirty_bitmaps for the
> compressed writes.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/io.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 10/15] block: simplify drive-backup
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 10/15] block: simplify drive-backup Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:21PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Now that we can support boxed commands, use it to greatly reduce the
> number of parameters (and likelihood of getting out of sync) when
> adjusting drive-backup parameters.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c           | 109 +++++++++++++++++----------------------------------
>  hmp.c                |  29 ++++++++------
>  qapi/block-core.json |   3 +-
>  3 files changed, 55 insertions(+), 86 deletions(-)

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

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

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

* Re: [Qemu-devel] [Qemu-block] [PATCH v5 11/15] block: simplify blockdev-backup
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 11/15] block: simplify blockdev-backup Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  2016-07-14 12:37   ` [Qemu-devel] " Eric Blake
  1 sibling, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin,
	Markus Armbruster, Stefan Hajnoczi

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

On Mon, Jul 04, 2016 at 03:28:22PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Now that we can support boxed commands, use it to greatly reduce the
> number of parameters (and likelihood of getting out of sync) when
> adjusting blockdev-backup parameters.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c           | 65 +++++++++++++++-------------------------------------
>  qapi/block-core.json |  6 ++++-
>  2 files changed, 24 insertions(+), 47 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 12/15] drive-backup: added support for data compression
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 12/15] drive-backup: added support for data compression Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:23PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> The idea is simple - backup is "written-once" data. It is written block
> by block and it is large enough. It would be nice to save storage
> space and compress it.
> 
> The patch adds a flag to the qmp/hmp drive-backup command which enables
> block compression. Compression should be implemented in the format driver
> to enable this feature.
> 
> There are some limitations of the format driver to allow compressed writes.
> We can write data only once. Though for backup this is perfectly fine.
> These limitations are maintained by the driver and the error will be
> reported if we are doing something wrong.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/backup.c            | 12 +++++++++++-
>  blockdev.c                |  9 ++++++---
>  hmp-commands.hx           |  8 +++++---
>  hmp.c                     |  3 +++
>  include/block/block_int.h |  1 +
>  qapi/block-core.json      |  5 ++++-
>  qmp-commands.hx           |  5 ++++-
>  7 files changed, 34 insertions(+), 9 deletions(-)

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

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

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

* Re: [Qemu-devel] [Qemu-block] [PATCH v5 13/15] blockdev-backup: added support for data compression
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 13/15] blockdev-backup: " Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin,
	Markus Armbruster, Stefan Hajnoczi

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

On Mon, Jul 04, 2016 at 03:28:24PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> The idea is simple - backup is "written-once" data. It is written block
> by block and it is large enough. It would be nice to save storage
> space and compress it.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c           | 9 ++++++---
>  qapi/block-core.json | 4 ++++
>  qmp-commands.hx      | 4 +++-
>  3 files changed, 13 insertions(+), 4 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 14/15] qemu-iotests: test backup compression in 055
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 14/15] qemu-iotests: test backup compression in 055 Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Mon, Jul 04, 2016 at 03:28:25PM +0300, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Added cases to check the backup compression out of qcow2, raw in qcow2
> on drive-backup and blockdev-backup.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/055        | 97 +++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/055.out    |  4 +-
>  tests/qemu-iotests/iotests.py | 10 ++---
>  3 files changed, 104 insertions(+), 7 deletions(-)

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

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

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

* Re: [Qemu-devel] [PATCH v5 00/15] backup compression
  2016-07-08 13:13 ` [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
@ 2016-07-14 12:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2016-07-14 12:17 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Kevin Wolf, Pavel Butsykin, Jeff Cody,
	Markus Armbruster, Stefan Hajnoczi, John Snow

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

On Fri, Jul 08, 2016 at 04:13:24PM +0300, Denis V. Lunev wrote:
> On 07/04/2016 03:28 PM, Denis V. Lunev wrote:
> > The idea is simple - backup is "written-once" data. It is written block
> > by block and it is large enough. It would be nice to save storage
> > space and compress it.
> > 
> > These patches add the ability to compress data during backup. This
> > functionality is implemented by means of adding options to the qmp/hmp
> > commands(drive-backup, blockdev-backup). The implementation is quite
> > simple, because the responsibility for data compression imposed on the
> > format driver.
> > 
> > This patchset is based on the Eric's QAPI changes
> >    http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
> > 
> > Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > CC: Jeff Cody <jcody@redhat.com>
> > CC: Markus Armbruster <armbru@redhat.com>
> > CC: Eric Blake <eblake@redhat.com>
> > CC: John Snow <jsnow@redhat.com>
> > CC: Stefan Hajnoczi <stefanha@redhat.com>
> > CC: Kevin Wolf <kwolf@redhat.com>
> > 
> > Changes from v1:
> > - added unittest for backup compression (12, 13)
> > 
> > Changes from v2:
> > - implemented a new .bdrv_co_write_compressed interface to replace the
> >    old .bdrv_write_compressed (2,3,4,5,6)
> > 
> > Changes from v3:
> > - added the byte-based interfaces:
> >    blk_pwrite_compressed()/blk_co_pwritev_compressed() (1, 7)
> > - fix drive/blockdev-backup documentation (10, 11)
> > 
> > Changes form v4:
> > - added assert that offset and count are aligned (1)
> > - reuse RwCo and bdrv_co_pwritev() for write compressed (2)
> > - converted interfaces to byte-based for format drivers (2, 3, 5, 6)
> > - move an unrelated cleanup in a separate patches (4, 7)
> > - turn on dirty_bitmaps for the compressed writes (9)
> > - added simplify drive/blockdev-backup by using the boxed commands (10, 11)
> > - reworded drive/blockdev-backup documentation about compression (12, 13)
> > - fix s/bakup/backup/ (14)
> > 
> > Pavel Butsykin (15):
> >    block: switch blk_write_compressed() to byte-based interface
> >    block/io: reuse bdrv_co_pwritev() for write compressed
> >    qcow2: add qcow2_co_pwritev_compressed
> >    qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
> >    vmdk: add vmdk_co_pwritev_compressed
> >    qcow: add qcow_co_pwritev_compressed
> >    qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
> >    block: remove BlockDriver.bdrv_write_compressed
> >    block/io: turn on dirty_bitmaps for the compressed writes
> >    block: simplify drive-backup
> >    block: simplify blockdev-backup
> >    drive-backup: added support for data compression
> >    blockdev-backup: added support for data compression
> >    qemu-iotests: test backup compression in 055
> >    qemu-iotests: add vmdk for test backup compression in 055
> > 
> >   block/backup.c                 |  12 ++-
> >   block/block-backend.c          |  12 +--
> >   block/io.c                     |  37 ++++-----
> >   block/qcow.c                   |  71 +++++++++-------
> >   block/qcow2.c                  |  83 +++++++++++--------
> >   block/vmdk.c                   |  55 ++-----------
> >   blockdev.c                     | 182 ++++++++++++++---------------------------
> >   hmp-commands.hx                |   8 +-
> >   hmp.c                          |  32 +++++---
> >   include/block/block.h          |   5 +-
> >   include/block/block_int.h      |   5 +-
> >   include/sysemu/block-backend.h |   4 +-
> >   qapi/block-core.json           |  18 +++-
> >   qemu-img.c                     |   8 +-
> >   qemu-io-cmds.c                 |   2 +-
> >   qmp-commands.hx                |   9 +-
> >   tests/qemu-iotests/055         | 118 ++++++++++++++++++++++++++
> >   tests/qemu-iotests/055.out     |   4 +-
> >   tests/qemu-iotests/iotests.py  |  10 +--
> >   19 files changed, 369 insertions(+), 306 deletions(-)
> > 
> ping
> 

Looks good.  I'm not sure what the QAPI 'box' dependency means in terms
of getting this into QEMU 2.7.  Hard freeze is on the 19th of July so
both QAPI 'box' and this series must be merged by then.

Stefan

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

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

* Re: [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
  2016-07-14 12:17   ` Stefan Hajnoczi
@ 2016-07-14 12:35   ` Eric Blake
  1 sibling, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-07-14 12:35 UTC (permalink / raw)
  To: Denis V. Lunev, qemu-block, qemu-devel
  Cc: Pavel Butsykin, Jeff Cody, Markus Armbruster, John Snow,
	Stefan Hajnoczi, Kevin Wolf

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

On 07/04/2016 06:28 AM, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Since the function became use a vector instead of a buffer there is no
> sense to use a recursive code.

Grammar suggestion:

Now that the function uses a vector instead of a buffer, there is no
need to use recursive code.


-- 
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 --]

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

* Re: [Qemu-devel] [PATCH v5 11/15] block: simplify blockdev-backup
  2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 11/15] block: simplify blockdev-backup Denis V. Lunev
  2016-07-14 12:17   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2016-07-14 12:37   ` Eric Blake
  1 sibling, 0 replies; 32+ messages in thread
From: Eric Blake @ 2016-07-14 12:37 UTC (permalink / raw)
  To: Denis V. Lunev, qemu-block, qemu-devel
  Cc: Pavel Butsykin, Jeff Cody, Markus Armbruster, John Snow,
	Stefan Hajnoczi, Kevin Wolf

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

On 07/04/2016 06:28 AM, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> Now that we can support boxed commands, use it to greatly reduce the
> number of parameters (and likelihood of getting out of sync) when
> adjusting blockdev-backup parameters.
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c           | 65 +++++++++++++++-------------------------------------
>  qapi/block-core.json |  6 ++++-
>  2 files changed, 24 insertions(+), 47 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -1088,9 +1088,13 @@
>  #
>  # For the arguments, see the documentation of BlockdevBackup.
>  #
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#
>  # Since 2.3
>  ##
> -{ 'command': 'blockdev-backup', 'data': 'BlockdevBackup' }
> +{ 'command': 'blockdev-backup', 'box': true,
> +  'data': 'BlockdevBackup' }

Will need a trivial rebase; my pending qapi v9 patches renamed the new
parameter 'boxed' instead of 'box'.

-- 
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 --]

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

* Re: [Qemu-devel] [PATCH v5 00/15] backup compression
  2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
                   ` (15 preceding siblings ...)
  2016-07-08 13:13 ` [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
@ 2016-07-19 21:58 ` Jeff Cody
  2016-07-20  9:55   ` Pavel Butsykin
  16 siblings, 1 reply; 32+ messages in thread
From: Jeff Cody @ 2016-07-19 21:58 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: qemu-block, qemu-devel, Pavel Butsykin, Markus Armbruster,
	Eric Blake, John Snow, Stefan Hajnoczi, Kevin Wolf

On Mon, Jul 04, 2016 at 03:28:11PM +0300, Denis V. Lunev wrote:
> The idea is simple - backup is "written-once" data. It is written block
> by block and it is large enough. It would be nice to save storage
> space and compress it.
> 
> These patches add the ability to compress data during backup. This
> functionality is implemented by means of adding options to the qmp/hmp
> commands(drive-backup, blockdev-backup). The implementation is quite
> simple, because the responsibility for data compression imposed on the
> format driver.
> 
> This patchset is based on the Eric's QAPI changes
>   http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
> 
> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> 
> Changes from v1:
> - added unittest for backup compression (12, 13)
> 
> Changes from v2:
> - implemented a new .bdrv_co_write_compressed interface to replace the
>   old .bdrv_write_compressed (2,3,4,5,6)
> 
> Changes from v3:
> - added the byte-based interfaces:
>   blk_pwrite_compressed()/blk_co_pwritev_compressed() (1, 7)
> - fix drive/blockdev-backup documentation (10, 11)
> 
> Changes form v4:
> - added assert that offset and count are aligned (1)
> - reuse RwCo and bdrv_co_pwritev() for write compressed (2)
> - converted interfaces to byte-based for format drivers (2, 3, 5, 6)
> - move an unrelated cleanup in a separate patches (4, 7)
> - turn on dirty_bitmaps for the compressed writes (9)
> - added simplify drive/blockdev-backup by using the boxed commands (10, 11)
> - reworded drive/blockdev-backup documentation about compression (12, 13)
> - fix s/bakup/backup/ (14)
> 
> Pavel Butsykin (15):
>   block: switch blk_write_compressed() to byte-based interface
>   block/io: reuse bdrv_co_pwritev() for write compressed
>   qcow2: add qcow2_co_pwritev_compressed
>   qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
>   vmdk: add vmdk_co_pwritev_compressed
>   qcow: add qcow_co_pwritev_compressed
>   qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
>   block: remove BlockDriver.bdrv_write_compressed
>   block/io: turn on dirty_bitmaps for the compressed writes
>   block: simplify drive-backup
>   block: simplify blockdev-backup
>   drive-backup: added support for data compression
>   blockdev-backup: added support for data compression
>   qemu-iotests: test backup compression in 055
>   qemu-iotests: add vmdk for test backup compression in 055
> 
>  block/backup.c                 |  12 ++-
>  block/block-backend.c          |  12 +--
>  block/io.c                     |  37 ++++-----
>  block/qcow.c                   |  71 +++++++++-------
>  block/qcow2.c                  |  83 +++++++++++--------
>  block/vmdk.c                   |  55 ++-----------
>  blockdev.c                     | 182 ++++++++++++++---------------------------
>  hmp-commands.hx                |   8 +-
>  hmp.c                          |  32 +++++---
>  include/block/block.h          |   5 +-
>  include/block/block_int.h      |   5 +-
>  include/sysemu/block-backend.h |   4 +-
>  qapi/block-core.json           |  18 +++-
>  qemu-img.c                     |   8 +-
>  qemu-io-cmds.c                 |   2 +-
>  qmp-commands.hx                |   9 +-
>  tests/qemu-iotests/055         | 118 ++++++++++++++++++++++++++
>  tests/qemu-iotests/055.out     |   4 +-
>  tests/qemu-iotests/iotests.py  |  10 +--
>  19 files changed, 369 insertions(+), 306 deletions(-)
> 
> -- 
> 2.1.4
>

This series needs a rebase, I cannot apply it to my branch (or qemu/master)
without a lot of conflicts.  Can you rebase?

Thanks,
Jeff

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

* Re: [Qemu-devel] [PATCH v5 00/15] backup compression
  2016-07-19 21:58 ` Jeff Cody
@ 2016-07-20  9:55   ` Pavel Butsykin
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Butsykin @ 2016-07-20  9:55 UTC (permalink / raw)
  To: Jeff Cody, Denis V. Lunev
  Cc: qemu-block, qemu-devel, Markus Armbruster, Eric Blake, John Snow,
	Stefan Hajnoczi, Kevin Wolf

On 20.07.2016 00:58, Jeff Cody wrote:
> On Mon, Jul 04, 2016 at 03:28:11PM +0300, Denis V. Lunev wrote:
>> The idea is simple - backup is "written-once" data. It is written block
>> by block and it is large enough. It would be nice to save storage
>> space and compress it.
>>
>> These patches add the ability to compress data during backup. This
>> functionality is implemented by means of adding options to the qmp/hmp
>> commands(drive-backup, blockdev-backup). The implementation is quite
>> simple, because the responsibility for data compression imposed on the
>> format driver.
>>
>> This patchset is based on the Eric's QAPI changes
>>    http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Jeff Cody <jcody@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> CC: John Snow <jsnow@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
>>
>> Changes from v1:
>> - added unittest for backup compression (12, 13)
>>
>> Changes from v2:
>> - implemented a new .bdrv_co_write_compressed interface to replace the
>>    old .bdrv_write_compressed (2,3,4,5,6)
>>
>> Changes from v3:
>> - added the byte-based interfaces:
>>    blk_pwrite_compressed()/blk_co_pwritev_compressed() (1, 7)
>> - fix drive/blockdev-backup documentation (10, 11)
>>
>> Changes form v4:
>> - added assert that offset and count are aligned (1)
>> - reuse RwCo and bdrv_co_pwritev() for write compressed (2)
>> - converted interfaces to byte-based for format drivers (2, 3, 5, 6)
>> - move an unrelated cleanup in a separate patches (4, 7)
>> - turn on dirty_bitmaps for the compressed writes (9)
>> - added simplify drive/blockdev-backup by using the boxed commands (10, 11)
>> - reworded drive/blockdev-backup documentation about compression (12, 13)
>> - fix s/bakup/backup/ (14)
>>
>> Pavel Butsykin (15):
>>    block: switch blk_write_compressed() to byte-based interface
>>    block/io: reuse bdrv_co_pwritev() for write compressed
>>    qcow2: add qcow2_co_pwritev_compressed
>>    qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
>>    vmdk: add vmdk_co_pwritev_compressed
>>    qcow: add qcow_co_pwritev_compressed
>>    qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
>>    block: remove BlockDriver.bdrv_write_compressed
>>    block/io: turn on dirty_bitmaps for the compressed writes
>>    block: simplify drive-backup
>>    block: simplify blockdev-backup
>>    drive-backup: added support for data compression
>>    blockdev-backup: added support for data compression
>>    qemu-iotests: test backup compression in 055
>>    qemu-iotests: add vmdk for test backup compression in 055
>>
>>   block/backup.c                 |  12 ++-
>>   block/block-backend.c          |  12 +--
>>   block/io.c                     |  37 ++++-----
>>   block/qcow.c                   |  71 +++++++++-------
>>   block/qcow2.c                  |  83 +++++++++++--------
>>   block/vmdk.c                   |  55 ++-----------
>>   blockdev.c                     | 182 ++++++++++++++---------------------------
>>   hmp-commands.hx                |   8 +-
>>   hmp.c                          |  32 +++++---
>>   include/block/block.h          |   5 +-
>>   include/block/block_int.h      |   5 +-
>>   include/sysemu/block-backend.h |   4 +-
>>   qapi/block-core.json           |  18 +++-
>>   qemu-img.c                     |   8 +-
>>   qemu-io-cmds.c                 |   2 +-
>>   qmp-commands.hx                |   9 +-
>>   tests/qemu-iotests/055         | 118 ++++++++++++++++++++++++++
>>   tests/qemu-iotests/055.out     |   4 +-
>>   tests/qemu-iotests/iotests.py  |  10 +--
>>   19 files changed, 369 insertions(+), 306 deletions(-)
>>
>> --
>> 2.1.4
>>
>
> This series needs a rebase, I cannot apply it to my branch (or qemu/master)
> without a lot of conflicts.  Can you rebase?
>
Ok, probably will have to add a separate patch for convert
bdrv_pwrite_compressed() to BdrvChild.

> Thanks,
> Jeff
>

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

end of thread, other threads:[~2016-07-20 11:29 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-04 12:28 [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 01/15] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 02/15] block/io: reuse bdrv_co_pwritev() for write compressed Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 03/15] qcow2: add qcow2_co_pwritev_compressed Denis V. Lunev
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 04/15] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-14 12:35   ` Eric Blake
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 05/15] vmdk: add vmdk_co_pwritev_compressed Denis V. Lunev
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 06/15] qcow: add qcow_co_pwritev_compressed Denis V. Lunev
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 07/15] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 08/15] block: remove BlockDriver.bdrv_write_compressed Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 09/15] block/io: turn on dirty_bitmaps for the compressed writes Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 10/15] block: simplify drive-backup Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 11/15] block: simplify blockdev-backup Denis V. Lunev
2016-07-14 12:17   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-07-14 12:37   ` [Qemu-devel] " Eric Blake
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 12/15] drive-backup: added support for data compression Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 13/15] blockdev-backup: " Denis V. Lunev
2016-07-14 12:17   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 14/15] qemu-iotests: test backup compression in 055 Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-04 12:28 ` [Qemu-devel] [PATCH v5 15/15] qemu-iotests: add vmdk for " Denis V. Lunev
2016-07-08 13:13 ` [Qemu-devel] [PATCH v5 00/15] backup compression Denis V. Lunev
2016-07-14 12:17   ` Stefan Hajnoczi
2016-07-19 21:58 ` Jeff Cody
2016-07-20  9:55   ` Pavel Butsykin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.