All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 00/16] backup compression
@ 2016-07-21 19:40 Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 01/16] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
                   ` (15 more replies)
  0 siblings, 16 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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.

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)

Changes form v5:
- rebased on master
- fix grammar (5, 8)

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>

Pavel Butsykin (16):
  block: switch blk_write_compressed() to byte-based interface
  block: Convert bdrv_pwrite_compressed() to BdrvChild
  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          |  27 +-----
 block/io.c                     |  48 ++++------
 block/qcow.c                   | 113 +++++++++---------------
 block/qcow2.c                  | 128 ++++++++++-----------------
 block/vmdk.c                   |  55 ++----------
 blockdev.c                     | 193 ++++++++++++++---------------------------
 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, 374 insertions(+), 425 deletions(-)

-- 
2.5.0

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

* [Qemu-devel] [PATCH v6 01/16] block: switch blk_write_compressed() to byte-based interface
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 23:08   ` Eric Blake
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 02/16] block: Convert bdrv_pwrite_compressed() to BdrvChild Denis V. Lunev
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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. bdrv_check_request() and
blk_check_requestis no longer used, so we can remove.

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          | 23 ++++-------------------
 block/io.c                     | 22 +++++++---------------
 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(+), 41 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index effa038..8f38ab4 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -727,21 +727,6 @@ static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
     return 0;
 }
 
-static int blk_check_request(BlockBackend *blk, int64_t sector_num,
-                             int nb_sectors)
-{
-    if (sector_num < 0 || sector_num > INT64_MAX / BDRV_SECTOR_SIZE) {
-        return -EIO;
-    }
-
-    if (nb_sectors < 0 || nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
-        return -EIO;
-    }
-
-    return blk_check_byte_request(blk, sector_num * BDRV_SECTOR_SIZE,
-                                  nb_sectors * BDRV_SECTOR_SIZE);
-}
-
 int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset,
                                unsigned int bytes, QEMUIOVector *qiov,
                                BdrvRequestFlags flags)
@@ -1484,15 +1469,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 7323f0f..e9f35c6 100644
--- a/block/io.c
+++ b/block/io.c
@@ -540,17 +540,6 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
     return 0;
 }
 
-static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
-                              int nb_sectors)
-{
-    if (nb_sectors < 0 || nb_sectors > BDRV_REQUEST_MAX_SECTORS) {
-        return -EIO;
-    }
-
-    return bdrv_check_byte_request(bs, sector_num * BDRV_SECTOR_SIZE,
-                                   nb_sectors * BDRV_SECTOR_SIZE);
-}
-
 typedef struct RwCo {
     BdrvChild *child;
     int64_t offset;
@@ -1878,8 +1867,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;
@@ -1890,14 +1879,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 11c162d..b4a97f2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -399,8 +399,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 2da4905..07285f6 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_pdiscard(BlockBackend *blk, int64_t offset, int count);
 int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
diff --git a/qemu-img.c b/qemu-img.c
index 2e40e1f..c0f939b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1590,7 +1590,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;
                 }
@@ -1727,7 +1729,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 25954f5..3a3838a 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.5.0

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

