[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v14 13/13] block: apply COR-filter to block-stream jobs
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
[PATCH v14 13/13] block: apply COR-filter to block-stream jobs |
Date: |
Sat, 5 Dec 2020 01:07:58 +0300 |
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 | 78 ++++++++++++++++++++++++++------------
tests/qemu-iotests/030 | 8 ++--
tests/qemu-iotests/141.out | 2 +-
tests/qemu-iotests/245 | 20 ++++++----
4 files changed, 72 insertions(+), 36 deletions(-)
diff --git a/block/stream.c b/block/stream.c
index a7fd8945ad..b92f7de55b 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -18,8 +18,10 @@
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
#include "qemu/error-report.h"
+#include "qapi/qmp/qdict.h"
#include "qemu/ratelimit.h"
#include "sysemu/block-backend.h"
+#include "block/copy-on-read.h"
enum {
/*
@@ -34,6 +36,7 @@ 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;
@@ -46,8 +49,7 @@ static int coroutine_fn stream_populate(BlockBackend *blk,
{
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, BDRV_REQ_PREFETCH);
}
static void stream_abort(Job *job)
@@ -55,7 +57,7 @@ static void stream_abort(Job *job)
StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
if (s->chain_frozen) {
- bdrv_unfreeze_backing_chain(s->target_bs, s->above_base);
+ bdrv_unfreeze_backing_chain(s->cor_filter_bs, s->above_base);
}
}
@@ -69,7 +71,7 @@ static int stream_prepare(Job *job)
Error *local_err = NULL;
int ret = 0;
- bdrv_unfreeze_backing_chain(s->target_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)) {
@@ -117,6 +119,8 @@ static void stream_clean(Job *job)
bdrv_reopen_set_read_only(s->target_bs, true, NULL);
}
+ bdrv_cor_filter_drop(s->cor_filter_bs);
+
g_free(s->backing_file_str);
}
@@ -125,7 +129,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
BlockBackend *blk = s->common.blk;
BlockDriverState *unfiltered_bs = bdrv_skip_filters(s->target_bs);
- bool enable_cor = !bdrv_cow_child(s->base_overlay);
int64_t len;
int64_t offset = 0;
uint64_t delay_ns = 0;
@@ -143,15 +146,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
}
job_progress_set_remaining(&s->common.job, len);
- /* Turn on copy-on-read for the whole block device so that guest read
- * requests help us make progress. Only do this when copying the entire
- * backing chain since the copy-on-read operation does not take base into
- * account.
- */
- if (enable_cor) {
- bdrv_enable_copy_on_read(s->target_bs);
- }
-
for ( ; offset < len; offset += n) {
bool copy;
int ret;
@@ -210,10 +204,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
}
}
- if (enable_cor) {
- bdrv_disable_copy_on_read(s->target_bs);
- }
-
/* Do not remove the backing file if an error was there but ignored. */
return error;
}
@@ -244,7 +234,9 @@ void stream_start(const char *job_id, BlockDriverState *bs,
bool bs_read_only;
int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
BlockDriverState *base_overlay;
+ BlockDriverState *cor_filter_bs = NULL;
BlockDriverState *above_base;
+ QDict *opts;
assert(!(base && bottom));
assert(!(backing_file_str && bottom));
@@ -295,17 +287,49 @@ 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,
+ opts = qdict_new();
+
+ qdict_put_str(opts, "driver", "copy-on-read");
+ qdict_put_str(opts, "file", bdrv_get_node_name(bs));
+ /* Pass the base_overlay node name as 'bottom' to COR driver */
+ qdict_put_str(opts, "bottom", base_overlay->node_name);
+ if (filter_node_name) {
+ qdict_put_str(opts, "node-name", filter_node_name);
+ }
+
+ cor_filter_bs = bdrv_insert_node(bs, opts, BDRV_O_RDWR, errp);
+ if (cor_filter_bs == NULL) {
+ goto fail;
+ }
+
+ if (!filter_node_name) {
+ cor_filter_bs->implicit = true;
+ }
+
+ 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,
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, 0,
+ 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
@@ -326,6 +350,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
s->base_overlay = base_overlay;
s->above_base = above_base;
s->backing_file_str = g_strdup(backing_file_str);
+ s->cor_filter_bs = cor_filter_bs;
s->target_bs = bs;
s->bs_read_only = bs_read_only;
s->chain_frozen = true;
@@ -339,5 +364,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 bd8cf9cff7..c576d55d07 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -278,12 +278,14 @@ class TestParallelOps(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
# Set a speed limit to make sure that this job blocks the rest
- result = self.vm.qmp('block-stream', device='node4',
job_id='stream-node4', base=self.imgs[1], speed=1024*1024)
+ result = self.vm.qmp('block-stream', device='node4',
+ job_id='stream-node4', base=self.imgs[1],
+ filter_node_name='stream-filter', speed=1024*1024)
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('block-stream', device='node5',
job_id='stream-node5', base=self.imgs[2])
self.assert_qmp(result, 'error/desc',
- "Node 'node4' is busy: block device is in use by block job:
stream")
+ "Node 'stream-filter' is busy: block device is in use by block
job: stream")
result = self.vm.qmp('block-stream', device='node3',
job_id='stream-node3', base=self.imgs[2])
self.assert_qmp(result, 'error/desc',
@@ -296,7 +298,7 @@ class TestParallelOps(iotests.QMPTestCase):
# block-commit should also fail if it touches nodes used by the stream
job
result = self.vm.qmp('block-commit', device='drive0',
base=self.imgs[4], job_id='commit-node4')
self.assert_qmp(result, 'error/desc',
- "Node 'node4' is busy: block device is in use by block job:
stream")
+ "Node 'stream-filter' is busy: block device is in use by block
job: stream")
result = self.vm.qmp('block-commit', device='drive0',
base=self.imgs[1], top=self.imgs[3], job_id='commit-node1')
self.assert_qmp(result, 'error/desc',
diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
index 08e0aecd65..028a16f365 100644
--- a/tests/qemu-iotests/141.out
+++ b/tests/qemu-iotests/141.out
@@ -99,7 +99,7 @@ wrote 1048576/1048576 bytes at offset 0
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
{'execute': 'blockdev-del', 'arguments': {'node-name': 'drv0'}}
-{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}}
+{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device
is in use by block job: stream"}}
{'execute': 'block-job-cancel', 'arguments': {'device': 'job0'}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "job0"}}
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index e60c8326d3..432e837e6c 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -892,20 +892,24 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# hd1 <- hd0
result = self.vm.qmp('block-stream', conv_keys = True, job_id =
'stream0',
- device = 'hd1', auto_finalize = False)
+ device = 'hd1', filter_node_name='cor',
+ auto_finalize = False)
self.assert_qmp(result, 'return', {})
- # We can't reopen with the original options because that would
- # make hd1 read-only and block-stream requires it to be read-write
- # (Which error message appears depends on whether the stream job is
- # already done with copying at this point.)
+ # We can't reopen with the original options because there is a filter
+ # inserted by stream job above hd1.
self.reopen(opts, {},
- ["Can't set node 'hd1' to r/o with copy-on-read enabled",
- "Cannot make block node read-only, there is a writer on it"])
+ "Cannot change the option
'backing.backing.file.node-name'")
+
+ # We can't reopen hd1 to read-only, as block-stream requires it to be
+ # read-write
+ self.reopen(opts['backing'], {'read-only': True},
+ "Cannot make block node read-only, there is a writer on
it")
# We can't remove hd2 while the stream job is ongoing
opts['backing']['backing'] = None
- self.reopen(opts, {'backing.read-only': False}, "Cannot change
'backing' link from 'hd1' to 'hd2'")
+ self.reopen(opts['backing'], {'read-only': False},
+ "Cannot change 'backing' link from 'hd1' to 'hd2'")
# We can detach hd1 from hd0 because it doesn't affect the stream job
opts['backing'] = None
--
2.21.3
- [PATCH v14 10/13] qapi: block-stream: add "bottom" argument, (continued)
[PATCH v14 12/13] block/stream: add s->target_bs, Vladimir Sementsov-Ogievskiy, 2020/12/04
[PATCH v14 11/13] iotests: 30: prepare to COR filter insertion by stream job, Vladimir Sementsov-Ogievskiy, 2020/12/04
[PATCH v14 13/13] block: apply COR-filter to block-stream jobs,
Vladimir Sementsov-Ogievskiy <=