qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 4/4] block: apply COR-filter to block-stream jobs


From: Andrey Shinkevich
Subject: Re: [PATCH v6 4/4] block: apply COR-filter to block-stream jobs
Date: Mon, 24 Aug 2020 11:49:37 +0300
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Thunderbird/68.9.0

On 24.08.2020 11:20, Vladimir Sementsov-Ogievskiy wrote:
23.08.2020 22:28, Andrey Shinkevich wrote:
On 19.08.2020 13:46, Vladimir Sementsov-Ogievskiy wrote:
19.08.2020 00:24, Andrey Shinkevich wrote:
The patch completes the series with the COR-filter insertion to any
block-stream operation. It also makes changes to the iotests 030.
The test case 'test_stream_parallel' was deleted due to multiple
errors.

...


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

diff --git a/block/stream.c b/block/stream.c
index 8bf6b6d..0b11979 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -19,6 +19,7 @@
  #include "qapi/qmp/qerror.h"
  #include "qemu/ratelimit.h"
  #include "sysemu/block-backend.h"
+#include "block/copy-on-read.h"
    enum {
      /*
@@ -33,8 +34,11 @@ typedef struct StreamBlockJob {
      BlockJob common;
      BlockDriverState *base_overlay; /* COW overlay (stream from this) */
      BlockDriverState *above_base;   /* Node directly above the base */
+    BlockDriverState *cor_filter_bs;
+    BlockDriverState *target_bs;
      BlockdevOnError on_error;
      char *backing_file_str;
+    char *base_fmt;
      bool bs_read_only;
      bool chain_frozen;
  } StreamBlockJob;
