From: Max Reitz <mre...@redhat.com>
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 no 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 <mre...@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 e34796d98f..0345f6f2d2 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.25.4
