[Qemu-devel] [PATCH v5 0/6] Discrad blocks during block-stream operation

[Qemu-devel] [PATCH v5 0/6] Discrad blocks during block-stream operation

[Andrey Shinkevich]

Dear collegues,

The given feature discards blocks with copy-on-read operation while the
streaming process runs. Adding the 'discard' argument to the QMP block-stream
command allows dropping a block in the backing chain after it has been copied
to the active layer. That will elude the block duplication in the intermediate
backing file. It saves the disk space while external snapshots are being
merged.
The method involves the filter insertion above the active layer to allow write
operation in the backing chain. The method is similar to that in the 'commit
active' command (mirror.c).
The permission to write into an inactive layer can not be obtained due to the
existing child permission mechanism. 

v5:
The existing copy-on-read filter driver was applied to the block-stream
operation in the separated patch. The code refactoring in 'stream.c' is big.
Unfortunately, there are issues with getting "write" lock in iotests -
subject to further resolution.

v4:
"[PATCH v4 0/5] Discrad blocks during block-stream operation"
The targeted version of the release at 'Since' word of the comment to 'discard'
argument changed to 4.0 in the file qapi/block-core.json as Eric Blake noticed.

v3:
"[PATCH v3 0/5] Discrad blocks during block-stream operation"
The series version number and the given changelog were added. 'Signed-off-by'
subscription of the patch [PATCH 3/5] amended as Peter Krempa requested.

v2:
"[PATCH 0/5] Discrad blocks during block-stream operation"
The suggestions of Dr. David Alan Gilbert and Alberto Garcia after their first
review have been applied.
The abort() callback function have been added to the filter driver and some
refactoring work done.
The test suite for the 'block-stream' command with 'discard' option was added
to the 030 file of iotests.

v1:
"[PATCH 0/2] Discrad blocks during block-stream operation"
The initial implementation of the feature.

Sincerely,

Andrey Shinkevich (6):
  Stream block job involves copy-on-read filter driver
  Discard blocks while copy-on-read
  The discard flag for block stream operation
  iotests: allow resume_drive by node name
  iotests: prepare 030 for graph change
  iotests: 030 with block-stream discard

 block/copy-on-read.c          |  24 ++++
 block/stream.c                | 290 ++++++++++++++++++++++++++++++++++++++++--
 blockdev.c                    |   8 +-
 hmp-commands.hx               |   4 +-
 hmp.c                         |   4 +-
 include/block/block_int.h     |   2 +-
 qapi/block-core.json          |   5 +-
 tests/qemu-iotests/030        | 163 ++++++++++++++++++------
 tests/qemu-iotests/030.out    |   4 +-
 tests/qemu-iotests/iotests.py |   9 +-
 10 files changed, 448 insertions(+), 65 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v5 1/6] Stream block job involves copy-on-read filter driver

[Andrey Shinkevich]

The copy-on-read filter is applied to block-stream operation.
It is necessary for further block discard option.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/stream.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index 7a49ac0..20e768e 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -16,6 +16,7 @@
 #include "block/block_int.h"
 #include "block/blockjob_int.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/ratelimit.h"
 #include "sysemu/block-backend.h"
@@ -35,8 +36,14 @@ typedef struct StreamBlockJob {
     BlockdevOnError on_error;
     char *backing_file_str;
     bool bs_read_only;
+    BlockDriverState *cor_filter_bs;
 } StreamBlockJob;
 
+static BlockDriverState *child_file_bs(BlockDriverState *bs)
+{
+    return bs->file ? bs->file->bs : NULL;
+}
+
 static int coroutine_fn stream_populate(BlockBackend *blk,
                                         int64_t offset, uint64_t bytes,
                                         void *buf)
@@ -54,7 +61,7 @@ static int coroutine_fn stream_populate(BlockBackend *blk,
     return blk_co_preadv(blk, offset, qiov.size, &qiov, BDRV_REQ_COPY_ON_READ);
 }
 
-static int stream_prepare(Job *job)
+static int stream_change_backing_file(Job *job)
 {
     StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
     BlockJob *bjob = &s->common;
@@ -82,6 +89,43 @@ static int stream_prepare(Job *job)
     return ret;
 }
 
