From patchwork Wed Jun 4 13:51:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Cody X-Patchwork-Id: 356037 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 99772140092 for ; Thu, 5 Jun 2014 02:56:25 +1000 (EST) Received: from localhost ([::1]:33592 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsBge-0003qX-N1 for incoming@patchwork.ozlabs.org; Wed, 04 Jun 2014 09:57:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48810) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsBbR-0005ES-8t for qemu-devel@nongnu.org; Wed, 04 Jun 2014 09:51:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsBbL-0005TO-5x for qemu-devel@nongnu.org; Wed, 04 Jun 2014 09:51:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18662) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsBbK-0005TG-RC for qemu-devel@nongnu.org; Wed, 04 Jun 2014 09:51:35 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s54DpW27029427 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 4 Jun 2014 09:51:32 -0400 Received: from localhost (ovpn-112-28.phx2.redhat.com [10.3.112.28]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s54DpTd4007005 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Wed, 4 Jun 2014 09:51:31 -0400 From: Jeff Cody To: qemu-devel@nongnu.org Date: Wed, 4 Jun 2014 09:51:08 -0400 Message-Id: <5f59341336f3bbdf8e52413d2266f79a99ae7da8.1401886089.git.jcody@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, benoit.canet@irqsave.net, pkrempa@redhat.com, famz@redhat.com, stefanha@redhat.com Subject: [Qemu-devel] [PATCH v4 06/10] block: extend block-commit to accept a string for the backing file X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On some image chains, QEMU may not always be able to resolve the filenames properly, when updating the backing file of an image after a block commit. For instance, certain relative pathnames may fail, or drives may have been specified originally by file descriptor (e.g. /dev/fd/???), or a relative protocol pathname may have been used. In these instances, QEMU may lack the information to be able to make the correct choice, but the user or management layer most likely does have that knowledge. With this extension to the block-commit api, the user is able to change the backing file of the overlay image as part of the block-commit operation. This allows the change to be 'safe', in the sense that if the attempt to write the overlay image metadata fails, then the block-commit operation returns failure, without disrupting the guest. If the commit top is the active layer, then specifying the backing file string will be treated as an error (there is no overlay image to modify in that case). If a backing file string is not specified in the command, the backing file string to use is determined in the same manner as it was previously. Reviewed-by: Eric Blake Signed-off-by: Jeff Cody --- block.c | 8 ++++++-- block/commit.c | 9 ++++++--- blockdev.c | 8 +++++++- include/block/block.h | 3 ++- include/block/block_int.h | 3 ++- qapi-schema.json | 23 +++++++++++++++++++++-- qmp-commands.hx | 19 ++++++++++++++++++- 7 files changed, 62 insertions(+), 11 deletions(-) diff --git a/block.c b/block.c index 6b8fbe5..ccff942 100644 --- a/block.c +++ b/block.c @@ -2608,12 +2608,15 @@ typedef struct BlkIntermediateStates { * * base <- active * + * If backing_file_str is non-NULL, it will be used when modifying top's + * overlay image metadata. + * * Error conditions: * if active == top, that is considered an error * */ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, - BlockDriverState *base) + BlockDriverState *base, const char *backing_file_str) { BlockDriverState *intermediate; BlockDriverState *base_bs = NULL; @@ -2665,7 +2668,8 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, } /* success - we can delete the intermediate states, and link top->base */ - ret = bdrv_change_backing_file(new_top_bs, base_bs->filename, + backing_file_str = backing_file_str ? backing_file_str : base_bs->filename; + ret = bdrv_change_backing_file(new_top_bs, backing_file_str, base_bs->drv ? base_bs->drv->format_name : ""); if (ret) { goto exit; diff --git a/block/commit.c b/block/commit.c index 5c09f44..91517d3 100644 --- a/block/commit.c +++ b/block/commit.c @@ -37,6 +37,7 @@ typedef struct CommitBlockJob { BlockdevOnError on_error; int base_flags; int orig_overlay_flags; + char *backing_file_str; } CommitBlockJob; static int coroutine_fn commit_populate(BlockDriverState *bs, @@ -141,7 +142,7 @@ wait: if (!block_job_is_cancelled(&s->common) && sector_num == end) { /* success */ - ret = bdrv_drop_intermediate(active, top, base); + ret = bdrv_drop_intermediate(active, top, base, s->backing_file_str); } exit_free_buf: @@ -158,7 +159,7 @@ exit_restore_reopen: if (overlay_bs && s->orig_overlay_flags != bdrv_get_flags(overlay_bs)) { bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL); } - + g_free(s->backing_file_str); block_job_completed(&s->common, ret); } @@ -182,7 +183,7 @@ static const BlockJobDriver commit_job_driver = { void commit_start(BlockDriverState *bs, BlockDriverState *base, BlockDriverState *top, int64_t speed, BlockdevOnError on_error, BlockDriverCompletionFunc *cb, - void *opaque, Error **errp) + void *opaque, const char *backing_file_str, Error **errp) { CommitBlockJob *s; BlockReopenQueue *reopen_queue = NULL; @@ -244,6 +245,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, s->base_flags = orig_base_flags; s->orig_overlay_flags = orig_overlay_flags; + s->backing_file_str = g_strdup(backing_file_str); + s->on_error = on_error; s->common.co = qemu_coroutine_create(commit_run); diff --git a/blockdev.c b/blockdev.c index 50d4890..7a97b7f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1918,6 +1918,7 @@ void qmp_block_commit(const char *device, bool has_base_node_name, const char *base_node_name, bool has_top, const char *top, bool has_top_node_name, const char *top_node_name, + bool has_backing_file, const char *backing_file, bool has_speed, int64_t speed, Error **errp) { @@ -2009,11 +2010,16 @@ void qmp_block_commit(const char *device, } if (top_bs == bs) { + if (has_backing_file) { + error_setg(errp, "'backing-file' specified," + " but 'top' is the active layer"); + return; + } commit_active_start(bs, base_bs, speed, on_error, block_job_cb, bs, &local_err); } else { commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs, - &local_err); + has_backing_file ? backing_file : NULL, &local_err); } if (local_err != NULL) { error_propagate(errp, local_err); diff --git a/include/block/block.h b/include/block/block.h index 4dc68be..c07a85f 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -289,7 +289,8 @@ int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, const char *backing_fmt); void bdrv_register(BlockDriver *bdrv); int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, - BlockDriverState *base); + BlockDriverState *base, + const char *backing_file_str); BlockDriverState *bdrv_find_overlay(BlockDriverState *active, BlockDriverState *bs); BlockDriverState *bdrv_find_base(BlockDriverState *bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index f2e753f..87c2e02 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -436,13 +436,14 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base, * @on_error: The action to take upon error. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. + * @backing_file_str: String to use as the backing file in @top's overlay * @errp: Error object. * */ void commit_start(BlockDriverState *bs, BlockDriverState *base, BlockDriverState *top, int64_t speed, BlockdevOnError on_error, BlockDriverCompletionFunc *cb, - void *opaque, Error **errp); + void *opaque, const char *backing_file_str, Error **errp); /** * commit_active_start: * @bs: Active block device to be committed. diff --git a/qapi-schema.json b/qapi-schema.json index 67a627f..b449aec 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2121,6 +2121,23 @@ # topmost data to be committed down. # (Since 2.1) # +# @backing-file: #optional 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. +# +# 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 +# there is no obvious choice. Care should be taken +# when specifying the string, to specify a valid +# 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 @@ -2133,7 +2150,6 @@ # size of the smaller top, you can safely truncate it # yourself once the commit operation successfully completes. # -# # @speed: #optional the maximum speed, in bytes per second # # Returns: Nothing on success @@ -2144,13 +2160,16 @@ # If @speed is invalid, InvalidParameter # If both @base and @base-node-name are specified, GenericError # If both @top and @top-node-name are specified, GenericError +# If @top or @top-node-name is the active layer, and @backing-file is +# specified, GenericError # # Since: 1.3 # ## { 'command': 'block-commit', 'data': { 'device': 'str', '*base': 'str', '*base-node-name': 'str', - '*top': 'str', '*top-node-name': 'str', '*speed': 'int' } } + '*top': 'str', '*top-node-name': 'str', '*backing-file': 'str', + '*speed': 'int' } } ## # @drive-backup diff --git a/qmp-commands.hx b/qmp-commands.hx index c61f4cb..1014b68 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -985,7 +985,7 @@ EQMP { .name = "block-commit", - .args_type = "device:B,base:s?,base-node-name:s?,top:s?,top-node-name:s?,speed:o?", + .args_type = "device:B,base:s?,base-node-name:s?,top:s?,top-node-name:s?,backing-file:s?,speed:o?", .mhandler.cmd_new = qmp_marshal_input_block_commit, }, @@ -1021,6 +1021,23 @@ neither is specified, this is the active layer topmost data to be committed down. (json-string, optional) (Since 2.1) +- 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. + + 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 + there is no obvious choice. Care should be taken + when specifying the string, to specify a valid + filename or protocol. + (json-string, optional) (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