From patchwork Mon Oct 26 12:27:16 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 535911 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 14DCD141310 for ; Mon, 26 Oct 2015 23:30:31 +1100 (AEDT) Received: from localhost ([::1]:52585 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqguz-0007NB-5p for incoming@patchwork.ozlabs.org; Mon, 26 Oct 2015 08:30:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37840) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqgsv-0003le-IO for qemu-devel@nongnu.org; Mon, 26 Oct 2015 08:28:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zqgso-0002BH-PP for qemu-devel@nongnu.org; Mon, 26 Oct 2015 08:28:21 -0400 Received: from smtp3.mundo-r.com ([212.51.32.191]:30187 helo=smtp4.mundo-r.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqgso-00023y-EZ; Mon, 26 Oct 2015 08:28:14 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2AhBADpGi5W/5tjdVteGQEBAg4BAQGDCIFDqUgBAQEBAQEFAYENAY8+hCEBDYFahh0CgS44FAEBAQEBAQGBCoQzAQEEJ1IQPxI8GxkbiBkBxDoBLIYwiWZsB4QuBYdChVaJHo0inC8fAQFCghEdgVdwhxgBAQE X-IPAS-Result: A2AhBADpGi5W/5tjdVteGQEBAg4BAQGDCIFDqUgBAQEBAQEFAYENAY8+hCEBDYFahh0CgS44FAEBAQEBAQGBCoQzAQEEJ1IQPxI8GxkbiBkBxDoBLIYwiWZsB4QuBYdChVaJHo0inC8fAQFCghEdgVdwhxgBAQE X-IronPort-AV: E=Sophos;i="5.20,201,1444687200"; d="scan'208";a="38253690" Received: from fanzine.igalia.com ([91.117.99.155]) by smtp4.mundo-r.com with ESMTP; 26 Oct 2015 13:27:35 +0100 Received: from 85-76-104-95-nat.elisa-mobile.fi ([85.76.104.95] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim) id 1Zqgs9-00023E-HX; Mon, 26 Oct 2015 13:27:34 +0100 Received: from berto by perseus.local with local (Exim 4.86) (envelope-from ) id 1Zqgrv-0005rG-QW; Mon, 26 Oct 2015 14:27:19 +0200 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Mon, 26 Oct 2015 14:27:16 +0200 Message-Id: <3884c224cd92d067df8a07c875a098eb7e80a051.1445861886.git.berto@igalia.com> X-Mailer: git-send-email 2.6.1 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 212.51.32.191 Cc: Kevin Wolf , Alberto Garcia , qemu-block@nongnu.org, Markus Armbruster , Max Reitz Subject: [Qemu-devel] [PATCH v8 4/5] block: add a 'blockdev-snapshot' QMP command 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 One of the limitations of the 'blockdev-snapshot-sync' command is that it does not allow passing BlockdevOptions to the newly created snapshots, so they are always opened using the default values. Extending the command to allow passing options is not a practical solution because there is overlap between those options and some of the existing parameters of the command. This patch introduces a new 'blockdev-snapshot' command with a simpler interface: it just takes two references to existing block devices that will be used as the source and target for the snapshot. Since the main difference between the two commands is that one of them creates and opens the target image, while the other uses an already opened one, the bulk of the implementation is shared. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- blockdev.c | 165 ++++++++++++++++++++++++++++++++------------------- qapi-schema.json | 2 + qapi/block-core.json | 28 +++++++++ qmp-commands.hx | 38 ++++++++++++ 4 files changed, 172 insertions(+), 61 deletions(-) diff --git a/blockdev.c b/blockdev.c index 06a5e8a..a567a05 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1174,6 +1174,18 @@ void qmp_blockdev_snapshot_sync(bool has_device, const char *device, &snapshot, errp); } +void qmp_blockdev_snapshot(const char *node, const char *overlay, + Error **errp) +{ + BlockdevSnapshot snapshot_data = { + .node = (char *) node, + .overlay = (char *) overlay + }; + + blockdev_do_action(TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT, + &snapshot_data, errp); +} + void qmp_blockdev_snapshot_internal_sync(const char *device, const char *name, Error **errp) @@ -1519,58 +1531,48 @@ typedef struct ExternalSnapshotState { static void external_snapshot_prepare(BlkTransactionState *common, Error **errp) { - int flags, ret; - QDict *options; + int flags = 0, ret; + QDict *options = NULL; Error *local_err = NULL; - bool has_device = false; + /* Device and node name of the image to generate the snapshot from */ const char *device; - bool has_node_name = false; const char *node_name; - bool has_snapshot_node_name = false; - const char *snapshot_node_name; + /* Reference to the new image (for 'blockdev-snapshot') */ + const char *snapshot_ref; + /* File name of the new image (for 'blockdev-snapshot-sync') */ const char *new_image_file; - const char *format = "qcow2"; - enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); TransactionAction *action = common->action; - /* get parameters */ - g_assert(action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC); - - has_device = action->blockdev_snapshot_sync->has_device; - device = action->blockdev_snapshot_sync->device; - has_node_name = action->blockdev_snapshot_sync->has_node_name; - node_name = action->blockdev_snapshot_sync->node_name; - has_snapshot_node_name = - action->blockdev_snapshot_sync->has_snapshot_node_name; - snapshot_node_name = action->blockdev_snapshot_sync->snapshot_node_name; - - new_image_file = action->blockdev_snapshot_sync->snapshot_file; - if (action->blockdev_snapshot_sync->has_format) { - format = action->blockdev_snapshot_sync->format; - } - if (action->blockdev_snapshot_sync->has_mode) { - mode = action->blockdev_snapshot_sync->mode; + /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar + * purpose but a different set of parameters */ + switch (action->kind) { + case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT: + { + BlockdevSnapshot *s = action->blockdev_snapshot; + device = s->node; + node_name = s->node; + new_image_file = NULL; + snapshot_ref = s->overlay; + } + break; + case TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC: + { + BlockdevSnapshotSync *s = action->blockdev_snapshot_sync; + device = s->has_device ? s->device : NULL; + node_name = s->has_node_name ? s->node_name : NULL; + new_image_file = s->snapshot_file; + snapshot_ref = NULL; + } + break; + default: + g_assert_not_reached(); } /* start processing */ - state->old_bs = bdrv_lookup_bs(has_device ? device : NULL, - has_node_name ? node_name : NULL, - &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - if (has_node_name && !has_snapshot_node_name) { - error_setg(errp, "New snapshot node name missing"); - return; - } - - if (has_snapshot_node_name && - bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) { - error_setg(errp, "New snapshot node name already in use"); + state->old_bs = bdrv_lookup_bs(device, node_name, errp); + if (!state->old_bs) { return; } @@ -1601,35 +1603,70 @@ static void external_snapshot_prepare(BlkTransactionState *common, return; } - flags = state->old_bs->open_flags; + if (action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC) { + BlockdevSnapshotSync *s = action->blockdev_snapshot_sync; + const char *format = s->has_format ? s->format : "qcow2"; + enum NewImageMode mode; + const char *snapshot_node_name = + s->has_snapshot_node_name ? s->snapshot_node_name : NULL; - /* create new image w/backing file */ - if (mode != NEW_IMAGE_MODE_EXISTING) { - bdrv_img_create(new_image_file, format, - state->old_bs->filename, - state->old_bs->drv->format_name, - NULL, -1, flags, &local_err, false); - if (local_err) { - error_propagate(errp, local_err); + if (node_name && !snapshot_node_name) { + error_setg(errp, "New snapshot node name missing"); return; } - } - options = qdict_new(); - if (has_snapshot_node_name) { - qdict_put(options, "node-name", - qstring_from_str(snapshot_node_name)); + if (snapshot_node_name && + bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) { + error_setg(errp, "New snapshot node name already in use"); + return; + } + + flags = state->old_bs->open_flags; + + /* create new image w/backing file */ + mode = s->has_mode ? s->mode : NEW_IMAGE_MODE_ABSOLUTE_PATHS; + if (mode != NEW_IMAGE_MODE_EXISTING) { + bdrv_img_create(new_image_file, format, + state->old_bs->filename, + state->old_bs->drv->format_name, + NULL, -1, flags, &local_err, false); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } + + options = qdict_new(); + if (s->has_snapshot_node_name) { + qdict_put(options, "node-name", + qstring_from_str(snapshot_node_name)); + } + qdict_put(options, "driver", qstring_from_str(format)); + + flags |= BDRV_O_NO_BACKING; } - qdict_put(options, "driver", qstring_from_str(format)); - /* TODO Inherit bs->options or only take explicit options with an - * extended QMP command? */ assert(state->new_bs == NULL); - ret = bdrv_open(&state->new_bs, new_image_file, NULL, options, - flags | BDRV_O_NO_BACKING, &local_err); + ret = bdrv_open(&state->new_bs, new_image_file, snapshot_ref, options, + flags, errp); /* We will manually add the backing_hd field to the bs later */ if (ret != 0) { - error_propagate(errp, local_err); + return; + } + + if (state->new_bs->blk != NULL) { + error_setg(errp, "The snapshot is already in use by %s", + blk_name(state->new_bs->blk)); + return; + } + + if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, + errp)) { + return; + } + + if (state->new_bs->backing != NULL) { + error_setg(errp, "The snapshot already has a backing image"); } } @@ -1832,6 +1869,12 @@ static void abort_commit(BlkTransactionState *common) } static const BdrvActionOps actions[] = { + [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT] = { + .instance_size = sizeof(ExternalSnapshotState), + .prepare = external_snapshot_prepare, + .commit = external_snapshot_commit, + .abort = external_snapshot_abort, + }, [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC] = { .instance_size = sizeof(ExternalSnapshotState), .prepare = external_snapshot_prepare, diff --git a/qapi-schema.json b/qapi-schema.json index 08f2487..5d73687 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1531,9 +1531,11 @@ # abort since 1.6 # blockdev-snapshot-internal-sync since 1.7 # blockdev-backup since 2.3 +# blockdev-snapshot since 2.5 ## { 'union': 'TransactionAction', 'data': { + 'blockdev-snapshot': 'BlockdevSnapshot', 'blockdev-snapshot-sync': 'BlockdevSnapshotSync', 'drive-backup': 'DriveBackup', 'blockdev-backup': 'BlockdevBackup', diff --git a/qapi/block-core.json b/qapi/block-core.json index 34f8f98..d4f7afe 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -705,6 +705,21 @@ '*format': 'str', '*mode': 'NewImageMode' } } ## +# @BlockdevSnapshot +# +# @node: device or node name that will have a snapshot created. +# +# @overlay: reference to the existing block device that will become +# the overlay of @node, as part of creating the snapshot. +# It must not have a current backing file (this can be +# achieved by passing "backing": "" to blockdev-add). +# +# Since 2.5 +## +{ 'struct': 'BlockdevSnapshot', + 'data': { 'node': 'str', 'overlay': 'str' } } + +## # @DriveBackup # # @device: the name of the device which should be copied. @@ -800,6 +815,19 @@ { 'command': 'blockdev-snapshot-sync', 'data': 'BlockdevSnapshotSync' } + +## +# @blockdev-snapshot +# +# Generates a snapshot of a block device. +# +# For the arguments, see the documentation of BlockdevSnapshot. +# +# Since 2.5 +## +{ 'command': 'blockdev-snapshot', + 'data': 'BlockdevSnapshot' } + ## # @change-backing-file # diff --git a/qmp-commands.hx b/qmp-commands.hx index d7cf0ff..92b8662 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1476,6 +1476,44 @@ Example: EQMP { + .name = "blockdev-snapshot", + .args_type = "node:s,overlay:s", + .mhandler.cmd_new = qmp_marshal_blockdev_snapshot, + }, + +SQMP +blockdev-snapshot +----------------- +Since 2.5 + +Create a snapshot, by installing 'node' as the backing image of +'overlay'. Additionally, if 'node' is associated with a block +device, the block device changes to using 'overlay' as its new active +image. + +Arguments: + +- "node": device that will have a snapshot created (json-string) +- "overlay": device that will have 'node' as its backing image (json-string) + +Example: + +-> { "execute": "blockdev-add", + "arguments": { "options": { "driver": "qcow2", + "node-name": "node1534", + "file": { "driver": "file", + "filename": "hd1.qcow2" }, + "backing": "" } } } + +<- { "return": {} } + +-> { "execute": "blockdev-snapshot", "arguments": { "node": "ide-hd0", + "overlay": "node1534" } } +<- { "return": {} } + +EQMP + + { .name = "blockdev-snapshot-internal-sync", .args_type = "device:B,name:s", .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_internal_sync,