+static void remove_filter(BlockDriverState *cor_filter_bs)
+{
+    BlockDriverState *bs = child_file_bs(cor_filter_bs);
+
+    /* Hold a guest back from writing until we remove the filter */
+    bdrv_drained_begin(bs);
+    bdrv_child_try_set_perm(cor_filter_bs->file, 0, BLK_PERM_ALL,
+                            &error_abort);
+    bdrv_replace_node(cor_filter_bs, bs, &error_abort);
+    bdrv_drained_end(bs);
+
+    bdrv_unref(cor_filter_bs);
+}
+
+static void stream_exit(Job *job)
+{
+    StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
+    if (s->cor_filter_bs == NULL) {
+        return;
+    }
+    /* Remove the filter driver from the graph */
+    remove_filter(s->cor_filter_bs);
+    s->cor_filter_bs = NULL;
+}
+
+static int stream_prepare(Job *job)
+{
+    stream_exit(job);
+
+    return stream_change_backing_file(job);
+}
+
+static void stream_abort(Job *job)
+{
+    stream_exit(job);
+}
+
 static void stream_clean(Job *job)
 {
     StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
@@ -102,7 +146,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
 {
     StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
     BlockBackend *blk = s->common.blk;
-    BlockDriverState *bs = blk_bs(blk);
+    BlockDriverState *bs = child_file_bs(s->cor_filter_bs);
     BlockDriverState *base = s->base;
     int64_t len;
     int64_t offset = 0;
@@ -206,6 +250,42 @@ out:
     return ret;
 }
 
+static BlockDriverState *create_filter_node(BlockDriverState *bs, Error **errp)
+{
+    QDict *opts = qdict_new();
+
+    qdict_put_str(opts, "driver", "copy-on-read");
+    qdict_put_str(opts, "file", bdrv_get_node_name(bs));
+
+    return bdrv_open(NULL, NULL, opts, BDRV_O_RDWR, errp);
+}
+
+static BlockDriverState *insert_filter(BlockDriverState *bs, Error **errp)
+{
+    BlockDriverState *cor_filter_bs;
+    Error *local_err = NULL;
+
+    cor_filter_bs = create_filter_node(bs, errp);
+    if (cor_filter_bs == NULL) {
+        error_prepend(errp, "Could not create filter node: ");
+        return NULL;
+    }
+
+    bdrv_set_aio_context(cor_filter_bs, bdrv_get_aio_context(bs));
+
+    bdrv_drained_begin(bs);
+    bdrv_replace_node(bs, cor_filter_bs, &local_err);
+    bdrv_drained_end(bs);
+
+    if (local_err) {
+        bdrv_unref(cor_filter_bs);
+        error_propagate(errp, local_err);
+        return NULL;
+    }
+
+    return cor_filter_bs;
+}
+
 static const BlockJobDriver stream_job_driver = {
     .job_driver = {
         .instance_size = sizeof(StreamBlockJob),
@@ -213,6 +293,7 @@ static const BlockJobDriver stream_job_driver = {
         .free          = block_job_free,
         .run           = stream_run,
         .prepare       = stream_prepare,
+        .abort         = stream_abort,
         .clean         = stream_clean,
         .user_resume   = block_job_user_resume,
         .drain         = block_job_drain,
@@ -259,6 +340,11 @@ void stream_start(const char *job_id, BlockDriverState *bs,
                            &error_abort);
     }
 
+    s->cor_filter_bs = insert_filter(bs, errp);
+    if (s->cor_filter_bs == NULL) {
+        goto fail;
+    }
+
     s->base = base;
     s->backing_file_str = g_strdup(backing_file_str);
     s->bs_read_only = bs_read_only;
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 2/6] Discard blocks while copy-on-read

[Andrey Shinkevich]

Discards the block duplicated in an intermediate backing file
after the block have been copied into the active layer during
QMP block-stream operation.
It saves the disk space while merging external snapshots.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/stream.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 195 insertions(+), 12 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index 20e768e..af2eebf 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -12,6 +12,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "trace.h"
 #include "block/block_int.h"
 #include "block/blockjob_int.h"
@@ -37,14 +38,68 @@ typedef struct StreamBlockJob {
     char *backing_file_str;
     bool bs_read_only;
     BlockDriverState *cor_filter_bs;
+    bool discard;
+    GSList *im_nodes;
 } StreamBlockJob;
 
+typedef struct IntermediateNode {
+    BlockBackend *blk;
+    bool bs_read_only;
+} IntermediateNode;
+
 static BlockDriverState *child_file_bs(BlockDriverState *bs)
 {
     return bs->file ? bs->file->bs : NULL;
 }
 
-static int coroutine_fn stream_populate(BlockBackend *blk,
+static void restore_all_im_nodes(StreamBlockJob *s)
+{
+    GSList *l;
+    BlockDriverState *bs_active;
+    BlockDriverState *bs_im;
+    IntermediateNode *im_node;
+    QDict *opts;
+    BlockReopenQueue *queue = NULL;
+    Error *local_err = NULL;
+
+    assert(s->cor_filter_bs);
+    bs_active = child_file_bs(s->cor_filter_bs);
+    assert(bs_active && backing_bs(bs_active));
+
+    bdrv_subtree_drained_begin(backing_bs(bs_active));
+
+    for (l = s->im_nodes; l; l = l->next) {
+        im_node = l->data;
+        if (im_node->blk) {
+            bs_im = blk_bs(im_node->blk);
+
+            if (im_node->bs_read_only && bs_im && !bdrv_is_read_only(bs_im)) {
+                opts = qdict_new();
+                qdict_put_bool(opts, BDRV_OPT_READ_ONLY, true);
+                queue = bdrv_reopen_queue(queue, bs_im, opts);
+            }
+            /* Give up write permissions before making it read-only */
+            blk_set_perm(im_node->blk, 0, BLK_PERM_ALL, &error_abort);
+            blk_unref(im_node->blk);
+            bdrv_unref(bs_im);
+        }
+        g_free(im_node);
+    }
+    g_slist_free(s->im_nodes);
+    s->im_nodes = NULL;
+
+    if (queue) {
+        bdrv_reopen_multiple(bdrv_get_aio_context(bs_active), queue,
+                             &local_err);
+        if (local_err != NULL) {
+            error_report_err(local_err);
+        }
+    }
+
+    bdrv_subtree_drained_end(backing_bs(bs_active));
+}
+
+static int coroutine_fn stream_populate(const StreamBlockJob *s,
                                         int64_t offset, uint64_t bytes,
                                         void *buf)
 {
@@ -53,12 +108,28 @@ static int coroutine_fn stream_populate(BlockBackend *blk,
         .iov_len  = bytes,
     };
     QEMUIOVector qiov;
+    GSList *l;
+    IntermediateNode *im_node;
+    int ret;
 
+    assert(s);
     assert(bytes < SIZE_MAX);
     qemu_iovec_init_external(&qiov, &iov, 1);
 
     /* Copy-on-read the unallocated clusters */
-    return blk_co_preadv(blk, offset, qiov.size, &qiov, BDRV_REQ_COPY_ON_READ);
+    ret = blk_co_preadv(s->common.blk, offset, qiov.size, &qiov,
+                        BDRV_REQ_COPY_ON_READ);
+
+    if (ret < 0 || !s->discard) {
+        return ret;
+    }
+
+    for (l = s->im_nodes; l; l = l->next) {
+        im_node = l->data;
+        blk_co_pdiscard(im_node->blk, offset, bytes);
+    }
+
+    return ret;
 }
 
 static int stream_change_backing_file(Job *job)
@@ -109,6 +180,8 @@ static void stream_exit(Job *job)
     if (s->cor_filter_bs == NULL) {
         return;
     }
+    /* Reopen intermediate images back in read-only mode */
+    restore_all_im_nodes(s);
     /* Remove the filter driver from the graph */
     remove_filter(s->cor_filter_bs);
     s->cor_filter_bs = NULL;
@@ -145,7 +218,6 @@ static void stream_clean(Job *job)
 static int coroutine_fn stream_run(Job *job, Error **errp)
 {
     StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
-    BlockBackend *blk = s->common.blk;
     BlockDriverState *bs = child_file_bs(s->cor_filter_bs);
     BlockDriverState *base = s->base;
     int64_t len;
@@ -209,7 +281,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, buf);
+            ret = stream_populate(s, offset, n, buf);
         }
         if (ret < 0) {
             BlockErrorAction action =
@@ -250,22 +322,27 @@ out:
     return ret;
 }
 
-static BlockDriverState *create_filter_node(BlockDriverState *bs, Error **errp)
+static BlockDriverState *create_filter_node(BlockDriverState *bs, bool discard,
+                                            Error **errp)
 {
     QDict *opts = qdict_new();
 
     qdict_put_str(opts, "driver", "copy-on-read");
     qdict_put_str(opts, "file", bdrv_get_node_name(bs));
+    if (discard) {
+        qdict_put_bool(opts, "driver.discard", true);
+    }
 
     return bdrv_open(NULL, NULL, opts, BDRV_O_RDWR, errp);
 }
 
-static BlockDriverState *insert_filter(BlockDriverState *bs, Error **errp)
+static BlockDriverState *insert_filter(BlockDriverState *bs, bool discard,
+                                       Error **errp)
 {
     BlockDriverState *cor_filter_bs;
     Error *local_err = NULL;
 
-    cor_filter_bs = create_filter_node(bs, errp);
+    cor_filter_bs = create_filter_node(bs, discard, errp);
     if (cor_filter_bs == NULL) {
         error_prepend(errp, "Could not create filter node: ");
         return NULL;
@@ -286,6 +363,92 @@ static BlockDriverState *insert_filter(BlockDriverState *bs, Error **errp)
     return cor_filter_bs;
 }
 
+/* Makes intermediate block chain writable */
+static int init_intermediate_nodes(StreamBlockJob *s,
+                                   BlockDriverState *bs,
+                                   BlockDriverState *base, Error **errp)
+{
+    BlockDriverState *iter;
+    bool bs_read_only;
+    IntermediateNode *im_node;
+    BlockBackend *blk;
+    QDict *opts;
+    BlockReopenQueue *queue = NULL;
+    Error *local_err = NULL;
+    int ret;
+
+    /* Sanity check */
+    if (!backing_bs(bs)) {
+        error_setg(errp, "Top BDS does not have a backing file.");
+        return -EINVAL;
+    }
+    if (base && !bdrv_chain_contains(bs, base)) {
+        error_setg(errp, "The backing chain does not contain the base file.");
+        return -EINVAL;
+    }
+
+    /* Reopen intermediate images in read-write mode */
+    bdrv_subtree_drained_begin(backing_bs(bs));
+
+    for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) {
+        bs_read_only = bdrv_is_read_only(iter);
+        im_node = g_new0(IntermediateNode, 1);
+        im_node->blk = NULL;
+        im_node->bs_read_only = bs_read_only;
+        bdrv_ref(iter);
+        s->im_nodes = g_slist_prepend(s->im_nodes, im_node);
+
+        if (bs_read_only) {
+            opts = qdict_new();
+            qdict_put_bool(opts, BDRV_OPT_READ_ONLY, false);
+            queue = bdrv_reopen_queue(queue, iter, opts);
+        }
+    }
+
+    if (queue) {
+        ret = bdrv_reopen_multiple(bdrv_get_aio_context(bs), queue, &local_err);
+        if (local_err != NULL) {
+            error_propagate(errp, local_err);
+            bdrv_subtree_drained_end(backing_bs(bs));
+            restore_all_im_nodes(s);
+            return -1;
+        }
+    }
+
+    bdrv_subtree_drained_end(backing_bs(bs));
+
+    s->im_nodes = g_slist_reverse(s->im_nodes);
+    GSList *l = s->im_nodes;
+
+    for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) {
+        blk = blk_new(BLK_PERM_WRITE, BLK_PERM_CONSISTENT_READ |
+                      BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED |
+                      BLK_PERM_GRAPH_MOD);
+        if (!blk) {
+            error_setg(errp,
+                       "Block Stream: failed to create new Block Backend.");
+            goto fail;
+        }
+
+        ret = blk_insert_bs(blk, iter, errp);
+        if (ret < 0) {
+            goto fail;
+        }
+
+        assert(l);
+        im_node = l->data;
+        im_node->blk = blk;
+        l = l->next;
+    }
+
+    return 0;
+
+fail:
+    restore_all_im_nodes(s);
+
+    return -1;
+}
+
 static const BlockJobDriver stream_job_driver = {
     .job_driver = {
         .instance_size = sizeof(StreamBlockJob),
@@ -308,6 +471,9 @@ void stream_start(const char *job_id, BlockDriverState *bs,
     StreamBlockJob *s;
     BlockDriverState *iter;
     bool bs_read_only;
+    const bool discard = false;
+    int node_shared_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
+    int ret;
 
     /* Make sure that the image is opened in read-write mode */
     bs_read_only = bdrv_is_read_only(bs);
@@ -330,21 +496,34 @@ void stream_start(const char *job_id, BlockDriverState *bs,
         goto fail;
     }
 
-    /* Block all intermediate nodes between bs and base, because they will
+    /*
+     * Block all intermediate nodes between bs and base, because they will
      * disappear from the chain after this operation. The streaming job reads
-     * every block only once, assuming that it doesn't change, so block writes
-     * and resizes. */
+     * every block only once, assuming that it doesn't change, so forbid writes
+     * and resizes. Allow writing in case of discard.
+     */
+    if (discard) {
+        node_shared_flags |= BLK_PERM_WRITE;
+    }
     for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) {
         block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
-                           BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED,
+                           node_shared_flags,
                            &error_abort);
     }
 
-    s->cor_filter_bs = insert_filter(bs, errp);
+    s->cor_filter_bs = insert_filter(bs, discard, errp);
     if (s->cor_filter_bs == NULL) {
         goto fail;
     }
 
+    if (discard) {
+        ret = init_intermediate_nodes(s, bs, base, errp);
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+
+    s->discard = discard;
     s->base = base;
     s->backing_file_str = g_strdup(backing_file_str);
     s->bs_read_only = bs_read_only;
@@ -355,6 +534,10 @@ void stream_start(const char *job_id, BlockDriverState *bs,
     return;
 
 fail:
+    if (s && s->cor_filter_bs) {
+        remove_filter(s->cor_filter_bs);
+        job_early_fail(&s->common.job);
+    }
     if (bs_read_only) {
         bdrv_reopen_set_read_only(bs, true, NULL);
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 3/6] The discard flag for block stream operation

[Andrey Shinkevich]

Adding a parameter to QMP block-stream command to allow discarding
blocks in the backing chain while blocks are being copied to the
active layer.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/copy-on-read.c      | 24 ++++++++++++++++++++++++
 block/stream.c            |  3 +--
 blockdev.c                |  8 +++++++-
 hmp-commands.hx           |  4 ++--
 hmp.c                     |  4 +++-
 include/block/block_int.h |  2 +-
 qapi/block-core.json      |  5 ++++-
 7 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 64dcc42..bf7580e 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -22,11 +22,22 @@
 
 #include "qemu/osdep.h"
 #include "block/block_int.h"
+#include "qapi/qmp/qdict.h"
+
+typedef struct BDRVCORState {
+    bool discard;
+} BDRVCORState;
 
 
 static int cor_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
+    if (qdict_haskey(options, "driver.discard")) {
+        BDRVCORState *s = bs->opaque;
+        s->discard = qdict_get_bool(options, "driver.discard");
+        qdict_del(options, "driver.discard");
+    }
+
     bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false,
                                errp);
     if (!bs->file) {
@@ -66,6 +77,11 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
              (c->perm & PERM_UNCHANGED);
     *nshared = (shared & PERM_PASSTHROUGH) |
                (c->shared_perm & PERM_UNCHANGED);
+
+    BDRVCORState *s = bs->opaque;
+    if (s->discard) {
+        *nshared |= BLK_PERM_WRITE;
+    }
 }
 
 
@@ -134,8 +150,15 @@ static bool cor_recurse_is_first_non_filter(BlockDriverState *bs,
 }
 
 
+static int cor_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+    return bdrv_get_info(bs->file->bs, bdi);
+}
+
+
 BlockDriver bdrv_copy_on_read = {
     .format_name                        = "copy-on-read",
+    .instance_size                      = sizeof(BDRVCORState),
 
     .bdrv_open                          = cor_open,
     .bdrv_child_perm                    = cor_child_perm,
@@ -148,6 +171,7 @@ BlockDriver bdrv_copy_on_read = {
     .bdrv_co_pwrite_zeroes              = cor_co_pwrite_zeroes,
     .bdrv_co_pdiscard                   = cor_co_pdiscard,
 
+    .bdrv_get_info                      = cor_get_info,
     .bdrv_eject                         = cor_eject,
     .bdrv_lock_medium                   = cor_lock_medium,
 
diff --git a/block/stream.c b/block/stream.c
index af2eebf..b160eee 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -465,13 +465,12 @@ 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 discard,
                   BlockdevOnError on_error, Error **errp)
 {
     StreamBlockJob *s;
     BlockDriverState *iter;
     bool bs_read_only;
-    const bool discard = false;
     int node_shared_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
     int ret;
 
diff --git a/blockdev.c b/blockdev.c
index a6f71f9..6a53cb7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3138,6 +3138,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_discard, bool discard,
                       bool has_on_error, BlockdevOnError on_error,
                       bool has_auto_finalize, bool auto_finalize,
                       bool has_auto_dismiss, bool auto_dismiss,
@@ -3154,6 +3155,10 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
         on_error = BLOCKDEV_ON_ERROR_REPORT;
     }
 
+    if (!has_discard) {
+        discard = false;
+    }
+
     bs = bdrv_lookup_bs(device, device, errp);
     if (!bs) {
         return;
@@ -3218,7 +3223,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,
+                 discard, 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 ba71558..12806db 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?,discard:-d",
+        .params     = "device [speed [base]] [-d]",
         .help       = "copy data from a backing file into a block device",
         .cmd        = hmp_block_stream,
     },
diff --git a/hmp.c b/hmp.c
index 80aa5ab..353a164 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1923,9 +1923,11 @@ 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 discard = qdict_get_try_bool(qdict, "discard", false);
 
     qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
-                     false, NULL, qdict_haskey(qdict, "speed"), speed, true,
+                     false, NULL, qdict_haskey(qdict, "speed"), speed,
+                     true, discard, true,
                      BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
                      &error);
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f605622..2660336 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -970,7 +970,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 discard,
                   BlockdevOnError on_error, Error **errp);
 
 /**
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 762000f..5005c57 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2337,6 +2337,9 @@
 #
 # @speed:  the maximum speed, in bytes per second
 #
+# @discard: true to delete blocks duplicated in old backing files.
+#           (default: false). Since 4.0.
+#
 # @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.
@@ -2369,7 +2372,7 @@
 { 'command': 'block-stream',
   'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
             '*base-node': 'str', '*backing-file': 'str', '*speed': 'int',
-            '*on-error': 'BlockdevOnError',
+            '*discard': 'bool', '*on-error': 'BlockdevOnError',
             '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
 
 ##
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 4/6] iotests: allow resume_drive by node name

[Andrey Shinkevich]

After node graph changes, we may not be able to resume_drive by device
name (backing files are not recursively searched). So, lets allow to
resume by node-name. Set constant name for breakpoints, to avoid
introducing extra parameters.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index d537538..3fe046c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -407,11 +407,11 @@ class VM(qtest.QEMUQtestMachine):
             self.pause_drive(drive, "write_aio")
             return
         self.qmp('human-monitor-command',
-                    command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))
+                    command_line='qemu-io %s "break %s bp_0"' % (drive, event))
 
     def resume_drive(self, drive):
         self.qmp('human-monitor-command',
-                    command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))
+                    command_line='qemu-io %s "remove_break bp_0"' % (drive))
 
     def hmp_qemu_io(self, drive, cmd):
         '''Write to a given drive using an HMP command'''
@@ -535,13 +535,14 @@ class QMPTestCase(unittest.TestCase):
         self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),
                          self.vm.flatten_qmp_object(reference))
 
-    def cancel_and_wait(self, drive='drive0', force=False, resume=False):
+    def cancel_and_wait(self, drive='drive0', force=False,
+                        resume=False,resume_node=None):
         '''Cancel a block job and wait for it to finish, returning the event'''
         result = self.vm.qmp('block-job-cancel', device=drive, force=force)
         self.assert_qmp(result, 'return', {})
 
         if resume:
-            self.vm.resume_drive(drive)
+            self.vm.resume_drive(resume_node or drive)
 
         cancelled = False
         result = None
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 5/6] iotests: prepare 030 for graph change

[Andrey Shinkevich]

The discard option for block-stream command requires insertion of the
filter to write into the backing chain. In that case, the job will not
resume by device name. So, the node name is specified.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/030 | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 276e06b..5d148b0 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -36,7 +36,8 @@ class TestSingleDrive(iotests.QMPTestCase):
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
         qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 512', backing_img)
         qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 524288 512', mid_img)
-        self.vm = iotests.VM().add_drive("blkdebug::" + test_img, "backing.node-name=mid")
+        self.vm = iotests.VM().add_drive("blkdebug::" + test_img,
+                               "node-name=source,backing.node-name=mid")
         self.vm.launch()
 
     def tearDown(self):
@@ -87,7 +88,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         self.pause_job('drive0', wait=False)
-        self.vm.resume_drive('drive0')
+        self.vm.resume_drive('source')
         self.pause_wait('drive0')
 
         result = self.vm.qmp('query-block-jobs')
@@ -743,7 +744,8 @@ class TestStreamStop(iotests.QMPTestCase):
         qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 32M', backing_img)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
         qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 32M 32M', test_img)
-        self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
+        self.vm = iotests.VM().add_drive("blkdebug::" + test_img,
+                                         "node-name=source")
         self.vm.launch()
 
     def tearDown(self):
@@ -764,7 +766,7 @@ class TestStreamStop(iotests.QMPTestCase):
             self.assert_qmp(e, 'event', 'JOB_STATUS_CHANGE')
             self.assert_qmp(e, 'data/id', 'drive0')
 
-        self.cancel_and_wait(resume=True)
+        self.cancel_and_wait(resume=True, resume_node='source')
 
 class TestSetSpeed(iotests.QMPTestCase):
     image_len = 80 * 1024 * 1024 # MB
@@ -774,7 +776,8 @@ class TestSetSpeed(iotests.QMPTestCase):
         qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 32M', backing_img)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
         qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 32M 32M', test_img)
-        self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
+        self.vm = iotests.VM().add_drive('blkdebug::' + test_img,
+                                         "node-name=source")
         self.vm.launch()
 
     def tearDown(self):
@@ -817,7 +820,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 8 * 1024 * 1024)
 
-        self.cancel_and_wait(resume=True)
+        self.cancel_and_wait(resume=True, resume_node='source')
         self.vm.pause_drive('drive0')
 
         # Check setting speed in block-stream works
@@ -828,7 +831,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_qmp(result, 'return[0]/device', 'drive0')
         self.assert_qmp(result, 'return[0]/speed', 4 * 1024 * 1024)
 
-        self.cancel_and_wait(resume=True)
+        self.cancel_and_wait(resume=True, resume_node='source')
 
     def test_set_speed_invalid(self):
         self.assert_no_active_block_jobs()
@@ -845,7 +848,8 @@ class TestSetSpeed(iotests.QMPTestCase):
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        self.cancel_and_wait(resume=True)
+        self.cancel_and_wait(resume=True, resume_node='source')
+
 
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'])
-- 
1.8.3.1

[Qemu-devel] [PATCH v5 6/6] iotests: 030 with block-stream discard

[Andrey Shinkevich]

The classes that set tests for the block-stream command with discard
option on are inherited from the existent classes in the 030 file.
Some QMP commands do not have the optional 'discard' argument because
the WRITE permission is not being granted when the filter is inserted.
For instance, it is true while streaming into an inactive layer.

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

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 5d148b0..eba2fff 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -29,6 +29,7 @@ test_img = os.path.join(iotests.test_dir, 'test.img')
 
 class TestSingleDrive(iotests.QMPTestCase):
     image_len = 1 * 1024 * 1024 # MB
+    do_discard = False
 
     def setUp(self):
         iotests.create_image(backing_img, TestSingleDrive.image_len)
@@ -49,7 +50,8 @@ class TestSingleDrive(iotests.QMPTestCase):
     def test_stream(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         self.wait_until_completed()
@@ -84,7 +86,8 @@ class TestSingleDrive(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         self.vm.pause_drive('drive0')
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         self.pause_job('drive0', wait=False)
@@ -117,7 +120,8 @@ class TestSingleDrive(iotests.QMPTestCase):
         empty_map = qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', test_img)
 
         # This is a no-op: no data should ever be copied from the base image
-        result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', base=mid_img)
         self.assert_qmp(result, 'return', {})
 
         self.wait_until_completed()
@@ -131,7 +135,8 @@ class TestSingleDrive(iotests.QMPTestCase):
     def test_stream_partial(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0', base=backing_img)
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', base=backing_img)
         self.assert_qmp(result, 'return', {})
 
         self.wait_until_completed()
@@ -144,11 +149,13 @@ class TestSingleDrive(iotests.QMPTestCase):
                          'image file map does not match backing file after streaming')
 
     def test_device_not_found(self):
-        result = self.vm.qmp('block-stream', device='nonexistent')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='nonexistent')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_job_id_missing(self):
-        result = self.vm.qmp('block-stream', device='mid')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='mid')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
 
@@ -157,6 +164,7 @@ class TestParallelOps(iotests.QMPTestCase):
     num_imgs = num_ops * 2 + 1
     image_len = num_ops * 512 * 1024
     imgs = []
+    do_discard = False
 
     def setUp(self):
         opts = []
@@ -241,13 +249,16 @@ class TestParallelOps(iotests.QMPTestCase):
         result = self.vm.qmp('block-stream', device='node4', job_id='stream-node4', base=self.imgs[1], speed=1024*1024)
         self.assert_qmp(result, 'return', {})
 
-        result = self.vm.qmp('block-stream', device='node5', job_id='stream-node5', base=self.imgs[2])
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node5', job_id='stream-node5', base=self.imgs[2])
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3', base=self.imgs[2])
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node3', job_id='stream-node3', base=self.imgs[2])
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        result = self.vm.qmp('block-stream', device='node4', job_id='stream-node4-v2')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node4', job_id='stream-node4-v2')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         # block-commit should also fail if it touches nodes used by the stream job
@@ -274,20 +285,29 @@ class TestParallelOps(iotests.QMPTestCase):
         result = self.vm.qmp('block-commit', device='drive0', top=self.imgs[5], base=self.imgs[3], job_id='commit-node3', speed=1024*1024)
         self.assert_qmp(result, 'return', {})
 
-        result = self.vm.qmp('block-stream', device='node3', job_id='stream-node3')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node3', job_id='stream-node3')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        result = self.vm.qmp('block-stream', device='node6', base=self.imgs[2], job_id='stream-node6')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node6', base=self.imgs[2],
+                             job_id='stream-node6')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], job_id='stream-node4')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node4', base=self.imgs[2],
+                             job_id='stream-node4')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-        result = self.vm.qmp('block-stream', device='node6', base=self.imgs[4], job_id='stream-node6-v2')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node6', base=self.imgs[4],
+                             job_id='stream-node6-v2')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         # This fails because block-commit currently blocks the active layer even if it's not used
-        result = self.vm.qmp('block-stream', device='drive0', base=self.imgs[5], job_id='stream-drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', base=self.imgs[5],
+                             job_id='stream-drive0')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         self.wait_until_completed(drive='commit-node3')
@@ -302,7 +322,9 @@ class TestParallelOps(iotests.QMPTestCase):
         result = self.vm.qmp('block-commit', device='drive0', base=self.imgs[3], job_id='commit-drive0', speed=1024*1024)
         self.assert_qmp(result, 'return', {})
 
-        result = self.vm.qmp('block-stream', device='node5', base=self.imgs[3], job_id='stream-node6')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node5', base=self.imgs[3],
+                             job_id='stream-node6')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         event = self.vm.event_wait(name='BLOCK_JOB_READY')
@@ -389,19 +411,24 @@ class TestParallelOps(iotests.QMPTestCase):
                             'image file map matches backing file before streaming')
 
         # Error: the base node does not exist
-        result = self.vm.qmp('block-stream', device='node4', base_node='none', job_id='stream')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node4', base_node='none', job_id='stream')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         # Error: the base node is not a backing file of the top node
-        result = self.vm.qmp('block-stream', device='node4', base_node='node6', job_id='stream')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node4',base_node='node6', job_id='stream')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         # Error: the base node is the same as the top node
-        result = self.vm.qmp('block-stream', device='node4', base_node='node4', job_id='stream')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node4',base_node='node4', job_id='stream')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         # Error: cannot specify 'base' and 'base-node' at the same time
-        result = self.vm.qmp('block-stream', device='node4', base=self.imgs[2], base_node='node2', job_id='stream')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node4', base=self.imgs[2],
+                             base_node='node2', job_id='stream')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         # Success: the base node is a backing file of the top node
@@ -421,6 +448,7 @@ class TestQuorum(iotests.QMPTestCase):
     num_children = 3
     children = []
     backing = []
+    do_discard = False
 
     def setUp(self):
         opts = ['driver=quorum', 'vote-threshold=2']
@@ -446,10 +474,12 @@ class TestQuorum(iotests.QMPTestCase):
 
     def tearDown(self):
         self.vm.shutdown()
-        for img in self.children:
+        for img in self.children[:]:
             os.remove(img)
-        for img in self.backing:
+            self.children.remove(img)
+        for img in self.backing[:]:
             os.remove(img)
+            self.backing.remove(img)
 
     def test_stream_quorum(self):
         if not iotests.supports_quorum():
@@ -461,7 +491,8 @@ class TestQuorum(iotests.QMPTestCase):
 
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='node0', job_id='stream-node0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='node0', job_id='stream-node0')
         self.assert_qmp(result, 'return', {})
 
         self.wait_until_completed(drive='stream-node0')
@@ -476,6 +507,7 @@ class TestQuorum(iotests.QMPTestCase):
 class TestSmallerBackingFile(iotests.QMPTestCase):
     backing_len = 1 * 1024 * 1024 # MB
     image_len = 2 * backing_len
+    do_discard = False
 
     def setUp(self):
         iotests.create_image(backing_img, self.backing_len)
@@ -488,7 +520,8 @@ class TestSmallerBackingFile(iotests.QMPTestCase):
     def test_stream(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         self.wait_until_completed()
@@ -526,6 +559,8 @@ new_state = "1"
         file.close()
 
 class TestEIO(TestErrors):
+    do_discard = False
+
     def setUp(self):
         self.blkdebug_file = backing_img + ".blkdebug"
         iotests.create_image(backing_img, TestErrors.image_len)
@@ -546,7 +581,8 @@ class TestEIO(TestErrors):
     def test_report(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         completed = False
@@ -574,7 +610,8 @@ class TestEIO(TestErrors):
     def test_ignore(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0', on_error='ignore')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', on_error='ignore')
         self.assert_qmp(result, 'return', {})
 
         error = False
@@ -607,7 +644,8 @@ class TestEIO(TestErrors):
     def test_stop(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0', on_error='stop')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', on_error='stop')
         self.assert_qmp(result, 'return', {})
 
         error = False
@@ -650,7 +688,8 @@ class TestEIO(TestErrors):
     def test_enospc(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0', on_error='enospc')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', on_error='enospc')
         self.assert_qmp(result, 'return', {})
 
         completed = False
@@ -676,6 +715,8 @@ class TestEIO(TestErrors):
         self.vm.shutdown()
 
 class TestENOSPC(TestErrors):
+    do_discard = False
+
     def setUp(self):
         self.blkdebug_file = backing_img + ".blkdebug"
         iotests.create_image(backing_img, TestErrors.image_len)
@@ -696,7 +737,8 @@ class TestENOSPC(TestErrors):
     def test_enospc(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0', on_error='enospc')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', on_error='enospc')
         self.assert_qmp(result, 'return', {})
 
         error = False
@@ -738,6 +780,7 @@ class TestENOSPC(TestErrors):
 
 class TestStreamStop(iotests.QMPTestCase):
     image_len = 8 * 1024 * 1024 * 1024 # GB
+    do_discard = False
 
     def setUp(self):
         qemu_img('create', backing_img, str(TestStreamStop.image_len))
@@ -757,7 +800,8 @@ class TestStreamStop(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         self.vm.pause_drive('drive0')
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         time.sleep(0.1)
@@ -770,6 +814,7 @@ class TestStreamStop(iotests.QMPTestCase):
 
 class TestSetSpeed(iotests.QMPTestCase):
     image_len = 80 * 1024 * 1024 # MB
+    do_discard = False
 
     def setUp(self):
         qemu_img('create', backing_img, str(TestSetSpeed.image_len))
@@ -790,7 +835,8 @@ class TestSetSpeed(iotests.QMPTestCase):
     def perf_test_throughput(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=8 * 1024 * 1024)
@@ -804,7 +850,8 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         self.vm.pause_drive('drive0')
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         # Default speed is 0
@@ -824,7 +871,8 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.vm.pause_drive('drive0')
 
         # Check setting speed in block-stream works
-        result = self.vm.qmp('block-stream', device='drive0', speed=4 * 1024 * 1024)
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', speed=4 * 1024 * 1024)
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('query-block-jobs')
@@ -836,13 +884,15 @@ class TestSetSpeed(iotests.QMPTestCase):
     def test_set_speed_invalid(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0', speed=-1)
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0', speed=-1)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         self.assert_no_active_block_jobs()
 
         self.vm.pause_drive('drive0')
-        result = self.vm.qmp('block-stream', device='drive0')
+        result = self.vm.qmp('block-stream', discard=self.do_discard,
+                             device='drive0')
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
@@ -851,5 +901,30 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.cancel_and_wait(resume=True, resume_node='source')
 
 
+class TestSingleDriveDiscard(TestSingleDrive):
+    do_discard = True
+
+class TestParallelOpsDiscard(TestParallelOps):
+    do_discard = True
+
+class TestQuorumDiscard(TestQuorum):
+    do_discard = True
+
+class TestSmallerBackingFileDiscard(TestSmallerBackingFile):
+    do_discard = True
+
+class TestEIODiscard(TestEIO):
+    do_discard = True
+
+class TestENOSPCDiscard(TestENOSPC):
+    do_discard = True
+
+class TestStreamStopDiscard(TestStreamStop):
+    do_discard = True
+
+class TestSetSpeedDiscard(TestSetSpeed):
+    do_discard = True
+
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index 42314e9..dea5c5a 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -1,5 +1,5 @@
-........................
+................................................
 ----------------------------------------------------------------------
-Ran 24 tests
+Ran 48 tests
 
 OK
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v5 3/6] The discard flag for block stream operation

[Dr. David Alan Gilbert]

* Andrey Shinkevich (andrey.shinkevich@virtuozzo.com) wrote:
> Adding a parameter to QMP block-stream command to allow discarding
> blocks in the backing chain while blocks are being copied to the
> active layer.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  block/copy-on-read.c      | 24 ++++++++++++++++++++++++
>  block/stream.c            |  3 +--
>  blockdev.c                |  8 +++++++-
>  hmp-commands.hx           |  4 ++--
>  hmp.c                     |  4 +++-
>  include/block/block_int.h |  2 +-
>  qapi/block-core.json      |  5 ++++-
>  7 files changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 64dcc42..bf7580e 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -22,11 +22,22 @@
>  
>  #include "qemu/osdep.h"
>  #include "block/block_int.h"
> +#include "qapi/qmp/qdict.h"
> +
> +typedef struct BDRVCORState {
> +    bool discard;
> +} BDRVCORState;
>  
>  
>  static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>                      Error **errp)
>  {
> +    if (qdict_haskey(options, "driver.discard")) {
> +        BDRVCORState *s = bs->opaque;
> +        s->discard = qdict_get_bool(options, "driver.discard");
> +        qdict_del(options, "driver.discard");
> +    }
> +
>      bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false,
>                                 errp);
>      if (!bs->file) {
> @@ -66,6 +77,11 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
>               (c->perm & PERM_UNCHANGED);
>      *nshared = (shared & PERM_PASSTHROUGH) |
>                 (c->shared_perm & PERM_UNCHANGED);
> +
> +    BDRVCORState *s = bs->opaque;
> +    if (s->discard) {
> +        *nshared |= BLK_PERM_WRITE;
> +    }
>  }
>  
>  
> @@ -134,8 +150,15 @@ static bool cor_recurse_is_first_non_filter(BlockDriverState *bs,
>  }
>  
>  
> +static int cor_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
> +{
> +    return bdrv_get_info(bs->file->bs, bdi);
> +}
> +
> +
>  BlockDriver bdrv_copy_on_read = {
>      .format_name                        = "copy-on-read",
> +    .instance_size                      = sizeof(BDRVCORState),
>  
>      .bdrv_open                          = cor_open,
>      .bdrv_child_perm                    = cor_child_perm,
> @@ -148,6 +171,7 @@ BlockDriver bdrv_copy_on_read = {
>      .bdrv_co_pwrite_zeroes              = cor_co_pwrite_zeroes,
>      .bdrv_co_pdiscard                   = cor_co_pdiscard,
>  
> +    .bdrv_get_info                      = cor_get_info,
>      .bdrv_eject                         = cor_eject,
>      .bdrv_lock_medium                   = cor_lock_medium,
>  
> diff --git a/block/stream.c b/block/stream.c
> index af2eebf..b160eee 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -465,13 +465,12 @@ 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 discard,
>                    BlockdevOnError on_error, Error **errp)
>  {
>      StreamBlockJob *s;
>      BlockDriverState *iter;
>      bool bs_read_only;
> -    const bool discard = false;
>      int node_shared_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
>      int ret;
>  
> diff --git a/blockdev.c b/blockdev.c
> index a6f71f9..6a53cb7 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3138,6 +3138,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_discard, bool discard,
>                        bool has_on_error, BlockdevOnError on_error,
>                        bool has_auto_finalize, bool auto_finalize,
>                        bool has_auto_dismiss, bool auto_dismiss,
> @@ -3154,6 +3155,10 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
>          on_error = BLOCKDEV_ON_ERROR_REPORT;
>      }
>  
> +    if (!has_discard) {
> +        discard = false;
> +    }
> +
>      bs = bdrv_lookup_bs(device, device, errp);
>      if (!bs) {
>          return;
> @@ -3218,7 +3223,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,
> +                 discard, 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 ba71558..12806db 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?,discard:-d",
> +        .params     = "device [speed [base]] [-d]",
>          .help       = "copy data from a backing file into a block device",
>          .cmd        = hmp_block_stream,
>      },
> diff --git a/hmp.c b/hmp.c
> index 80aa5ab..353a164 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1923,9 +1923,11 @@ 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 discard = qdict_get_try_bool(qdict, "discard", false);
>  
>      qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
> -                     false, NULL, qdict_haskey(qdict, "speed"), speed, true,
> +                     false, NULL, qdict_haskey(qdict, "speed"), speed,
> +                     true, discard, true,
>                       BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
>                       &error);

Yes, I think that's fine from the HMP side (that line is 'true' for
has_discard, discard, true for has_on_error which is the true that you
moved from the end of the previous line).


Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


>  
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index f605622..2660336 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -970,7 +970,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 discard,
>                    BlockdevOnError on_error, Error **errp);
>  
>  /**
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 762000f..5005c57 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2337,6 +2337,9 @@
>  #
>  # @speed:  the maximum speed, in bytes per second
>  #
> +# @discard: true to delete blocks duplicated in old backing files.
> +#           (default: false). Since 4.0.
> +#
>  # @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.
> @@ -2369,7 +2372,7 @@
>  { 'command': 'block-stream',
>    'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
>              '*base-node': 'str', '*backing-file': 'str', '*speed': 'int',
> -            '*on-error': 'BlockdevOnError',
> +            '*discard': 'bool', '*on-error': 'BlockdevOnError',
>              '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
>  
>  ##
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v5 1/6] Stream block job involves copy-on-read filter driver

[Vladimir Sementsov-Ogievskiy]

30.12.2018 23:09, Andrey Shinkevich wrote:
> The copy-on-read filter is applied to block-stream operation.
> It is necessary for further block discard option.

If we move to c-o-r filter in stream, I think we should:
1. get rid of COPY_ON_READ flag in stream code (so, this thing to be done in one place)
2. add base parameter to c-o-r filter, to not copy things that are below base.

Current behavior of stream is enabling copy-on-read on bs during stream if base == NULL.
So, if we just add c-o-r filter, we will copy extra blocks. On the other hand, if we
support base parameter in c-o-r filter we will involve guest reads to the stream
process for cases with base != NULL, so stream operation should become more efficient.

So for me the following sequence seems reasonable:

- prepare iotests for graph changes
- add base parameter to c-o-r filter
- use c-o-r in stream and drop COPY_ON_READ flag from stream code

then (may be in separate series) add discard functionality. And looks like again, it should
be property of c-o-r filter, not stream job itself, to involve guest reads to discarding too.

In general, Max, is that correspond to what you mean?

Any suggestions?

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v5 1/6] Stream block job involves copy-on-read filter driver

[Max Reitz]

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

On 08.01.19 14:45, Vladimir Sementsov-Ogievskiy wrote:
> 30.12.2018 23:09, Andrey Shinkevich wrote:
>> The copy-on-read filter is applied to block-stream operation.
>> It is necessary for further block discard option.
> 
> If we move to c-o-r filter in stream, I think we should:
> 1. get rid of COPY_ON_READ flag in stream code (so, this thing to be done in one place)
> 2. add base parameter to c-o-r filter, to not copy things that are below base.
> 
> Current behavior of stream is enabling copy-on-read on bs during stream if base == NULL.
> So, if we just add c-o-r filter, we will copy extra blocks. On the other hand, if we
> support base parameter in c-o-r filter we will involve guest reads to the stream
> process for cases with base != NULL, so stream operation should become more efficient.
> 
> So for me the following sequence seems reasonable:
> 
> - prepare iotests for graph changes
> - add base parameter to c-o-r filter
> - use c-o-r in stream and drop COPY_ON_READ flag from stream code
> 
> then (may be in separate series) add discard functionality. And looks like again, it should
> be property of c-o-r filter, not stream job itself, to involve guest reads to discarding too.
> 
> In general, Max, is that correspond to what you mean?

Yep, that sounds good to me.

Max


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

Re: [Qemu-devel] [Qemu-block] [PATCH v5 1/6] Stream block job involves copy-on-read filter driver

[Alberto Garcia]

On Sun 30 Dec 2018 09:09:12 PM CET, Andrey Shinkevich wrote:
> +static BlockDriverState *child_file_bs(BlockDriverState *bs)
> +{
> +    return bs->file ? bs->file->bs : NULL;
> +}

In addition to Vladimir's comments, I wonder if it's useful to put this
in block_int.h (together with backing_bs()). It seems that there's a
couple of more places where this can be used.

Berto