qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v15 13/13] block: apply COR-filter to block-stream jobs


From: Max Reitz
Subject: Re: [PATCH v15 13/13] block: apply COR-filter to block-stream jobs
Date: Tue, 5 Jan 2021 13:52:22 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0

On 22.12.20 19:07, Vladimir Sementsov-Ogievskiy wrote:
22.12.2020 19:20, Max Reitz wrote:
On 16.12.20 07:17, Vladimir Sementsov-Ogievskiy wrote:
From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

This patch completes the series with the COR-filter applied to
block-stream operations.

Adding the filter makes it possible in future implement discarding
copied regions in backing files during the block-stream job, to reduce
the disk overuse (we need control on permissions).

Also, the filter now is smart enough to do copy-on-read with specified
base, so we have benefit on guest reads even when doing block-stream of
the part of the backing chain.

Several iotests are slightly modified due to filter insertion.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
  block/stream.c             | 105 ++++++++++++++++++++++---------------
  tests/qemu-iotests/030     |   8 +--
  tests/qemu-iotests/141.out |   2 +-
  tests/qemu-iotests/245     |  20 ++++---
  4 files changed, 80 insertions(+), 55 deletions(-)

diff --git a/block/stream.c b/block/stream.c
index 626dfa2b22..1fa742b0db 100644
--- a/block/stream.c
+++ b/block/stream.c

[...]

@@ -266,30 +251,62 @@ void stream_start(const char *job_id, BlockDriverState *bs,

[...]

      /* Make sure that the image is opened in read-write mode */
      bs_read_only = bdrv_is_read_only(bs);
      if (bs_read_only) {
-        if (bdrv_reopen_set_read_only(bs, false, errp) != 0) {
-            bs_read_only = false;
-            goto fail;
+        int ret;
+        /* Hold the chain during reopen */
+        if (bdrv_freeze_backing_chain(bs, above_base, errp) < 0) {
+            return;
+        }
+
+        ret = bdrv_reopen_set_read_only(bs, false, errp);
+
+        /* failure, or cor-filter will hold the chain */
+        bdrv_unfreeze_backing_chain(bs, above_base);
+
+        if (ret < 0) {

Shouldn’t we keep the “bs_read_only = false;” here?


No, as we don't goto fail.

Ah, right, then it won’t do anything.

(pre-patch, we goto fail here, and don't want fail: code path to reopend back to RW (as reopening to RO is failed anyway (and we hope it's transactional enough)))

That’s why we had bs_read_only = false; pre-patch, so the reopen back to RW is skipped.

And with this patch, we don’t need anything else from the “fail” path (freezing is done by the filter, and the filter doesn’t exist yet), so it’s correct to condense the “bs_read_only = false; goto fail;” into a plain “return”.

Reviewed-by: Max Reitz <mreitz@redhat.com>




reply via email to

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