From patchwork Mon Nov 20 20:09:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 10067231 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4D26E603FA for ; Mon, 20 Nov 2017 20:29:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3DDFD2883F for ; Mon, 20 Nov 2017 20:29:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3060F29332; Mon, 20 Nov 2017 20:29:28 +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=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9D0892883F for ; Mon, 20 Nov 2017 20:29:27 +0000 (UTC) Received: from localhost ([::1]:59594 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eGsh4-0007Bf-T7 for patchwork-qemu-devel@patchwork.kernel.org; Mon, 20 Nov 2017 15:29:26 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35556) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eGsPv-0000WT-SW for qemu-devel@nongnu.org; Mon, 20 Nov 2017 15:11:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eGsPu-0007ja-G6 for qemu-devel@nongnu.org; Mon, 20 Nov 2017 15:11:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44324) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eGsPo-0007gy-Rj; Mon, 20 Nov 2017 15:11:37 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1906F7F7AB; Mon, 20 Nov 2017 20:11:36 +0000 (UTC) Received: from localhost (ovpn-204-75.brq.redhat.com [10.40.204.75]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 86CD460C80; Mon, 20 Nov 2017 20:11:35 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 20 Nov 2017 21:09:58 +0100 Message-Id: <20171120201004.14999-20-mreitz@redhat.com> In-Reply-To: <20171120201004.14999-1-mreitz@redhat.com> References: <20171120201004.14999-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 20 Nov 2017 20:11:36 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v7 for-2.12 19/25] block: Add BlockDriver.bdrv_gather_child_options 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 , John Snow , qemu-devel@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 Some follow-up patches will rework the way bs->full_open_options is refreshed in bdrv_refresh_filename(). The new implementation will remove the need for the block drivers' bdrv_refresh_filename() implementations to set bs->full_open_options; instead, it will be generic and use static information from each block driver. However, by implementing bdrv_gather_child_options(), block drivers will still be able to override the way the full_open_options of their children are incorporated into their own. We need to implement this function for VMDK because we have to prevent the generic implementation from gathering the options of all children: It is not possible to specify options for the extents through the runtime options. For quorum, the child names that would be used by the generic implementation and the ones that we actually (currently) want to use differ. See quorum_gather_child_options() for more information. Note that both of these are cases which are not ideal: In case of VMDK it would probably be nice to be able to specify options for all extents. In case of quorum, the current runtime option structure is simply broken and needs to be fixed (but that is left for another patch). Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia --- include/block/block_int.h | 17 +++++++++++++++++ block/quorum.c | 38 ++++++++++++++++++++++++++++++++++++++ block/vmdk.c | 13 +++++++++++++ 3 files changed, 68 insertions(+) diff --git a/include/block/block_int.h b/include/block/block_int.h index d99ded8891..f81516b615 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -136,6 +136,23 @@ struct BlockDriver { void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options); /* + * Gathers the open options for all children into @target. + * A simple format driver (without backing file support) might + * implement this function like this: + * + * QINCREF(bs->file->bs->full_open_options); + * qdict_put(target, "file", bs->file->bs->full_open_options); + * + * If not specified, the generic implementation will simply put + * all children's options under their respective name. + * + * Note that ideally this function would not be needed. Every + * block driver which implements it is probably doing something + * shady regarding its runtime option structure. + */ + void (*bdrv_gather_child_options)(BlockDriverState *bs, QDict *target); + + /* * Returns an allocated string which is the directory name of this BDS: It * will be used to make relative filenames absolute by prepending this * function's return value to them. diff --git a/block/quorum.c b/block/quorum.c index 4b38201aa2..9d680addc6 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1095,6 +1095,43 @@ static void quorum_refresh_filename(BlockDriverState *bs, QDict *options) bs->full_open_options = opts; } +static void quorum_gather_child_options(BlockDriverState *bs, QDict *target) +{ + BDRVQuorumState *s = bs->opaque; + QList *children_list; + int i; + + /* The generic implementation for gathering child options in + * bdrv_refresh_filename() would use the names of the children + * as specified for bdrv_open_child() or bdrv_attach_child(), + * which is "children.%u" with %u being a value + * (s->next_child_index) that is incremented each time a new child + * is added (and never decremented). Since children can be + * deleted at runtime, there may be gaps in that enumeration. + * When creating a new quorum BDS and specifying the children for + * it through runtime options, the enumeration used there may not + * have any gaps, though. + * + * Therefore, we have to create a new gap-less enumeration here + * (which we can achieve by simply putting all of the children's + * full_open_options into a QList). + * + * XXX: Note that there are issues with the current child option + * structure quorum uses (such as the fact that children do + * not really have unique permanent names). Therefore, this + * is going to have to change in the future and ideally we + * want quorum to be covered by the generic implementation. + */ + + children_list = qlist_new(); + qdict_put(target, "children", children_list); + + for (i = 0; i < s->num_children; i++) { + QINCREF(s->children[i]->bs->full_open_options); + qlist_append(children_list, s->children[i]->bs->full_open_options); + } +} + static char *quorum_dirname(BlockDriverState *bs, Error **errp) { /* In general, there are multiple BDSs with different dirnames below this @@ -1123,6 +1160,7 @@ static BlockDriver bdrv_quorum = { .bdrv_file_open = quorum_open, .bdrv_close = quorum_close, .bdrv_refresh_filename = quorum_refresh_filename, + .bdrv_gather_child_options = quorum_gather_child_options, .bdrv_dirname = quorum_dirname, .bdrv_co_flush_to_disk = quorum_co_flush, diff --git a/block/vmdk.c b/block/vmdk.c index c1dbcfe133..dffab90a8b 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -2327,6 +2327,18 @@ static int vmdk_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) return 0; } +static void vmdk_gather_child_options(BlockDriverState *bs, QDict *target) +{ + /* No children but file and backing can be explicitly specified (TODO) */ + QINCREF(bs->file->bs->full_open_options); + qdict_put(target, "file", bs->file->bs->full_open_options); + + if (bs->backing && bs->backing_overridden) { + QINCREF(bs->backing->bs->full_open_options); + qdict_put(target, "backing", bs->backing->bs->full_open_options); + } +} + static QemuOptsList vmdk_create_opts = { .name = "vmdk-create-opts", .head = QTAILQ_HEAD_INITIALIZER(vmdk_create_opts.head), @@ -2397,6 +2409,7 @@ static BlockDriver bdrv_vmdk = { .bdrv_get_specific_info = vmdk_get_specific_info, .bdrv_refresh_limits = vmdk_refresh_limits, .bdrv_get_info = vmdk_get_info, + .bdrv_gather_child_options = vmdk_gather_child_options, .supports_backing = true, .create_opts = &vmdk_create_opts,