From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PATCH v8 35/43] blockdev: Fix active commit choice
Date: Tue, 1 Sep 2020 16:34:16 +0200 [thread overview]
Message-ID: <20200901143424.884735-36-mreitz@redhat.com> (raw)
In-Reply-To: <20200901143424.884735-1-mreitz@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 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
next prev parent reply other threads:[~2020-09-01 14:56 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-01 14:33 [PATCH v8 00/43] block: Deal with filters Max Reitz
2020-09-01 14:33 ` [PATCH v8 01/43] block: Add child access functions Max Reitz
2020-09-01 14:33 ` [PATCH v8 02/43] block: Add chain helper functions Max Reitz
2020-09-01 14:33 ` [PATCH v8 03/43] block: bdrv_cow_child() for bdrv_has_zero_init() Max Reitz
2020-09-01 14:33 ` [PATCH v8 04/43] block: bdrv_set_backing_hd() is about bs->backing Max Reitz
2020-09-01 14:33 ` [PATCH v8 05/43] block: Include filters when freezing backing chain Max Reitz
2020-09-01 14:33 ` [PATCH v8 06/43] block: Drop bdrv_is_encrypted() Max Reitz
2020-09-01 14:33 ` [PATCH v8 07/43] block: Add bdrv_supports_compressed_writes() Max Reitz
2020-09-01 14:33 ` [PATCH v8 08/43] throttle: Support compressed writes Max Reitz
2020-09-01 14:33 ` [PATCH v8 09/43] copy-on-read: " Max Reitz
2020-09-01 14:33 ` [PATCH v8 10/43] block: Use bdrv_filter_(bs|child) where obvious Max Reitz
2020-09-01 14:33 ` [PATCH v8 11/43] block: Use CAFs in block status functions Max Reitz
2020-09-01 14:33 ` [PATCH v8 12/43] stream: Deal with filters Max Reitz
2020-09-01 14:33 ` [PATCH v8 13/43] block: Use CAFs when working with backing chains Max Reitz
2020-09-01 14:33 ` [PATCH v8 14/43] block: Use bdrv_cow_child() in bdrv_co_truncate() Max Reitz
2020-09-01 14:33 ` [PATCH v8 15/43] block: Re-evaluate backing file handling in reopen Max Reitz
2020-09-01 14:33 ` [PATCH v8 16/43] block: Flush all children in generic code Max Reitz
2020-09-01 14:33 ` [PATCH v8 17/43] vmdk: Drop vmdk_co_flush() Max Reitz
2020-09-01 14:33 ` [PATCH v8 18/43] block: Iterate over children in refresh_limits Max Reitz
2020-09-01 14:34 ` [PATCH v8 19/43] block: Use CAFs in bdrv_refresh_filename() Max Reitz
2020-09-01 14:34 ` [PATCH v8 20/43] block: Use CAF in bdrv_co_rw_vmstate() Max Reitz
2020-09-01 14:34 ` [PATCH v8 21/43] block/snapshot: Fix fallback Max Reitz
2020-09-01 14:34 ` [PATCH v8 22/43] block: Use CAFs for debug breakpoints Max Reitz
2020-09-01 14:34 ` [PATCH v8 23/43] block: Improve get_allocated_file_size's default Max Reitz
2020-09-01 14:34 ` [PATCH v8 24/43] block/null: Implement bdrv_get_allocated_file_size Max Reitz
2020-09-01 14:34 ` [PATCH v8 25/43] blockdev: Use CAF in external_snapshot_prepare() Max Reitz
2020-09-01 14:34 ` [PATCH v8 26/43] block: Report data child for query-blockstats Max Reitz
2020-09-01 14:34 ` [PATCH v8 27/43] block: Use child access functions for QAPI queries Max Reitz
2020-09-01 14:34 ` [PATCH v8 28/43] block-copy: Use CAF to find sync=top base Max Reitz
2020-09-01 14:34 ` [PATCH v8 29/43] mirror: Deal with filters Max Reitz
2020-09-02 8:53 ` Kevin Wolf
2020-09-02 10:19 ` Max Reitz
2020-09-01 14:34 ` [PATCH v8 30/43] backup: " Max Reitz
2020-09-01 14:34 ` [PATCH v8 31/43] commit: " Max Reitz
2020-09-01 14:34 ` [PATCH v8 32/43] nbd: Use CAF when looking for dirty bitmap Max Reitz
2020-09-01 14:34 ` [PATCH v8 33/43] qemu-img: Use child access functions Max Reitz
2020-09-01 14:34 ` [PATCH v8 34/43] block: Drop backing_bs() Max Reitz
2020-09-01 14:34 ` Max Reitz [this message]
2020-09-02 9:10 ` [PATCH v8 35/43] blockdev: Fix active commit choice Kevin Wolf
2020-09-01 14:34 ` [PATCH v8 36/43] block: Inline bdrv_co_block_status_from_*() Max Reitz
2020-09-01 14:34 ` [PATCH v8 37/43] block: Leave BDS.backing_{file,format} constant Max Reitz
2020-09-01 14:34 ` [PATCH v8 38/43] iotests: Test that qcow2's data-file is flushed Max Reitz
2020-09-01 14:34 ` [PATCH v8 39/43] iotests: Let complete_and_wait() work with commit Max Reitz
2020-09-01 14:34 ` [PATCH v8 40/43] iotests: Add filter commit test cases Max Reitz
2020-09-01 14:34 ` [PATCH v8 41/43] iotests: Add filter mirror " Max Reitz
2020-09-01 14:34 ` [PATCH v8 42/43] iotests: Add test for commit in sub directory Max Reitz
2020-09-01 14:34 ` [PATCH v8 43/43] iotests: Test committing to overridden backing Max Reitz
2020-09-02 10:23 ` [PATCH v8 00/43] block: Deal with filters Kevin Wolf
2020-09-02 12:26 ` Max Reitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200901143424.884735-36-mreitz@redhat.com \
--to=mreitz@redhat.com \
--cc=andrey.shinkevich@virtuozzo.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.