[PATCH 0/6] qcow2: advanced compression options

[PATCH 0/6] qcow2: advanced compression options

[Andrey Shinkevich]

New enhancements of writing compressed data to QCOW2 image.

Based on: message ID <20190916175324.18478-1-vsementsov@virtuozzo.com>
          https://lists.nongnu.org/archive/html/qemu-block/2019-09/msg00751.html

Andrey Shinkevich (6):
  qcow2: multiple clusters write compressed
  tests/qemu-iotests: add case of writing compressed data to multiple
    clusters.
  block: support compressed write for copy-on-read
  qemu-nbd: add compression flag support
  block-stream: add compress option
  tests/qemu-iotests: add case for block-stream compress

 block/io.c                 |  21 +++++++--
 block/qcow2.c              | 113 ++++++++++++++++++++++++++++++++++-----------
 block/stream.c             |  19 +++++---
 block/trace-events         |   2 +-
 blockdev-nbd.c             |   2 +-
 blockdev.c                 |  14 +++++-
 hmp-commands.hx            |   4 +-
 include/block/block_int.h  |   3 +-
 include/block/nbd.h        |   7 ++-
 monitor/hmp-cmds.c         |   5 +-
 nbd/server.c               |   8 +++-
 qapi/block-core.json       |   5 +-
 qemu-nbd.c                 |  18 +++++++-
 qemu-nbd.texi              |   2 +
 tests/qemu-iotests/030     |  49 +++++++++++++++++++-
 tests/qemu-iotests/030.out |   4 +-
 tests/qemu-iotests/214     |   9 ++++
 tests/qemu-iotests/214.out |   6 +++
 18 files changed, 236 insertions(+), 55 deletions(-)

-- 
1.8.3.1

[PATCH 1/6] qcow2: multiple clusters write compressed

[Andrey Shinkevich]

QEMU currently supports writing compressed data of size less than or
equal to one cluster. This patch allows writing QCOW2 compressed data
that exceed one cluster. The implementation is simple, we split buffered
data into separate clusters and write them using the existing
functionality. For unaligned requests, we use a workaround that writes
the data without compression.

Suggested-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/qcow2.c | 113 +++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 85 insertions(+), 28 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 7961c05..54ccaf6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4152,10 +4152,8 @@ fail:
     return ret;
 }
 
-/* XXX: put compressed sectors first, then all the cluster aligned
-   tables to avoid losing bytes in alignment */
 static coroutine_fn int
-qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
+qcow2_co_pwritev_compressed_task(BlockDriverState *bs,
                                  uint64_t offset, uint64_t bytes,
                                  QEMUIOVector *qiov, size_t qiov_offset)
 {
@@ -4165,36 +4163,14 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
     uint8_t *buf, *out_buf;
     uint64_t cluster_offset;
 
-    if (has_data_file(bs)) {
-        return -ENOTSUP;
-    }
-
-    if (bytes == 0) {
-        /* align end of file to a sector boundary to ease reading with
-           sector based I/Os */
-        int64_t len = bdrv_getlength(bs->file->bs);
-        if (len < 0) {
-            return len;
-        }
-        return bdrv_co_truncate(bs->file, len, PREALLOC_MODE_OFF, NULL);
-    }
-
-    if (offset_into_cluster(s, offset)) {
-        return -EINVAL;
-    }
+    assert(bytes <= s->cluster_size);
 
     buf = qemu_blockalign(bs, s->cluster_size);
-    if (bytes != s->cluster_size) {
-        if (bytes > s->cluster_size ||
-            offset + bytes != bs->total_sectors << BDRV_SECTOR_BITS)
-        {
-            qemu_vfree(buf);
-            return -EINVAL;
-        }
+    if (bytes < s->cluster_size) {
         /* Zero-pad last write if image size is not cluster aligned */
         memset(buf + bytes, 0, s->cluster_size - bytes);
     }
-    qemu_iovec_to_buf(qiov, qiov_offset, buf, bytes);
+    qemu_iovec_to_buf(qiov, qiov_offset, buf, s->cluster_size);
 
     out_buf = g_malloc(s->cluster_size);
 
@@ -4228,6 +4204,9 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
 
     BLKDBG_EVENT(s->data_file, BLKDBG_WRITE_COMPRESSED);
     ret = bdrv_co_pwrite(s->data_file, cluster_offset, out_len, out_buf, 0);
+    if (ret == -ENOTSUP) {
+        ret = qcow2_co_pwritev_part(bs, offset, bytes, qiov, qiov_offset, 0);
+    }
     if (ret < 0) {
         goto fail;
     }
@@ -4239,6 +4218,84 @@ fail:
     return ret;
 }
 
+static coroutine_fn int qcow2_co_pwritev_compressed_task_entry(AioTask *task)
+{
+    Qcow2AioTask *t = container_of(task, Qcow2AioTask, task);
+
+    assert(!t->cluster_type);
+
+    return qcow2_co_pwritev_compressed_task(t->bs, t->offset, t->bytes, t->qiov,
+                                            t->qiov_offset);
+}
+
+/*
+ * XXX: put compressed sectors first, then all the cluster aligned
+   tables to avoid losing bytes in alignment
+ */
+static coroutine_fn int
+qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
+                                 uint64_t offset, uint64_t bytes,
+                                 QEMUIOVector *qiov, size_t qiov_offset)
+{
+    BDRVQcow2State *s = bs->opaque;
+    QCowL2Meta *l2meta = NULL;
+    AioTaskPool *aio = NULL;
+    uint64_t curr_off = 0;
+    int ret;
+
+    if (has_data_file(bs)) {
+        return -ENOTSUP;
+    }
+
+    if (bytes == 0) {
+        /*
+         * align end of file to a sector boundary to ease reading with
+         * sector based I/Os
+         */
+        int64_t cluster_offset = bdrv_getlength(bs->file->bs);
+        if (cluster_offset < 0) {
+            return cluster_offset;
+        }
+        return bdrv_co_truncate(bs->file, cluster_offset, PREALLOC_MODE_OFF,
+                                NULL);
+    }
+
+    if (offset_into_cluster(s, offset)) {
+        return -EINVAL;
+    }
+
+    while (bytes && aio_task_pool_status(aio) == 0) {
+        uint32_t chunk_size = MIN(bytes, s->cluster_size);
+
+        assert((curr_off & (BDRV_SECTOR_SIZE - 1)) == 0);
+        assert((chunk_size & (BDRV_SECTOR_SIZE - 1)) == 0);
+
+        if (!aio && chunk_size != bytes) {
+            aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
+        }
+
+        ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
+                             0, 0, offset + curr_off, chunk_size,
+                             qiov, qiov_offset, l2meta);
+        if (ret < 0) {
+            break;
+        }
+        qiov_offset += chunk_size;
+        curr_off += chunk_size;
+        bytes -= chunk_size;
+    }
+
+    if (aio) {
+        aio_task_pool_wait_all(aio);
+        if (ret == 0) {
+            ret = aio_task_pool_status(aio);
+        }
+        g_free(aio);
+    }
+
+    return ret;
+}
+
 static int coroutine_fn
 qcow2_co_preadv_compressed(BlockDriverState *bs,
                            uint64_t file_cluster_offset,
-- 
1.8.3.1

[PATCH 2/6] tests/qemu-iotests: add case of writing compressed data to multiple clusters.

[Andrey Shinkevich]

Add the test case to the iotest #214 that checks possibility of writing
compressed data to more than one cluster.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/214     | 9 +++++++++
 tests/qemu-iotests/214.out | 6 ++++++
 2 files changed, 15 insertions(+)

diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index 21ec8a2..5f437e4 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -89,6 +89,15 @@ _check_test_img -r all
 $QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
 $QEMU_IO -c "read  -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "=== Write compressed data to multiple clusters ==="
+echo
+# Create an empty image and fill three and half clusters with compressed data.
+_make_test_img 2M -o cluster_size=0x10000
+data_size=3*0x10000+0x8000
+$QEMU_IO -c "write -c -P 0x11 0 256k" "$TEST_IMG" \
+         2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo '*** done'
 rm -f $seq.full
diff --git a/tests/qemu-iotests/214.out b/tests/qemu-iotests/214.out
index 0fcd8dc..2b908f6 100644
--- a/tests/qemu-iotests/214.out
+++ b/tests/qemu-iotests/214.out
@@ -32,4 +32,10 @@ read 4194304/4194304 bytes at offset 0
 4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 4194304/4194304 bytes at offset 4194304
 4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Write compressed data to multiple clusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2097152
+wrote 262144/262144 bytes at offset 0
+256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
1.8.3.1

[PATCH 3/6] block: support compressed write for copy-on-read

[Andrey Shinkevich]

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/io.c         | 21 ++++++++++++++++-----
 block/trace-events |  2 +-
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/block/io.c b/block/io.c
index f8c3596..a7cd24f 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1264,12 +1264,13 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
      * allocating cluster in the image file.  Note that this value may exceed
      * BDRV_REQUEST_MAX_BYTES (even when the original read did not), which
      * is one reason we loop rather than doing it all at once.
+     * Also, this is crucial for compressed copy-on-read.
      */
     bdrv_round_to_clusters(bs, offset, bytes, &cluster_offset, &cluster_bytes);
     skip_bytes = offset - cluster_offset;
 
     trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
-                                   cluster_offset, cluster_bytes);
+                                   cluster_offset, cluster_bytes, flags);
 
     while (cluster_bytes) {
         int64_t pnum;
@@ -1328,9 +1329,15 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
                 /* This does not change the data on the disk, it is not
                  * necessary to flush even in cache=writethrough mode.
                  */
-                ret = bdrv_driver_pwritev(bs, cluster_offset, pnum,
-                                          &local_qiov, 0,
-                                          BDRV_REQ_WRITE_UNCHANGED);
+                if (flags & BDRV_REQ_WRITE_COMPRESSED) {
+                    ret = bdrv_driver_pwritev_compressed(bs, cluster_offset,
+                                                         pnum, &local_qiov,
+                                                         qiov_offset);
+                } else {
+                    ret = bdrv_driver_pwritev(bs, cluster_offset, pnum,
+                                              &local_qiov, 0,
+                                              BDRV_REQ_WRITE_UNCHANGED);
+                }
             }
 
             if (ret < 0) {
@@ -1396,7 +1403,11 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
      * to pass through to drivers.  For now, there aren't any
      * passthrough flags.  */
     assert(!(flags & ~(BDRV_REQ_NO_SERIALISING | BDRV_REQ_COPY_ON_READ |
-                       BDRV_REQ_PREFETCH)));
+                       BDRV_REQ_PREFETCH | BDRV_REQ_WRITE_COMPRESSED)));
+
+    /* write compressed only makes sense with copy on read */
+    assert(!(flags & BDRV_REQ_WRITE_COMPRESSED) ||
+           (flags & BDRV_REQ_COPY_ON_READ));
 
     /* Handle Copy on Read and associated serialisation */
     if (flags & BDRV_REQ_COPY_ON_READ) {
diff --git a/block/trace-events b/block/trace-events
index 3aa27e6..f444548 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -14,7 +14,7 @@ blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
 bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
 bdrv_co_pwritev(void *bs, int64_t offset, int64_t nbytes, unsigned int flags) "bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
 bdrv_co_pwrite_zeroes(void *bs, int64_t offset, int count, int flags) "bs %p offset %"PRId64" count %d flags 0x%x"
-bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, int64_t cluster_bytes) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %"PRId64
+bdrv_co_do_copy_on_readv(void *bs, int64_t offset, unsigned int bytes, int64_t cluster_offset, int64_t cluster_bytes, int flags) "bs %p offset %"PRId64" bytes %u cluster_offset %"PRId64" cluster_bytes %"PRId64" flags 0x%x"
 bdrv_co_copy_range_from(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
 bdrv_co_copy_range_to(void *src, uint64_t src_offset, void *dst, uint64_t dst_offset, uint64_t bytes, int read_flags, int write_flags) "src %p offset %"PRIu64" dst %p offset %"PRIu64" bytes %"PRIu64" rw flags 0x%x 0x%x"
 
-- 
1.8.3.1

[PATCH 4/6] qemu-nbd: add compression flag support

[Andrey Shinkevich]

Added possibility to write compressed data by using the
blk_write_compressed. This action has the limitations of the format
driver. For example we can't write compressed data over other.

$ ./qemu-img create -f qcow2 -o size=10G ./image.qcow2
$ sudo ./qemu-nbd -c /dev/nbd0 ./image.qcow2
$ sudo dd if=/dev/sda1 of=/dev/nbd0 bs=10M count=10
10+0 records in
10+0 records out
104857600 bytes (105 MB) copied, 0,0890581 s, 1,2 GB/s
$ sudo ./qemu-nbd -d /dev/nbd0
$ du -sh ./image.qcow2
101M    ./image.qcow2

$ ./qemu-img create -f qcow2 -o size=10G ./image.qcow2
$ sudo ./qemu-nbd -C -c /dev/nbd0 ./image.qcow2
$ sudo dd if=/dev/sda1 of=/dev/nbd0 bs=10M count=10
10+0 records in
10+0 records out
104857600 bytes (105 MB) copied, 0,076046 s, 1,4 GB/s
$ sudo ./qemu-nbd -d /dev/nbd0
$ du -sh ./image.qcow2
5,3M    ./image.qcow2

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 blockdev-nbd.c      |  2 +-
 include/block/nbd.h |  7 ++++++-
 nbd/server.c        |  8 +++++++-
 qemu-nbd.c          | 18 ++++++++++++++++--
 qemu-nbd.texi       |  2 ++
 5 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 6a8b206..2a01a04 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -191,7 +191,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
     }
 
     exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, !writable,
