blockdev: Fix active commit choice

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 <mreitz@redhat.com>
This commit is contained in:
Max Reitz 2019-06-11 21:19:26 +02:00 committed by Kevin Wolf
parent f1a7f18f07
commit 05ea385afd
2 changed files with 50 additions and 24 deletions

View File

@ -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);

View File

@ -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