From patchwork Tue Mar 12 16:48:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10849561 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 071B61850 for ; Tue, 12 Mar 2019 17:04:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DD17728DF8 for ; Tue, 12 Mar 2019 17:04:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CF2A3290CC; Tue, 12 Mar 2019 17:04:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 05DCC28DF8 for ; Tue, 12 Mar 2019 17:04:22 +0000 (UTC) Received: from localhost ([127.0.0.1]:55972 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h3kpB-0002nE-CJ for patchwork-qemu-devel@patchwork.kernel.org; Tue, 12 Mar 2019 13:04:21 -0400 Received: from eggs.gnu.org ([209.51.188.92]:43499) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h3kaY-0006em-UP for qemu-devel@nongnu.org; Tue, 12 Mar 2019 12:49:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h3kaV-0006hO-O4 for qemu-devel@nongnu.org; Tue, 12 Mar 2019 12:49:14 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:33713) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1h3kaV-0006dJ-5s; Tue, 12 Mar 2019 12:49:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=8vsOrY49+F1FPTT1GtTLowlHTxSJoIZTWaZrXpb476k=; b=RvxGALFgtyWFbo81R3luYeLc/aHlumnOyNvAoGHTO5Nuf2/pdF/DmCtYPboRpjpsqsjEOU/n4l0n7fK5l01TwKi0tUWZJBfb/0AxjOcoh8Wrj6D/biU2ZPZY8iO1G7F3mleeNOmXJOfbx1h7D9H2cKWgXE1VShltDhwN0vWL1x/iqFyxqpv7whjaaucR/jFmy14azGsqgsVKV6/NuQobsFWGQSSVgaxEr/MyTaUmlC084v55QXQfny8DGcjqP6wv9YPZdJrdXgKSaMFfkFoc4/uX6KfOOwIRWwn0Bp5y/p5Iu/8xydgHvFlsfUiN22ahriDQsxG4OinI/U7WT5zBjg==; Received: from 87-100-202-60.bb.dnainternet.fi ([87.100.202.60] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1h3kaS-0006oo-2f; Tue, 12 Mar 2019 17:49:08 +0100 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1h3kaF-0007vw-5c; Tue, 12 Mar 2019 18:48:55 +0200 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Tue, 12 Mar 2019 18:48:47 +0200 Message-Id: <3980c9561cf42a11f7723fa60b0e9f8acf0fd971.1552409054.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH v3 08/13] block: Allow changing the backing file on reopen X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Alberto Garcia , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP This patch allows the user to change the backing file of an image that is being reopened. Here's what it does: - In bdrv_reopen_prepare(): check that the value of 'backing' points to an existing node or is null. If it points to an existing node it also needs to make sure that replacing the backing file will not create a cycle in the node graph (i.e. you cannot reach the parent from the new backing file). - In bdrv_reopen_commit(): perform the actual node replacement by calling bdrv_set_backing_hd(). There may be temporary implicit nodes between a BDS and its backing file (e.g. a commit filter node). In these cases bdrv_reopen_prepare() looks for the real (non-implicit) backing file and requires that the 'backing' option points to it. Replacing or detaching a backing file is forbidden if there are implicit nodes in the middle. Although x-blockdev-reopen is meant to be used like blockdev-add, there's an important thing that must be taken into account: the only way to set a new backing file is by using a reference to an existing node (previously added with e.g. blockdev-add). If 'backing' contains a dictionary with a new set of options ({"driver": "qcow2", "file": { ... }}) then it is interpreted that the _existing_ backing file must be reopened with those options. Signed-off-by: Alberto Garcia --- block.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++++ include/block/block.h | 2 + 2 files changed, 168 insertions(+) diff --git a/block.c b/block.c index c043f5eadd..c6786cd35b 100644 --- a/block.c +++ b/block.c @@ -2984,6 +2984,27 @@ BlockDriverState *bdrv_open(const char *filename, const char *reference, } /* + * Returns true if @child can be reached recursively from @bs + */ +static bool bdrv_recurse_has_child(BlockDriverState *bs, + BlockDriverState *child) +{ + BdrvChild *c; + + if (bs == child) { + return true; + } + + QLIST_FOREACH(c, &bs->children, next) { + if (bdrv_recurse_has_child(c->bs, child)) { + return true; + } + } + + return false; +} + +/* * Adds a BlockDriverState to a simple queue for an atomic, transactional * reopen of multiple devices. * @@ -3208,6 +3229,19 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er if (ret < 0) { goto cleanup_perm; } + /* Check if new_backing_bs would accept the new permissions */ + if (state->replace_backing_bs && state->new_backing_bs) { + uint64_t nperm, nshared; + bdrv_child_perm(state->bs, state->new_backing_bs, + NULL, &child_backing, bs_queue, + state->perm, state->shared_perm, + &nperm, &nshared); + ret = bdrv_check_update_perm(state->new_backing_bs, NULL, + nperm, nshared, NULL, errp); + if (ret < 0) { + goto cleanup_perm; + } + } bs_entry->perms_checked = true; } @@ -3231,6 +3265,9 @@ cleanup_perm: bdrv_set_perm(state->bs, state->perm, state->shared_perm); } else { bdrv_abort_perm_update(state->bs); + if (state->replace_backing_bs && state->new_backing_bs) { + bdrv_abort_perm_update(state->new_backing_bs); + } } } cleanup: @@ -3242,6 +3279,9 @@ cleanup: qobject_unref(bs_entry->state.explicit_options); qobject_unref(bs_entry->state.options); } + if (bs_entry->state.new_backing_bs) { + bdrv_unref(bs_entry->state.new_backing_bs); + } g_free(bs_entry); } g_free(bs_queue); @@ -3314,6 +3354,101 @@ static void bdrv_reopen_perm(BlockReopenQueue *q, BlockDriverState *bs, } /* + * Take a BDRVReopenState and check if the value of 'backing' in the + * reopen_state->options QDict is valid or not. + * + * If 'backing' is missing from the QDict then return 0. + * + * If 'backing' contains the node name of the backing file of + * reopen_state->bs then return 0. + * + * If 'backing' contains a different node name (or is null) then check + * whether the current backing file can be replaced with the new one. + * If that's the case then reopen_state->replace_backing_bs is set to + * true and reopen_state->new_backing_bs contains a pointer to the new + * backing BlockDriverState (or NULL). + * + * Return 0 on success, otherwise return < 0 and set @errp. + */ +static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state, + Error **errp) +{ + BlockDriverState *bs = reopen_state->bs; + BlockDriverState *overlay_bs, *new_backing_bs; + QObject *value; + const char *str; + + value = qdict_get(reopen_state->options, "backing"); + if (value == NULL) { + return 0; + } + + switch (qobject_type(value)) { + case QTYPE_QNULL: + new_backing_bs = NULL; + break; + case QTYPE_QSTRING: + str = qobject_get_try_str(value); + new_backing_bs = bdrv_lookup_bs(NULL, str, errp); + if (new_backing_bs == NULL) { + return -EINVAL; + } else if (bdrv_recurse_has_child(new_backing_bs, bs)) { + error_setg(errp, "Making '%s' a backing file of '%s' " + "would create a cycle", str, bs->node_name); + return -EINVAL; + } + break; + default: + /* 'backing' does not allow any other data type */ + g_assert_not_reached(); + } + + /* + * TODO: before removing the x- prefix from x-blockdev-reopen we + * should move the new backing file into the right AioContext + * instead of returning an error. + */ + if (new_backing_bs) { + if (bdrv_get_aio_context(new_backing_bs) != bdrv_get_aio_context(bs)) { + error_setg(errp, "Cannot use a new backing file " + "with a different AioContext"); + return -EINVAL; + } + } + + /* + * Find the "actual" backing file by skipping all links that point + * to an implicit node, if any (e.g. a commit filter node). + */ + overlay_bs = bs; + while (backing_bs(overlay_bs) && backing_bs(overlay_bs)->implicit) { + overlay_bs = backing_bs(overlay_bs); + } + + /* If we want to replace the backing file we need some extra checks */ + if (new_backing_bs != backing_bs(overlay_bs)) { + /* Check for implicit nodes between bs and its backing file */ + if (bs != overlay_bs) { + error_setg(errp, "Cannot change backing link if '%s' has " + "an implicit backing file", bs->node_name); + return -EPERM; + } + /* Check if the backing link that we want to replace is frozen */ + if (bdrv_is_backing_chain_frozen(overlay_bs, backing_bs(overlay_bs), + errp)) { + return -EPERM; + } + reopen_state->replace_backing_bs = true; + if (new_backing_bs) { + bdrv_ref(new_backing_bs); + reopen_state->new_backing_bs = new_backing_bs; + } + } + + return 0; +} + +/* * Prepares a BlockDriverState for reopen. All changes are staged in the * 'opaque' field of the BDRVReopenState, which is used and allocated by * the block driver layer .bdrv_reopen_prepare() @@ -3447,6 +3582,17 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, goto error; } + /* + * Allow changing the 'backing' option. The new value can be + * either a reference to an existing node (using its node name) + * or NULL to simply detach the current backing file. + */ + ret = bdrv_reopen_parse_backing(reopen_state, errp); + if (ret < 0) { + goto error; + } + qdict_del(reopen_state->options, "backing"); + /* Options that are not handled are only okay if they are unchanged * compared to the old state. It is expected that some options are only * used for the initial open, but not reopen (e.g. filename) */ @@ -3556,6 +3702,11 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) bs->read_only = !(reopen_state->flags & BDRV_O_RDWR); bs->detect_zeroes = reopen_state->detect_zeroes; + if (reopen_state->replace_backing_bs) { + qdict_del(bs->explicit_options, "backing"); + qdict_del(bs->options, "backing"); + } + /* Remove child references from bs->options and bs->explicit_options. * Child options were already removed in bdrv_reopen_queue_child() */ QLIST_FOREACH(child, &bs->children, next) { @@ -3563,6 +3714,21 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) qdict_del(bs->options, child->name); } + /* + * Change the backing file if a new one was specified. We do this + * after updating bs->options, so bdrv_refresh_filename() (called + * from bdrv_set_backing_hd()) has the new values. + */ + if (reopen_state->replace_backing_bs) { + BlockDriverState *old_backing_bs = backing_bs(bs); + assert(!old_backing_bs || !old_backing_bs->implicit); + /* Abort the permission update on the backing bs we're detaching */ + if (old_backing_bs) { + bdrv_abort_perm_update(old_backing_bs); + } + bdrv_set_backing_hd(bs, reopen_state->new_backing_bs, &error_abort); + } + bdrv_refresh_limits(bs, NULL); new_can_write = diff --git a/include/block/block.h b/include/block/block.h index fcc61a3a19..68a3efbb43 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -188,6 +188,8 @@ typedef struct BDRVReopenState { int flags; BlockdevDetectZeroesOptions detect_zeroes; bool backing_missing; + bool replace_backing_bs; /* new_backing_bs is ignored if this is false */ + BlockDriverState *new_backing_bs; /* If NULL then detach the current bs */ uint64_t perm, shared_perm; QDict *options; QDict *explicit_options;