-                         NULL, false, on_eject_blk, errp);
+                         0, NULL, false, on_eject_blk, errp);
     if (!exp) {
         goto out;
     }
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 316fd70..0032f73 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -25,6 +25,10 @@
 #include "crypto/tlscreds.h"
 #include "qapi/error.h"
 
+enum {
+  NBD_INTERNAL_FLAG_COMPRESS = 1 << 1, /* Use write compressed */
+};
+
 /* Handshake phase structs - this struct is passed on the wire */
 
 struct NBDOption {
@@ -330,7 +334,8 @@ typedef struct NBDClient NBDClient;
 
 NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
                           uint64_t size, const char *name, const char *desc,
-                          const char *bitmap, bool readonly, bool shared,
+                          const char *bitmap, bool readonly,
+                          bool shared, uint32_t iflags,
                           void (*close)(NBDExport *), bool writethrough,
                           BlockBackend *on_eject_blk, Error **errp);
 void nbd_export_close(NBDExport *exp);
diff --git a/nbd/server.c b/nbd/server.c
index d8d1e62..96d581d 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -91,6 +91,7 @@ struct NBDExport {
     uint16_t nbdflags;
     QTAILQ_HEAD(, NBDClient) clients;
     QTAILQ_ENTRY(NBDExport) next;
+    uint32_t iflags;
 
     AioContext *ctx;
 
@@ -1471,7 +1472,8 @@ static void nbd_eject_notifier(Notifier *n, void *data)
 
 NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
                           uint64_t size, const char *name, const char *desc,
-                          const char *bitmap, bool readonly, bool shared,
+                          const char *bitmap, bool readonly,
+                          bool shared, uint32_t iflags,
                           void (*close)(NBDExport *), bool writethrough,
                           BlockBackend *on_eject_blk, Error **errp)
 {
@@ -1525,6 +1527,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
         exp->nbdflags |= (NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_WRITE_ZEROES |
                           NBD_FLAG_SEND_FAST_ZERO);
     }
+    exp->iflags = iflags;
     assert(size <= INT64_MAX - dev_offset);
     exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
 
@@ -2312,6 +2315,9 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
         if (request->flags & NBD_CMD_FLAG_FUA) {
             flags |= BDRV_REQ_FUA;
         }
+        if (exp->iflags & NBD_INTERNAL_FLAG_COMPRESS) {
+            flags |= BDRV_REQ_WRITE_COMPRESSED;
+        }
         ret = blk_pwrite(exp->blk, request->from + exp->dev_offset,
                          data, request->len, flags);
         return nbd_send_generic_reply(client, request->handle, ret,
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9032b6d..3765c4b 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -139,6 +139,7 @@ static void usage(const char *name)
 "      --discard=MODE        set discard mode (ignore, unmap)\n"
 "      --detect-zeroes=MODE  set detect-zeroes mode (off, on, unmap)\n"
 "      --image-opts          treat FILE as a full set of image options\n"
+"  -C, --compress            use data compression (if the target format supports it)\n"
 "\n"
 QEMU_HELP_BOTTOM "\n"
     , name, name, NBD_DEFAULT_PORT, "DEVICE");
