* [PATCH v5 00/15] Apply COR-filter to the block-stream permanently
@ 2020-05-13 9:50 Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 01/15] block: Mark commit and mirror as filter drivers Andrey Shinkevich
` (15 more replies)
0 siblings, 16 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
With this series, all the block-stream COR operations pass through
the COR-filter. The patches 01-08/15 are taken from the series
"block: Deal with filters" by Max Reitz, the full version of that
can be found in the branches:
https://git.xanclic.moe/XanClic/qemu child-access-functions-v6
https://github.com/XanClic/qemu child-access-functions-v6
When running iotests, apply "char-socket: Fix race condition"
to avoid sporadic segmentation faults.
v5:
01: Included forgotten file block/copy-on-read.h
v4:
01: Initialization of the COR-filter BDRVStateCOR member.
v3:
01: The COR filter insert/remove functions moved to block/copy-on-read.c
to be a part of API.
02: block/stream.c code refactoring.
03: The separate call to block_job_add_bdrv() is used to block operations
on the active node after the filter inserted and the job created.
04: The iotests case 030::test_overlapping_4 was modified to unbound
the block-stream job from the base node.
05: The COR driver functions preadv/pwritev replaced with their analogous
preadv/pwritev_part.
v2:
01: No more skipping filters while checking for operation blockers.
However, we exclude filters between the bottom node and base
because we do not set the operation blockers for filters anymore.
02: As stated above, we do not set the operation blockers for filters
anymore. So, skip filters when we block operations for the target
node.
03: The comment added for the patch 4/7.
04: The QAPI target version changed to 5.1.
05: The 'filter-node-name' now excluded from using in the test #030.
If we need it no more in a final version of the series, the patch
5/7 may be removed.
06: The COR-filter included into the frozen chain of a block-stream job.
The 'above_base' node pointer is left because it is essential for
finding the base node in case of filters above.
Andrey Shinkevich (7):
block: prepare block-stream for using COR-filter
copy-on-read: Support change filename functions
copy-on-read: Support preadv/pwritev_part functions
copy-on-read: add filter append/drop functions
qapi: add filter-node-name to block-stream
iotests: prepare 245 for using filter in block-stream
block: apply COR-filter to block-stream jobs
Max Reitz (8):
block: Mark commit and mirror as filter drivers
copy-on-read: Support compressed writes
block: Add child access functions
block: Add chain helper functions
block: Include filters when freezing backing chain
block: Use CAFs in block status functions
commit: Deal with filters when blocking intermediate nodes
block: Use CAFs when working with backing chains
block.c | 275 ++++++++++++++++++++++++++++++++++-------
block/commit.c | 85 ++++++++++---
block/copy-on-read.c | 159 ++++++++++++++++++++++--
block/copy-on-read.h | 36 ++++++
block/io.c | 19 +--
block/mirror.c | 6 +-
block/monitor/block-hmp-cmds.c | 4 +-
block/stream.c | 89 +++++++++----
blockdev.c | 19 ++-
include/block/block.h | 6 +-
include/block/block_int.h | 67 +++++++++-
qapi/block-core.json | 6 +
tests/qemu-iotests/030 | 17 +--
tests/qemu-iotests/141.out | 2 +-
tests/qemu-iotests/245 | 10 +-
15 files changed, 661 insertions(+), 139 deletions(-)
create mode 100644 block/copy-on-read.h
--
1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v5 01/15] block: Mark commit and mirror as filter drivers
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 02/15] copy-on-read: Support compressed writes Andrey Shinkevich
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
The commit and mirror block nodes are filters, so they should be marked
as such. (Strictly speaking, BDS.is_filter's documentation states that
a filter's child must be bs->file. The following patch will relax this
restriction, however.)
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/commit.c | 2 ++
block/mirror.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/block/commit.c b/block/commit.c
index 87f6096..445a280 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -240,6 +240,8 @@ static BlockDriver bdrv_commit_top = {
.bdrv_co_block_status = bdrv_co_block_status_from_backing,
.bdrv_refresh_filename = bdrv_commit_top_refresh_filename,
.bdrv_child_perm = bdrv_commit_top_child_perm,
+
+ .is_filter = true,
};
void commit_start(const char *job_id, BlockDriverState *bs,
diff --git a/block/mirror.c b/block/mirror.c
index aca95c9..b6de24b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1527,6 +1527,8 @@ static BlockDriver bdrv_mirror_top = {
.bdrv_co_block_status = bdrv_co_block_status_from_backing,
.bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
.bdrv_child_perm = bdrv_mirror_top_child_perm,
+
+ .is_filter = true,
};
static BlockJob *mirror_start_job(
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 02/15] copy-on-read: Support compressed writes
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 01/15] block: Mark commit and mirror as filter drivers Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 03/15] block: Add child access functions Andrey Shinkevich
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/copy-on-read.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 242d3ff..c4fa468 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -106,6 +106,16 @@ static int coroutine_fn cor_co_pdiscard(BlockDriverState *bs,
}
+static int coroutine_fn cor_co_pwritev_compressed(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov)
+{
+ return bdrv_co_pwritev(bs->file, offset, bytes, qiov,
+ BDRV_REQ_WRITE_COMPRESSED);
+}
+
+
static void cor_eject(BlockDriverState *bs, bool eject_flag)
{
bdrv_eject(bs->file->bs, eject_flag);
@@ -130,6 +140,7 @@ static BlockDriver bdrv_copy_on_read = {
.bdrv_co_pwritev = cor_co_pwritev,
.bdrv_co_pwrite_zeroes = cor_co_pwrite_zeroes,
.bdrv_co_pdiscard = cor_co_pdiscard,
+ .bdrv_co_pwritev_compressed = cor_co_pwritev_compressed,
.bdrv_eject = cor_eject,
.bdrv_lock_medium = cor_lock_medium,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 03/15] block: Add child access functions
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 01/15] block: Mark commit and mirror as filter drivers Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 02/15] copy-on-read: Support compressed writes Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 04/15] block: Add chain helper functions Andrey Shinkevich
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
There are BDS children that the general block layer code can access,
namely bs->file and bs->backing. Since the introduction of filters and
external data files, their meaning is not quite clear. bs->backing can
be a COW source, or it can be an R/W-filtered child; bs->file can be an
R/W-filtered child, it can be data and metadata storage, or it can be
just metadata storage.
This overloading really is not helpful. This patch adds function that
retrieve the correct child for each exact purpose. Later patches in
this series will make use of them. Doing so will allow us to handle
filter nodes and external data files in a meaningful way.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++
include/block/block_int.h | 57 +++++++++++++++++++++++++--
2 files changed, 153 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 0653ccb..b2aae2e 100644
--- a/block.c
+++ b/block.c
@@ -6764,3 +6764,102 @@ void bdrv_del_child(BlockDriverState *parent_bs, BdrvChild *child, Error **errp)
parent_bs->drv->bdrv_del_child(parent_bs, child, errp);
}
+
+/*
+ * Return the child that @bs acts as an overlay for, and from which data may be
+ * copied in COW or COR operations. Usually this is the backing file.
+ */
+BdrvChild *bdrv_filtered_cow_child(BlockDriverState *bs)
+{
+ if (!bs || !bs->drv) {
+ return NULL;
+ }
+
+ if (bs->drv->is_filter) {
+ return NULL;
+ }
+
+ return bs->backing;
+}
+
+/*
+ * If @bs acts as a pass-through filter for one of its children,
+ * return that child. "Pass-through" means that write operations to
+ * @bs are forwarded to that child instead of triggering COW.
+ */
+BdrvChild *bdrv_filtered_rw_child(BlockDriverState *bs)
+{
+ if (!bs || !bs->drv) {
+ return NULL;
+ }
+
+ if (!bs->drv->is_filter) {
+ return NULL;
+ }
+
+ /* Only one of @backing or @file may be used */
+ assert(!(bs->backing && bs->file));
+
+ return bs->backing ?: bs->file;
+}
+
+/*
+ * Return any filtered child, independently of how it reacts to write
+ * accesses and whether data is copied onto this BDS through COR.
+ */
+BdrvChild *bdrv_filtered_child(BlockDriverState *bs)
+{
+ BdrvChild *cow_child = bdrv_filtered_cow_child(bs);
+ BdrvChild *rw_child = bdrv_filtered_rw_child(bs);
+
+ /* There can only be one filtered child at a time */
+ assert(!(cow_child && rw_child));
+
+ return cow_child ?: rw_child;
+}
+
+/*
+ * Return the child that stores the metadata for this node.
+ */
+BdrvChild *bdrv_metadata_child(BlockDriverState *bs)
+{
+ if (!bs || !bs->drv) {
+ return NULL;
+ }
+
+ /* Filters do not have metadata */
+ if (bs->drv->is_filter) {
+ return NULL;
+ }
+
+ return bs->file;
+}
+
+/*
+ * Return the child that stores the data that is allocated on this
+ * node. This may or may not include metadata.
+ */
+BdrvChild *bdrv_storage_child(BlockDriverState *bs)
+{
+ if (!bs || !bs->drv) {
+ return NULL;
+ }
+
+ if (bs->drv->bdrv_storage_child) {
+ return bs->drv->bdrv_storage_child(bs);
+ }
+
+ return bdrv_filtered_rw_child(bs) ?: bs->file;
+}
+
+/*
+ * Return the primary child of this node: For filters, that is the
+ * filtered child. For other nodes, that is usually the child storing
+ * metadata.
+ * (A generally more helpful description is that this is (usually) the
+ * child that has the same filename as @bs.)
+ */
+BdrvChild *bdrv_primary_child(BlockDriverState *bs)
+{
+ return bdrv_filtered_rw_child(bs) ?: bs->file;
+}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index df6d027..dca59e9 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -89,9 +89,11 @@ struct BlockDriver {
int instance_size;
/* set to true if the BlockDriver is a block filter. Block filters pass
- * certain callbacks that refer to data (see block.c) to their bs->file if
- * the driver doesn't implement them. Drivers that do not wish to forward
- * must implement them and return -ENOTSUP.
+ * certain callbacks that refer to data (see block.c) to their bs->file
+ * or bs->backing (whichever one exists) if the driver doesn't implement
+ * them. Drivers that do not wish to forward must implement them and return
+ * -ENOTSUP.
+ * Note that filters are not allowed to modify data.
*/
bool is_filter;
/*
@@ -585,6 +587,13 @@ struct BlockDriver {
* If this pointer is NULL, the array is considered empty.
* "filename" and "driver" are always considered strong. */
const char *const *strong_runtime_opts;
+
+ /**
+ * Return the data storage child, if there is exactly one. If
+ * this function is not implemented, the block layer will assume
+ * bs->file to be this child.
+ */
+ BdrvChild *(*bdrv_storage_child)(BlockDriverState *bs);
};
static inline bool block_driver_can_compress(BlockDriver *drv)
@@ -1332,6 +1341,48 @@ int refresh_total_sectors(BlockDriverState *bs, int64_t hint);
void bdrv_set_monitor_owned(BlockDriverState *bs);
BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp);
+BdrvChild *bdrv_filtered_cow_child(BlockDriverState *bs);
+BdrvChild *bdrv_filtered_rw_child(BlockDriverState *bs);
+BdrvChild *bdrv_filtered_child(BlockDriverState *bs);
+BdrvChild *bdrv_metadata_child(BlockDriverState *bs);
+BdrvChild *bdrv_storage_child(BlockDriverState *bs);
+BdrvChild *bdrv_primary_child(BlockDriverState *bs);
+
+static inline BlockDriverState *child_bs(BdrvChild *child)
+{
+ return child ? child->bs : NULL;
+}
+
+static inline BlockDriverState *bdrv_filtered_cow_bs(BlockDriverState *bs)
+{
+ return child_bs(bdrv_filtered_cow_child(bs));
+}
+
+static inline BlockDriverState *bdrv_filtered_rw_bs(BlockDriverState *bs)
+{
+ return child_bs(bdrv_filtered_rw_child(bs));
+}
+
+static inline BlockDriverState *bdrv_filtered_bs(BlockDriverState *bs)
+{
+ return child_bs(bdrv_filtered_child(bs));
+}
+
+static inline BlockDriverState *bdrv_metadata_bs(BlockDriverState *bs)
+{
+ return child_bs(bdrv_metadata_child(bs));
+}
+
+static inline BlockDriverState *bdrv_storage_bs(BlockDriverState *bs)
+{
+ return child_bs(bdrv_storage_child(bs));
+}
+
+static inline BlockDriverState *bdrv_primary_bs(BlockDriverState *bs)
+{
+ return child_bs(bdrv_primary_child(bs));
+}
+
/**
* Simple implementation of bdrv_co_create_opts for protocol drivers
* which only support creation via opening a file
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 04/15] block: Add chain helper functions
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (2 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 03/15] block: Add child access functions Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 05/15] block: Include filters when freezing backing chain Andrey Shinkevich
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
Add some helper functions for skipping filters in a chain of block
nodes.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
include/block/block_int.h | 3 +++
2 files changed, 58 insertions(+)
diff --git a/block.c b/block.c
index b2aae2e..5b4ebfe 100644
--- a/block.c
+++ b/block.c
@@ -6863,3 +6863,58 @@ BdrvChild *bdrv_primary_child(BlockDriverState *bs)
{
return bdrv_filtered_rw_child(bs) ?: bs->file;
}
+
+static BlockDriverState *bdrv_skip_filters(BlockDriverState *bs,
+ bool stop_on_explicit_filter)
+{
+ BdrvChild *filtered;
+
+ if (!bs) {
+ return NULL;
+ }
+
+ while (!(stop_on_explicit_filter && !bs->implicit)) {
+ filtered = bdrv_filtered_rw_child(bs);
+ if (!filtered) {
+ break;
+ }
+ bs = filtered->bs;
+ }
+ /*
+ * Note that this treats nodes with bs->drv == NULL as not being
+ * R/W filters (bs->drv == NULL should be replaced by something
+ * else anyway).
+ * The advantage of this behavior is that this function will thus
+ * always return a non-NULL value (given a non-NULL @bs).
+ */
+
+ return bs;
+}
+
+/*
+ * Return the first BDS that has not been added implicitly or that
+ * does not have an RW-filtered child down the chain starting from @bs
+ * (including @bs itself).
+ */
+BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs)
+{
+ return bdrv_skip_filters(bs, true);
+}
+
+/*
+ * Return the first BDS that does not have an RW-filtered child down
+ * the chain starting from @bs (including @bs itself).
+ */
+BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs)
+{
+ return bdrv_skip_filters(bs, false);
+}
+
+/*
+ * For a backing chain, return the first non-filter backing image of
+ * the first non-filter image.
+ */
+BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs)
+{
+ return bdrv_skip_rw_filters(bdrv_filtered_cow_bs(bdrv_skip_rw_filters(bs)));
+}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index dca59e9..86f7666 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1347,6 +1347,9 @@ BdrvChild *bdrv_filtered_child(BlockDriverState *bs);
BdrvChild *bdrv_metadata_child(BlockDriverState *bs);
BdrvChild *bdrv_storage_child(BlockDriverState *bs);
BdrvChild *bdrv_primary_child(BlockDriverState *bs);
+BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
+BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs);
+BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs);
static inline BlockDriverState *child_bs(BdrvChild *child)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 05/15] block: Include filters when freezing backing chain
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (3 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 04/15] block: Add chain helper functions Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 06/15] block: Use CAFs in block status functions Andrey Shinkevich
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
In order to make filters work in backing chains, the associated
functions must be able to deal with them and freeze all filter links, be
they COW or R/W filter links.
In the process, rename these functions to reflect that they now act on
generalized chains of filter nodes instead of backing chains alone.
While at it, add some comments that note which functions require their
caller to ensure that a given child link is not frozen, and how the
callers do so.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block.c | 81 ++++++++++++++++++++++++++++++---------------------
block/commit.c | 8 ++---
block/mirror.c | 4 +--
block/stream.c | 8 ++---
include/block/block.h | 6 ++--
5 files changed, 60 insertions(+), 47 deletions(-)
diff --git a/block.c b/block.c
index 5b4ebfe..459412e 100644
--- a/block.c
+++ b/block.c
@@ -2513,12 +2513,15 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
* If @new_bs is not NULL, bdrv_check_perm() must be called beforehand, as this
* function uses bdrv_set_perm() to update the permissions according to the new
* reference that @new_bs gets.
+ *
+ * Callers must ensure that child->frozen is false.
*/
static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs)
{
BlockDriverState *old_bs = child->bs;
uint64_t perm, shared_perm;
+ /* Asserts that child->frozen == false */
bdrv_replace_child_noperm(child, new_bs);
/*
@@ -2676,6 +2679,7 @@ static void bdrv_detach_child(BdrvChild *child)
g_free(child);
}
+/* Callers must ensure that child->frozen is false. */
void bdrv_root_unref_child(BdrvChild *child)
{
BlockDriverState *child_bs;
@@ -2685,10 +2689,6 @@ void bdrv_root_unref_child(BdrvChild *child)
bdrv_unref(child_bs);
}
-/**
- * Clear all inherits_from pointers from children and grandchildren of
- * @root that point to @root, where necessary.
- */
static void bdrv_unset_inherits_from(BlockDriverState *root, BdrvChild *child)
{
BdrvChild *c;
@@ -2713,6 +2713,7 @@ static void bdrv_unset_inherits_from(BlockDriverState *root, BdrvChild *child)
}
}
+/* Callers must ensure that child->frozen is false. */
void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child)
{
if (child == NULL) {
@@ -2756,7 +2757,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) &&
bdrv_inherits_from_recursive(backing_hd, bs);
- if (bdrv_is_backing_chain_frozen(bs, backing_bs(bs), errp)) {
+ if (bdrv_is_chain_frozen(bs, child_bs(bs->backing), errp)) {
return;
}
@@ -2765,6 +2766,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
}
if (bs->backing) {
+ /* Cannot be frozen, we checked that above */
bdrv_unref_child(bs, bs->backing);
bs->backing = NULL;
}
@@ -3912,8 +3914,7 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
return -EPERM;
}
/* Check if the backing link that we want to replace is frozen */
- if (bdrv_is_backing_chain_frozen(overlay_bs, backing_bs(overlay_bs),
- errp)) {
+ if (bdrv_is_chain_frozen(overlay_bs, backing_bs(overlay_bs), errp)) {
return -EPERM;
}
reopen_state->replace_backing_bs = true;
@@ -4248,6 +4249,7 @@ static void bdrv_close(BlockDriverState *bs)
if (bs->drv) {
if (bs->drv->bdrv_close) {
+ /* Must unfreeze all children, so bdrv_unref_child() works */
bs->drv->bdrv_close(bs);
}
bs->drv = NULL;
@@ -4617,20 +4619,22 @@ BlockDriverState *bdrv_find_base(BlockDriverState *bs)
}
/*
- * Return true if at least one of the backing links between @bs and
- * @base is frozen. @errp is set if that's the case.
+ * Return true if at least one of the (COW and R/W) filter links
+ * between @bs and @base is frozen. @errp is set if that's the case.
* @base must be reachable from @bs, or NULL.
*/
-bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
- Error **errp)
+bool bdrv_is_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
+ Error **errp)
{
BlockDriverState *i;
+ BdrvChild *child;
+
+ for (i = bs; i != base; i = child_bs(child)) {
+ child = bdrv_filtered_child(i);
- for (i = bs; i != base; i = backing_bs(i)) {
- if (i->backing && i->backing->frozen) {
+ if (child && child->frozen) {
error_setg(errp, "Cannot change '%s' link from '%s' to '%s'",
- i->backing->name, i->node_name,
- backing_bs(i)->node_name);
+ child->name, i->node_name, child->bs->node_name);
return true;
}
}
@@ -4639,32 +4643,35 @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
}
/*
- * Freeze all backing links between @bs and @base.
+ * Freeze all (COW and R/W) filter links between @bs and @base.
* If any of the links is already frozen the operation is aborted and
* none of the links are modified.
* @base must be reachable from @bs, or NULL.
* Returns 0 on success. On failure returns < 0 and sets @errp.
*/
-int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
- Error **errp)
+int bdrv_freeze_chain(BlockDriverState *bs, BlockDriverState *base,
+ Error **errp)
{
BlockDriverState *i;
+ BdrvChild *child;
- if (bdrv_is_backing_chain_frozen(bs, base, errp)) {
+ if (bdrv_is_chain_frozen(bs, base, errp)) {
return -EPERM;
}
- for (i = bs; i != base; i = backing_bs(i)) {
- if (i->backing && backing_bs(i)->never_freeze) {
+ for (i = bs; i != base; i = child_bs(child)) {
+ child = bdrv_filtered_child(i);
+ if (child && child->bs->never_freeze) {
error_setg(errp, "Cannot freeze '%s' link to '%s'",
- i->backing->name, backing_bs(i)->node_name);
+ child->name, child->bs->node_name);
return -EPERM;
}
}
- for (i = bs; i != base; i = backing_bs(i)) {
- if (i->backing) {
- i->backing->frozen = true;
+ for (i = bs; i != base; i = child_bs(child)) {
+ child = bdrv_filtered_child(i);
+ if (child) {
+ child->frozen = true;
}
}
@@ -4672,18 +4679,21 @@ int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
}
/*
- * Unfreeze all backing links between @bs and @base. The caller must
- * ensure that all links are frozen before using this function.
+ * Unfreeze all (COW and R/W) filter links between @bs and @base. The
+ * caller must ensure that all links are frozen before using this
+ * function.
* @base must be reachable from @bs, or NULL.
*/
-void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base)
+void bdrv_unfreeze_chain(BlockDriverState *bs, BlockDriverState *base)
{
BlockDriverState *i;
+ BdrvChild *child;
- for (i = bs; i != base; i = backing_bs(i)) {
- if (i->backing) {
- assert(i->backing->frozen);
- i->backing->frozen = false;
+ for (i = bs; i != base; i = child_bs(child)) {
+ child = bdrv_filtered_child(i);
+ if (child) {
+ assert(child->frozen);
+ child->frozen = false;
}
}
}
@@ -4786,8 +4796,11 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
}
}
- /* Do the actual switch in the in-memory graph.
- * Completes bdrv_check_update_perm() transaction internally. */
+ /*
+ * Do the actual switch in the in-memory graph.
+ * Completes bdrv_check_update_perm() transaction internally.
+ * c->frozen is false, we have checked that above.
+ */
bdrv_ref(base);
bdrv_replace_child(c, base);
bdrv_unref(top);
diff --git a/block/commit.c b/block/commit.c
index 445a280..e1f45a4 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -47,7 +47,7 @@ static int commit_prepare(Job *job)
{
CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
- bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);
+ bdrv_unfreeze_chain(s->commit_top_bs, s->base_bs);
s->chain_frozen = false;
/* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before
@@ -67,7 +67,7 @@ static void commit_abort(Job *job)
BlockDriverState *top_bs = blk_bs(s->top);
if (s->chain_frozen) {
- bdrv_unfreeze_backing_chain(s->commit_top_bs, s->base_bs);
+ bdrv_unfreeze_chain(s->commit_top_bs, s->base_bs);
}
/* Make sure commit_top_bs and top stay around until bdrv_replace_node() */
@@ -317,7 +317,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
}
}
- if (bdrv_freeze_backing_chain(commit_top_bs, base, errp) < 0) {
+ if (bdrv_freeze_chain(commit_top_bs, base, errp) < 0) {
goto fail;
}
s->chain_frozen = true;
@@ -358,7 +358,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
fail:
if (s->chain_frozen) {
- bdrv_unfreeze_backing_chain(commit_top_bs, base);
+ bdrv_unfreeze_chain(commit_top_bs, base);
}
if (s->base) {
blk_unref(s->base);
diff --git a/block/mirror.c b/block/mirror.c
index b6de24b..4b187e6 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -648,7 +648,7 @@ static int mirror_exit_common(Job *job)
target_bs = blk_bs(s->target);
if (bdrv_chain_contains(src, target_bs)) {
- bdrv_unfreeze_backing_chain(mirror_top_bs, target_bs);
+ bdrv_unfreeze_chain(mirror_top_bs, target_bs);
}
bdrv_release_dirty_bitmap(s->dirty_bitmap);
@@ -1713,7 +1713,7 @@ static BlockJob *mirror_start_job(
}
}
- if (bdrv_freeze_backing_chain(mirror_top_bs, target, errp) < 0) {
+ if (bdrv_freeze_chain(mirror_top_bs, target, errp) < 0) {
goto fail;
}
}
diff --git a/block/stream.c b/block/stream.c
index aa2e7af..3cca1f9 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -53,7 +53,7 @@ static void stream_abort(Job *job)
if (s->chain_frozen) {
BlockJob *bjob = &s->common;
- bdrv_unfreeze_backing_chain(blk_bs(bjob->blk), s->bottom);
+ bdrv_unfreeze_chain(blk_bs(bjob->blk), s->bottom);
}
}
@@ -66,7 +66,7 @@ static int stream_prepare(Job *job)
Error *local_err = NULL;
int ret = 0;
- bdrv_unfreeze_backing_chain(bs, s->bottom);
+ bdrv_unfreeze_chain(bs, s->bottom);
s->chain_frozen = false;
if (bs->backing) {
@@ -225,7 +225,7 @@ void stream_start(const char *job_id, BlockDriverState *bs,
int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
BlockDriverState *bottom = bdrv_find_overlay(bs, base);
- if (bdrv_freeze_backing_chain(bs, bottom, errp) < 0) {
+ if (bdrv_freeze_chain(bs, bottom, errp) < 0) {
return;
}
@@ -276,5 +276,5 @@ fail:
if (bs_read_only) {
bdrv_reopen_set_read_only(bs, true, NULL);
}
- bdrv_unfreeze_backing_chain(bs, bottom);
+ bdrv_unfreeze_chain(bs, bottom);
}
diff --git a/include/block/block.h b/include/block/block.h
index 4de8d8f..e55143d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -360,11 +360,11 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
BlockDriverState *bdrv_find_overlay(BlockDriverState *active,
BlockDriverState *bs);
BlockDriverState *bdrv_find_base(BlockDriverState *bs);
-bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
+bool bdrv_is_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
Error **errp);
-int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
+int bdrv_freeze_chain(BlockDriverState *bs, BlockDriverState *base,
Error **errp);
-void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base);
+void bdrv_unfreeze_chain(BlockDriverState *bs, BlockDriverState *base);
int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 06/15] block: Use CAFs in block status functions
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (4 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 05/15] block: Include filters when freezing backing chain Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 07/15] commit: Deal with filters when blocking intermediate nodes Andrey Shinkevich
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
Use the child access functions in the block status inquiry functions as
appropriate.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/io.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/block/io.c b/block/io.c
index 7d30e61..47d8096 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2387,11 +2387,12 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
ret |= BDRV_BLOCK_ALLOCATED;
} else if (want_zero) {
+ BlockDriverState *cow_bs = bdrv_filtered_cow_bs(bs);
+
if (bdrv_unallocated_blocks_are_zero(bs)) {
ret |= BDRV_BLOCK_ZERO;
- } else if (bs->backing) {
- BlockDriverState *bs2 = bs->backing->bs;
- int64_t size2 = bdrv_getlength(bs2);
+ } else if (cow_bs) {
+ int64_t size2 = bdrv_getlength(cow_bs);
if (size2 >= 0 && offset >= size2) {
ret |= BDRV_BLOCK_ZERO;
@@ -2457,7 +2458,7 @@ static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs,
bool first = true;
assert(bs != base);
- for (p = bs; p != base; p = backing_bs(p)) {
+ for (p = bs; p != base; p = bdrv_filtered_bs(p)) {
ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
file);
if (ret < 0) {
@@ -2543,7 +2544,7 @@ int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base,
int bdrv_block_status(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum, int64_t *map, BlockDriverState **file)
{
- return bdrv_block_status_above(bs, backing_bs(bs),
+ return bdrv_block_status_above(bs, bdrv_filtered_bs(bs),
offset, bytes, pnum, map, file);
}
@@ -2553,9 +2554,9 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset,
int ret;
int64_t dummy;
- ret = bdrv_common_block_status_above(bs, backing_bs(bs), false, offset,
- bytes, pnum ? pnum : &dummy, NULL,
- NULL);
+ ret = bdrv_common_block_status_above(bs, bdrv_filtered_bs(bs), false,
+ offset, bytes, pnum ? pnum : &dummy,
+ NULL, NULL);
if (ret < 0) {
return ret;
}
@@ -2618,7 +2619,7 @@ int bdrv_is_allocated_above(BlockDriverState *top,
break;
}
- intermediate = backing_bs(intermediate);
+ intermediate = bdrv_filtered_bs(intermediate);
}
*pnum = n;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 07/15] commit: Deal with filters when blocking intermediate nodes
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (5 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 06/15] block: Use CAFs in block status functions Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 08/15] block: Use CAFs when working with backing chains Andrey Shinkevich
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
This includes some permission limiting (for example, we only need to
take the RESIZE permission if the base is smaller than the top).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/commit.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 60 insertions(+), 15 deletions(-)
diff --git a/block/commit.c b/block/commit.c
index e1f45a4..4df145d 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -37,6 +37,7 @@ typedef struct CommitBlockJob {
BlockBackend *top;
BlockBackend *base;
BlockDriverState *base_bs;
+ BlockDriverState *above_base;
BlockdevOnError on_error;
bool base_read_only;
bool chain_frozen;
@@ -153,7 +154,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
break;
}
/* Copy if allocated above the base */
- ret = bdrv_is_allocated_above(blk_bs(s->top), blk_bs(s->base), false,
+ ret = bdrv_is_allocated_above(blk_bs(s->top), s->above_base, true,
offset, COMMIT_BUFFER_SIZE, &n);
copy = (ret == 1);
trace_commit_one_iteration(s, offset, n, ret);
@@ -253,15 +254,35 @@ void commit_start(const char *job_id, BlockDriverState *bs,
CommitBlockJob *s;
BlockDriverState *iter;
BlockDriverState *commit_top_bs = NULL;
+ BlockDriverState *filtered_base;
Error *local_err = NULL;
+ int64_t base_size, top_size;
+ uint64_t perms, iter_shared_perms;
int ret;
assert(top != bs);
- if (top == base) {
+ if (bdrv_skip_rw_filters(top) == bdrv_skip_rw_filters(base)) {
error_setg(errp, "Invalid files for merge: top and base are the same");
return;
}
+ base_size = bdrv_getlength(base);
+ if (base_size < 0) {
+ error_setg_errno(errp, -base_size, "Could not inquire base image size");
+ return;
+ }
+
+ top_size = bdrv_getlength(top);
+ if (top_size < 0) {
+ error_setg_errno(errp, -top_size, "Could not inquire top image size");
+ return;
+ }
+
+ perms = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE;
+ if (base_size < top_size) {
+ perms |= BLK_PERM_RESIZE;
+ }
+
s = block_job_create(job_id, &commit_job_driver, NULL, bs, 0, BLK_PERM_ALL,
speed, creation_flags, NULL, NULL, errp);
if (!s) {
@@ -301,17 +322,43 @@ void commit_start(const char *job_id, BlockDriverState *bs,
s->commit_top_bs = commit_top_bs;
- /* Block all nodes between top and base, because they will
- * disappear from the chain after this operation. */
- assert(bdrv_chain_contains(top, base));
- for (iter = top; iter != base; iter = backing_bs(iter)) {
- /* XXX BLK_PERM_WRITE needs to be allowed so we don't block ourselves
- * at s->base (if writes are blocked for a node, they are also blocked
- * for its backing file). The other options would be a second filter
- * driver above s->base. */
+ /*
+ * Block all nodes between top and base, because they will
+ * disappear from the chain after this operation.
+ * Note that this assumes that the user is fine with removing all
+ * nodes (including R/W filters) between top and base. Assuring
+ * this is the responsibility of the interface (i.e. whoever calls
+ * commit_start()).
+ */
+ s->above_base = bdrv_find_overlay(top, base);
+ assert(s->above_base);
+
+ /*
+ * The topmost node with
+ * bdrv_skip_rw_filters(filtered_base) == bdrv_skip_rw_filters(base)
+ */
+ filtered_base = bdrv_filtered_cow_bs(s->above_base);
+ assert(bdrv_skip_rw_filters(filtered_base) == bdrv_skip_rw_filters(base));
+
+ /*
+ * XXX BLK_PERM_WRITE needs to be allowed so we don't block ourselves
+ * at s->base (if writes are blocked for a node, they are also blocked
+ * for its backing file). The other options would be a second filter
+ * driver above s->base.
+ */
+ iter_shared_perms = BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE;
+
+ for (iter = top; iter != base; iter = bdrv_filtered_bs(iter)) {
+ if (iter == filtered_base) {
+ /*
+ * From here on, all nodes are filters on the base. This
+ * allows us to share BLK_PERM_CONSISTENT_READ.
+ */
+ iter_shared_perms |= BLK_PERM_CONSISTENT_READ;
+ }
+
ret = block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
- BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE,
- errp);
+ iter_shared_perms, errp);
if (ret < 0) {
goto fail;
}
@@ -328,9 +375,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
}
s->base = blk_new(s->common.job.aio_context,
- BLK_PERM_CONSISTENT_READ
- | BLK_PERM_WRITE
- | BLK_PERM_RESIZE,
+ perms,
BLK_PERM_CONSISTENT_READ
| BLK_PERM_GRAPH_MOD
| BLK_PERM_WRITE_UNCHANGED);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 08/15] block: Use CAFs when working with backing chains
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (6 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 07/15] commit: Deal with filters when blocking intermediate nodes Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 09/15] block: prepare block-stream for using COR-filter Andrey Shinkevich
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
From: Max Reitz <mreitz@redhat.com>
Use child access functions when iterating through backing chains so
filters do not break the chain.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/block.c b/block.c
index 459412e..69e9152 100644
--- a/block.c
+++ b/block.c
@@ -4593,7 +4593,8 @@ int bdrv_change_backing_file(BlockDriverState *bs,
}
/*
- * Finds the image layer in the chain that has 'bs' as its backing file.
+ * Finds the image layer in the chain that has 'bs' (or a filter on
+ * top of it) as its backing file.
*
* active is the current topmost image.
*
@@ -4605,11 +4606,18 @@ int bdrv_change_backing_file(BlockDriverState *bs,
BlockDriverState *bdrv_find_overlay(BlockDriverState *active,
BlockDriverState *bs)
{
- while (active && bs != backing_bs(active)) {
- active = backing_bs(active);
+ bs = bdrv_skip_rw_filters(bs);
+ active = bdrv_skip_rw_filters(active);
+
+ while (active) {
+ BlockDriverState *next = bdrv_backing_chain_next(active);
+ if (bs == next) {
+ return active;
+ }
+ active = next;
}
- return active;
+ return NULL;
}
/* Given a BDS, searches for the base layer. */
@@ -4761,9 +4769,7 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
* other intermediate nodes have been dropped.
* If 'top' is an implicit node (e.g. "commit_top") we should skip
* it because no one inherits from it. We use explicit_top for that. */
- while (explicit_top && explicit_top->implicit) {
- explicit_top = backing_bs(explicit_top);
- }
+ explicit_top = bdrv_skip_implicit_filters(explicit_top);
update_inherits_from = bdrv_inherits_from_recursive(base, explicit_top);
/* success - we can delete the intermediate states, and link top->base */
@@ -5205,7 +5211,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base)
{
while (top && top != base) {
- top = backing_bs(top);
+ top = bdrv_filtered_bs(top);
}
return top != NULL;
@@ -5466,7 +5472,17 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
is_protocol = path_has_protocol(backing_file);
- for (curr_bs = bs; curr_bs->backing; curr_bs = curr_bs->backing->bs) {
+ /*
+ * Being largely a legacy function, skip any filters here
+ * (because filters do not have normal filenames, so they cannot
+ * match anyway; and allowing json:{} filenames is a bit out of
+ * scope).
+ */
+ for (curr_bs = bdrv_skip_rw_filters(bs);
+ bdrv_filtered_cow_child(curr_bs) != NULL;
+ curr_bs = bdrv_backing_chain_next(curr_bs))
+ {
+ BlockDriverState *bs_below = bdrv_backing_chain_next(curr_bs);
/* If either of the filename paths is actually a protocol, then
* compare unmodified paths; otherwise make paths relative */
@@ -5474,7 +5490,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
char *backing_file_full_ret;
if (strcmp(backing_file, curr_bs->backing_file) == 0) {
- retval = curr_bs->backing->bs;
+ retval = bs_below;
break;
}
/* Also check against the full backing filename for the image */
@@ -5484,7 +5500,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
bool equal = strcmp(backing_file, backing_file_full_ret) == 0;
g_free(backing_file_full_ret);
if (equal) {
- retval = curr_bs->backing->bs;
+ retval = bs_below;
break;
}
}
@@ -5510,7 +5526,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
g_free(filename_tmp);
if (strcmp(backing_file_full, filename_full) == 0) {
- retval = curr_bs->backing->bs;
+ retval = bs_below;
break;
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 09/15] block: prepare block-stream for using COR-filter
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (7 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 08/15] block: Use CAFs when working with backing chains Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 10/15] copy-on-read: Support change filename functions Andrey Shinkevich
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
This patch is the first one in the series where the COR-filter node
will be hard-coded for using in the block-stream job. The block jobs
may be run in parallel. Exclude conflicts with filter nodes used for
a concurrent job while checking for the blocked operations. It incurs
changes in the iotests 030::test_overlapping_4 that now passes with no
conflict because the stream job does not have a real dependency on its
base and on a filter above it.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
blockdev.c | 11 +++++++++--
tests/qemu-iotests/030 | 9 ++++-----
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index b3c840e..97c2ba2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2763,6 +2763,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
Error *local_err = NULL;
const char *base_name = NULL;
int job_flags = JOB_DEFAULT;
+ BlockDriverState *bottom_cow_node;
if (!has_on_error) {
on_error = BLOCKDEV_ON_ERROR_REPORT;
@@ -2807,8 +2808,14 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
base_name = base_bs->filename;
}
- /* Check for op blockers in the whole chain between bs and base */
- for (iter = bs; iter && iter != base_bs; iter = backing_bs(iter)) {
+ bottom_cow_node = bdrv_find_overlay(bs, base_bs);
+ if (!bottom_cow_node) {
+ error_setg(errp, "bottom node is not found, nothing to stream");
+ goto out;
+ }
+ /* Check for op blockers in the whole chain between bs and bottom */
+ for (iter = bs; iter && iter != bdrv_filtered_bs(bottom_cow_node);
+ iter = bdrv_filtered_bs(iter)) {
if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_STREAM, errp)) {
goto out;
}
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 104e3ce..d7638cd 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -368,8 +368,7 @@ class TestParallelOps(iotests.QMPTestCase):
self.wait_until_completed(drive='commit-drive0')
# In this case the base node of the stream job is the same as the
- # top node of commit job. Since this results in the commit filter
- # node being part of the stream chain, this is not allowed.
+ # top node of commit job.
def test_overlapping_4(self):
self.assert_no_active_block_jobs()
@@ -381,13 +380,13 @@ class TestParallelOps(iotests.QMPTestCase):
# Stream from node2 into node4
result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='node4')
- self.assert_qmp(result, 'error/desc',
- "Cannot freeze 'backing' link to 'commit-filter'")
+ self.assert_qmp(result, 'return', {})
result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
self.assert_qmp(result, 'return', {})
- self.wait_until_completed()
+ self.vm.run_job(job='drive0', auto_dismiss=True)
+ self.vm.run_job(job='node4', auto_dismiss=True)
self.assert_no_active_block_jobs()
# In this case the base node of the stream job is the commit job's
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 10/15] copy-on-read: Support change filename functions
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (8 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 09/15] block: prepare block-stream for using COR-filter Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 11/15] copy-on-read: Support preadv/pwritev_part functions Andrey Shinkevich
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
The COR-filter driver should support a redirecting function to refresh
filenames. Otherwise, a file name of the filter will be copied
instead of the one of a data node. It is also true for the function
bdrv_change_backing_file().
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/copy-on-read.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index c4fa468..74e01ee 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -21,6 +21,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include "block/block_int.h"
#include "qemu/module.h"
@@ -128,6 +129,21 @@ static void cor_lock_medium(BlockDriverState *bs, bool locked)
}
+static void cor_refresh_filename(BlockDriverState *bs)
+{
+ pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
+ bs->file->bs->filename);
+}
+
+
+static int cor_change_backing_file(BlockDriverState *bs,
+ const char *backing_file,
+ const char *backing_fmt)
+{
+ return bdrv_change_backing_file(bs->file->bs, backing_file, backing_fmt);
+}
+
+
static BlockDriver bdrv_copy_on_read = {
.format_name = "copy-on-read",
@@ -146,6 +162,8 @@ static BlockDriver bdrv_copy_on_read = {
.bdrv_lock_medium = cor_lock_medium,
.bdrv_co_block_status = bdrv_co_block_status_from_file,
+ .bdrv_refresh_filename = cor_refresh_filename,
+ .bdrv_change_backing_file = cor_change_backing_file,
.has_variable_length = true,
.is_filter = true,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 11/15] copy-on-read: Support preadv/pwritev_part functions
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (9 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 10/15] copy-on-read: Support change filename functions Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 12/15] copy-on-read: add filter append/drop functions Andrey Shinkevich
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
Add support for the recently introduced functions
bdrv_co_preadv_part()
and
bdrv_co_pwritev_part()
to the COR-filter driver.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/copy-on-read.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 74e01ee..4030d56 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -74,21 +74,25 @@ static int64_t cor_getlength(BlockDriverState *bs)
}
-static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+static int coroutine_fn cor_co_preadv_part(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov,
+ size_t qiov_offset,
+ int flags)
{
- return bdrv_co_preadv(bs->file, offset, bytes, qiov,
- flags | BDRV_REQ_COPY_ON_READ);
+ return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
+ flags | BDRV_REQ_COPY_ON_READ);
}
-static int coroutine_fn cor_co_pwritev(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
- QEMUIOVector *qiov, int flags)
+static int coroutine_fn cor_co_pwritev_part(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov,
+ size_t qiov_offset, int flags)
{
-
- return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
+ return bdrv_co_pwritev_part(bs->file, offset, bytes, qiov, qiov_offset,
+ flags);
}
@@ -152,8 +156,8 @@ static BlockDriver bdrv_copy_on_read = {
.bdrv_getlength = cor_getlength,
- .bdrv_co_preadv = cor_co_preadv,
- .bdrv_co_pwritev = cor_co_pwritev,
+ .bdrv_co_preadv_part = cor_co_preadv_part,
+ .bdrv_co_pwritev_part = cor_co_pwritev_part,
.bdrv_co_pwrite_zeroes = cor_co_pwrite_zeroes,
.bdrv_co_pdiscard = cor_co_pdiscard,
.bdrv_co_pwritev_compressed = cor_co_pwritev_compressed,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 12/15] copy-on-read: add filter append/drop functions
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (10 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 11/15] copy-on-read: Support preadv/pwritev_part functions Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 13/15] qapi: add filter-node-name to block-stream Andrey Shinkevich
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
Provide API for the COR-filter insertion/removal.
Also, drop the filter child permissions for an inactive state when the
filter node is being removed.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/copy-on-read.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++
block/copy-on-read.h | 36 ++++++++++++++++++
2 files changed, 138 insertions(+)
create mode 100644 block/copy-on-read.h
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 4030d56..229a97c 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -24,11 +24,20 @@
#include "qemu/cutils.h"
#include "block/block_int.h"
#include "qemu/module.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "block/copy-on-read.h"
+typedef struct BDRVStateCOR {
+ bool active;
+} BDRVStateCOR;
+
static int cor_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
+ BDRVStateCOR *state = bs->opaque;
+
bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false,
errp);
if (!bs->file) {
@@ -42,6 +51,8 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
bs->file->bs->supported_zero_flags);
+ state->active = true;
+
return 0;
}
@@ -57,6 +68,17 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared)
{
+ BDRVStateCOR *s = bs->opaque;
+
+ if (!s->active) {
+ /*
+ * While the filter is being removed
+ */
+ *nperm = 0;
+ *nshared = BLK_PERM_ALL;
+ return;
+ }
+
*nperm = perm & PERM_PASSTHROUGH;
*nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
@@ -150,6 +172,7 @@ static int cor_change_backing_file(BlockDriverState *bs,
static BlockDriver bdrv_copy_on_read = {
.format_name = "copy-on-read",
+ .instance_size = sizeof(BDRVStateCOR),
.bdrv_open = cor_open,
.bdrv_child_perm = cor_child_perm,
@@ -178,4 +201,83 @@ static void bdrv_copy_on_read_init(void)
bdrv_register(&bdrv_copy_on_read);
}
+
+static BlockDriverState *create_filter_node(BlockDriverState *bs,
+ const char *filter_node_name,
+ 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 (filter_node_name) {
+ qdict_put_str(opts, "node-name", filter_node_name);
+ }
+
+ return bdrv_open(NULL, NULL, opts, BDRV_O_RDWR, errp);
+}
+
+
+BlockDriverState *bdrv_cor_filter_append(BlockDriverState *bs,
+ const char *filter_node_name,
+ Error **errp)
+{
+ BlockDriverState *cor_filter_bs;
+ BDRVStateCOR *state;
+ Error *local_err = NULL;
+
+ cor_filter_bs = create_filter_node(bs, filter_node_name, errp);
+ if (cor_filter_bs == NULL) {
+ error_prepend(errp, "Could not create filter node: ");
+ return NULL;
+ }
+
+ if (!filter_node_name) {
+ cor_filter_bs->implicit = true;
+ }
+
+ 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;
+ }
+
+ state = cor_filter_bs->opaque;
+ state->active = true;
+
+ return cor_filter_bs;
+}
+
+
+void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs)
+{
+ BdrvChild *child;
+ BlockDriverState *bs;
+ BDRVStateCOR *s = cor_filter_bs->opaque;
+
+ child = bdrv_filtered_rw_child(cor_filter_bs);
+ if (!child) {
+ return;
+ }
+ bs = child->bs;
+
+ /* Retain the BDS until we complete the graph change. */
+ bdrv_ref(bs);
+ /* Hold a guest back from writing while permissions are being reset. */
+ bdrv_drained_begin(bs);
+ /* Drop permissions before the graph change. */
+ s->active = false;
+ bdrv_child_refresh_perms(cor_filter_bs, child, &error_abort);
+ bdrv_replace_node(cor_filter_bs, bs, &error_abort);
+
+ bdrv_drained_end(bs);
+ bdrv_unref(bs);
+ bdrv_unref(cor_filter_bs);
+}
+
+
block_init(bdrv_copy_on_read_init);
diff --git a/block/copy-on-read.h b/block/copy-on-read.h
new file mode 100644
index 0000000..db03c6c
--- /dev/null
+++ b/block/copy-on-read.h
@@ -0,0 +1,36 @@
+/*
+ * Copy-on-read filter block driver
+ *
+ * The filter driver performs Copy-On-Read (COR) operations
+ *
+ * Copyright (c) 2018-2020 Virtuozzo International GmbH.
+ *
+ * Author:
+ * Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef COPY_ON_READ_FILTER
+#define COPY_ON_READ_FILTER
+
+#include "block/block_int.h"
+#include "block/block-copy.h"
+
+BlockDriverState *bdrv_cor_filter_append(BlockDriverState *bs,
+ const char *filter_node_name,
+ Error **errp);
+void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs);
+
+#endif /* COPY_ON_READ_FILTER */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 13/15] qapi: add filter-node-name to block-stream
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (11 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 12/15] copy-on-read: add filter append/drop functions Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 14/15] iotests: prepare 245 for using filter in block-stream Andrey Shinkevich
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
Provide the possibility to pass the 'filter-node-name' parameter to the
block-stream job as it is done for the commit block job. That will be
needed for further iotests implementations.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/monitor/block-hmp-cmds.c | 4 ++--
block/stream.c | 4 +++-
blockdev.c | 8 +++++++-
include/block/block_int.h | 7 ++++++-
qapi/block-core.json | 6 ++++++
5 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 4c8c375..6f6636b 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -507,8 +507,8 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
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);
+ BLOCKDEV_ON_ERROR_REPORT, false, NULL, false, false, false,
+ false, &error);
hmp_handle_error(mon, error);
}
diff --git a/block/stream.c b/block/stream.c
index 3cca1f9..e0b35f8 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -217,7 +217,9 @@ 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,
- BlockdevOnError on_error, Error **errp)
+ BlockdevOnError on_error,
+ const char *filter_node_name,
+ Error **errp)
{
StreamBlockJob *s;
BlockDriverState *iter;
diff --git a/blockdev.c b/blockdev.c
index 97c2ba2..67918fc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2753,6 +2753,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
bool has_backing_file, const char *backing_file,
bool has_speed, int64_t speed,
bool has_on_error, BlockdevOnError on_error,
+ bool has_filter_node_name, const char *filter_node_name,
bool has_auto_finalize, bool auto_finalize,
bool has_auto_dismiss, bool auto_dismiss,
Error **errp)
@@ -2769,6 +2770,10 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
on_error = BLOCKDEV_ON_ERROR_REPORT;
}
+ if (!has_filter_node_name) {
+ filter_node_name = NULL;
+ }
+
bs = bdrv_lookup_bs(device, device, errp);
if (!bs) {
return;
@@ -2840,7 +2845,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, on_error,
+ filter_node_name, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 86f7666..0fdfa9c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1094,6 +1094,9 @@ int is_windows_drive(const char *filename);
* See @BlockJobCreateFlags
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
* @on_error: The action to take upon error.
+ * @filter_node_name: The node name that should be assigned to the filter
+ * driver that the commit job inserts into the graph above @bs. NULL means
+ * that a node name should be autogenerated.
* @errp: Error object.
*
* Start a streaming operation on @bs. Clusters that are unallocated
@@ -1106,7 +1109,9 @@ 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,
- BlockdevOnError on_error, Error **errp);
+ BlockdevOnError on_error,
+ const char *filter_node_name,
+ Error **errp);
/**
* commit_start:
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 943df19..df6981e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2509,6 +2509,11 @@
# 'stop' and 'enospc' can only be used if the block device
# supports io-status (see BlockInfo). Since 1.3.
#
+# @filter-node-name: the node name that should be assigned to the
+# filter driver that the stream job inserts into the graph
+# above @device. If this option is not given, a node name is
+# autogenerated. (Since: 5.1)
+#
# @auto-finalize: When false, this job will wait in a PENDING state after it has
# finished its work, waiting for @block-job-finalize before
# making any block graph changes.
@@ -2539,6 +2544,7 @@
'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
'*base-node': 'str', '*backing-file': 'str', '*speed': 'int',
'*on-error': 'BlockdevOnError',
+ '*filter-node-name': 'str',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
##
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 14/15] iotests: prepare 245 for using filter in block-stream
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (12 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 13/15] qapi: add filter-node-name to block-stream Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 15/15] block: apply COR-filter to block-stream jobs Andrey Shinkevich
2020-06-01 13:57 ` [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
The preliminary patch modifies the test 245 to prepare the block-stream
job for using COR-filter. The filter breaks the backing chain being
connected to the underlying node by file child link.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/245 | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 4f5f0bb..5bcddc4 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -896,15 +896,19 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# 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.)
- self.reopen(opts, {},
+ opts = hd_opts(1)
+ opts['backing'] = hd_opts(2)
+ opts['backing']['backing'] = None
+ self.reopen(opts, {'read-only': True},
["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"])
# 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'")
+ opts['backing'] = None
+ self.reopen(opts, {'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 = hd_opts(0)
opts['backing'] = None
self.reopen(opts)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v5 15/15] block: apply COR-filter to block-stream jobs
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (13 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 14/15] iotests: prepare 245 for using filter in block-stream Andrey Shinkevich
@ 2020-05-13 9:50 ` Andrey Shinkevich
2020-06-01 13:57 ` [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-05-13 9:50 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, vsementsov, armbru, qemu-devel, stefanha,
andrey.shinkevich, den, mreitz, jsnow
The patch completes the series with the COR-filter insertion to any
block-stream operation. It also makes changes to the iotests 030 and
141.out.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/stream.c | 83 ++++++++++++++++++++++++++++++++--------------
tests/qemu-iotests/030 | 8 +++--
tests/qemu-iotests/141.out | 2 +-
3 files changed, 64 insertions(+), 29 deletions(-)
diff --git a/block/stream.c b/block/stream.c
index e0b35f8..a74a07b 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 {
/*
@@ -32,8 +33,11 @@ enum {
typedef struct StreamBlockJob {
BlockJob common;
BlockDriverState *bottom;
+ 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;
@@ -52,33 +56,25 @@ static void stream_abort(Job *job)
StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
if (s->chain_frozen) {
- BlockJob *bjob = &s->common;
- bdrv_unfreeze_chain(blk_bs(bjob->blk), s->bottom);
+ bdrv_unfreeze_chain(s->cor_filter_bs, s->bottom);
}
}
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 *base = backing_bs(s->bottom);
Error *local_err = NULL;
int ret = 0;
- bdrv_unfreeze_chain(bs, s->bottom);
+ bdrv_unfreeze_chain(s->cor_filter_bs, s->bottom);
s->chain_frozen = false;
if (bs->backing) {
- 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(bs, base, &local_err);
- ret = bdrv_change_backing_file(bs, base_id, base_fmt);
+ ret = bdrv_change_backing_file(bs, s->backing_file_str,
+ s->base_fmt);
if (local_err) {
error_report_err(local_err);
return -EPERM;
@@ -92,7 +88,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) {
@@ -102,13 +100,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;
bool enable_cor = !backing_bs(s->bottom);
int64_t len;
int64_t offset = 0;
@@ -156,8 +155,8 @@ static int coroutine_fn stream_run(Job *job, Error **errp)
} else if (ret >= 0) {
/* Copy if allocated in the intermediate images. Limit to the
* known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE). */
- ret = bdrv_is_allocated_above(backing_bs(bs), s->bottom, true,
- offset, n, &n);
+ ret = bdrv_is_allocated_above(bdrv_filtered_cow_bs(bs), s->bottom,
+ true, offset, n, &n);
/* Finish early if end of backing file has been reached */
if (ret == 0 && n == 0) {
n = len - offset;
@@ -225,7 +224,13 @@ void stream_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *iter;
bool bs_read_only;
int basic_flags = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED;
+ BlockDriverState *cor_filter_bs = NULL;
BlockDriverState *bottom = bdrv_find_overlay(bs, base);
+ char *base_fmt = NULL;
+
+ if (base && base->drv) {
+ base_fmt = g_strdup(base->drv->format_name);
+ }
if (bdrv_freeze_chain(bs, bottom, errp) < 0) {
return;
@@ -240,17 +245,35 @@ 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_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
@@ -259,13 +282,17 @@ void stream_start(const char *job_id, BlockDriverState *bs,
* due to parallel block jobs running.
*/
base = backing_bs(bottom);
- for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) {
+ for (iter = bdrv_filtered_bs(bs); iter && iter != base;
+ iter = bdrv_filtered_bs(iter)) {
block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
basic_flags, &error_abort);
}
+ s->cor_filter_bs = cor_filter_bs;
+ s->target_bs = bs;
s->bottom = bottom;
s->backing_file_str = g_strdup(backing_file_str);
+ s->base_fmt = base_fmt;
s->bs_read_only = bs_read_only;
s->chain_frozen = true;
@@ -278,5 +305,11 @@ fail:
if (bs_read_only) {
bdrv_reopen_set_read_only(bs, true, NULL);
}
- bdrv_unfreeze_chain(bs, bottom);
+
+ if (cor_filter_bs) {
+ bdrv_unfreeze_chain(cor_filter_bs, bottom);
+ bdrv_cor_filter_drop(cor_filter_bs);
+ } else {
+ bdrv_unfreeze_chain(bs, bottom);
+ }
}
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index d7638cd..9856a08 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -269,12 +269,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',
@@ -287,7 +289,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 263b680..cf18558 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"}}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v5 00/15] Apply COR-filter to the block-stream permanently
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
` (14 preceding siblings ...)
2020-05-13 9:50 ` [PATCH v5 15/15] block: apply COR-filter to block-stream jobs Andrey Shinkevich
@ 2020-06-01 13:57 ` Andrey Shinkevich
15 siblings, 0 replies; 17+ messages in thread
From: Andrey Shinkevich @ 2020-06-01 13:57 UTC (permalink / raw)
To: qemu-block
Cc: kwolf, fam, Vladimir Sementsov-Ogievskiy, Denis Lunev, armbru,
qemu-devel, stefanha, mreitz, jsnow
[-- Attachment #1: Type: text/plain, Size: 4263 bytes --]
PINGING...
Please
________________________________
From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Sent: Wednesday, May 13, 2020 12:50 PM
To: qemu-block@nongnu.org <qemu-block@nongnu.org>
Cc: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; kwolf@redhat.com <kwolf@redhat.com>; mreitz@redhat.com <mreitz@redhat.com>; armbru@redhat.com <armbru@redhat.com>; fam@euphon.net <fam@euphon.net>; jsnow@redhat.com <jsnow@redhat.com>; stefanha@redhat.com <stefanha@redhat.com>; eblake@redhat.com <eblake@redhat.com>; Denis Lunev <den@virtuozzo.com>; Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>; Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Subject: [PATCH v5 00/15] Apply COR-filter to the block-stream permanently
With this series, all the block-stream COR operations pass through
the COR-filter. The patches 01-08/15 are taken from the series
"block: Deal with filters" by Max Reitz, the full version of that
can be found in the branches:
https://git.xanclic.moe/XanClic/qemu child-access-functions-v6
https://github.com/XanClic/qemu child-access-functions-v6
When running iotests, apply "char-socket: Fix race condition"
to avoid sporadic segmentation faults.
v5:
01: Included forgotten file block/copy-on-read.h
v4:
01: Initialization of the COR-filter BDRVStateCOR member.
v3:
01: The COR filter insert/remove functions moved to block/copy-on-read.c
to be a part of API.
02: block/stream.c code refactoring.
03: The separate call to block_job_add_bdrv() is used to block operations
on the active node after the filter inserted and the job created.
04: The iotests case 030::test_overlapping_4 was modified to unbound
the block-stream job from the base node.
05: The COR driver functions preadv/pwritev replaced with their analogous
preadv/pwritev_part.
v2:
01: No more skipping filters while checking for operation blockers.
However, we exclude filters between the bottom node and base
because we do not set the operation blockers for filters anymore.
02: As stated above, we do not set the operation blockers for filters
anymore. So, skip filters when we block operations for the target
node.
03: The comment added for the patch 4/7.
04: The QAPI target version changed to 5.1.
05: The 'filter-node-name' now excluded from using in the test #030.
If we need it no more in a final version of the series, the patch
5/7 may be removed.
06: The COR-filter included into the frozen chain of a block-stream job.
The 'above_base' node pointer is left because it is essential for
finding the base node in case of filters above.
Andrey Shinkevich (7):
block: prepare block-stream for using COR-filter
copy-on-read: Support change filename functions
copy-on-read: Support preadv/pwritev_part functions
copy-on-read: add filter append/drop functions
qapi: add filter-node-name to block-stream
iotests: prepare 245 for using filter in block-stream
block: apply COR-filter to block-stream jobs
Max Reitz (8):
block: Mark commit and mirror as filter drivers
copy-on-read: Support compressed writes
block: Add child access functions
block: Add chain helper functions
block: Include filters when freezing backing chain
block: Use CAFs in block status functions
commit: Deal with filters when blocking intermediate nodes
block: Use CAFs when working with backing chains
block.c | 275 ++++++++++++++++++++++++++++++++++-------
block/commit.c | 85 ++++++++++---
block/copy-on-read.c | 159 ++++++++++++++++++++++--
block/copy-on-read.h | 36 ++++++
block/io.c | 19 +--
block/mirror.c | 6 +-
block/monitor/block-hmp-cmds.c | 4 +-
block/stream.c | 89 +++++++++----
blockdev.c | 19 ++-
include/block/block.h | 6 +-
include/block/block_int.h | 67 +++++++++-
qapi/block-core.json | 6 +
tests/qemu-iotests/030 | 17 +--
tests/qemu-iotests/141.out | 2 +-
tests/qemu-iotests/245 | 10 +-
15 files changed, 661 insertions(+), 139 deletions(-)
create mode 100644 block/copy-on-read.h
--
1.8.3.1
[-- Attachment #2: Type: text/html, Size: 7306 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-06-01 13:58 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 9:50 [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 01/15] block: Mark commit and mirror as filter drivers Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 02/15] copy-on-read: Support compressed writes Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 03/15] block: Add child access functions Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 04/15] block: Add chain helper functions Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 05/15] block: Include filters when freezing backing chain Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 06/15] block: Use CAFs in block status functions Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 07/15] commit: Deal with filters when blocking intermediate nodes Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 08/15] block: Use CAFs when working with backing chains Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 09/15] block: prepare block-stream for using COR-filter Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 10/15] copy-on-read: Support change filename functions Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 11/15] copy-on-read: Support preadv/pwritev_part functions Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 12/15] copy-on-read: add filter append/drop functions Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 13/15] qapi: add filter-node-name to block-stream Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 14/15] iotests: prepare 245 for using filter in block-stream Andrey Shinkevich
2020-05-13 9:50 ` [PATCH v5 15/15] block: apply COR-filter to block-stream jobs Andrey Shinkevich
2020-06-01 13:57 ` [PATCH v5 00/15] Apply COR-filter to the block-stream permanently Andrey Shinkevich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.