@@ -53,34 +57,26 @@ static void stream_abort(Job *job)
      StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
        if (s->chain_frozen) {
-        BlockJob *bjob = &s->common;
-        bdrv_unfreeze_backing_chain(blk_bs(bjob->blk), s->above_base);
+        bdrv_unfreeze_backing_chain(s->cor_filter_bs, s->above_base);
      }
  }
    static int stream_prepare(Job *job)
  {
      StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
-    BlockJob *bjob = &s->common;
-    BlockDriverState *bs = blk_bs(bjob->blk);
+    BlockDriverState *bs = s->target_bs;
      BlockDriverState *unfiltered_bs = bdrv_skip_filters(bs);
      BlockDriverState *base = bdrv_filter_or_cow_bs(s->above_base);
      Error *local_err = NULL;
      int ret = 0;
  -    bdrv_unfreeze_backing_chain(bs, s->above_base);
+    bdrv_unfreeze_backing_chain(s->cor_filter_bs, s->above_base);
      s->chain_frozen = false;
        if (bdrv_cow_child(unfiltered_bs)) {
-        const char *base_id = NULL, *base_fmt = NULL;
-        if (base) {
-            base_id = s->backing_file_str;
-            if (base->drv) {
-                base_fmt = base->drv->format_name;
-            }
-        }
          bdrv_set_backing_hd(unfiltered_bs, base, &local_err);
-        ret = bdrv_change_backing_file(unfiltered_bs, base_id, base_fmt);
+        ret = bdrv_change_backing_file(unfiltered_bs, s->backing_file_str,
+                                       s->base_fmt);
          if (local_err) {
              error_report_err(local_err);
              return -EPERM;
@@ -94,7 +90,9 @@ static void stream_clean(Job *job)
  {
      StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
      BlockJob *bjob = &s->common;
-    BlockDriverState *bs = blk_bs(bjob->blk);
+    BlockDriverState *bs = s->target_bs;
+
+    bdrv_cor_filter_drop(s->cor_filter_bs);
        /* Reopen the image back in read-only mode if necessary */
      if (s->bs_read_only) {
@@ -104,13 +102,14 @@ static void stream_clean(Job *job)
      }
        g_free(s->backing_file_str);
+    g_free(s->base_fmt);
  }
    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 = s->target_bs;
      BlockDriverState *unfiltered_bs = bdrv_skip_filters(bs);
      bool enable_cor = !bdrv_cow_child(s->base_overlay);
      int64_t len;
@@ -231,6 +230,12 @@ void stream_start(const char *job_id, BlockDriverState *bs,
      int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
      BlockDriverState *base_overlay = bdrv_find_overlay(bs, base);
      BlockDriverState *above_base;
+    BlockDriverState *cor_filter_bs = NULL;
+    char *base_fmt = NULL;
+
+    if (base && base->drv) {
+        base_fmt = g_strdup(base->drv->format_name);
+    }
        if (!base_overlay) {
          error_setg(errp, "'%s' is not in the backing chain of '%s'",
@@ -264,17 +269,36 @@ void stream_start(const char *job_id, BlockDriverState *bs,
          }
      }
  -    /* Prevent concurrent jobs trying to modify the graph structure here, we
-     * already have our own plans. Also don't allow resize as the image size is
-     * queried only at the job start and then cached. */
-    s = block_job_create(job_id, &stream_job_driver, NULL, bs,
-                         basic_flags | BLK_PERM_GRAPH_MOD,
-                         basic_flags | BLK_PERM_WRITE,
+    cor_filter_bs = bdrv_cor_filter_append(bs, filter_node_name, errp);
+    if (cor_filter_bs == NULL) {
+        goto fail;
+    }
+
+    if (bdrv_freeze_backing_chain(cor_filter_bs, bs, errp) < 0) {
+        bdrv_cor_filter_drop(cor_filter_bs);
+        cor_filter_bs = NULL;
+        goto fail;
+    }
+
+    s = block_job_create(job_id, &stream_job_driver, NULL, cor_filter_bs,
+                         BLK_PERM_CONSISTENT_READ,
+                         basic_flags | BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD,
                           speed, creation_flags, NULL, NULL, errp);
      if (!s) {
          goto fail;
      }
  +    /*
+     * Prevent concurrent jobs trying to modify the graph structure here, we
+     * already have our own plans. Also don't allow resize as the image size is
+     * queried only at the job start and then cached.
+     */
+    if (block_job_add_bdrv(&s->common, "active node", bs,
+                           basic_flags | BLK_PERM_GRAPH_MOD,
+                           basic_flags | BLK_PERM_WRITE, &error_abort)) {
+        goto fail;
+    }
+
      /* 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 forbid writes
@@ -294,6 +318,9 @@ void stream_start(const char *job_id, BlockDriverState *bs,
        s->base_overlay = base_overlay;
      s->above_base = above_base;
+    s->cor_filter_bs = cor_filter_bs;
+    s->target_bs = bs;
+    s->base_fmt = base_fmt;

it's wrong to keep base_fmt during the job, as base may change


So, the format can differ from that of the backing_file_str given as the input parameter of the stream_start()...

...while the backing_file_str path is written to the QCOW2 header on a disk.


Or better, let's try to revert c624b015bf14f and freeze base again.


The reversion won't help as the patch "stream: Deal with filters" did that work already. We have to freeze the base again. I guess it will discard the concept of the 'base_overlay' and the  'above_base'  introduced by Max in the series "block: Deal with filters".

Andrey






      s->backing_file_str = g_strdup(backing_file_str);
      s->bs_read_only = bs_read_only;
      s->chain_frozen = true;
@@ -307,5 +334,10 @@ fail:
      if (bs_read_only) {
          bdrv_reopen_set_read_only(bs, true, NULL);
      }
-    bdrv_unfreeze_backing_chain(bs, above_base);
+    if (cor_filter_bs) {
+        bdrv_unfreeze_backing_chain(cor_filter_bs, above_base);
+        bdrv_cor_filter_drop(cor_filter_bs);
+    } else {
+        bdrv_unfreeze_backing_chain(bs, above_base);
+    }
  }
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 1cdd7e2..fec9d89 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -221,60 +221,20 @@ class TestParallelOps(iotests.QMPTestCase):
          for img in self.imgs:
              os.remove(img)
...



reply via email to

[Prev in Thread] Current Thread [Next in Thread]