@@ -602,6 +603,7 @@ int main(int argc, char **argv)
     BlockDriverState *bs;
     uint64_t dev_offset = 0;
     bool readonly = false;
+    uint32_t iflags = 0;
     bool disconnect = false;
     const char *bindto = NULL;
     const char *port = NULL;
@@ -610,7 +612,7 @@ int main(int argc, char **argv)
     int64_t fd_size;
     QemuOpts *sn_opts = NULL;
     const char *sn_id_or_name = NULL;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:B:L";
+    const char *sopt = "hVb:o:p:CrsnP:c:dvk:e:f:tl:x:T:D:B:L";
     struct option lopt[] = {
         { "help", no_argument, NULL, 'h' },
         { "version", no_argument, NULL, 'V' },
@@ -619,6 +621,7 @@ int main(int argc, char **argv)
         { "socket", required_argument, NULL, 'k' },
         { "offset", required_argument, NULL, 'o' },
         { "read-only", no_argument, NULL, 'r' },
+        { "compress", no_argument, NULL, 'C'},
         { "partition", required_argument, NULL, 'P' },
         { "bitmap", required_argument, NULL, 'B' },
         { "connect", required_argument, NULL, 'c' },
@@ -786,6 +789,9 @@ int main(int argc, char **argv)
             readonly = true;
             flags &= ~BDRV_O_RDWR;
             break;
+        case 'C':
+            iflags |= NBD_INTERNAL_FLAG_COMPRESS;
+            break;
         case 'P':
             warn_report("The '-P' option is deprecated; use --image-opts with "
                         "a raw device wrapper for subset exports instead");
@@ -1117,6 +1123,14 @@ int main(int argc, char **argv)
 
     blk_set_enable_write_cache(blk, !writethrough);
 
+    if ((iflags & NBD_INTERNAL_FLAG_COMPRESS) &&
+        !(bs->drv && bs->drv->bdrv_co_pwritev_compressed_part))
+    {
+        error_report("Compression not supported for this file format %s",
+                     argv[optind]);
+        exit(EXIT_FAILURE);
+    }
+
     if (sn_opts) {
         ret = bdrv_snapshot_load_tmp(bs,
                                      qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
@@ -1175,7 +1189,7 @@ int main(int argc, char **argv)
 
     export = nbd_export_new(bs, dev_offset, fd_size, export_name,
                             export_description, bitmap, readonly, shared > 1,
-                            nbd_export_closed, writethrough, NULL,
+                            iflags, nbd_export_closed, writethrough, NULL,
                             &error_fatal);
 
     if (device) {
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 7f55657..26ea1ec 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -55,6 +55,8 @@ Force the use of the block driver for format @var{fmt} instead of
 auto-detecting.
 @item -r, --read-only
 Export the disk as read-only.
+@item -C, --compress
+Use data compression (if the target format supports it)
 @item -P, --partition=@var{num}
 Deprecated: Only expose MBR partition @var{num}.  Understands physical
 partitions 1-4 and logical partition 5. New code should instead use
-- 
1.8.3.1

[PATCH 5/6] block-stream: add compress option

[Andrey Shinkevich]

Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/stream.c            | 19 +++++++++++++------
 blockdev.c                | 14 +++++++++++++-
 hmp-commands.hx           |  4 ++--
 include/block/block_int.h |  3 ++-
 monitor/hmp-cmds.c        |  5 +++--
 qapi/block-core.json      |  5 ++++-
 6 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index 5562ccb..51cc49e 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -36,15 +36,21 @@ typedef struct StreamBlockJob {
     char *backing_file_str;
     bool bs_read_only;
     bool chain_frozen;
+    bool compress;
 } StreamBlockJob;
 
-static int coroutine_fn stream_populate(BlockBackend *blk,
-                                        int64_t offset, uint64_t bytes)
+static int coroutine_fn stream_populate(BlockBackend *blk, int64_t offset,
+                                        uint64_t bytes, bool compress)
 {
+    int flags = BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH;
+
+    if (compress) {
+        flags |= BDRV_REQ_WRITE_COMPRESSED;
+    }
+
     assert(bytes < SIZE_MAX);
 
-    return blk_co_preadv(blk, offset, bytes, NULL,
-                         BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH);
+    return blk_co_preadv(blk, offset, bytes, NULL, flags);
 }
 
 static void stream_abort(Job *job)
@@ -167,7 +173,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
         }
         trace_stream_one_iteration(s, offset, n, ret);
         if (copy) {
-            ret = stream_populate(blk, offset, n);
+            ret = stream_populate(blk, offset, n, s->compress);
         }
         if (ret < 0) {
             BlockErrorAction action =
@@ -217,7 +223,7 @@ static const BlockJobDriver stream_job_driver = {
 
 void stream_start(const char *job_id, BlockDriverState *bs,
                   BlockDriverState *base, const char *backing_file_str,
-                  int creation_flags, int64_t speed,
+                  int creation_flags, int64_t speed, bool compress,
                   BlockdevOnError on_error, Error **errp)
 {
     StreamBlockJob *s;
@@ -267,6 +273,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
     s->backing_file_str = g_strdup(backing_file_str);
     s->bs_read_only = bs_read_only;
     s->chain_frozen = true;
+    s->compress = compress;
 
     s->on_error = on_error;
     trace_stream_start(bs, base, s);
diff --git a/blockdev.c b/blockdev.c
index fbef684..290ee4b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3238,6 +3238,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
                       bool has_base_node, const char *base_node,
                       bool has_backing_file, const char *backing_file,
                       bool has_speed, int64_t speed,
+                      bool has_compress, bool compress,
                       bool has_on_error, BlockdevOnError on_error,
                       bool has_auto_finalize, bool auto_finalize,
                       bool has_auto_dismiss, bool auto_dismiss,
@@ -3254,6 +3255,10 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
         on_error = BLOCKDEV_ON_ERROR_REPORT;
     }
 
+    if (!has_compress) {
+        compress = false;
+    }
+
     bs = bdrv_lookup_bs(device, device, errp);
     if (!bs) {
         return;
@@ -3308,6 +3313,12 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
         goto out;
     }
 
+    if (compress && bs->drv->bdrv_co_pwritev_compressed_part == NULL) {
+        error_setg(errp, "Compression is not supported for this drive %s",
+                   bdrv_get_device_name(bs));
+        goto out;
+    }
+
     /* backing_file string overrides base bs filename */
     base_name = has_backing_file ? backing_file : base_name;
 
@@ -3319,7 +3330,8 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
     }
 
     stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name,
-                 job_flags, has_speed ? speed : 0, on_error, &local_err);
+                 job_flags, has_speed ? speed : 0, compress, on_error,
+                 &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index cfcc044..3a347fd 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -95,8 +95,8 @@ ETEXI
 
     {
         .name       = "block_stream",
-        .args_type  = "device:B,speed:o?,base:s?",
-        .params     = "device [speed [base]]",
+        .args_type  = "device:B,speed:o?,base:s?,compress:o?",
+        .params     = "device [speed [base]] [compress]",
         .help       = "copy data from a backing file into a block device",
         .cmd        = hmp_block_stream,
     },
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0422acd..5e7fce8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1065,6 +1065,7 @@ int is_windows_drive(const char *filename);
  * @creation_flags: Flags that control the behavior of the Job lifetime.
  *                  See @BlockJobCreateFlags
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
+ * @compress: True to compress data.
  * @on_error: The action to take upon error.
  * @errp: Error object.
  *
@@ -1077,7 +1078,7 @@ int is_windows_drive(const char *filename);
  */
 void stream_start(const char *job_id, BlockDriverState *bs,
                   BlockDriverState *base, const char *backing_file_str,
-                  int creation_flags, int64_t speed,
+                  int creation_flags, int64_t speed, bool compress,
                   BlockdevOnError on_error, Error **errp);
 
 /**
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index b2551c1..91201fe 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -2025,11 +2025,12 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
     const char *device = qdict_get_str(qdict, "device");
     const char *base = qdict_get_try_str(qdict, "base");
     int64_t speed = qdict_get_try_int(qdict, "speed", 0);
+    bool compress = qdict_get_try_bool(qdict, "compress", false);
 
     qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
                      false, NULL, qdict_haskey(qdict, "speed"), speed, true,
-                     BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
-                     &error);
+                     compress, true, BLOCKDEV_ON_ERROR_REPORT,
+                     false, false, false, false, &error);
 
     hmp_handle_error(mon, &error);
 }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e6edd64..f41513f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2544,6 +2544,9 @@
 #
 # @speed:  the maximum speed, in bytes per second
 #
+# @compress: true to compress data, if the target format supports it.
+#            (default: false). Since 4.1.
+#
 # @on-error: the action to take on an error (default report).
 #            'stop' and 'enospc' can only be used if the block device
 #            supports io-status (see BlockInfo).  Since 1.3.
@@ -2576,7 +2579,7 @@
 { 'command': 'block-stream',
   'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
             '*base-node': 'str', '*backing-file': 'str', '*speed': 'int',
-            '*on-error': 'BlockdevOnError',
+            '*compress': 'bool', '*on-error': 'BlockdevOnError',
             '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
 
 ##
-- 
1.8.3.1

[PATCH 6/6] tests/qemu-iotests: add case for block-stream compress

[Andrey Shinkevich]

Add a test case to the iotest #030 that checks 'compress' option for a
block-stream job.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/030     | 49 +++++++++++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/030.out |  4 ++--
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index f3766f2..13fe5a2 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -21,7 +21,8 @@
 import time
 import os
 import iotests
-from iotests import qemu_img, qemu_io
+from iotests import qemu_img, qemu_io, qemu_img_pipe
+import json
 
 backing_img = os.path.join(iotests.test_dir, 'backing.img')
 mid_img = os.path.join(iotests.test_dir, 'mid.img')
@@ -956,6 +957,52 @@ class TestSetSpeed(iotests.QMPTestCase):
 
         self.cancel_and_wait(resume=True)
 
+class TestCompressed(iotests.QMPTestCase):
+
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, backing_img, '1M')
+        qemu_img('create', '-f', iotests.imgfmt, '-o',
+                 'backing_file=%s' % backing_img, mid_img)
+        qemu_img('create', '-f', iotests.imgfmt, '-o',
+                 'backing_file=%s' % mid_img, test_img)
+        qemu_io('-c', 'write -P 0x1 0 512k', backing_img)
+        self.vm = iotests.VM().add_drive(test_img, "backing.node-name=mid," +
+                                         "backing.backing.node-name=base")
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(mid_img)
+        os.remove(backing_img)
+
+    def test_stream_compress(self):
+        self.assert_no_active_block_jobs()
+
+        result = self.vm.qmp('block-stream', device='mid', job_id='stream-mid')
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_until_completed(drive='stream-mid')
+        for event in self.vm.get_qmp_events(wait=True):
+            if event['event'] == 'BLOCK_JOB_COMPLETED':
+                self.dictpath(event, 'data/device')
+                self.assert_qmp_absent(event, 'data/error')
+
+        result = self.vm.qmp('block-stream', device='drive0', base=mid_img,
+                             job_id='stream-top', compress=True)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_until_completed(drive='stream-top')
+        self.assert_no_active_block_jobs()
+        self.vm.shutdown()
+
+        top = json.loads(qemu_img_pipe('info', '--output=json', test_img))
+        mid = json.loads(qemu_img_pipe('info', '--output=json', mid_img))
+        base = json.loads(qemu_img_pipe('info', '--output=json', backing_img))
+
+        self.assertEqual(mid['actual-size'], base['actual-size'])
+        self.assertLess(top['actual-size'], mid['actual-size'])
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'],
                  supported_protocols=['file'])
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index 6d9bee1..af8dac1 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -1,5 +1,5 @@
-...........................
+............................
 ----------------------------------------------------------------------
-Ran 27 tests
+Ran 28 tests
 
 OK
-- 
1.8.3.1

Re: [PATCH 4/6] qemu-nbd: add compression flag support

[Eric Blake]

On 10/1/19 2:27 PM, Andrey Shinkevich wrote:
> Added possibility to write compressed data by using the
> blk_write_compressed. This action has the limitations of the format
> driver. For example we can't write compressed data over other.
> 

> +++ b/blockdev-nbd.c
> @@ -191,7 +191,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
>       }
>   
>       exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, !writable,
> -                         NULL, false, on_eject_blk, errp);
> +                         0, NULL, false, on_eject_blk, errp);

This is a lot of parameters.  Should we be combining some of them into a 
struct, or even at least the booleans into a flags parameter?


> +++ b/include/block/nbd.h
> @@ -25,6 +25,10 @@
>   #include "crypto/tlscreds.h"
>   #include "qapi/error.h"
>   
> +enum {
> +  NBD_INTERNAL_FLAG_COMPRESS = 1 << 1, /* Use write compressed */
> +};

What happened to flag 1 << 0?  What other flags do you anticipate adding?


> +++ b/nbd/server.c
> @@ -91,6 +91,7 @@ struct NBDExport {
>       uint16_t nbdflags;
>       QTAILQ_HEAD(, NBDClient) clients;
>       QTAILQ_ENTRY(NBDExport) next;
> +    uint32_t iflags;
>   
>       AioContext *ctx;
>   
> @@ -1471,7 +1472,8 @@ static void nbd_eject_notifier(Notifier *n, void *data)
>   
>   NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>                             uint64_t size, const char *name, const char *desc,
> -                          const char *bitmap, bool readonly, bool shared,
> +                          const char *bitmap, bool readonly,
> +                          bool shared, uint32_t iflags,
>                             void (*close)(NBDExport *), bool writethrough,
>                             BlockBackend *on_eject_blk, Error **errp)

Again, this feels like a lot of parameters, combining more through 
iflags may make sense.

>   {
> @@ -1525,6 +1527,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
>           exp->nbdflags |= (NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_WRITE_ZEROES |
>                             NBD_FLAG_SEND_FAST_ZERO);
>       }
> +    exp->iflags = iflags;
>       assert(size <= INT64_MAX - dev_offset);
>       exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
>   
> @@ -2312,6 +2315,9 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
>           if (request->flags & NBD_CMD_FLAG_FUA) {
>               flags |= BDRV_REQ_FUA;
>           }
> +        if (exp->iflags & NBD_INTERNAL_FLAG_COMPRESS) {
> +            flags |= BDRV_REQ_WRITE_COMPRESSED;
> +        }

This unconditionally tries to make all writes compressed if the option 
was selected when starting qemu-nbd.  Should we at least sanity check 
that it will work during nbd_export_new, rather than waiting to find out 
on the first (failed) write, whether it actually works?

/me looks ahead [1]

>           ret = blk_pwrite(exp->blk, request->from + exp->dev_offset,
>                            data, request->len, flags);
>           return nbd_send_generic_reply(client, request->handle, ret,
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 9032b6d..3765c4b 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -139,6 +139,7 @@ static void usage(const char *name)
>   "      --discard=MODE        set discard mode (ignore, unmap)\n"
>   "      --detect-zeroes=MODE  set detect-zeroes mode (off, on, unmap)\n"
>   "      --image-opts          treat FILE as a full set of image options\n"
> +"  -C, --compress            use data compression (if the target format supports it)\n"

I'm not necessarily opposed to burning a short option.  But it's a shame 
that we can't use -c to be similar to 'qemu-img convert -c'.  Requiring 
the use of a long option is also okay (short options have to be for the 
more likely uses, although it does seem like this use case might qualify).

> @@ -610,7 +612,7 @@ int main(int argc, char **argv)
>       int64_t fd_size;
>       QemuOpts *sn_opts = NULL;
>       const char *sn_id_or_name = NULL;
> -    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:B:L";
> +    const char *sopt = "hVb:o:p:CrsnP:c:dvk:e:f:tl:x:T:D:B:L";

Pre-existing, but we don't sort this very well.

>       struct option lopt[] = {
>           { "help", no_argument, NULL, 'h' },
>           { "version", no_argument, NULL, 'V' },
> @@ -619,6 +621,7 @@ int main(int argc, char **argv)
>           { "socket", required_argument, NULL, 'k' },
>           { "offset", required_argument, NULL, 'o' },
>           { "read-only", no_argument, NULL, 'r' },
> +        { "compress", no_argument, NULL, 'C'},
>           { "partition", required_argument, NULL, 'P' },

Above you put 'C' between 'p' and 'r', but here between 'r' and 'P'.  We 
really don't sort very well :)

>           { "bitmap", required_argument, NULL, 'B' },
>           { "connect", required_argument, NULL, 'c' },
> @@ -786,6 +789,9 @@ int main(int argc, char **argv)
>               readonly = true;
>               flags &= ~BDRV_O_RDWR;
>               break;
> +        case 'C':
> +            iflags |= NBD_INTERNAL_FLAG_COMPRESS;
> +            break;
>           case 'P':

At least this matches your lopt[] ordering.

>               warn_report("The '-P' option is deprecated; use --image-opts with "
>                           "a raw device wrapper for subset exports instead");
> @@ -1117,6 +1123,14 @@ int main(int argc, char **argv)
>   
>       blk_set_enable_write_cache(blk, !writethrough);
>   
> +    if ((iflags & NBD_INTERNAL_FLAG_COMPRESS) &&
> +        !(bs->drv && bs->drv->bdrv_co_pwritev_compressed_part))
> +    {
> +        error_report("Compression not supported for this file format %s",
> +                     argv[optind]);
> +        exit(EXIT_FAILURE);
> +    }
> +

[1] ah, you DO make sure that compression is supported before passing 
the option through.

The idea seems reasonable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 5/6] block-stream: add compress option

[Eric Blake]

On 10/1/19 2:27 PM, Andrey Shinkevich wrote:
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---

The commit summary says 'what', but left out a body stating 'why'. 
Reading the whole series makes it obvious, but landing on this commit in 
isolation during a future 'git bisect' less so.  It's not necessarily a 
show-stopper, but food for thought in writing future commit messages.


> +++ b/qapi/block-core.json
> @@ -2544,6 +2544,9 @@
>   #
>   # @speed:  the maximum speed, in bytes per second
>   #
> +# @compress: true to compress data, if the target format supports it.

Looking at neighbors, it would be more consistent to drop the trailing dot.

> +#            (default: false). Since 4.1.
> +#

4.2, now.

>   # @on-error: the action to take on an error (default report).
>   #            'stop' and 'enospc' can only be used if the block device
>   #            supports io-status (see BlockInfo).  Since 1.3.
> @@ -2576,7 +2579,7 @@
>   { 'command': 'block-stream',
>     'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
>               '*base-node': 'str', '*backing-file': 'str', '*speed': 'int',
> -            '*on-error': 'BlockdevOnError',
> +            '*compress': 'bool', '*on-error': 'BlockdevOnError',
>               '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
>   
>   ##
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 0/6] qcow2: advanced compression options

[no-reply]

Patchew URL: https://patchew.org/QEMU/1569958040-697220-1-git-send-email-andrey.shinkevich@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      block/qed-l2-cache.o
  CC      block/qed-table.o
  CC      block/qed-cluster.o
/tmp/qemu-test/src/block/qcow2.c:4077:64: error: unknown type name 'AioTask'
 static coroutine_fn int qcow2_co_pwritev_compressed_task_entry(AioTask *task)
                                                                ^
/tmp/qemu-test/src/block/qcow2.c: In function 'qcow2_co_pwritev_compressed_part':
/tmp/qemu-test/src/block/qcow2.c:4098:5: error: unknown type name 'AioTaskPool'
     AioTaskPool *aio = NULL;
     ^
/tmp/qemu-test/src/block/qcow2.c:4123:5: error: implicit declaration of function 'aio_task_pool_status' [-Werror=implicit-function-declaration]
     while (bytes && aio_task_pool_status(aio) == 0) {
     ^
/tmp/qemu-test/src/block/qcow2.c:4123:5: error: nested extern declaration of 'aio_task_pool_status' [-Werror=nested-externs]
/tmp/qemu-test/src/block/qcow2.c:4130:13: error: implicit declaration of function 'aio_task_pool_new' [-Werror=implicit-function-declaration]
             aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
             ^
/tmp/qemu-test/src/block/qcow2.c:4130:13: error: nested extern declaration of 'aio_task_pool_new' [-Werror=nested-externs]
/tmp/qemu-test/src/block/qcow2.c:4130:37: error: 'QCOW2_MAX_WORKERS' undeclared (first use in this function)
             aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
                                     ^
/tmp/qemu-test/src/block/qcow2.c:4130:37: note: each undeclared identifier is reported only once for each function it appears in
/tmp/qemu-test/src/block/qcow2.c:4133:9: error: implicit declaration of function 'qcow2_add_task' [-Werror=implicit-function-declaration]
         ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
         ^
/tmp/qemu-test/src/block/qcow2.c:4133:9: error: nested extern declaration of 'qcow2_add_task' [-Werror=nested-externs]
/tmp/qemu-test/src/block/qcow2.c:4133:39: error: 'qcow2_co_pwritev_compressed_task_entry' undeclared (first use in this function)
         ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
                                       ^
/tmp/qemu-test/src/block/qcow2.c:4145:9: error: implicit declaration of function 'aio_task_pool_wait_all' [-Werror=implicit-function-declaration]
         aio_task_pool_wait_all(aio);
         ^
/tmp/qemu-test/src/block/qcow2.c:4145:9: error: nested extern declaration of 'aio_task_pool_wait_all' [-Werror=nested-externs]
/tmp/qemu-test/src/block/qcow2.c: At top level:
/tmp/qemu-test/src/block/qcow2.c:4012:1: error: 'qcow2_co_pwritev_compressed_task' defined but not used [-Werror=unused-function]
 qcow2_co_pwritev_compressed_task(BlockDriverState *bs,
 ^
cc1: all warnings being treated as errors
make: *** [block/qcow2.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 662, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=c0ff6dd6fb3f46d1b9c07a106c9dd26a', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-8m_nebab/src/docker-src.2019-10-01-22.22.08.6455:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=c0ff6dd6fb3f46d1b9c07a106c9dd26a
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-8m_nebab/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    1m40.390s
user    0m8.966s


The full log is available at
http://patchew.org/logs/1569958040-697220-1-git-send-email-andrey.shinkevich@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 0/6] qcow2: advanced compression options

[no-reply]

Patchew URL: https://patchew.org/QEMU/1569958040-697220-1-git-send-email-andrey.shinkevich@virtuozzo.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      block/qed-table.o
  CC      block/qed-cluster.o
  CC      block/qed-check.o
/tmp/qemu-test/src/block/qcow2.c:4077:64: error: unknown type name 'AioTask'; did you mean 'AioWait'?
 static coroutine_fn int qcow2_co_pwritev_compressed_task_entry(AioTask *task)
                                                                ^~~~~~~
                                                                AioWait
/tmp/qemu-test/src/block/qcow2.c: In function 'qcow2_co_pwritev_compressed_part':
/tmp/qemu-test/src/block/qcow2.c:4098:5: error: unknown type name 'AioTaskPool'
     AioTaskPool *aio = NULL;
     ^~~~~~~~~~~
/tmp/qemu-test/src/block/qcow2.c:4123:21: error: implicit declaration of function 'aio_task_pool_status' [-Werror=implicit-function-declaration]
     while (bytes && aio_task_pool_status(aio) == 0) {
                     ^~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/block/qcow2.c:4123:21: error: nested extern declaration of 'aio_task_pool_status' [-Werror=nested-externs]
/tmp/qemu-test/src/block/qcow2.c:4130:19: error: implicit declaration of function 'aio_task_pool_new'; did you mean 'aio_timer_new'? [-Werror=implicit-function-declaration]
             aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
                   ^~~~~~~~~~~~~~~~~
                   aio_timer_new
/tmp/qemu-test/src/block/qcow2.c:4130:19: error: nested extern declaration of 'aio_task_pool_new' [-Werror=nested-externs]
/tmp/qemu-test/src/block/qcow2.c:4130:37: error: 'QCOW2_MAX_WORKERS' undeclared (first use in this function); did you mean 'QCOW2_MAX_THREADS'?
             aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
                                     ^~~~~~~~~~~~~~~~~
                                     QCOW2_MAX_THREADS
/tmp/qemu-test/src/block/qcow2.c:4130:37: note: each undeclared identifier is reported only once for each function it appears in
/tmp/qemu-test/src/block/qcow2.c:4133:15: error: implicit declaration of function 'qcow2_add_task'; did you mean 'qcow2_do_open'? [-Werror=implicit-function-declaration]
         ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
               ^~~~~~~~~~~~~~
               qcow2_do_open
/tmp/qemu-test/src/block/qcow2.c:4133:15: error: nested extern declaration of 'qcow2_add_task' [-Werror=nested-externs]
/tmp/qemu-test/src/block/qcow2.c:4133:39: error: 'qcow2_co_pwritev_compressed_task_entry' undeclared (first use in this function); did you mean 'qcow2_co_pwritev_compressed_task'?
         ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                       qcow2_co_pwritev_compressed_task
/tmp/qemu-test/src/block/qcow2.c:4145:9: error: implicit declaration of function 'aio_task_pool_wait_all' [-Werror=implicit-function-declaration]
         aio_task_pool_wait_all(aio);
         ^~~~~~~~~~~~~~~~~~~~~~
/tmp/qemu-test/src/block/qcow2.c:4145:9: error: nested extern declaration of 'aio_task_pool_wait_all' [-Werror=nested-externs]
At top level:
/tmp/qemu-test/src/block/qcow2.c:4012:1: error: 'qcow2_co_pwritev_compressed_task' defined but not used [-Werror=unused-function]
 qcow2_co_pwritev_compressed_task(BlockDriverState *bs,
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: block/qcow2.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 662, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=5e43e6e4ab784c32a96f618a29b7ceef', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-yubxkwpj/src/docker-src.2019-10-01-22.23.28.10478:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=5e43e6e4ab784c32a96f618a29b7ceef
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-yubxkwpj/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m9.622s
user    0m7.777s


The full log is available at
http://patchew.org/logs/1569958040-697220-1-git-send-email-andrey.shinkevich@virtuozzo.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 4/6] qemu-nbd: add compression flag support

[Vladimir Sementsov-Ogievskiy]

01.10.2019 23:47, Eric Blake wrote:
> On 10/1/19 2:27 PM, Andrey Shinkevich wrote:
>> Added possibility to write compressed data by using the
>> blk_write_compressed. This action has the limitations of the format
>> driver. For example we can't write compressed data over other.
>>
> 
>> +++ b/blockdev-nbd.c
>> @@ -191,7 +191,7 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name,
>>       }
>>       exp = nbd_export_new(bs, 0, len, name, NULL, bitmap, !writable, !writable,
>> -                         NULL, false, on_eject_blk, errp);
>> +                         0, NULL, false, on_eject_blk, errp);
> 
> This is a lot of parameters.  Should we be combining some of them into a struct, or even at least the booleans into a flags parameter?
> 
> 
>> +++ b/include/block/nbd.h
>> @@ -25,6 +25,10 @@
>>   #include "crypto/tlscreds.h"
>>   #include "qapi/error.h"
>> +enum {
>> +  NBD_INTERNAL_FLAG_COMPRESS = 1 << 1, /* Use write compressed */
>> +};
> 
> What happened to flag 1 << 0?  What other flags do you anticipate adding?

Hmm, any way, creating flags parameter for only one flag seems useless.

I think, I'd prefer to cover all nbd_export_new parameters to a structure
with boolean fields, to avoid extra &/| arithmetic.

nbd_export_new is called from qmp_nbd_server_add and qemu-nbd main(), and
both places seems to benefit, if they will fill structure instead of local
variables.

-- 
Best regards,
Vladimir