From patchwork Fri May 30 17:35:25 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Cody X-Patchwork-Id: 354310 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 794D91400CF for ; Sat, 31 May 2014 04:58:10 +1000 (EST) Received: from localhost ([::1]:55795 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WqQmN-0001ZK-Ni for incoming@patchwork.ozlabs.org; Fri, 30 May 2014 13:39:43 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WqQir-0004rg-Ej for qemu-devel@nongnu.org; Fri, 30 May 2014 13:36:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WqQij-0002fp-Qc for qemu-devel@nongnu.org; Fri, 30 May 2014 13:36:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7631) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WqQij-0002fj-JR for qemu-devel@nongnu.org; Fri, 30 May 2014 13:35:57 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4UHZt0Y021979 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 30 May 2014 13:35:55 -0400 Received: from localhost (ovpn-112-54.phx2.redhat.com [10.3.112.54]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4UHZr6C007570 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Fri, 30 May 2014 13:35:54 -0400 From: Jeff Cody To: qemu-devel@nongnu.org Date: Fri, 30 May 2014 13:35:25 -0400 Message-Id: <174235c7d8defd0a71496a38a505e40ef2e1c683.1401471188.git.jcody@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 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 v3 07/12] block: Accept node-name arguments for block-commit 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 This modifies the block operation block-commit so that it will accept node-name arguments for either 'top' or 'base' BDS. The filename and node-name are mutually exclusive to each other; i.e.: "top" and "top-node-name" are mutually exclusive (enforced) "base" and "base-node-name" are mutually exclusive (enforced) The "device" argument now becomes optional as well, because with a node-name we can identify the block device chain. It is only optional if a node-name is not specified. The preferred and recommended method now of using block-commit is to specify the BDS to operate on via their node-name arguments. This also adds an explicit check that base_bs is in the chain of top_bs, because if they are referenced by their unique node-name arguments, the discovery process does not intrinsically verify they are in the same chain. Reviewed-by: Eric Blake Signed-off-by: Jeff Cody --- blockdev.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++------- qapi-schema.json | 38 +++++++++++++++++++------- qmp-commands.hx | 33 +++++++++++++++++------ 3 files changed, 125 insertions(+), 27 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8007fd9..4399cb5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1910,14 +1910,17 @@ void qmp_block_stream(const char *device, bool has_base, trace_qmp_block_stream(bs, bs->job); } -void qmp_block_commit(const char *device, +void qmp_block_commit(bool has_device, const char *device, bool has_base, const char *base, + 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_speed, int64_t speed, Error **errp) { - BlockDriverState *bs; - BlockDriverState *base_bs, *top_bs; + BlockDriverState *bs = NULL; + BlockDriverState *base_bs = NULL; + BlockDriverState *top_bs = NULL; Error *local_err = NULL; /* This will be part of the QMP command, if/when the * BlockdevOnError change for blkmirror makes it in @@ -1931,18 +1934,61 @@ void qmp_block_commit(const char *device, /* drain all i/o before commits */ bdrv_drain_all(); - bs = bdrv_find(device); - if (!bs) { - error_set(errp, QERR_DEVICE_NOT_FOUND, device); + /* argument combination validation */ + if (has_base && has_base_node_name) { + error_setg(errp, "'base' and 'base-node-name' are mutually exclusive"); + return; + } + if (has_top && has_top_node_name) { + error_setg(errp, "'top' and 'top-node-name' are mutually exclusive"); return; } - if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) { + if ((has_base || has_top) && !has_device) { + error_setg(errp, "'device' required if 'top' or 'base' specified"); + return; + } + + if (!has_top && !has_top_node_name && + !has_base && !has_base_node_name && !has_device) { + error_setg(errp, "'device' required if no node-name specified"); return; } - /* default top_bs is the active layer */ - top_bs = bs; + if (has_device) { + /* device lookups */ + bs = bdrv_find(device); + if (!bs) { + error_set(errp, QERR_DEVICE_NOT_FOUND, device); + return; + } + } + + if (has_top_node_name) { + top_bs = bdrv_lookup_bs(NULL, top_node_name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + bs = bs ?: bdrv_find_active(top_bs); + } + + if (has_base_node_name) { + base_bs = bdrv_lookup_bs(NULL, base_node_name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + bs = bs ?: bdrv_find_active(base_bs); + } + + if (!bs) { + error_setg(errp, "Could not find active layer"); + return; + } + + /* default top_bs is the active layer, if NULL */ + top_bs = top_bs ?: bs; if (has_top && top) { if (strcmp(bs->filename, top) != 0) { @@ -1966,6 +2012,23 @@ void qmp_block_commit(const char *device, return; } + /* Verify that 'base' is in the same chain as 'top' */ + if (!bdrv_chain_contains(top_bs, base_bs)) { + error_setg(errp, "'base' and 'top' are not in the same chain"); + return; + } + + /* This should technically be caught in commit_start, but + * check here for validation completeness */ + if (!bdrv_chain_contains(bs, top_bs)) { + error_setg(errp, "'%s' and 'top' are not in the same chain", device); + return; + } + + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) { + return; + } + if (top_bs == bs) { commit_active_start(bs, base_bs, speed, on_error, block_job_cb, bs, &local_err); diff --git a/qapi-schema.json b/qapi-schema.json index 97cf053..2858155 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2097,14 +2097,30 @@ # Live commit of data from overlay image nodes into backing nodes - i.e., # writes data between 'top' and 'base' into 'base'. # -# @device: the name of the device +# @device: #optional The name of the device. Optional only if +# node-names are used for both base and top. # -# @base: #optional The file name of the backing image to write data into. -# If not specified, this is the deepest backing image +# For 'base', either @base or @base-node-name may be set but not both. If +# neither is specified, this is the deepest backing image. # -# @top: #optional The file name of the backing image within the image chain, -# which contains the topmost data to be committed down. If -# not specified, this is the active layer. +# @base: #optional The file name of the backing image to write data +# into. +# +# @base-node-name #optional The name of the block driver state node of the +# backing image to write data into. +# (Since 2.1) +# +# For 'top', either @top or @top-node-name must be set but not both. If +# neither is specified, this is the active layer. +# +# @top: #optional The file name of the backing image within the image +# chain, which contains the topmost data to be +# committed down. +# +# @top-node-name: #optional The block driver state node name of the backing +# image within the image chain, which contains the +# topmost data to be committed down. +# (Since 2.1) # # If top == base, that is an error. # If top == active, the job will not be completed by itself, @@ -2123,17 +2139,19 @@ # # Returns: Nothing on success # If commit or stream is already active on this device, DeviceInUse -# If @device does not exist, DeviceNotFound +# If @device does not exist or cannot be determined, DeviceNotFound # If image commit is not supported by this device, NotSupported -# If @base or @top is invalid, a generic error is returned +# If @base, @top, @base-node-name, @top-node-name invalid, GenericError # 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 # # Since: 1.3 # ## { 'command': 'block-commit', - 'data': { 'device': 'str', '*base': 'str', '*top': 'str', - '*speed': 'int' } } + 'data': { '*device': 'str', '*base': 'str', '*base-node-name': 'str', + '*top': 'str', '*top-node-name': 'str', '*speed': 'int' } } ## # @drive-backup diff --git a/qmp-commands.hx b/qmp-commands.hx index 6b67d2f..e0b6916 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -985,7 +985,7 @@ EQMP { .name = "block-commit", - .args_type = "device:B,base:s?,top:s?,speed:o?", + .args_type = "device:B?,base:s?,base-node-name:s?,top:s?,top-node-name:s?,speed:o?", .mhandler.cmd_new = qmp_marshal_input_block_commit, }, @@ -998,13 +998,30 @@ data between 'top' and 'base' into 'base'. Arguments: -- "device": The device's ID, must be unique (json-string) -- "base": The file name of the backing image to write data into. - If not specified, this is the deepest backing image - (json-string, optional) -- "top": The file name of the backing image within the image chain, - which contains the topmost data to be committed down. If - not specified, this is the active layer. (json-string, optional) +- "device": The device's ID, must be unique (json-string, optional) + Optional only if node-names are used to identify + "top" and "base" + +For 'base', either base or base-node-name may be set but not both. If +neither is specified, this is the deepest backing image + +- "base": The file name of the backing image to write data into. + (json-string, optional) +- "base-node-name": The name of the block driver state node of the + backing image to write data into. + (json-string, optional) (Since 2.1) + +For 'top', either top or top-node-name must be set but not both. If +neither is specified, this is the active layer + +- "top": The file name of the backing image within the image chain, + which contains the topmost data to be committed down. + (json-string, optional) + +- "top-node-name": The block driver state node name of the backing + image within the image chain, which contains the + topmost data to be committed down. + (json-string, optional) (Since 2.1) If top == base, that is an error. If top == active, the job will not be completed by itself,