* [Qemu-devel] [PATCH v6 02/16] block: Convert bdrv_pwrite_compressed() to BdrvChild
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 01/16] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 23:10   ` Eric Blake
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 03/16] block/io: reuse bdrv_co_pwritev() for write compressed Denis V. Lunev
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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>

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 | 2 +-
 block/io.c            | 3 ++-
 include/block/block.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 8f38ab4..4bfc2eb 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1477,7 +1477,7 @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
         return ret;
     }
 
-    return bdrv_pwrite_compressed(blk_bs(blk), offset, buf, count);
+    return bdrv_pwrite_compressed(blk->root, offset, buf, count);
 }
 
 int blk_truncate(BlockBackend *blk, int64_t offset)
diff --git a/block/io.c b/block/io.c
index e9f35c6..1503e09 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1867,9 +1867,10 @@ int bdrv_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
-int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset,
+int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset,
                            const void *buf, int bytes)
 {
+    BlockDriverState *bs = child->bs;
     BlockDriver *drv = bs->drv;
     int ret;
 
diff --git a/include/block/block.h b/include/block/block.h
index b4a97f2..7bb5ddb 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -399,7 +399,7 @@ 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,
+int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset,
                            const void *buf, int bytes);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
-- 
2.5.0

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

* [Qemu-devel] [PATCH v6 03/16] block/io: reuse bdrv_co_pwritev() for write compressed
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 01/16] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 02/16] block: Convert bdrv_pwrite_compressed() to BdrvChild Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 04/16] qcow2: add qcow2_co_pwritev_compressed Denis V. Lunev
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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>
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/io.c                | 56 +++++++++++++++++++++++++++++++++--------------
 include/block/block.h     |  3 ++-
 include/block/block_int.h |  3 +++
 qemu-img.c                |  2 +-
 4 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/block/io.c b/block/io.c
index 1503e09..7fad5b7 100644
--- a/block/io.c
+++ b/block/io.c
@@ -886,6 +886,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)
 {
@@ -1554,9 +1568,14 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
         bytes = ROUND_UP(bytes, align);
     }
 
-    ret = bdrv_aligned_pwritev(bs, &req, offset, bytes, align,
-                               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, align,
+                                   use_local_qiov ? &local_qiov : qiov,
+                                   flags);
+    }
 
 fail:
 
@@ -1872,25 +1891,30 @@ int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset,
 {
     BlockDriverState *bs = child->bs;
     BlockDriver *drv = bs->drv;
-    int ret;
+    QEMUIOVector qiov;
+    struct iovec iov;
 
     if (!drv) {
         return -ENOMEDIUM;
     }
-    if (!drv->bdrv_write_compressed) {
-        return -ENOTSUP;
-    }
-    ret = bdrv_check_byte_request(bs, offset, bytes);
-    if (ret < 0) {
-        return ret;
+    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);
 
-    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);
+    return bdrv_prwv_co(child, offset, &qiov, true, BDRV_REQ_WRITE_COMPRESSED);
 }
 
 typedef struct BdrvVmstateCo {
diff --git a/include/block/block.h b/include/block/block.h
index 7bb5ddb..d8dacd2 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 1fe0fd9..d2673a1 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 c0f939b..d5676a5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2034,7 +2034,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.5.0

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

* [Qemu-devel] [PATCH v6 04/16] qcow2: add qcow2_co_pwritev_compressed
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (2 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 03/16] block/io: reuse bdrv_co_pwritev() for write compressed Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 05/16] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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 | 124 +++++++++++++++++++++++-----------------------------------
 1 file changed, 50 insertions(+), 74 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index d620d0a..b5c69df 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2533,84 +2533,49 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
     return 0;
 }
 
-typedef struct Qcow2WriteCo {
-    BlockDriverState *bs;
-    int64_t sector_num;
-    const uint8_t *buf;
-    int nb_sectors;
-    int ret;
-} Qcow2WriteCo;
-
-static void qcow2_write_co_entry(void *opaque)
-{
-    Qcow2WriteCo *co = opaque;
-    QEMUIOVector 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(&qiov, &iov, 1);
-
-    co->ret = qcow2_co_pwritev(co->bs, offset, bytes, &qiov, 0);
-}
-
-/* Wrapper for non-coroutine contexts */
-static int qcow2_write(BlockDriverState *bs, int64_t sector_num,
-                       const uint8_t *buf, int nb_sectors)
-{
-    Coroutine *co;
-    AioContext *aio_context = bdrv_get_aio_context(bs);
-    Qcow2WriteCo data = {
-        .bs         = bs,
-        .sector_num = sector_num,
-        .buf        = buf,
-        .nb_sectors = nb_sectors,
-        .ret        = -EINPROGRESS,
-    };
-    co = qemu_coroutine_create(qcow2_write_co_entry, &data);
-    qemu_coroutine_enter(co);
-    while (data.ret == -EINPROGRESS) {
-        aio_poll(aio_context, true);
-    }
-    return data.ret;
-}
-
 /* 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);
 
@@ -2641,33 +2606,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 = qcow2_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, 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, 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;
 }
@@ -3412,7 +3388,7 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_co_pwrite_zeroes  = qcow2_co_pwrite_zeroes,
     .bdrv_co_pdiscard       = qcow2_co_pdiscard,
     .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.5.0

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

* [Qemu-devel] [PATCH v6 05/16] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (3 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 04/16] qcow2: add qcow2_co_pwritev_compressed Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 06/16] vmdk: add vmdk_co_pwritev_compressed Denis V. Lunev
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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 the function uses a vector instead of a buffer, there is no
need to use recursive code.

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 | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b5c69df..01bc003 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2554,27 +2554,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.5.0

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

* [Qemu-devel] [PATCH v6 06/16] vmdk: add vmdk_co_pwritev_compressed
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (4 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 05/16] qcow2: cleanup qcow2_co_pwritev_compressed to avoid the recursion Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 07/16] qcow: add qcow_co_pwritev_compressed Denis V. Lunev
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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 46d474e..a11c27a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1645,56 +1645,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, &data);
-        qemu_coroutine_enter(co);
-        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,
@@ -2393,7 +2348,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.5.0

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

* [Qemu-devel] [PATCH v6 07/16] qcow: add qcow_co_pwritev_compressed
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (5 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 06/16] vmdk: add vmdk_co_pwritev_compressed Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 08/16] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion Denis V. Lunev
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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 | 109 +++++++++++++++++++++++------------------------------------
 1 file changed, 42 insertions(+), 67 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 0c7b75b..e1d335d 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -913,75 +913,42 @@ static int qcow_make_empty(BlockDriverState *bs)
     return 0;
 }
 
-typedef struct QcowWriteCo {
-    BlockDriverState *bs;
-    int64_t sector_num;
-    const uint8_t *buf;
-    int nb_sectors;
-    int ret;
-} QcowWriteCo;
-
-static void qcow_write_co_entry(void *opaque)
-{
-    QcowWriteCo *co = opaque;
-    QEMUIOVector qiov;
-
-    struct iovec iov = (struct iovec) {
-        .iov_base   = (uint8_t*) co->buf,
-        .iov_len    = co->nb_sectors * BDRV_SECTOR_SIZE,
-    };
-    qemu_iovec_init_external(&qiov, &iov, 1);
-
-    co->ret = qcow_co_writev(co->bs, co->sector_num, co->nb_sectors, &qiov);
-}
-
-/* Wrapper for non-coroutine contexts */
-static int qcow_write(BlockDriverState *bs, int64_t sector_num,
-                      const uint8_t *buf, int nb_sectors)
-{
-    Coroutine *co;
-    AioContext *aio_context = bdrv_get_aio_context(bs);
-    QcowWriteCo data = {
-        .bs         = bs,
-        .sector_num = sector_num,
-        .buf        = buf,
-        .nb_sectors = nb_sectors,
-        .ret        = -EINPROGRESS,
-    };
-    co = qemu_coroutine_create(qcow_write_co_entry, &data);
-    qemu_coroutine_enter(co);
-    while (data.ret == -EINPROGRESS) {
-        aio_poll(aio_context, true);
-    }
-    return data.ret;
-}
-
 /* 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);
 
@@ -1012,27 +979,35 @@ 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 = qcow_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, 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, 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;
 }
@@ -1085,7 +1060,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.5.0

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

* [Qemu-devel] [PATCH v6 08/16] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (6 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 07/16] qcow: add qcow_co_pwritev_compressed Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 09/16] block: remove BlockDriver.bdrv_write_compressed Denis V. Lunev
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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 the function uses a vector instead of a buffer, there is no
need to use recursive code.

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 | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index e1d335d..20d2e15 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -927,27 +927,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.5.0

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

* [Qemu-devel] [PATCH v6 09/16] block: remove BlockDriver.bdrv_write_compressed
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (7 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 08/16] qcow: cleanup qcow_co_pwritev_compressed to avoid the recursion Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 10/16] block/io: turn on dirty_bitmaps for the compressed writes Denis V. Lunev
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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>
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                | 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 4bfc2eb..53f7971 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1472,12 +1472,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->root, 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 7fad5b7..60922ed 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1886,37 +1886,6 @@ int bdrv_is_allocated_above(BlockDriverState *top,
     return 0;
 }
 
-int bdrv_pwrite_compressed(BdrvChild *child, int64_t offset,
-                           const void *buf, int bytes)
-{
-    BlockDriverState *bs = child->bs;
-    BlockDriver *drv = bs->drv;
-    QEMUIOVector qiov;
-    struct iovec iov;
-
-    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(child, 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 d8dacd2..7edce5c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -400,8 +400,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(BdrvChild *child, 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 d2673a1..378c966 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 d5676a5..014c408 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2034,7 +2034,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.5.0

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

* [Qemu-devel] [PATCH v6 10/16] block/io: turn on dirty_bitmaps for the compressed writes
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (8 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 09/16] block: remove BlockDriver.bdrv_write_compressed Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup Denis V. Lunev
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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>
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/io.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/block/io.c b/block/io.c
index 60922ed..bc9eee5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -896,7 +896,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);
 }
 
@@ -1317,6 +1316,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 if (bytes <= max_transfer) {
         bdrv_debug_event(bs, BLKDBG_PWRITEV);
         ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, flags);
@@ -1568,14 +1569,9 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
         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, align,
-                                   use_local_qiov ? &local_qiov : qiov,
-                                   flags);
-    }
+    ret = bdrv_aligned_pwritev(bs, &req, offset, bytes, align,
+                               use_local_qiov ? &local_qiov : qiov,
+                               flags);
 
 fail:
 
-- 
2.5.0

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

* [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (9 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 10/16] block/io: turn on dirty_bitmaps for the compressed writes Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 23:21   ` Eric Blake
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 12/16] block: simplify blockdev-backup Denis V. Lunev
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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>
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           | 115 +++++++++++++++++----------------------------------
 hmp.c                |  29 ++++++++-----
 qapi/block-core.json |   3 +-
 3 files changed, 58 insertions(+), 89 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index eafeba9..e29147a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1838,17 +1838,8 @@ typedef struct DriveBackupState {
     BlockJob *job;
 } DriveBackupState;
 
