[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v8 35/43] blockdev: Fix active commit choice
From: |
Max Reitz |
Subject: |
[PATCH v8 35/43] blockdev: Fix active commit choice |
Date: |
Tue, 1 Sep 2020 16:34:16 +0200 |
We have to perform an active commit whenever the top node has a parent
that has taken the WRITE permission on it.
This means that block-commit's @backing-file parameter is longer allowed
for such nodes, and that users will have to issue a block-job-complete
command. Neither should pose a problem in practice, because this case
was basically just broken until now.
(Since this commit already touches block-commit's documentation, it also
moves up the chunk explaining general block-commit behavior that for
some reason was situated under @backing-file.)
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 39 ++++++++++++++++++++-------------------
blockdev.c | 35 ++++++++++++++++++++++++++++++-----
2 files changed, 50 insertions(+), 24 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8d180b584c..32d4aa0ddc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1569,6 +1569,18 @@
# Live commit of data from overlay image nodes into backing nodes - i.e.,
# writes data between 'top' and 'base' into 'base'.
#
+# If top == base, that is an error.
+# If top has no overlays on top of it, or if it is in use by a writer,
+# the job will not be completed by itself. The user needs to complete
+# the job with the block-job-complete command after getting the ready
+# event. (Since 2.0)
+#
+# If the base image is smaller than top, then the base image will be
+# resized to be the same size as top. If top is smaller than the base
+# image, the base will not be truncated. If you want the base image
+# size to match the size of the smaller top, you can safely truncate
+# it yourself once the commit operation successfully completes.
+#
# @job-id: identifier for the newly-created block job. If
# omitted, the device name will be used. (Since 2.7)
#
@@ -1593,14 +1605,15 @@
# accepted
#
# @backing-file: The backing file string to write into the overlay
-# image of 'top'. If 'top' is the active layer,
-# specifying a backing file string is an error. This
-# filename is not validated.
+# image of 'top'. If 'top' does not have an overlay
+# image, or if 'top' is in use by a writer, specifying
+# a backing file string is an error.
#
-# If a pathname string is such that it cannot be
-# resolved by QEMU, that means that subsequent QMP or
-# HMP commands must use node-names for the image in
-# question, as filename lookup methods will fail.
+# This filename is not validated. If a pathname string
+# is such that it cannot be resolved by QEMU, that
+# means that subsequent QMP or HMP commands must use
+# node-names for the image in question, as filename
+# lookup methods will fail.
#
# If not specified, QEMU will automatically determine
# the backing file string to use, or error out if
@@ -1609,18 +1622,6 @@
# filename or protocol.
# (Since 2.1)
#
-# If top == base, that is an error.
-# If top == active, the job will not be completed by itself,
-# user needs to complete the job with the block-job-complete
-# command after getting the ready event. (Since 2.0)
-#
-# If the base image is smaller than top, then the base image
-# will be resized to be the same size as top. If top is
-# smaller than the base image, the base will not be
-# truncated. If you want the base image size to match the
-# size of the smaller top, you can safely truncate it
-# yourself once the commit operation successfully completes.
-#
# @speed: the maximum speed, in bytes per second
#
# @on-error: the action to take on an error. 'ignore' means that the request
diff --git a/blockdev.c b/blockdev.c
index f308adc9aa..7f2561081e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2602,6 +2602,7 @@ void qmp_block_commit(bool has_job_id, const char
*job_id, const char *device,
AioContext *aio_context;
Error *local_err = NULL;
int job_flags = JOB_DEFAULT;
+ uint64_t top_perm, top_shared;
if (!has_speed) {
speed = 0;
@@ -2717,14 +2718,38 @@ void qmp_block_commit(bool has_job_id, const char
*job_id, const char *device,
goto out;
}
- if (top_bs == bs) {
+ /*
+ * Active commit is required if and only if someone has taken a
+ * WRITE permission on the top node. Historically, we have always
+ * used active commit for top nodes, so continue that practice
+ * lest we possibly break clients that rely on this behavior, e.g.
+ * to later attach this node to a writing parent.
+ * (Active commit is never really wrong.)
+ */
+ bdrv_get_cumulative_perm(top_bs, &top_perm, &top_shared);
+ if (top_perm & BLK_PERM_WRITE ||
+ bdrv_skip_filters(top_bs) == bdrv_skip_filters(bs))
+ {
if (has_backing_file) {
- error_setg(errp, "'backing-file' specified,"
- " but 'top' is the active layer");
+ if (bdrv_skip_filters(top_bs) == bdrv_skip_filters(bs)) {
+ error_setg(errp, "'backing-file' specified,"
+ " but 'top' is the active layer");
+ } else {
+ error_setg(errp, "'backing-file' specified, but 'top' has a "
+ "writer on it");
+ }
goto out;
}
- commit_active_start(has_job_id ? job_id : NULL, bs, base_bs,
- job_flags, speed, on_error,
+ if (!has_job_id) {
+ /*
+ * Emulate here what block_job_create() does, because it
+ * is possible that @bs != @top_bs (the block job should
+ * be named after @bs, even if @top_bs is the actual
+ * source)
+ */
+ job_id = bdrv_get_device_name(bs);
+ }
+ commit_active_start(job_id, top_bs, base_bs, job_flags, speed,
on_error,
filter_node_name, NULL, NULL, false, &local_err);
} else {
BlockDriverState *overlay_bs = bdrv_find_overlay(bs, top_bs);
--
2.26.2
- [PATCH v8 27/43] block: Use child access functions for QAPI queries, (continued)
- [PATCH v8 27/43] block: Use child access functions for QAPI queries, Max Reitz, 2020/09/01
- [PATCH v8 28/43] block-copy: Use CAF to find sync=top base, Max Reitz, 2020/09/01
- [PATCH v8 29/43] mirror: Deal with filters, Max Reitz, 2020/09/01
- [PATCH v8 30/43] backup: Deal with filters, Max Reitz, 2020/09/01
- [PATCH v8 31/43] commit: Deal with filters, Max Reitz, 2020/09/01
- [PATCH v8 32/43] nbd: Use CAF when looking for dirty bitmap, Max Reitz, 2020/09/01
- [PATCH v8 33/43] qemu-img: Use child access functions, Max Reitz, 2020/09/01
- [PATCH v8 34/43] block: Drop backing_bs(), Max Reitz, 2020/09/01
- [PATCH v8 35/43] blockdev: Fix active commit choice,
Max Reitz <=
- [PATCH v8 36/43] block: Inline bdrv_co_block_status_from_*(), Max Reitz, 2020/09/01
- [PATCH v8 37/43] block: Leave BDS.backing_{file,format} constant, Max Reitz, 2020/09/01
- [PATCH v8 38/43] iotests: Test that qcow2's data-file is flushed, Max Reitz, 2020/09/01
- [PATCH v8 39/43] iotests: Let complete_and_wait() work with commit, Max Reitz, 2020/09/01
- [PATCH v8 41/43] iotests: Add filter mirror test cases, Max Reitz, 2020/09/01
- [PATCH v8 40/43] iotests: Add filter commit test cases, Max Reitz, 2020/09/01
- [PATCH v8 42/43] iotests: Add test for commit in sub directory, Max Reitz, 2020/09/01
- [PATCH v8 43/43] iotests: Test committing to overridden backing, Max Reitz, 2020/09/01
- Re: [PATCH v8 00/43] block: Deal with filters, Kevin Wolf, 2020/09/02