-static void do_drive_backup(const char *job_id, 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)
 {
@@ -1878,16 +1869,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->has_job_id ? backup->job_id : NULL,
-                    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;
@@ -3155,17 +3137,7 @@ out:
     aio_context_release(aio_context);
 }
 
-static void do_drive_backup(const char *job_id, 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;
@@ -3178,23 +3150,26 @@ static void do_drive_backup(const char *job_id, const char *device,
     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 (!backup->has_mode) {
+        backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
     }
-    if (!has_mode) {
-        mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+    if (!backup->has_job_id) {
+        backup->job_id = NULL;
     }
 
-    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;
     }
 
@@ -3204,13 +3179,14 @@ static void do_drive_backup(const char *job_id, const char *device,
     /* 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 */
@@ -3222,13 +3198,13 @@ static void do_drive_backup(const char *job_id, const char *device,
 
     /* 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;
     }
 
@@ -3238,14 +3214,14 @@ static void do_drive_backup(const char *job_id, const char *device,
         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);
         }
     }
@@ -3255,29 +3231,29 @@ static void do_drive_backup(const char *job_id, const char *device,
         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(job_id, bs, target_bs, speed, sync, bmap,
-                 on_source_error, on_target_error,
+    backup_start(backup->job_id, 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) {
@@ -3289,24 +3265,9 @@ out:
     aio_context_release(aio_context);
 }
 
-void qmp_drive_backup(bool has_job_id, const char *job_id,
-                      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(has_job_id ? job_id : NULL, 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 cc2056e..9ac49b2 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(false, NULL, 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 f462345..121a540 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1086,7 +1086,8 @@
 #
 # Since 1.6
 ##
-{ 'command': 'drive-backup', 'data': 'DriveBackup' }
+{ 'command': 'drive-backup', 'boxed': true,
+  'data': 'DriveBackup' }
 
 ##
 # @blockdev-backup
-- 
2.5.0

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

* [Qemu-devel] [PATCH v6 12/16] block: simplify blockdev-backup
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (10 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 13/16] drive-backup: added support for data compression Denis V. Lunev
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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>
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           | 70 +++++++++++++++++-----------------------------------
 qapi/block-core.json |  6 ++++-
 2 files changed, 27 insertions(+), 49 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e29147a..0c5ea25 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1906,14 +1906,8 @@ typedef struct BlockdevBackupState {
     AioContext *aio_context;
 } BlockdevBackupState;
 
-static void do_blockdev_backup(const char *job_id, 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)
 {
@@ -1953,12 +1947,7 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
     state->bs = blk_bs(blk);
     bdrv_drained_begin(state->bs);
 
-    do_blockdev_backup(backup->has_job_id ? backup->job_id : NULL,
-                       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;
@@ -3275,14 +3264,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
     return bdrv_named_nodes_list(errp);
 }
 
-void do_blockdev_backup(const char *job_id, 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;
@@ -3290,19 +3272,22 @@ void do_blockdev_backup(const char *job_id, const char *device,
     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;
+    }
+    if (!backup->has_job_id) {
+        backup->job_id = NULL;
     }
 
-    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;
     }
 
@@ -3310,12 +3295,12 @@ void do_blockdev_backup(const char *job_id, const char *device,
     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;
     }
@@ -3331,8 +3316,9 @@ void do_blockdev_backup(const char *job_id, const char *device,
             goto out;
         }
     }
-    backup_start(job_id, bs, target_bs, speed, sync, NULL, on_source_error,
-                 on_target_error, block_job_cb, bs, txn, &local_err);
+    backup_start(backup->job_id, 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);
     }
@@ -3340,21 +3326,9 @@ out:
     aio_context_release(aio_context);
 }
 
-void qmp_blockdev_backup(bool has_job_id, const char *job_id,
-                         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(has_job_id ? job_id : NULL, 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 121a540..44cf9c6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1100,9 +1100,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', 'boxed': true,
+  'data': 'BlockdevBackup' }
 
 
 ##
-- 
2.5.0

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

* [Qemu-devel] [PATCH v6 13/16] drive-backup: added support for data compression
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (11 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 12/16] block: simplify blockdev-backup Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 14/16] blockdev-backup: " Denis V. Lunev
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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 2c05323..bb3bb9a 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(const char *job_id, 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,
@@ -507,6 +510,12 @@ void backup_start(const char *job_id, BlockDriverState *bs,
         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(const char *job_id, BlockDriverState *bs,
     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 0c5ea25..587d76b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3154,6 +3154,9 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp)
     if (!backup->has_job_id) {
         backup->job_id = NULL;
     }
+    if (!backup->has_compress) {
+        backup->compress = false;
+    }
 
     blk = blk_by_name(backup->device);
     if (!blk) {
@@ -3242,8 +3245,8 @@ static void do_drive_backup(DriveBackup *backup, BlockJobTxn *txn, Error **errp)
     }
 
     backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
-                 bmap, backup->on_source_error, backup->on_target_error,
-                 block_job_cb, bs, txn, &local_err);
+                 bmap, 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);
@@ -3317,7 +3320,7 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
         }
     }
     backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
-                 NULL, backup->on_source_error, backup->on_target_error,
+                 NULL, false, 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/hmp-commands.hx b/hmp-commands.hx
index 848efee..74f32e5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1182,8 +1182,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"
@@ -1191,7 +1191,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 9ac49b2..3a9d8d9 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 378c966..da94066 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -762,6 +762,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
 void backup_start(const char *job_id, 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 44cf9c6..6d98da7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -894,6 +894,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).
@@ -911,7 +914,7 @@
 { 'struct': 'DriveBackup',
   'data': { '*job-id': 'str', '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 c8d360a..ff72194 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1217,7 +1217,8 @@ EQMP
     {
         .name       = "drive-backup",
         .args_type  = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
-                      "format:s?,bitmap:s?,on-source-error:s?,on-target-error:s?",
+                      "format:s?,bitmap:s?,compress:b?,"
+                      "on-source-error:s?,on-target-error:s?",
         .mhandler.cmd_new = qmp_marshal_drive_backup,
     },
 
@@ -1253,6 +1254,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.5.0

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

* [Qemu-devel] [PATCH v6 14/16] blockdev-backup: added support for data compression
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (12 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 13/16] drive-backup: added support for data compression Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 15/16] qemu-iotests: test backup compression in 055 Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 16/16] qemu-iotests: add vmdk for " Denis V. Lunev
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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           | 7 +++++--
 qapi/block-core.json | 4 ++++
 qmp-commands.hx      | 4 +++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 587d76b..89c403f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3287,6 +3287,9 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
     if (!backup->has_job_id) {
         backup->job_id = NULL;
     }
+    if (!backup->has_compress) {
+        backup->compress = false;
+    }
 
     blk = blk_by_name(backup->device);
     if (!blk) {
@@ -3320,8 +3323,8 @@ void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
         }
     }
     backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
-                 NULL, false, backup->on_source_error, backup->on_target_error,
-                 block_job_cb, bs, txn, &local_err);
+                 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 6d98da7..7f3424b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -935,6 +935,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).
@@ -953,6 +956,7 @@
   'data': { '*job-id': 'str', '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 ff72194..d06ced5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1275,7 +1275,7 @@ EQMP
 
     {
         .name       = "blockdev-backup",
-        .args_type  = "job-id:s?,sync:s,device:B,target:B,speed:i?,"
+        .args_type  = "job-id:s?,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,
     },
@@ -1299,6 +1299,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.5.0

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

* [Qemu-devel] [PATCH v6 15/16] qemu-iotests: test backup compression in 055
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (13 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 14/16] blockdev-backup: " Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 16/16] qemu-iotests: add vmdk for " Denis V. Lunev
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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>
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        | 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.5.0

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

* [Qemu-devel] [PATCH v6 16/16] qemu-iotests: add vmdk for test backup compression in 055
  2016-07-21 19:40 [Qemu-devel] [PATCH v6 00/16] backup compression Denis V. Lunev
                   ` (14 preceding siblings ...)
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 15/16] qemu-iotests: test backup compression in 055 Denis V. Lunev
@ 2016-07-21 19:40 ` Denis V. Lunev
  15 siblings, 0 replies; 20+ messages in thread
From: Denis V. Lunev @ 2016-07-21 19:40 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.5.0

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

* Re: [Qemu-devel] [PATCH v6 01/16] block: switch blk_write_compressed() to byte-based interface
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 01/16] block: switch blk_write_compressed() to byte-based interface Denis V. Lunev
@ 2016-07-21 23:08   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2016-07-21 23:08 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: 1292 bytes --]

On 07/21/2016 01:40 PM, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> This is a preparatory patch, which continues the general trend of the
> transition to the byte-based interfaces. bdrv_check_request() and
> blk_check_requestis no longer used, so we can remove.

s/requestis/request() are/
s/remove./remove them./

> 
> 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          | 23 ++++-------------------
>  block/io.c                     | 22 +++++++---------------
>  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(+), 41 deletions(-)
> 

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH v6 02/16] block: Convert bdrv_pwrite_compressed() to BdrvChild
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 02/16] block: Convert bdrv_pwrite_compressed() to BdrvChild Denis V. Lunev
@ 2016-07-21 23:10   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2016-07-21 23:10 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: 765 bytes --]

On 07/21/2016 01:40 PM, Denis V. Lunev wrote:
> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
> 
> 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 | 2 +-
>  block/io.c            | 3 ++-
>  include/block/block.h | 2 +-
>  3 files changed, 4 insertions(+), 3 deletions(-)

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup
  2016-07-21 19:40 ` [Qemu-devel] [PATCH v6 11/16] block: simplify drive-backup Denis V. Lunev
@ 2016-07-21 23:21   ` Eric Blake
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Blake @ 2016-07-21 23:21 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: 2274 bytes --]

On 07/21/2016 01:40 PM, 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..

s/.././

> 
> 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           | 115 +++++++++++++++++----------------------------------
>  hmp.c                |  29 ++++++++-----
>  qapi/block-core.json |   3 +-
>  3 files changed, 58 insertions(+), 89 deletions(-)
> 

> +++ 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,

Good.

> +        .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,

I'd drop these. C guarantees that they'll be zero-initialized, and
unless HMP has a way to set them to non-default values, it's not worth
wasting the lines of code.

With those fixups,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

end of thread, other threads:[~2016-07-21 23:21 UTC | newest]

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

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.