From patchwork Wed Feb 13 22:53:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041659 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FQD1Jdqz9sMr for ; Thu, 14 Feb 2019 10:01:40 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36244 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3X8-0001uj-2g for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 18:01:38 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Pt-0004m0-5x for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3P6-0005vu-4Q for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:53:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58906) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3P2-0005t7-TA; Wed, 13 Feb 2019 17:53:17 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1C48537F46; Wed, 13 Feb 2019 22:53:16 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A31E360856; Wed, 13 Feb 2019 22:53:15 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:00 +0100 Message-Id: <20190213225311.17876-2-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 13 Feb 2019 22:53:16 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 01/12] block: Mark commit and mirror as filter drivers 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The commit and mirror block nodes are filters, so they should be marked as such. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia Reviewed-by: Eric Blake --- block/commit.c | 2 ++ block/mirror.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/block/commit.c b/block/commit.c index 2b876bf6e9..87d0b208ea 100644 --- a/block/commit.c +++ b/block/commit.c @@ -254,6 +254,8 @@ static BlockDriver bdrv_commit_top = { .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_commit_top_refresh_filename, .bdrv_child_perm = bdrv_commit_top_child_perm, + + .is_filter = true, }; void commit_start(const char *job_id, BlockDriverState *bs, diff --git a/block/mirror.c b/block/mirror.c index 726d3c27fb..c7ca8078d1 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1469,6 +1469,8 @@ static BlockDriver bdrv_mirror_top = { .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_mirror_top_refresh_filename, .bdrv_child_perm = bdrv_mirror_top_child_perm, + + .is_filter = true, }; static void mirror_start_job(const char *job_id, BlockDriverState *bs, From patchwork Wed Feb 13 22:53:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041657 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FMY3MPRz9rxp for ; Thu, 14 Feb 2019 09:59:21 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36189 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Ut-0008Ny-Bx for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 17:59:19 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37824) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Pt-0004ls-6J for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3P6-0005w4-8K for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:53:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55092) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3P5-0005ur-6i; Wed, 13 Feb 2019 17:53:19 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7E6AC89AC6; Wed, 13 Feb 2019 22:53:18 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0F49960856; Wed, 13 Feb 2019 22:53:17 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:01 +0100 Message-Id: <20190213225311.17876-3-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 13 Feb 2019 22:53:18 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 02/12] blockdev: Check @replaces in blockdev_mirror_common 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" There is no reason why the constraints we put on @replaces should be limited to drive-mirror. Therefore, move the sanity checks from qmp_drive_mirror() to blockdev_mirror_common() so they apply to blockdev-mirror as well. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia Reviewed-by: Kevin Wolf --- blockdev.c | 55 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/blockdev.c b/blockdev.c index 3bab7e4f5a..22e41fb862 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3771,6 +3771,39 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, sync = MIRROR_SYNC_MODE_FULL; } + if (has_replaces) { + BlockDriverState *to_replace_bs; + AioContext *replace_aio_context; + int64_t bs_size, replace_size; + + bs_size = bdrv_getlength(bs); + if (bs_size < 0) { + error_setg_errno(errp, -bs_size, "Failed to query device's size"); + return; + } + + to_replace_bs = check_to_replace_node(bs, replaces, errp); + if (!to_replace_bs) { + return; + } + + replace_aio_context = bdrv_get_aio_context(to_replace_bs); + aio_context_acquire(replace_aio_context); + replace_size = bdrv_getlength(to_replace_bs); + aio_context_release(replace_aio_context); + + if (replace_size < 0) { + error_setg_errno(errp, -replace_size, + "Failed to query the replacement node's size"); + return; + } + if (bs_size != replace_size) { + error_setg(errp, "cannot replace image with a mirror image of " + "different size"); + return; + } + } + /* pass the node name to replace to mirror start since it's loose coupling * and will allow to check whether the node still exist at mirror completion */ @@ -3831,33 +3864,11 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) } if (arg->has_replaces) { - BlockDriverState *to_replace_bs; - AioContext *replace_aio_context; - int64_t replace_size; - if (!arg->has_node_name) { error_setg(errp, "a node-name must be provided when replacing a" " named node of the graph"); goto out; } - - to_replace_bs = check_to_replace_node(bs, arg->replaces, &local_err); - - if (!to_replace_bs) { - error_propagate(errp, local_err); - goto out; - } - - replace_aio_context = bdrv_get_aio_context(to_replace_bs); - aio_context_acquire(replace_aio_context); - replace_size = bdrv_getlength(to_replace_bs); - aio_context_release(replace_aio_context); - - if (size != replace_size) { - error_setg(errp, "cannot replace image with a mirror image of " - "different size"); - goto out; - } } if (arg->mode == NEW_IMAGE_MODE_ABSOLUTE_PATHS) { From patchwork Wed Feb 13 22:53:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041655 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FLd6vQtz9rxp for ; Thu, 14 Feb 2019 09:58:33 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36175 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3U7-0007hB-LV for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 17:58:31 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37735) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Pn-0004ib-TF for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3PE-00069c-44 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:53:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60266) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3P8-0005yG-V5; Wed, 13 Feb 2019 17:53:23 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6EAB4C049E20; Wed, 13 Feb 2019 22:53:21 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6B14B5D9C6; Wed, 13 Feb 2019 22:53:20 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:02 +0100 Message-Id: <20190213225311.17876-4-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 13 Feb 2019 22:53:21 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 03/12] block: Filtered children access functions 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" What bs->file and bs->backing mean depends on the node. For filter nodes, both signify a node that will eventually receive all R/W accesses. For format nodes, bs->file contains metadata and data, and bs->backing will not receive writes -- instead, writes are COWed to bs->file. Usually. In any case, it is not trivial to guess what a child means exactly with our currently limited form of expression. It is better to introduce some functions that actually guarantee a meaning: - bdrv_filtered_cow_child() will return the child that receives requests filtered through COW. That is, reads may or may not be forwarded (depending on the overlay's allocation status), but writes never go to this child. - bdrv_filtered_rw_child() will return the child that receives requests filtered through some very plain process. Reads and writes issued to the parent will go to the child as well (although timing, etc. may be modified). - All drivers but quorum (but quorum is pretty opaque to the general block layer anyway) always only have one of these children: All read requests must be served from the filtered_rw_child (if it exists), so if there was a filtered_cow_child in addition, it would not receive any requests at all. (The closest here is mirror, where all requests are passed on to the source, but with write-blocking, write requests are "COWed" to the target. But that just means that the target is a special child that cannot be introspected by the generic block layer functions, and that source is a filtered_rw_child.) Therefore, we can also add bdrv_filtered_child() which returns that one child (or NULL, if there is no filtered child). Also, many places in the current block layer should be skipping filters (all filters or just the ones added implicitly, it depends) when going through a block node chain. They do not do that currently, but this patch makes them. One example for this is qemu-img map, which should skip filters and only look at the COW elements in the graph. The change to iotest 204's reference output shows how using blkdebug on top of a COW node used to make qemu-img map disregard the rest of the backing chain, but with this patch, the allocation in the base image is reported correctly. Furthermore, a note should be made that sometimes we do want to access bs->backing directly. This is whenever the operation in question is not about accessing the COW child, but the "backing" child, be it COW or not. This is the case in functions such as bdrv_open_backing_file() or whenever we have to deal with the special behavior of @backing as a blockdev option, which is that it does not default to null like all other child references do. Finally, the query functions (query-block and query-named-block-nodes) are modified to return any filtered child under "backing", not just bs->backing or COW children. This is so that filters do not interrupt the reported backing chain. This changes the output of iotest 184, as the throttled node now appears as a backing child. Signed-off-by: Max Reitz Reviewed-by: Eric Blake --- qapi/block-core.json | 4 + include/block/block.h | 1 + include/block/block_int.h | 33 +++++- block.c | 183 ++++++++++++++++++++++++++++----- block/backup.c | 8 +- block/block-backend.c | 16 ++- block/commit.c | 33 +++--- block/io.c | 45 +++++--- block/mirror.c | 21 ++-- block/qapi.c | 30 +++--- block/stream.c | 13 ++- blockdev.c | 88 +++++++++++++--- migration/block-dirty-bitmap.c | 4 +- nbd/server.c | 6 +- qemu-img.c | 29 +++--- tests/qemu-iotests/184.out | 7 +- tests/qemu-iotests/204.out | 1 + 17 files changed, 392 insertions(+), 130 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 5f17d67d71..52ebf5b5f6 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2417,6 +2417,10 @@ # On successful completion the image file is updated to drop the backing file # and the BLOCK_JOB_COMPLETED event is emitted. # +# In case @device is a filter node, block-stream modifies the first non-filter +# overlay node below it to point to base's backing node (or NULL if @base was +# not specified) instead of modifying @device itself. +# # @job-id: identifier for the newly-created block job. If # omitted, the device name will be used. (Since 2.7) # diff --git a/include/block/block.h b/include/block/block.h index 5334ba1261..c97b330e10 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -453,6 +453,7 @@ BlockDriverState *bdrv_lookup_bs(const char *device, const char *node_name, Error **errp); bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base); +bool bdrv_legacy_chain_contains(BlockDriverState *top, BlockDriverState *base); BlockDriverState *bdrv_next_node(BlockDriverState *bs); BlockDriverState *bdrv_next_all_states(BlockDriverState *bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index 620581d236..d5b74181be 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -91,6 +91,7 @@ struct BlockDriver { * certain callbacks that refer to data (see block.c) to their bs->file if * the driver doesn't implement them. Drivers that do not wish to forward * must implement them and return -ENOTSUP. + * Note that filters are not allowed to modify data. */ bool is_filter; /* for snapshots block filter like Quorum can implement the @@ -884,11 +885,6 @@ typedef enum BlockMirrorBackingMode { MIRROR_LEAVE_BACKING_CHAIN, } BlockMirrorBackingMode; -static inline BlockDriverState *backing_bs(BlockDriverState *bs) -{ - return bs->backing ? bs->backing->bs : NULL; -} - /* Essential block drivers which must always be statically linked into qemu, and * which therefore can be accessed without using bdrv_find_format() */ @@ -1221,4 +1217,31 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, uint64_t src_offset, int refresh_total_sectors(BlockDriverState *bs, int64_t hint); +BdrvChild *bdrv_filtered_cow_child(BlockDriverState *bs); +BdrvChild *bdrv_filtered_rw_child(BlockDriverState *bs); +BdrvChild *bdrv_filtered_child(BlockDriverState *bs); +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs); +BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs); +BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs); + +static inline BlockDriverState *child_bs(BdrvChild *child) +{ + return child ? child->bs : NULL; +} + +static inline BlockDriverState *bdrv_filtered_cow_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_filtered_cow_child(bs)); +} + +static inline BlockDriverState *bdrv_filtered_rw_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_filtered_rw_child(bs)); +} + +static inline BlockDriverState *bdrv_filtered_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_filtered_child(bs)); +} + #endif /* BLOCK_INT_H */ diff --git a/block.c b/block.c index d496debda4..1f9c01d1c5 100644 --- a/block.c +++ b/block.c @@ -551,11 +551,12 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz) { BlockDriver *drv = bs->drv; + BlockDriverState *filtered = bdrv_filtered_rw_bs(bs); if (drv && drv->bdrv_probe_blocksizes) { return drv->bdrv_probe_blocksizes(bs, bsz); - } else if (drv && drv->is_filter && bs->file) { - return bdrv_probe_blocksizes(bs->file->bs, bsz); + } else if (filtered) { + return bdrv_probe_blocksizes(filtered, bsz); } return -ENOTSUP; @@ -570,11 +571,12 @@ int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz) int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo) { BlockDriver *drv = bs->drv; + BlockDriverState *filtered = bdrv_filtered_rw_bs(bs); if (drv && drv->bdrv_probe_geometry) { return drv->bdrv_probe_geometry(bs, geo); - } else if (drv && drv->is_filter && bs->file) { - return bdrv_probe_geometry(bs->file->bs, geo); + } else if (filtered) { + return bdrv_probe_geometry(filtered, geo); } return -ENOTSUP; @@ -2309,7 +2311,7 @@ static bool bdrv_inherits_from_recursive(BlockDriverState *child, } /* - * Sets the backing file link of a BDS. A new reference is created; callers + * Sets the bs->backing link of a BDS. A new reference is created; callers * which don't need their own reference any more must call bdrv_unref(). */ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, @@ -3854,8 +3856,8 @@ int bdrv_change_backing_file(BlockDriverState *bs, BlockDriverState *bdrv_find_overlay(BlockDriverState *active, BlockDriverState *bs) { - while (active && bs != backing_bs(active)) { - active = backing_bs(active); + while (active && bs != bdrv_filtered_bs(active)) { + active = bdrv_filtered_bs(active); } return active; @@ -3921,9 +3923,7 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base, * other intermediate nodes have been dropped. * If 'top' is an implicit node (e.g. "commit_top") we should skip * it because no one inherits from it. We use explicit_top for that. */ - while (explicit_top && explicit_top->implicit) { - explicit_top = backing_bs(explicit_top); - } + explicit_top = bdrv_skip_implicit_filters(explicit_top); update_inherits_from = bdrv_inherits_from_recursive(base, explicit_top); /* success - we can delete the intermediate states, and link top->base */ @@ -4074,10 +4074,14 @@ bool bdrv_is_sg(BlockDriverState *bs) bool bdrv_is_encrypted(BlockDriverState *bs) { - if (bs->backing && bs->backing->bs->encrypted) { + BlockDriverState *filtered = bdrv_filtered_bs(bs); + if (bs->encrypted) { + return true; + } + if (filtered && bdrv_is_encrypted(filtered)) { return true; } - return bs->encrypted; + return false; } const char *bdrv_get_format_name(BlockDriverState *bs) @@ -4364,7 +4368,21 @@ BlockDriverState *bdrv_lookup_bs(const char *device, bool bdrv_chain_contains(BlockDriverState *top, BlockDriverState *base) { while (top && top != base) { - top = backing_bs(top); + top = bdrv_filtered_bs(top); + } + + return top != NULL; +} + +/* + * Same as bdrv_chain_contains(), but skip implicitly added R/W filter + * nodes and do not move past explicitly added R/W filters. + */ +bool bdrv_legacy_chain_contains(BlockDriverState *top, BlockDriverState *base) +{ + top = bdrv_skip_implicit_filters(top); + while (top && top != base) { + top = bdrv_skip_implicit_filters(bdrv_filtered_cow_bs(top)); } return top != NULL; @@ -4436,20 +4454,24 @@ int bdrv_has_zero_init_1(BlockDriverState *bs) int bdrv_has_zero_init(BlockDriverState *bs) { + BlockDriverState *filtered; + if (!bs->drv) { return 0; } /* If BS is a copy on write image, it is initialized to the contents of the base image, which may not be zeroes. */ - if (bs->backing) { + if (bdrv_filtered_cow_child(bs)) { return 0; } if (bs->drv->bdrv_has_zero_init) { return bs->drv->bdrv_has_zero_init(bs); } - if (bs->file && bs->drv->is_filter) { - return bdrv_has_zero_init(bs->file->bs); + + filtered = bdrv_filtered_rw_bs(bs); + if (filtered) { + return bdrv_has_zero_init(filtered); } /* safe default */ @@ -4460,7 +4482,7 @@ bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs) { BlockDriverInfo bdi; - if (bs->backing) { + if (bdrv_filtered_cow_child(bs)) { return false; } @@ -4494,8 +4516,9 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) return -ENOMEDIUM; } if (!drv->bdrv_get_info) { - if (bs->file && drv->is_filter) { - return bdrv_get_info(bs->file->bs, bdi); + BlockDriverState *filtered = bdrv_filtered_rw_bs(bs); + if (filtered) { + return bdrv_get_info(filtered, bdi); } return -ENOTSUP; } @@ -4597,7 +4620,17 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, is_protocol = path_has_protocol(backing_file); - for (curr_bs = bs; curr_bs->backing; curr_bs = curr_bs->backing->bs) { + /* + * Being largely a legacy function, skip any filters here + * (because filters do not have normal filenames, so they cannot + * match anyway; and allowing json:{} filenames is a bit out of + * scope). + */ + for (curr_bs = bdrv_skip_rw_filters(bs); + bdrv_filtered_cow_child(curr_bs) != NULL; + curr_bs = bdrv_backing_chain_next(curr_bs)) + { + BlockDriverState *bs_below = bdrv_backing_chain_next(curr_bs); /* If either of the filename paths is actually a protocol, then * compare unmodified paths; otherwise make paths relative */ @@ -4605,7 +4638,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, char *backing_file_full_ret; if (strcmp(backing_file, curr_bs->backing_file) == 0) { - retval = curr_bs->backing->bs; + retval = bs_below; break; } /* Also check against the full backing filename for the image */ @@ -4615,7 +4648,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, bool equal = strcmp(backing_file, backing_file_full_ret) == 0; g_free(backing_file_full_ret); if (equal) { - retval = curr_bs->backing->bs; + retval = bs_below; break; } } @@ -4641,7 +4674,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, g_free(filename_tmp); if (strcmp(backing_file_full, filename_full) == 0) { - retval = curr_bs->backing->bs; + retval = bs_below; break; } } @@ -5802,3 +5835,107 @@ bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, return drv->bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp); } + +/* + * Return the child that @bs acts as an overlay for, and from which data may be + * copied in COW or COR operations. Usually this is the backing file. + */ +BdrvChild *bdrv_filtered_cow_child(BlockDriverState *bs) +{ + if (!bs || !bs->drv) { + return NULL; + } + + if (bs->drv->is_filter) { + return NULL; + } + + return bs->backing; +} + +/* + * If @bs acts as a pass-through filter for one of its children, + * return that child. "Pass-through" means that write operations to + * @bs are forwarded to that child instead of triggering COW. + */ +BdrvChild *bdrv_filtered_rw_child(BlockDriverState *bs) +{ + if (!bs || !bs->drv) { + return NULL; + } + + if (!bs->drv->is_filter) { + return NULL; + } + + return bs->backing ?: bs->file; +} + +/* + * Return any filtered child, independently of how it reacts to write + * accesses and whether data is copied onto this BDS through COR. + */ +BdrvChild *bdrv_filtered_child(BlockDriverState *bs) +{ + BdrvChild *cow_child = bdrv_filtered_cow_child(bs); + BdrvChild *rw_child = bdrv_filtered_rw_child(bs); + + /* There can only be one filtered child at a time */ + assert(!(cow_child && rw_child)); + + return cow_child ?: rw_child; +} + +static BlockDriverState *bdrv_skip_filters(BlockDriverState *bs, + bool stop_on_explicit_filter) +{ + BdrvChild *filtered; + + if (!bs) { + return NULL; + } + + while (!(stop_on_explicit_filter && !bs->implicit)) { + filtered = bdrv_filtered_rw_child(bs); + if (!filtered) { + break; + } + bs = filtered->bs; + } + /* + * Note that this treats nodes with bs->drv == NULL as not being + * R/W filters (bs->drv == NULL should be replaced by something + * else anyway). + * The advantage of this behavior is that this function will thus + * always return a non-NULL value (given a non-NULL @bs). + */ + + return bs; +} + +/* + * Return the first BDS that has not been added implicitly or that + * does not have an RW-filtered child down the chain starting from @bs + * (including @bs itself). + */ +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs) +{ + return bdrv_skip_filters(bs, true); +} + +/* + * Return the first BDS that does not have an RW-filtered child down + * the chain starting from @bs (including @bs itself). + */ +BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs) +{ + return bdrv_skip_filters(bs, false); +} + +/* + * For a backing chain, return the first non-filter backing image. + */ +BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs) +{ + return bdrv_skip_rw_filters(bdrv_filtered_cow_bs(bdrv_skip_rw_filters(bs))); +} diff --git a/block/backup.c b/block/backup.c index 435414e964..484f8fccc6 100644 --- a/block/backup.c +++ b/block/backup.c @@ -580,6 +580,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, int64_t len; BlockDriverInfo bdi; BackupBlockJob *job = NULL; + bool target_does_cow; int ret; assert(bs); @@ -674,8 +675,9 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, /* If there is no backing file on the target, we cannot rely on COW if our * backup cluster size is smaller than the target cluster size. Even for * targets with a backing file, try to avoid COW if possible. */ + target_does_cow = bdrv_filtered_cow_child(target); ret = bdrv_get_info(target, &bdi); - if (ret == -ENOTSUP && !target->backing) { + if (ret == -ENOTSUP && !target_does_cow) { /* Cluster size is not defined */ warn_report("The target block device doesn't provide " "information about the block size and it doesn't have a " @@ -684,14 +686,14 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, "this default, the backup may be unusable", BACKUP_CLUSTER_SIZE_DEFAULT); job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT; - } else if (ret < 0 && !target->backing) { + } else if (ret < 0 && !target_does_cow) { error_setg_errno(errp, -ret, "Couldn't determine the cluster size of the target image, " "which has no backing file"); error_append_hint(errp, "Aborting, since this may create an unusable destination image\n"); goto error; - } else if (ret < 0 && target->backing) { + } else if (ret < 0 && target_does_cow) { /* Not fatal; just trudge on ahead. */ job->cluster_size = BACKUP_CLUSTER_SIZE_DEFAULT; } else { diff --git a/block/block-backend.c b/block/block-backend.c index f6ea824308..0ea1215668 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -2091,11 +2091,17 @@ int blk_commit_all(void) AioContext *aio_context = blk_get_aio_context(blk); aio_context_acquire(aio_context); - if (blk_is_inserted(blk) && blk->root->bs->backing) { - int ret = bdrv_commit(blk->root->bs); - if (ret < 0) { - aio_context_release(aio_context); - return ret; + if (blk_is_inserted(blk)) { + BlockDriverState *non_filter; + + /* Legacy function, so skip implicit filters */ + non_filter = bdrv_skip_implicit_filters(blk->root->bs); + if (bdrv_filtered_cow_child(non_filter)) { + int ret = bdrv_commit(non_filter); + if (ret < 0) { + aio_context_release(aio_context); + return ret; + } } } aio_context_release(aio_context); diff --git a/block/commit.c b/block/commit.c index 87d0b208ea..4482ea8486 100644 --- a/block/commit.c +++ b/block/commit.c @@ -110,7 +110,7 @@ static void commit_abort(Job *job) * something to base, the intermediate images aren't valid any more. */ bdrv_child_try_set_perm(s->commit_top_bs->backing, 0, BLK_PERM_ALL, &error_abort); - bdrv_replace_node(s->commit_top_bs, backing_bs(s->commit_top_bs), + bdrv_replace_node(s->commit_top_bs, s->commit_top_bs->backing->bs, &error_abort); bdrv_unref(s->commit_top_bs); @@ -321,10 +321,16 @@ void commit_start(const char *job_id, BlockDriverState *bs, s->commit_top_bs = commit_top_bs; bdrv_unref(commit_top_bs); - /* Block all nodes between top and base, because they will - * disappear from the chain after this operation. */ + /* + * Block all nodes between top and base, because they will + * disappear from the chain after this operation. + * Note that this assumes that the user is fine with removing all + * nodes (including R/W filters) between top and base. Assuring + * this is the responsibility of the interface (i.e. whoever calls + * commit_start()). + */ assert(bdrv_chain_contains(top, base)); - for (iter = top; iter != base; iter = backing_bs(iter)) { + for (iter = top; iter != base; iter = bdrv_filtered_bs(iter)) { /* XXX BLK_PERM_WRITE needs to be allowed so we don't block ourselves * at s->base (if writes are blocked for a node, they are also blocked * for its backing file). The other options would be a second filter @@ -401,19 +407,22 @@ int bdrv_commit(BlockDriverState *bs) if (!drv) return -ENOMEDIUM; - if (!bs->backing) { + backing_file_bs = bdrv_filtered_cow_bs(bs); + + if (!backing_file_bs) { return -ENOTSUP; } if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, NULL) || - bdrv_op_is_blocked(bs->backing->bs, BLOCK_OP_TYPE_COMMIT_TARGET, NULL)) { + bdrv_op_is_blocked(backing_file_bs, BLOCK_OP_TYPE_COMMIT_TARGET, NULL)) + { return -EBUSY; } - ro = bs->backing->bs->read_only; + ro = backing_file_bs->read_only; if (ro) { - if (bdrv_reopen_set_read_only(bs->backing->bs, false, NULL)) { + if (bdrv_reopen_set_read_only(backing_file_bs, false, NULL)) { return -EACCES; } } @@ -428,8 +437,6 @@ int bdrv_commit(BlockDriverState *bs) } /* Insert commit_top block node above backing, so we can write to it */ - backing_file_bs = backing_bs(bs); - commit_top_bs = bdrv_new_open_driver(&bdrv_commit_top, NULL, BDRV_O_RDWR, &local_err); if (commit_top_bs == NULL) { @@ -515,15 +522,13 @@ ro_cleanup: qemu_vfree(buf); blk_unref(backing); - if (backing_file_bs) { - bdrv_set_backing_hd(bs, backing_file_bs, &error_abort); - } + bdrv_set_backing_hd(bs, backing_file_bs, &error_abort); bdrv_unref(commit_top_bs); blk_unref(src); if (ro) { /* ignoring error return here */ - bdrv_reopen_set_read_only(bs->backing->bs, true, NULL); + bdrv_reopen_set_read_only(backing_file_bs, true, NULL); } return ret; diff --git a/block/io.c b/block/io.c index 213ca03d8d..806f9b4933 100644 --- a/block/io.c +++ b/block/io.c @@ -118,8 +118,17 @@ static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src) void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs; + BlockDriverState *cow_bs = bdrv_filtered_cow_bs(bs); Error *local_err = NULL; + /* + * FIXME: There should be a function for this, and in fact there + * will be as of a follow-up patch. + */ + storage_bs = + child_bs(bs->file) ?: bdrv_filtered_rw_bs(bs); + memset(&bs->bl, 0, sizeof(bs->bl)); if (!drv) { @@ -131,13 +140,13 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) drv->bdrv_aio_preadv) ? 1 : 512; /* Take some limits from the children as a default */ - if (bs->file) { - bdrv_refresh_limits(bs->file->bs, &local_err); + if (storage_bs) { + bdrv_refresh_limits(storage_bs, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - bdrv_merge_limits(&bs->bl, &bs->file->bs->bl); + bdrv_merge_limits(&bs->bl, &storage_bs->bl); } else { bs->bl.min_mem_alignment = 512; bs->bl.opt_mem_alignment = getpagesize(); @@ -146,13 +155,13 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) bs->bl.max_iov = IOV_MAX; } - if (bs->backing) { - bdrv_refresh_limits(bs->backing->bs, &local_err); + if (cow_bs) { + bdrv_refresh_limits(cow_bs, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - bdrv_merge_limits(&bs->bl, &bs->backing->bs->bl); + bdrv_merge_limits(&bs->bl, &cow_bs->bl); } /* Then let the driver override it */ @@ -2175,11 +2184,12 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs, if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) { ret |= BDRV_BLOCK_ALLOCATED; } else if (want_zero) { + BlockDriverState *cow_bs = bdrv_filtered_cow_bs(bs); + if (bdrv_unallocated_blocks_are_zero(bs)) { ret |= BDRV_BLOCK_ZERO; - } else if (bs->backing) { - BlockDriverState *bs2 = bs->backing->bs; - int64_t size2 = bdrv_getlength(bs2); + } else if (cow_bs) { + int64_t size2 = bdrv_getlength(cow_bs); if (size2 >= 0 && offset >= size2) { ret |= BDRV_BLOCK_ZERO; @@ -2244,7 +2254,7 @@ static int coroutine_fn bdrv_co_block_status_above(BlockDriverState *bs, bool first = true; assert(bs != base); - for (p = bs; p != base; p = backing_bs(p)) { + for (p = bs; p != base; p = bdrv_filtered_bs(p)) { ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map, file); if (ret < 0) { @@ -2330,7 +2340,7 @@ int bdrv_block_status_above(BlockDriverState *bs, BlockDriverState *base, int bdrv_block_status(BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum, int64_t *map, BlockDriverState **file) { - return bdrv_block_status_above(bs, backing_bs(bs), + return bdrv_block_status_above(bs, bdrv_filtered_bs(bs), offset, bytes, pnum, map, file); } @@ -2340,9 +2350,9 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int ret; int64_t dummy; - ret = bdrv_common_block_status_above(bs, backing_bs(bs), false, offset, - bytes, pnum ? pnum : &dummy, NULL, - NULL); + ret = bdrv_common_block_status_above(bs, bdrv_filtered_bs(bs), false, + offset, bytes, pnum ? pnum : &dummy, + NULL, NULL); if (ret < 0) { return ret; } @@ -2396,7 +2406,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, n = pnum_inter; } - intermediate = backing_bs(intermediate); + intermediate = bdrv_filtered_bs(intermediate); } *pnum = n; @@ -3178,8 +3188,9 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, } if (!drv->bdrv_co_truncate) { - if (bs->file && drv->is_filter) { - ret = bdrv_co_truncate(bs->file, offset, prealloc, errp); + BdrvChild *filtered = bdrv_filtered_rw_child(bs); + if (filtered) { + ret = bdrv_co_truncate(filtered, offset, prealloc, errp); goto out; } error_setg(errp, "Image format driver does not support resize"); diff --git a/block/mirror.c b/block/mirror.c index c7ca8078d1..06b68f5cf2 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -655,8 +655,9 @@ static int mirror_exit_common(Job *job) &error_abort); if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) { BlockDriverState *backing = s->is_none_mode ? src : s->base; - if (backing_bs(target_bs) != backing) { - bdrv_set_backing_hd(target_bs, backing, &local_err); + if (bdrv_backing_chain_next(target_bs) != backing) { + bdrv_set_backing_hd(bdrv_skip_rw_filters(target_bs), backing, + &local_err); if (local_err) { error_report_err(local_err); ret = -EPERM; @@ -705,7 +706,7 @@ static int mirror_exit_common(Job *job) block_job_remove_all_bdrv(bjob); bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, &error_abort); - bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); + bdrv_replace_node(mirror_top_bs, mirror_top_bs->backing->bs, &error_abort); /* We just changed the BDS the job BB refers to (with either or both of the * bdrv_replace_node() calls), so switch the BB back so the cleanup does @@ -896,7 +897,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp) } else { s->target_cluster_size = BDRV_SECTOR_SIZE; } - if (backing_filename[0] && !target_bs->backing && + if (backing_filename[0] && !bdrv_filtered_cow_child(target_bs) && s->granularity < s->target_cluster_size) { s->buf_size = MAX(s->buf_size, s->target_cluster_size); s->cow_bitmap = bitmap_new(length); @@ -1073,7 +1074,7 @@ static void mirror_complete(Job *job, Error **errp) if (s->backing_mode == MIRROR_OPEN_BACKING_CHAIN) { int ret; - assert(!target->backing); + assert(!bdrv_filtered_cow_child(target)); ret = bdrv_open_backing_file(target, NULL, "backing", errp); if (ret < 0) { return; @@ -1629,7 +1630,9 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, * any jobs in them must be blocked */ if (target_is_backing) { BlockDriverState *iter; - for (iter = backing_bs(bs); iter != target; iter = backing_bs(iter)) { + for (iter = bdrv_filtered_bs(bs); iter != target; + iter = bdrv_filtered_bs(iter)) + { /* XXX BLK_PERM_WRITE needs to be allowed so we don't block * ourselves at s->base (if writes are blocked for a node, they are * also blocked for its backing file). The other options would be a @@ -1666,7 +1669,7 @@ fail: bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, &error_abort); - bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); + bdrv_replace_node(mirror_top_bs, mirror_top_bs->backing->bs, &error_abort); bdrv_unref(mirror_top_bs); } @@ -1682,14 +1685,14 @@ void mirror_start(const char *job_id, BlockDriverState *bs, MirrorCopyMode copy_mode, Error **errp) { bool is_none_mode; - BlockDriverState *base; + BlockDriverState *base = NULL; if (mode == MIRROR_SYNC_MODE_INCREMENTAL) { error_setg(errp, "Sync mode 'incremental' not supported"); return; } is_none_mode = mode == MIRROR_SYNC_MODE_NONE; - base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL; + base = mode == MIRROR_SYNC_MODE_TOP ? bdrv_backing_chain_next(bs) : NULL; mirror_start_job(job_id, bs, creation_flags, target, replaces, speed, granularity, buf_size, backing_mode, on_source_error, on_target_error, unmap, NULL, NULL, diff --git a/block/qapi.c b/block/qapi.c index 287e38e9de..509c023847 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -149,9 +149,13 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, return NULL; } - if (bs0->drv && bs0->backing) { + if (bs0->drv && bdrv_filtered_child(bs0)) { + /* + * Put any filtered child here (for backwards compatibility to when + * we put bs0->backing here, which might be any filtered child). + */ info->backing_file_depth++; - bs0 = bs0->backing->bs; + bs0 = bdrv_filtered_bs(bs0); (*p_image_info)->has_backing_image = true; p_image_info = &((*p_image_info)->backing_image); } else { @@ -160,9 +164,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, /* Skip automatically inserted nodes that the user isn't aware of for * query-block (blk != NULL), but not for query-named-block-nodes */ - while (blk && bs0->drv && bs0->implicit) { - bs0 = backing_bs(bs0); - assert(bs0); + if (blk) { + bs0 = bdrv_skip_implicit_filters(bs0); } } @@ -342,9 +345,9 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, BlockDriverState *bs = blk_bs(blk); char *qdev; - /* Skip automatically inserted nodes that the user isn't aware of */ - while (bs && bs->drv && bs->implicit) { - bs = backing_bs(bs); + if (bs) { + /* Skip automatically inserted nodes that the user isn't aware of */ + bs = bdrv_skip_implicit_filters(bs); } info->device = g_strdup(blk_name(blk)); @@ -501,6 +504,7 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk) static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs, bool blk_level) { + BlockDriverState *cow_bs; BlockStats *s = NULL; s = g_malloc0(sizeof(*s)); @@ -513,9 +517,8 @@ static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs, /* Skip automatically inserted nodes that the user isn't aware of in * a BlockBackend-level command. Stay at the exact node for a node-level * command. */ - while (blk_level && bs->drv && bs->implicit) { - bs = backing_bs(bs); - assert(bs); + if (blk_level) { + bs = bdrv_skip_implicit_filters(bs); } if (bdrv_get_node_name(bs)[0]) { @@ -530,9 +533,10 @@ static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs, s->parent = bdrv_query_bds_stats(bs->file->bs, blk_level); } - if (blk_level && bs->backing) { + cow_bs = bdrv_filtered_cow_bs(bs); + if (blk_level && cow_bs) { s->has_backing = true; - s->backing = bdrv_query_bds_stats(bs->backing->bs, blk_level); + s->backing = bdrv_query_bds_stats(cow_bs, blk_level); } return s; diff --git a/block/stream.c b/block/stream.c index 7a49ac0992..f4a7495dc9 100644 --- a/block/stream.c +++ b/block/stream.c @@ -59,11 +59,12 @@ static int stream_prepare(Job *job) StreamBlockJob *s = container_of(job, StreamBlockJob, common.job); BlockJob *bjob = &s->common; BlockDriverState *bs = blk_bs(bjob->blk); + BlockDriverState *unfiltered = bdrv_skip_rw_filters(bs); BlockDriverState *base = s->base; Error *local_err = NULL; int ret = 0; - if (bs->backing) { + if (bdrv_filtered_cow_child(unfiltered)) { const char *base_id = NULL, *base_fmt = NULL; if (base) { base_id = s->backing_file_str; @@ -71,7 +72,7 @@ static int stream_prepare(Job *job) base_fmt = base->drv->format_name; } } - ret = bdrv_change_backing_file(bs, base_id, base_fmt); + ret = bdrv_change_backing_file(unfiltered, base_id, base_fmt); bdrv_set_backing_hd(bs, base, &local_err); if (local_err) { error_report_err(local_err); @@ -112,7 +113,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp) int64_t n = 0; /* bytes */ void *buf; - if (!bs->backing) { + if (!bdrv_filtered_child(bs)) { goto out; } @@ -153,7 +154,7 @@ static int coroutine_fn stream_run(Job *job, Error **errp) } else if (ret >= 0) { /* Copy if allocated in the intermediate images. Limit to the * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE). */ - ret = bdrv_is_allocated_above(backing_bs(bs), base, + ret = bdrv_is_allocated_above(bdrv_filtered_bs(bs), base, offset, n, &n); /* Finish early if end of backing file has been reached */ @@ -253,7 +254,9 @@ void stream_start(const char *job_id, BlockDriverState *bs, * disappear from the chain after this operation. The streaming job reads * every block only once, assuming that it doesn't change, so block writes * and resizes. */ - for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) { + for (iter = bdrv_filtered_bs(bs); iter && iter != base; + iter = bdrv_filtered_bs(iter)) + { block_job_add_bdrv(&s->common, "intermediate node", iter, 0, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED, &error_abort); diff --git a/blockdev.c b/blockdev.c index 22e41fb862..cd7b536519 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1092,7 +1092,7 @@ void hmp_commit(Monitor *mon, const QDict *qdict) return; } - bs = blk_bs(blk); + bs = bdrv_skip_implicit_filters(blk_bs(blk)); aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); @@ -1662,7 +1662,7 @@ static void external_snapshot_prepare(BlkActionState *common, goto out; } - if (state->new_bs->backing != NULL) { + if (bdrv_filtered_cow_child(state->new_bs)) { error_setg(errp, "The snapshot already has a backing image"); goto out; } @@ -3211,6 +3211,13 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, if (!base_bs) { goto out; } + /* + * Streaming copies data through COR, so all of the filters + * between the target and the base are considered. Therefore, + * we can use bdrv_chain_contains() and do not have to use + * bdrv_legacy_chain_contains() (which does not go past + * explicitly added filters). + */ if (bs == base_bs || !bdrv_chain_contains(bs, base_bs)) { error_setg(errp, "Node '%s' is not a backing image of '%s'", base_node, device); @@ -3222,7 +3229,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, } /* Check for op blockers in the whole chain between bs and base */ - for (iter = bs; iter && iter != base_bs; iter = backing_bs(iter)) { + for (iter = bs; iter && iter != base_bs; iter = bdrv_filtered_bs(iter)) { if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_STREAM, errp)) { goto out; } @@ -3379,7 +3386,9 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, assert(bdrv_get_aio_context(base_bs) == aio_context); - for (iter = top_bs; iter != backing_bs(base_bs); iter = backing_bs(iter)) { + for (iter = top_bs; iter != bdrv_filtered_bs(base_bs); + iter = bdrv_filtered_bs(iter)) + { if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_COMMIT_TARGET, errp)) { goto out; } @@ -3390,6 +3399,11 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, error_setg(errp, "cannot commit an image into itself"); goto out; } + if (!bdrv_legacy_chain_contains(top_bs, base_bs)) { + /* We have to disallow this until the user can give explicit consent */ + error_setg(errp, "Cannot commit through explicit filter nodes"); + goto out; + } if (top_bs == bs) { if (has_backing_file) { @@ -3481,7 +3495,13 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn, /* See if we have a backing HD we can use to create our new image * on top of. */ if (backup->sync == MIRROR_SYNC_MODE_TOP) { - source = backing_bs(bs); + /* + * Backup will not replace the source by the target, so none + * of the filters skipped here will be removed (in contrast to + * mirror). Therefore, we can skip all of them when looking + * for the first COW relationship. + */ + source = bdrv_filtered_cow_bs(bdrv_skip_rw_filters(bs)); if (!source) { backup->sync = MIRROR_SYNC_MODE_FULL; } @@ -3501,9 +3521,14 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn, if (backup->mode != NEW_IMAGE_MODE_EXISTING) { assert(backup->format); if (source) { - bdrv_refresh_filename(source); - bdrv_img_create(backup->target, backup->format, source->filename, - source->drv->format_name, NULL, + /* Implicit filters should not appear in the filename */ + BlockDriverState *explicit_backing = + bdrv_skip_implicit_filters(source); + + bdrv_refresh_filename(explicit_backing); + bdrv_img_create(backup->target, backup->format, + explicit_backing->filename, + explicit_backing->drv->format_name, NULL, size, flags, false, &local_err); } else { bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL, @@ -3767,7 +3792,7 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, return; } - if (!bs->backing && sync == MIRROR_SYNC_MODE_TOP) { + if (!bdrv_backing_chain_next(bs) && sync == MIRROR_SYNC_MODE_TOP) { sync = MIRROR_SYNC_MODE_FULL; } @@ -3816,7 +3841,7 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, void qmp_drive_mirror(DriveMirror *arg, Error **errp) { - BlockDriverState *bs; + BlockDriverState *bs, *unfiltered_bs; BlockDriverState *source, *target_bs; AioContext *aio_context; BlockMirrorBackingMode backing_mode; @@ -3825,6 +3850,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) int flags; int64_t size; const char *format = arg->format; + const char *replaces_node_name = NULL; bs = qmp_get_root_bs(arg->device, errp); if (!bs) { @@ -3836,6 +3862,16 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) return; } + /* + * If the user has not instructed us otherwise, we should let the + * block job run from @bs (thus taking into account all filters on + * it) but replace @unfiltered_bs when it finishes (thus not + * removing those filters). + * (And if there are any explicit filters, we should assume the + * user knows how to use the @replaces option.) + */ + unfiltered_bs = bdrv_skip_implicit_filters(bs); + aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); @@ -3849,8 +3885,14 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) } flags = bs->open_flags | BDRV_O_RDWR; - source = backing_bs(bs); + source = bdrv_filtered_cow_bs(unfiltered_bs); if (!source && arg->sync == MIRROR_SYNC_MODE_TOP) { + if (bdrv_filtered_bs(unfiltered_bs)) { + /* @unfiltered_bs is an explicit filter */ + error_setg(errp, "Cannot perform sync=top mirror through an " + "explicitly added filter node on the source"); + goto out; + } arg->sync = MIRROR_SYNC_MODE_FULL; } if (arg->sync == MIRROR_SYNC_MODE_NONE) { @@ -3869,6 +3911,9 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) " named node of the graph"); goto out; } + replaces_node_name = arg->replaces; + } else if (unfiltered_bs != bs) { + replaces_node_name = unfiltered_bs->node_name; } if (arg->mode == NEW_IMAGE_MODE_ABSOLUTE_PATHS) { @@ -3888,6 +3933,9 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) bdrv_img_create(arg->target, format, NULL, NULL, NULL, size, flags, false, &local_err); } else { + /* Implicit filters should not appear in the filename */ + BlockDriverState *explicit_backing = bdrv_skip_implicit_filters(source); + switch (arg->mode) { case NEW_IMAGE_MODE_EXISTING: break; @@ -3895,8 +3943,8 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) /* create new image with backing file */ bdrv_refresh_filename(source); bdrv_img_create(arg->target, format, - source->filename, - source->drv->format_name, + explicit_backing->filename, + explicit_backing->drv->format_name, NULL, size, flags, false, &local_err); break; default: @@ -3928,7 +3976,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) bdrv_set_aio_context(target_bs, aio_context); blockdev_mirror_common(arg->has_job_id ? arg->job_id : NULL, bs, target_bs, - arg->has_replaces, arg->replaces, arg->sync, + !!replaces_node_name, replaces_node_name, arg->sync, backing_mode, arg->has_speed, arg->speed, arg->has_granularity, arg->granularity, arg->has_buf_size, arg->buf_size, @@ -3964,7 +4012,7 @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id, bool has_auto_dismiss, bool auto_dismiss, Error **errp) { - BlockDriverState *bs; + BlockDriverState *bs, *unfiltered_bs; BlockDriverState *target_bs; AioContext *aio_context; BlockMirrorBackingMode backing_mode = MIRROR_LEAVE_BACKING_CHAIN; @@ -3975,6 +4023,16 @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id, return; } + /* + * Same as in qmp_drive_mirror(): We want to run the job from @bs, + * but we want to replace @unfiltered_bs on completion. + */ + unfiltered_bs = bdrv_skip_implicit_filters(bs); + if (!has_replaces && unfiltered_bs != bs) { + replaces = unfiltered_bs->node_name; + has_replaces = true; + } + target_bs = bdrv_lookup_bs(target, target, errp); if (!target_bs) { return; diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index 6426151e4f..69c4e879bf 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -284,9 +284,7 @@ static int init_dirty_bitmap_migration(void) const char *drive_name = bdrv_get_device_or_node_name(bs); /* skip automatically inserted nodes */ - while (bs && bs->drv && bs->implicit) { - bs = backing_bs(bs); - } + bs = bdrv_skip_implicit_filters(bs); for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap; bitmap = bdrv_dirty_bitmap_next(bs, bitmap)) diff --git a/nbd/server.c b/nbd/server.c index 838c150d8c..2bd118707d 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1497,13 +1497,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset, BdrvDirtyBitmap *bm = NULL; BlockDriverState *bs = blk_bs(blk); - while (true) { + while (bs) { bm = bdrv_find_dirty_bitmap(bs, bitmap); - if (bm != NULL || bs->backing == NULL) { + if (bm != NULL) { break; } - bs = bs->backing->bs; + bs = bdrv_filtered_bs(bs); } if (bm == NULL) { diff --git a/qemu-img.c b/qemu-img.c index 8ee8dad771..92c08c3e1d 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -982,7 +982,7 @@ static int img_commit(int argc, char **argv) if (!blk) { return 1; } - bs = blk_bs(blk); + bs = bdrv_skip_implicit_filters(blk_bs(blk)); qemu_progress_init(progress, 1.f); qemu_progress_print(0.f, 100); @@ -999,7 +999,7 @@ static int img_commit(int argc, char **argv) /* This is different from QMP, which by default uses the deepest file in * the backing chain (i.e., the very base); however, the traditional * behavior of qemu-img commit is using the immediate backing file. */ - base_bs = backing_bs(bs); + base_bs = bdrv_filtered_cow_bs(bs); if (!base_bs) { error_setg(&local_err, "Image does not have a backing file"); goto done; @@ -1616,19 +1616,18 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) if (s->sector_next_status <= sector_num) { int64_t count = n * BDRV_SECTOR_SIZE; + BlockDriverState *src_bs = blk_bs(s->src[src_cur]); + BlockDriverState *base; if (s->target_has_backing) { - - ret = bdrv_block_status(blk_bs(s->src[src_cur]), - (sector_num - src_cur_offset) * - BDRV_SECTOR_SIZE, - count, &count, NULL, NULL); + base = bdrv_backing_chain_next(src_bs); } else { - ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL, - (sector_num - src_cur_offset) * - BDRV_SECTOR_SIZE, - count, &count, NULL, NULL); + base = NULL; } + ret = bdrv_block_status_above(src_bs, base, + (sector_num - src_cur_offset) * + BDRV_SECTOR_SIZE, + count, &count, NULL, NULL); if (ret < 0) { return ret; } @@ -2437,7 +2436,8 @@ static int img_convert(int argc, char **argv) * s.target_backing_sectors has to be negative, which it will * be automatically). The backing file length is used only * for optimizations, so such a case is not fatal. */ - s.target_backing_sectors = bdrv_nb_sectors(out_bs->backing->bs); + s.target_backing_sectors = + bdrv_nb_sectors(bdrv_filtered_cow_bs(out_bs)); } else { s.target_backing_sectors = -1; } @@ -2799,6 +2799,7 @@ static int get_block_status(BlockDriverState *bs, int64_t offset, depth = 0; for (;;) { + bs = bdrv_skip_rw_filters(bs); ret = bdrv_block_status(bs, offset, bytes, &bytes, &map, &file); if (ret < 0) { return ret; @@ -2807,7 +2808,7 @@ static int get_block_status(BlockDriverState *bs, int64_t offset, if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) { break; } - bs = backing_bs(bs); + bs = bdrv_filtered_cow_bs(bs); if (bs == NULL) { ret = 0; break; @@ -2946,7 +2947,7 @@ static int img_map(int argc, char **argv) if (!blk) { return 1; } - bs = blk_bs(blk); + bs = bdrv_skip_implicit_filters(blk_bs(blk)); if (output_format == OFORMAT_HUMAN) { printf("%-16s%-16s%-16s%s\n", "Offset", "Length", "Mapped to", "File"); diff --git a/tests/qemu-iotests/184.out b/tests/qemu-iotests/184.out index 3deb3cfb94..1d61f7e224 100644 --- a/tests/qemu-iotests/184.out +++ b/tests/qemu-iotests/184.out @@ -27,6 +27,11 @@ Testing: "iops_rd": 0, "detect_zeroes": "off", "image": { + "backing-image": { + "virtual-size": 1073741824, + "filename": "null-co://", + "format": "null-co" + }, "virtual-size": 1073741824, "filename": "json:{\"throttle-group\": \"group0\", \"driver\": \"throttle\", \"file\": {\"driver\": \"null-co\"}}", "format": "throttle" @@ -34,7 +39,7 @@ Testing: "iops_wr": 0, "ro": false, "node-name": "throttle0", - "backing_file_depth": 0, + "backing_file_depth": 1, "drv": "throttle", "iops": 0, "bps_wr": 0, diff --git a/tests/qemu-iotests/204.out b/tests/qemu-iotests/204.out index f3a10fbe90..684774d763 100644 --- a/tests/qemu-iotests/204.out +++ b/tests/qemu-iotests/204.out @@ -59,5 +59,6 @@ Offset Length File 0x900000 0x2400000 TEST_DIR/t.IMGFMT 0x3c00000 0x1100000 TEST_DIR/t.IMGFMT 0x6a00000 0x400000 TEST_DIR/t.IMGFMT +0x6e00000 0x1200000 TEST_DIR/t.IMGFMT.base No errors were found on the image. *** done From patchwork Wed Feb 13 22:53:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041654 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FHB0TwKz9sML for ; Thu, 14 Feb 2019 09:55:34 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36110 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3RD-0005Jq-WF for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 17:55:32 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Pp-0004m0-Q1 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3PD-000688-4f for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:53:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55110) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3PA-00063s-Vy; Wed, 13 Feb 2019 17:53:25 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3A60789AC4; Wed, 13 Feb 2019 22:53:24 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 62F5F19C7B; Wed, 13 Feb 2019 22:53:23 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:03 +0100 Message-Id: <20190213225311.17876-5-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 13 Feb 2019 22:53:24 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 04/12] block: Storage child access function 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" For completeness' sake, add a function for accessing a node's storage child, too. For filters, this is their filtered child; for non-filters, this is bs->file. Some places are deliberately left unconverted: - BDS opening/closing functions where bs->file is handled specially (which is basically wrong, but at least simplifies probing) - bdrv_co_block_status_from_file(), because its name implies that it points to ->file - bdrv_snapshot_goto() in one places unrefs bs->file. Such a modification is not covered by this patch and is therefore just safeguarded by an additional assert(), but otherwise kept as-is. Signed-off-by: Max Reitz --- include/block/block_int.h | 6 +++++ block.c | 53 ++++++++++++++++++++++++++++----------- block/io.c | 22 +++++++--------- block/qapi.c | 7 +++--- block/snapshot.c | 40 ++++++++++++++++------------- 5 files changed, 81 insertions(+), 47 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index d5b74181be..3e3b055c64 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1220,6 +1220,7 @@ int refresh_total_sectors(BlockDriverState *bs, int64_t hint); BdrvChild *bdrv_filtered_cow_child(BlockDriverState *bs); BdrvChild *bdrv_filtered_rw_child(BlockDriverState *bs); BdrvChild *bdrv_filtered_child(BlockDriverState *bs); +BdrvChild *bdrv_storage_child(BlockDriverState *bs); BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs); BlockDriverState *bdrv_skip_rw_filters(BlockDriverState *bs); BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs); @@ -1244,4 +1245,9 @@ static inline BlockDriverState *bdrv_filtered_bs(BlockDriverState *bs) return child_bs(bdrv_filtered_child(bs)); } +static inline BlockDriverState *bdrv_storage_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_storage_child(bs)); +} + #endif /* BLOCK_INT_H */ diff --git a/block.c b/block.c index 1f9c01d1c5..2bdef60370 100644 --- a/block.c +++ b/block.c @@ -3981,15 +3981,21 @@ exit: int64_t bdrv_get_allocated_file_size(BlockDriverState *bs) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs; + if (!drv) { return -ENOMEDIUM; } + if (drv->bdrv_get_allocated_file_size) { return drv->bdrv_get_allocated_file_size(bs); } - if (bs->file) { - return bdrv_get_allocated_file_size(bs->file->bs); + + storage_bs = bdrv_storage_bs(bs); + if (storage_bs) { + return bdrv_get_allocated_file_size(storage_bs); } + return -ENOTSUP; } @@ -4548,7 +4554,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event, const char *tag) { while (bs && bs->drv && !bs->drv->bdrv_debug_breakpoint) { - bs = bs->file ? bs->file->bs : NULL; + bs = bdrv_storage_bs(bs); } if (bs && bs->drv && bs->drv->bdrv_debug_breakpoint) { @@ -4561,7 +4567,7 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event, int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag) { while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) { - bs = bs->file ? bs->file->bs : NULL; + bs = bdrv_storage_bs(bs); } if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) { @@ -4574,7 +4580,7 @@ int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag) int bdrv_debug_resume(BlockDriverState *bs, const char *tag) { while (bs && (!bs->drv || !bs->drv->bdrv_debug_resume)) { - bs = bs->file ? bs->file->bs : NULL; + bs = bdrv_storage_bs(bs); } if (bs && bs->drv && bs->drv->bdrv_debug_resume) { @@ -4587,7 +4593,7 @@ int bdrv_debug_resume(BlockDriverState *bs, const char *tag) bool bdrv_debug_is_suspended(BlockDriverState *bs, const char *tag) { while (bs && bs->drv && !bs->drv->bdrv_debug_is_suspended) { - bs = bs->file ? bs->file->bs : NULL; + bs = bdrv_storage_bs(bs); } if (bs && bs->drv && bs->drv->bdrv_debug_is_suspended) { @@ -5704,14 +5710,23 @@ void bdrv_refresh_filename(BlockDriverState *bs) bs->exact_filename[0] = '\0'; drv->bdrv_refresh_filename(bs); - } else if (bs->file) { - /* Try to reconstruct valid information from the underlying file */ + } else if (bdrv_storage_child(bs)) { + /* + * Try to reconstruct valid information from the underlying + * file -- this only works for format nodes (filter nodes + * cannot be probed and as such must be selected by the user + * either through an options dict, or through a special + * filename which the filter driver must construct in its + * .bdrv_refresh_filename() implementation). + */ + BlockDriverState *storage_bs = bdrv_storage_bs(bs); bs->exact_filename[0] = '\0'; /* * We can use the underlying file's filename if: * - it has a filename, + * - the current BDS is not a filter, * - the file is a protocol BDS, and * - opening that file (as this BDS's format) will automatically create * the BDS tree we have right now, that is: @@ -5720,11 +5735,10 @@ void bdrv_refresh_filename(BlockDriverState *bs) * - no non-file child of this BDS has been overridden by the user * Both of these conditions are represented by generate_json_filename. */ - if (bs->file->bs->exact_filename[0] && - bs->file->bs->drv->bdrv_file_open && - !generate_json_filename) + if (storage_bs->exact_filename[0] && storage_bs->drv->bdrv_file_open && + !drv->is_filter && !generate_json_filename) { - strcpy(bs->exact_filename, bs->file->bs->exact_filename); + strcpy(bs->exact_filename, storage_bs->exact_filename); } } @@ -5741,6 +5755,7 @@ void bdrv_refresh_filename(BlockDriverState *bs) char *bdrv_dirname(BlockDriverState *bs, Error **errp) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs; if (!drv) { error_setg(errp, "Node '%s' is ejected", bs->node_name); @@ -5751,8 +5766,9 @@ char *bdrv_dirname(BlockDriverState *bs, Error **errp) return drv->bdrv_dirname(bs, errp); } - if (bs->file) { - return bdrv_dirname(bs->file->bs, errp); + storage_bs = bdrv_storage_bs(bs); + if (storage_bs) { + return bdrv_dirname(storage_bs, errp); } bdrv_refresh_filename(bs); @@ -5886,6 +5902,15 @@ BdrvChild *bdrv_filtered_child(BlockDriverState *bs) return cow_child ?: rw_child; } +/* + * Return the child that stores the data that is allocated on this + * node. This may or may not include metadata. + */ +BdrvChild *bdrv_storage_child(BlockDriverState *bs) +{ + return bdrv_filtered_rw_child(bs) ?: bs->file; +} + static BlockDriverState *bdrv_skip_filters(BlockDriverState *bs, bool stop_on_explicit_filter) { diff --git a/block/io.c b/block/io.c index 806f9b4933..e3750cd566 100644 --- a/block/io.c +++ b/block/io.c @@ -118,17 +118,10 @@ static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src) void bdrv_refresh_limits(BlockDriverState *bs, Error **errp) { BlockDriver *drv = bs->drv; - BlockDriverState *storage_bs; + BlockDriverState *storage_bs = bdrv_storage_bs(bs); BlockDriverState *cow_bs = bdrv_filtered_cow_bs(bs); Error *local_err = NULL; - /* - * FIXME: There should be a function for this, and in fact there - * will be as of a follow-up patch. - */ - storage_bs = - child_bs(bs->file) ?: bdrv_filtered_rw_bs(bs); - memset(&bs->bl, 0, sizeof(bs->bl)); if (!drv) { @@ -2426,6 +2419,7 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, bool is_read) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs = bdrv_storage_bs(bs); int ret = -ENOTSUP; bdrv_inc_in_flight(bs); @@ -2438,8 +2432,8 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos, } else { ret = drv->bdrv_save_vmstate(bs, qiov, pos); } - } else if (bs->file) { - ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read); + } else if (storage_bs) { + ret = bdrv_co_rw_vmstate(storage_bs, qiov, pos, is_read); } bdrv_dec_in_flight(bs); @@ -2577,6 +2571,7 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque) int coroutine_fn bdrv_co_flush(BlockDriverState *bs) { + BlockDriverState *storage_bs; int current_gen; int ret = 0; @@ -2606,7 +2601,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs) } /* Write back cached data to the OS even with cache=unsafe */ - BLKDBG_EVENT(bs->file, BLKDBG_FLUSH_TO_OS); + BLKDBG_EVENT(bdrv_storage_child(bs), BLKDBG_FLUSH_TO_OS); if (bs->drv->bdrv_co_flush_to_os) { ret = bs->drv->bdrv_co_flush_to_os(bs); if (ret < 0) { @@ -2624,7 +2619,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs) goto flush_parent; } - BLKDBG_EVENT(bs->file, BLKDBG_FLUSH_TO_DISK); + BLKDBG_EVENT(bdrv_storage_child(bs), BLKDBG_FLUSH_TO_DISK); if (!bs->drv) { /* bs->drv->bdrv_co_flush() might have ejected the BDS * (even in case of apparent success) */ @@ -2669,7 +2664,8 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs) * in the case of cache=unsafe, so there are no useless flushes. */ flush_parent: - ret = bs->file ? bdrv_co_flush(bs->file->bs) : 0; + storage_bs = bdrv_storage_bs(bs); + ret = storage_bs ? bdrv_co_flush(storage_bs) : 0; out: /* Notify any pending flushes that we have completed */ if (ret == 0) { diff --git a/block/qapi.c b/block/qapi.c index 509c023847..37597baa10 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -504,7 +504,7 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk) static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs, bool blk_level) { - BlockDriverState *cow_bs; + BlockDriverState *storage_bs, *cow_bs; BlockStats *s = NULL; s = g_malloc0(sizeof(*s)); @@ -528,9 +528,10 @@ static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs, s->stats->wr_highest_offset = stat64_get(&bs->wr_highest_offset); - if (bs->file) { + storage_bs = bdrv_storage_bs(bs); + if (storage_bs) { s->has_parent = true; - s->parent = bdrv_query_bds_stats(bs->file->bs, blk_level); + s->parent = bdrv_query_bds_stats(storage_bs, blk_level); } cow_bs = bdrv_filtered_cow_bs(bs); diff --git a/block/snapshot.c b/block/snapshot.c index 3218a542df..44c50c564c 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -154,8 +154,9 @@ int bdrv_can_snapshot(BlockDriverState *bs) } if (!drv->bdrv_snapshot_create) { - if (bs->file != NULL) { - return bdrv_can_snapshot(bs->file->bs); + BlockDriverState *storage_bs = bdrv_storage_bs(bs); + if (storage_bs) { + return bdrv_can_snapshot(storage_bs); } return 0; } @@ -167,14 +168,15 @@ int bdrv_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs = bdrv_storage_bs(bs); if (!drv) { return -ENOMEDIUM; } if (drv->bdrv_snapshot_create) { return drv->bdrv_snapshot_create(bs, sn_info); } - if (bs->file) { - return bdrv_snapshot_create(bs->file->bs, sn_info); + if (storage_bs) { + return bdrv_snapshot_create(storage_bs, sn_info); } return -ENOTSUP; } @@ -184,6 +186,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs, Error **errp) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs; int ret, open_ret; if (!drv) { @@ -204,39 +207,40 @@ int bdrv_snapshot_goto(BlockDriverState *bs, return ret; } - if (bs->file) { - BlockDriverState *file; + storage_bs = bdrv_storage_bs(bs); + if (storage_bs) { QDict *options = qdict_clone_shallow(bs->options); QDict *file_options; Error *local_err = NULL; - file = bs->file->bs; /* Prevent it from getting deleted when detached from bs */ - bdrv_ref(file); + bdrv_ref(storage_bs); qdict_extract_subqdict(options, &file_options, "file."); qobject_unref(file_options); - qdict_put_str(options, "file", bdrv_get_node_name(file)); + qdict_put_str(options, "file", bdrv_get_node_name(storage_bs)); if (drv->bdrv_close) { drv->bdrv_close(bs); } + + assert(bs->file->bs == storage_bs); bdrv_unref_child(bs, bs->file); bs->file = NULL; - ret = bdrv_snapshot_goto(file, snapshot_id, errp); + ret = bdrv_snapshot_goto(storage_bs, snapshot_id, errp); open_ret = drv->bdrv_open(bs, options, bs->open_flags, &local_err); qobject_unref(options); if (open_ret < 0) { - bdrv_unref(file); + bdrv_unref(storage_bs); bs->drv = NULL; /* A bdrv_snapshot_goto() error takes precedence */ error_propagate(errp, local_err); return ret < 0 ? ret : open_ret; } - assert(bs->file->bs == file); - bdrv_unref(file); + assert(bs->file->bs == storage_bs); + bdrv_unref(storage_bs); return ret; } @@ -272,6 +276,7 @@ int bdrv_snapshot_delete(BlockDriverState *bs, Error **errp) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs = bdrv_storage_bs(bs); int ret; if (!drv) { @@ -288,8 +293,8 @@ int bdrv_snapshot_delete(BlockDriverState *bs, if (drv->bdrv_snapshot_delete) { ret = drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp); - } else if (bs->file) { - ret = bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp); + } else if (storage_bs) { + ret = bdrv_snapshot_delete(storage_bs, snapshot_id, name, errp); } else { error_setg(errp, "Block format '%s' used by device '%s' " "does not support internal snapshot deletion", @@ -325,14 +330,15 @@ int bdrv_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_info) { BlockDriver *drv = bs->drv; + BlockDriverState *storage_bs = bdrv_storage_bs(bs); if (!drv) { return -ENOMEDIUM; } if (drv->bdrv_snapshot_list) { return drv->bdrv_snapshot_list(bs, psn_info); } - if (bs->file) { - return bdrv_snapshot_list(bs->file->bs, psn_info); + if (storage_bs) { + return bdrv_snapshot_list(storage_bs, psn_info); } return -ENOTSUP; } From patchwork Wed Feb 13 22:53:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041656 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FMV0JqDz9rxp for ; Thu, 14 Feb 2019 09:59:18 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36187 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Up-0008JX-Tk for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 17:59:15 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Pp-0004iX-5y for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3PF-0006AQ-6p for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:53:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49080) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3PD-000683-II; Wed, 13 Feb 2019 17:53:27 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D1E1759447; Wed, 13 Feb 2019 22:53:26 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 350C85D6AA; Wed, 13 Feb 2019 22:53:26 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:04 +0100 Message-Id: <20190213225311.17876-6-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 13 Feb 2019 22:53:26 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 05/12] block: Inline bdrv_co_block_status_from_*() 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" With bdrv_filtered_rw_bs(), we can easily handle this default filter behavior in bdrv_co_block_status(). blkdebug wants to have an additional assertion, so it keeps its own implementation, except bdrv_co_block_status_from_file() needs to be inlined there. Suggested-by: Eric Blake Signed-off-by: Max Reitz --- include/block/block_int.h | 22 ----------------- block/blkdebug.c | 7 ++++-- block/blklogwrites.c | 1 - block/commit.c | 1 - block/copy-on-read.c | 2 -- block/io.c | 51 +++++++++++++-------------------------- block/mirror.c | 1 - block/throttle.c | 1 - 8 files changed, 22 insertions(+), 64 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index 3e3b055c64..c81aa4132d 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1164,28 +1164,6 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared); -/* - * Default implementation for drivers to pass bdrv_co_block_status() to - * their file. - */ -int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs, - bool want_zero, - int64_t offset, - int64_t bytes, - int64_t *pnum, - int64_t *map, - BlockDriverState **file); -/* - * Default implementation for drivers to pass bdrv_co_block_status() to - * their backing file. - */ -int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, - bool want_zero, - int64_t offset, - int64_t bytes, - int64_t *pnum, - int64_t *map, - BlockDriverState **file); const char *bdrv_get_parent_name(const BlockDriverState *bs); void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); bool blk_dev_has_removable_media(BlockBackend *blk); diff --git a/block/blkdebug.c b/block/blkdebug.c index 1ea835c2b9..f3095b95ba 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -637,8 +637,11 @@ static int coroutine_fn blkdebug_co_block_status(BlockDriverState *bs, BlockDriverState **file) { assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment)); - return bdrv_co_block_status_from_file(bs, want_zero, offset, bytes, - pnum, map, file); + assert(bs->file && bs->file->bs); + *pnum = bytes; + *map = offset; + *file = bs->file->bs; + return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID; } static void blkdebug_close(BlockDriverState *bs) diff --git a/block/blklogwrites.c b/block/blklogwrites.c index eb2b4901a5..1eb4a5c613 100644 --- a/block/blklogwrites.c +++ b/block/blklogwrites.c @@ -518,7 +518,6 @@ static BlockDriver bdrv_blk_log_writes = { .bdrv_co_pwrite_zeroes = blk_log_writes_co_pwrite_zeroes, .bdrv_co_flush_to_disk = blk_log_writes_co_flush_to_disk, .bdrv_co_pdiscard = blk_log_writes_co_pdiscard, - .bdrv_co_block_status = bdrv_co_block_status_from_file, .is_filter = true, .strong_runtime_opts = blk_log_writes_strong_runtime_opts, diff --git a/block/commit.c b/block/commit.c index 4482ea8486..3d3ead5af3 100644 --- a/block/commit.c +++ b/block/commit.c @@ -251,7 +251,6 @@ static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c, static BlockDriver bdrv_commit_top = { .format_name = "commit_top", .bdrv_co_preadv = bdrv_commit_top_preadv, - .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_commit_top_refresh_filename, .bdrv_child_perm = bdrv_commit_top_child_perm, diff --git a/block/copy-on-read.c b/block/copy-on-read.c index 64dcc424b5..1b13ea9673 100644 --- a/block/copy-on-read.c +++ b/block/copy-on-read.c @@ -151,8 +151,6 @@ BlockDriver bdrv_copy_on_read = { .bdrv_eject = cor_eject, .bdrv_lock_medium = cor_lock_medium, - .bdrv_co_block_status = bdrv_co_block_status_from_file, - .bdrv_recurse_is_first_non_filter = cor_recurse_is_first_non_filter, .has_variable_length = true, diff --git a/block/io.c b/block/io.c index e3750cd566..a13af7ab4a 100644 --- a/block/io.c +++ b/block/io.c @@ -2029,36 +2029,6 @@ typedef struct BdrvCoBlockStatusData { bool done; } BdrvCoBlockStatusData; -int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs, - bool want_zero, - int64_t offset, - int64_t bytes, - int64_t *pnum, - int64_t *map, - BlockDriverState **file) -{ - assert(bs->file && bs->file->bs); - *pnum = bytes; - *map = offset; - *file = bs->file->bs; - return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID; -} - -int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, - bool want_zero, - int64_t offset, - int64_t bytes, - int64_t *pnum, - int64_t *map, - BlockDriverState **file) -{ - assert(bs->backing && bs->backing->bs); - *pnum = bytes; - *map = offset; - *file = bs->backing->bs; - return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID; -} - /* * Returns the allocation status of the specified sectors. * Drivers not implementing the functionality are assumed to not support @@ -2099,6 +2069,7 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs, BlockDriverState *local_file = NULL; int64_t aligned_offset, aligned_bytes; uint32_t align; + bool has_filtered_child; assert(pnum); *pnum = 0; @@ -2124,7 +2095,8 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs, /* Must be non-NULL or bdrv_getlength() would have failed */ assert(bs->drv); - if (!bs->drv->bdrv_co_block_status) { + has_filtered_child = bs->drv->is_filter && bdrv_filtered_rw_child(bs); + if (!bs->drv->bdrv_co_block_status && !has_filtered_child) { *pnum = bytes; ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED; if (offset + bytes == total_size) { @@ -2145,9 +2117,20 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs, aligned_offset = QEMU_ALIGN_DOWN(offset, align); aligned_bytes = ROUND_UP(offset + bytes, align) - aligned_offset; - ret = bs->drv->bdrv_co_block_status(bs, want_zero, aligned_offset, - aligned_bytes, pnum, &local_map, - &local_file); + if (bs->drv->bdrv_co_block_status) { + ret = bs->drv->bdrv_co_block_status(bs, want_zero, aligned_offset, + aligned_bytes, pnum, &local_map, + &local_file); + } else { + /* Default code for filters */ + + local_file = bdrv_filtered_rw_bs(bs); + assert(local_file); + + *pnum = aligned_bytes; + local_map = aligned_offset; + ret = BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID; + } if (ret < 0) { *pnum = 0; goto out; diff --git a/block/mirror.c b/block/mirror.c index 06b68f5cf2..c5609e3d68 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1467,7 +1467,6 @@ static BlockDriver bdrv_mirror_top = { .bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes, .bdrv_co_pdiscard = bdrv_mirror_top_pdiscard, .bdrv_co_flush = bdrv_mirror_top_flush, - .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_mirror_top_refresh_filename, .bdrv_child_perm = bdrv_mirror_top_child_perm, diff --git a/block/throttle.c b/block/throttle.c index f64dcc27b9..b6922e734f 100644 --- a/block/throttle.c +++ b/block/throttle.c @@ -259,7 +259,6 @@ static BlockDriver bdrv_throttle = { .bdrv_reopen_prepare = throttle_reopen_prepare, .bdrv_reopen_commit = throttle_reopen_commit, .bdrv_reopen_abort = throttle_reopen_abort, - .bdrv_co_block_status = bdrv_co_block_status_from_file, .bdrv_co_drain_begin = throttle_co_drain_begin, .bdrv_co_drain_end = throttle_co_drain_end, From patchwork Wed Feb 13 22:53:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041653 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FH30JLkz9sML for ; Thu, 14 Feb 2019 09:55:27 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36108 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3R6-0005Ak-T3 for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 17:55:25 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37764) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Pn-0004k0-Ny for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3PH-0006C8-Ja for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:53:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43964) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3PG-0006At-4h; Wed, 13 Feb 2019 17:53:30 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7074B81F19; Wed, 13 Feb 2019 22:53:29 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C3B5A60856; Wed, 13 Feb 2019 22:53:28 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:05 +0100 Message-Id: <20190213225311.17876-7-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 13 Feb 2019 22:53:29 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 06/12] block: Fix check_to_replace_node() 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Currently, check_to_replace_node() only allows mirror to replace a node in the chain of the source node, and only if it is the first non-filter node below the source. Well, technically, the idea is that you can exactly replace a quorum child by mirroring from quorum. This has (probably) two reasons: (1) We do not want to create loops. (2) @replaces and @device should have exactly the same content so replacing them does not cause visible data to change. This has two issues: (1) It is overly restrictive. It is completely fine for @replaces to be a filter. (2) It is not restrictive enough. You can create loops with this as follows: $ qemu-img create -f qcow2 /tmp/source.qcow2 64M $ qemu-system-x86_64 -qmp stdio {"execute": "qmp_capabilities"} {"execute": "object-add", "arguments": {"qom-type": "throttle-group", "id": "tg0"}} {"execute": "blockdev-add", "arguments": { "node-name": "source", "driver": "throttle", "throttle-group": "tg0", "file": { "node-name": "filtered", "driver": "qcow2", "file": { "driver": "file", "filename": "/tmp/source.qcow2" } } } } {"execute": "drive-mirror", "arguments": { "job-id": "mirror", "device": "source", "target": "/tmp/target.qcow2", "format": "qcow2", "node-name": "target", "sync" :"none", "replaces": "filtered" } } {"execute": "block-job-complete", "arguments": {"device": "mirror"}} And qemu crashes because of a stack overflow due to the loop being created (target's backing file is source, so when it replaces filtered, it points to itself through source). (blockdev-mirror can be broken similarly.) So let us make the checks for the two conditions above explicit, which makes the whole function exactly as restrictive as it needs to be. Signed-off-by: Max Reitz --- include/block/block.h | 1 + block.c | 83 +++++++++++++++++++++++++++++++++++++++---- blockdev.c | 34 ++++++++++++++++-- 3 files changed, 110 insertions(+), 8 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index c97b330e10..3b831f244a 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -388,6 +388,7 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate); /* check if a named node can be replaced when doing drive-mirror */ BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs, + BlockDriverState *backing_bs, const char *node_name, Error **errp); /* async block I/O */ diff --git a/block.c b/block.c index 2bdef60370..bf7cc4705e 100644 --- a/block.c +++ b/block.c @@ -5478,7 +5478,59 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate) return false; } +static bool is_child_of(BlockDriverState *child, BlockDriverState *parent) +{ + BdrvChild *c; + + if (!parent) { + return false; + } + + QLIST_FOREACH(c, &parent->children, next) { + if (c->bs == child || is_child_of(child, c->bs)) { + return true; + } + } + + return false; +} + +/* + * Return true if there are only filters in [@top, @base). Note that + * this may include quorum (which bdrv_chain_contains() cannot + * handle). + */ +static bool is_filtered_child(BlockDriverState *top, BlockDriverState *base) +{ + BdrvChild *c; + + if (!top) { + return false; + } + + if (top == base) { + return true; + } + + if (!top->drv->is_filter) { + return false; + } + + QLIST_FOREACH(c, &top->children, next) { + if (is_filtered_child(c->bs, base)) { + return true; + } + } + + return false; +} + +/* + * @parent_bs is mirror's source BDS, @backing_bs is the BDS which + * will be attached to the target when mirror completes. + */ BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs, + BlockDriverState *backing_bs, const char *node_name, Error **errp) { BlockDriverState *to_replace_bs = bdrv_find_node(node_name); @@ -5497,13 +5549,32 @@ BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs, goto out; } - /* We don't want arbitrary node of the BDS chain to be replaced only the top - * most non filter in order to prevent data corruption. - * Another benefit is that this tests exclude backing files which are - * blocked by the backing blockers. + /* + * If to_replace_bs is (recursively) a child of backing_bs, + * replacing it may create a loop. We cannot allow that. */ - if (!bdrv_recurse_is_first_non_filter(parent_bs, to_replace_bs)) { - error_setg(errp, "Only top most non filter can be replaced"); + if (to_replace_bs == backing_bs || is_child_of(to_replace_bs, backing_bs)) { + error_setg(errp, "Replacing this node would result in a loop"); + to_replace_bs = NULL; + goto out; + } + + /* + * Mirror is designed in such a way that when it completes, the + * source BDS is seamlessly replaced. It is therefore not allowed + * to replace a BDS where this condition would be violated, as that + * would defeat the purpose of mirror and could lead to data + * corruption. + * Therefore, between parent_bs and to_replace_bs there may be + * only filters (and the one on top must be a filter, too), so + * their data always stays in sync and mirror can complete and + * replace to_replace_bs without any possible corruptions. + */ + if (!is_filtered_child(parent_bs, to_replace_bs) && + !is_filtered_child(to_replace_bs, parent_bs)) + { + error_setg(errp, "The node to be replaced must be connected to the " + "source through filter nodes only"); to_replace_bs = NULL; goto out; } diff --git a/blockdev.c b/blockdev.c index cd7b536519..e620ea814b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3797,7 +3797,7 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, } if (has_replaces) { - BlockDriverState *to_replace_bs; + BlockDriverState *to_replace_bs, *backing_bs; AioContext *replace_aio_context; int64_t bs_size, replace_size; @@ -3807,7 +3807,37 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, return; } - to_replace_bs = check_to_replace_node(bs, replaces, errp); + if (backing_mode == MIRROR_SOURCE_BACKING_CHAIN || + backing_mode == MIRROR_OPEN_BACKING_CHAIN) + { + /* + * While we do not quite know what OPEN_BACKING_CHAIN + * (used for mode=existing) will yield, it is probably + * best to restrict it exactly like SOURCE_BACKING_CHAIN, + * because that is our best guess. + */ + switch (sync) { + case MIRROR_SYNC_MODE_FULL: + backing_bs = NULL; + break; + + case MIRROR_SYNC_MODE_TOP: + backing_bs = bdrv_filtered_cow_bs(bdrv_skip_rw_filters(bs)); + break; + + case MIRROR_SYNC_MODE_NONE: + backing_bs = bs; + break; + + default: + abort(); + } + } else { + assert(backing_mode == MIRROR_LEAVE_BACKING_CHAIN); + backing_bs = bdrv_filtered_cow_bs(bdrv_skip_rw_filters(target)); + } + + to_replace_bs = check_to_replace_node(bs, backing_bs, replaces, errp); if (!to_replace_bs) { return; } From patchwork Wed Feb 13 22:53:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041652 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FGh0ljZz9sMr for ; Thu, 14 Feb 2019 09:55:08 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36100 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Qn-0004mc-V6 for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 17:55:06 -0500 Received: from eggs.gnu.org ([209.51.188.92]:37732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Pl-0004iX-Q1 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3PJ-0006EW-T8 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:53:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51966) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3PI-0006Ce-Gz; Wed, 13 Feb 2019 17:53:32 -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 CA4748CD9; Wed, 13 Feb 2019 22:53:31 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5E57760C62; Wed, 13 Feb 2019 22:53:31 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:06 +0100 Message-Id: <20190213225311.17876-8-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 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.38]); Wed, 13 Feb 2019 22:53:31 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 07/12] iotests: Add tests for mirror @replaces loops 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This adds two tests for cases where our old check_to_replace_node() function failed to detect that executing this job with these parameters would result in a cyclic graph. Signed-off-by: Max Reitz Reviewed-by: Eric Blake --- tests/qemu-iotests/041 | 124 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/041.out | 4 +- 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 26bf1701eb..0c1432f189 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -1067,5 +1067,129 @@ class TestOrphanedSource(iotests.QMPTestCase): target='dest-ro') self.assert_qmp(result, 'error/class', 'GenericError') +# Various tests for the @replaces option (independent of quorum) +class TestReplaces(iotests.QMPTestCase): + def setUp(self): + self.vm = iotests.VM() + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + + def test_drive_mirror_loop(self): + qemu_img('create', '-f', iotests.imgfmt, test_img, '1M') + + result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'source', + 'driver': 'throttle', + 'throttle-group': 'tg', + 'file': { + 'node-name': 'filtered', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': test_img + } + } + }) + self.assert_qmp(result, 'return', {}) + + # Mirror from @source to @target in sync=none, so that @source + # will be @target's backing file; but replace @filtered. + # Then, @target's backing file will be @source, whose backing + # file is now @target instead of @filtered. That is a loop. + # (But apart from the loop, replacing @filtered instead of + # @source is fine, because both are just filtered versions of + # each other.) + result = self.vm.qmp('drive-mirror', + job_id='mirror', + device='source', + target=target_img, + format=iotests.imgfmt, + node_name='target', + sync='none', + replaces='filtered') + if 'error' in result: + # This is the correct result + self.assert_qmp(result, 'error/class', 'GenericError') + else: + # This is wrong, but let's run it to the bitter conclusion + self.complete_and_wait(drive='mirror') + # Fail for good measure, although qemu should have crashed + # anyway + self.fail('Loop creation was successful') + + os.remove(test_img) + try: + os.remove(target_img) + except OSError: + pass + + def test_blockdev_mirror_loop(self): + qemu_img('create', '-f', iotests.imgfmt, test_img, '1M') + qemu_img('create', '-f', iotests.imgfmt, target_img, '1M') + + result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'source', + 'driver': 'throttle', + 'throttle-group': 'tg', + 'file': { + 'node-name': 'middle', + 'driver': 'throttle', + 'throttle-group': 'tg', + 'file': { + 'node-name': 'bottom', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': test_img + } + } + } + }) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'target', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': target_img + }, + 'backing': 'middle' + }) + + # Mirror from @source to @target. With blockdev-mirror, the + # current (old) backing file is retained (which is @middle). + # By replacing @bottom, @middle's file will be @target, whose + # backing file is @middle again. That is a loop. + # (But apart from the loop, replacing @bottom instead of + # @source is fine, because both are just filtered versions of + # each other.) + result = self.vm.qmp('blockdev-mirror', + job_id='mirror', + device='source', + target='target', + sync='full', + replaces='bottom') + if 'error' in result: + # This is the correct result + self.assert_qmp(result, 'error/class', 'GenericError') + else: + # This is wrong, but let's run it to the bitter conclusion + self.complete_and_wait(drive='mirror') + # Fail for good measure, although qemu should have crashed + # anyway + self.fail('Loop creation was successful') + + os.remove(test_img) + os.remove(target_img) + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index e071d0b261..2c448b4239 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -........................................................................................ +.......................................................................................... ---------------------------------------------------------------------- -Ran 88 tests +Ran 90 tests OK From patchwork Wed Feb 13 22:53:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041669 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FVp54t9z9sN4 for ; Thu, 14 Feb 2019 10:05:38 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36294 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3ay-0004tt-If for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 18:05:36 -0500 Received: from eggs.gnu.org ([209.51.188.92]:38360) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3QI-0005DI-CG for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3QG-0007Er-FJ for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50738) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3Pf-0006GJ-UI; Wed, 13 Feb 2019 17:53:56 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6F9118535D; Wed, 13 Feb 2019 22:53:38 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EF0AE600C0; Wed, 13 Feb 2019 22:53:37 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:08 +0100 Message-Id: <20190213225311.17876-10-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 13 Feb 2019 22:53:38 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 09/12] iotests: Add filter commit test cases 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This patch adds some tests on how commit copes with filter nodes. Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 130 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/040.out | 4 +- 2 files changed, 132 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index b81133a474..dc3fe57fbd 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -394,5 +394,135 @@ class TestReopenOverlay(ImageCommitTestCase): def test_reopen_overlay(self): self.run_commit_test(self.img1, self.img0) +class TestCommitWithFilters(iotests.QMPTestCase): + img0 = os.path.join(iotests.test_dir, '0.img') + img1 = os.path.join(iotests.test_dir, '1.img') + img2 = os.path.join(iotests.test_dir, '2.img') + img3 = os.path.join(iotests.test_dir, '3.img') + + def setUp(self): + qemu_img('create', '-f', iotests.imgfmt, self.img0, '1M') + qemu_img('create', '-f', iotests.imgfmt, self.img1, '1M') + qemu_img('create', '-f', iotests.imgfmt, self.img2, '1M') + qemu_img('create', '-f', iotests.imgfmt, self.img3, '1M') + + self.vm = iotests.VM() + self.vm.launch() + result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'top-filter', + 'driver': 'throttle', + 'throttle-group': 'tg', + 'file': { + 'node-name': 'cow-3', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': self.img3 + }, + 'backing': { + 'node-name': 'cow-2', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': self.img2 + }, + 'backing': { + 'node-name': 'cow-1', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': self.img1 + }, + 'backing': { + 'node-name': 'bottom-filter', + 'driver': 'throttle', + 'throttle-group': 'tg', + 'file': { + 'node-name': 'cow-0', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': self.img0 + } + } + } + } + } + } + }) + self.assert_qmp(result, 'return', {}) + + def tearDown(self): + self.vm.shutdown() + os.remove(self.img3) + os.remove(self.img2) + os.remove(self.img1) + os.remove(self.img0) + + # Filters make for funny filenames, so we cannot just use + # self.imgX for the block-commit parameters + def get_filename(self, node): + return self.vm.node_info(node)['image']['filename'] + + def test_filterless_commit(self): + self.assert_no_active_block_jobs() + result = self.vm.qmp('block-commit', + job_id='commit', + device='top-filter', + top=self.get_filename('cow-2'), + base=self.get_filename('cow-1')) + self.assert_qmp(result, 'return', {}) + self.wait_until_completed(drive='commit') + + def test_commit_through_filter(self): + self.assert_no_active_block_jobs() + result = self.vm.qmp('block-commit', + job_id='commit', + device='top-filter', + top=self.get_filename('cow-1'), + base=self.get_filename('cow-0')) + # Cannot commit through explicitly added filters (yet, + # although in the future we probably want to make users use + # blockdev-copy for this) + self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', 'Cannot commit through explicit filter nodes') + + def test_filtered_active_commit_with_filter(self): + self.assert_no_active_block_jobs() + result = self.vm.qmp('block-commit', + job_id='commit', + device='top-filter', + base=self.get_filename('cow-2')) + # Not specifying @top means active commit, so including the + # filter on top (which is not allowed right now) + self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', 'Cannot commit through explicit filter nodes') + + def test_filtered_active_commit_without_filter(self): + cow3_name = self.get_filename('cow-3') + + self.assert_no_active_block_jobs() + result = self.vm.qmp('block-commit', + job_id='commit', + device='top-filter', + top=cow3_name, + base=self.get_filename('cow-2')) + # This is how you'd want to specify committing img3 into img2 + # disregarding the filter on top of img3 -- but that does not + # work, because you can only specify names of backing files + # (and img3 is not a backing file). The solution for this + # would be for block-commit to accept node names. + # Note that even if it did work, the above command would + # result in a non-active commit, because img3 is not the top + # node. Which is wrong, because img3 can still be written to, + # so it should be an active commit, but that is a different + # story. + self.assert_qmp(result, 'error/class', 'GenericError') + self.assert_qmp(result, 'error/desc', + 'Top image file %s not found' % cow3_name) + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out index 802ffaa0c0..220a5fa82c 100644 --- a/tests/qemu-iotests/040.out +++ b/tests/qemu-iotests/040.out @@ -1,5 +1,5 @@ -........................................... +............................................... ---------------------------------------------------------------------- -Ran 43 tests +Ran 47 tests OK From patchwork Wed Feb 13 22:53:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041661 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FR859wjz9sMr for ; Thu, 14 Feb 2019 10:02:28 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36259 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Xu-0002aA-Kz for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 18:02:26 -0500 Received: from eggs.gnu.org ([209.51.188.92]:38330) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3QG-0005DF-Jt for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3QE-0007BJ-IK for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:12683) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3Pj-0006Hf-TS; Wed, 13 Feb 2019 17:54:01 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 09C87C0669BB; Wed, 13 Feb 2019 22:53:41 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5E35760856; Wed, 13 Feb 2019 22:53:40 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:09 +0100 Message-Id: <20190213225311.17876-11-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 13 Feb 2019 22:53:41 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 10/12] iotests: Add filter mirror test cases 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This patch adds some test cases how mirroring relates to filters. One of them tests what happens when you mirror off a filtered COW node, two others use the mirror filter node as basically our only example of an implicitly created filter node so far (besides the commit filter). Signed-off-by: Max Reitz --- tests/qemu-iotests/041 | 146 ++++++++++++++++++++++++++++++++++++- tests/qemu-iotests/041.out | 4 +- 2 files changed, 147 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 0c1432f189..c2b5299f62 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -20,8 +20,9 @@ import time import os +import json import iotests -from iotests import qemu_img, qemu_io +from iotests import qemu_img, qemu_img_pipe, qemu_io backing_img = os.path.join(iotests.test_dir, 'backing.img') target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img') @@ -1191,5 +1192,148 @@ class TestReplaces(iotests.QMPTestCase): os.remove(test_img) os.remove(target_img) +# Tests for mirror with filters (and how the mirror filter behaves, as +# an example for an implicit filter) +class TestFilters(iotests.QMPTestCase): + def setUp(self): + qemu_img('create', '-f', iotests.imgfmt, backing_img, '1M') + qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, test_img) + qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, target_img) + + qemu_io('-c', 'write -P 1 0 512k', backing_img) + qemu_io('-c', 'write -P 2 512k 512k', test_img) + + self.vm = iotests.VM() + self.vm.launch() + + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'target', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': target_img + }, + 'backing': None + }) + self.assert_qmp(result, 'return', {}) + + self.filterless_chain = { + 'node-name': 'source', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': test_img + }, + 'backing': { + 'node-name': 'backing', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': backing_img + } + } + } + + def tearDown(self): + self.vm.shutdown() + + os.remove(test_img) + os.remove(target_img) + os.remove(backing_img) + + def test_cor(self): + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'filter', + 'driver': 'copy-on-read', + 'file': self.filterless_chain + }) + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-mirror', + job_id='mirror', + device='filter', + target='target', + sync='top') + self.assert_qmp(result, 'return', {}) + + self.complete_and_wait('mirror') + + self.vm.qmp('blockdev-del', node_name='target') + + target_map = qemu_img_pipe('map', '--output=json', target_img) + target_map = json.loads(target_map) + + assert target_map[0]['start'] == 0 + assert target_map[0]['length'] == 512 * 1024 + assert target_map[0]['depth'] == 1 + + assert target_map[1]['start'] == 512 * 1024 + assert target_map[1]['length'] == 512 * 1024 + assert target_map[1]['depth'] == 0 + + def test_implicit_mirror_filter(self): + result = self.vm.qmp('blockdev-add', **self.filterless_chain) + self.assert_qmp(result, 'return', {}) + + # We need this so we can query from above the mirror node + result = self.vm.qmp('device_add', + driver='virtio-blk', + id='virtio', + bus='pci.0', + drive='source') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-mirror', + job_id='mirror', + device='source', + target='target', + sync='top') + self.assert_qmp(result, 'return', {}) + + # The mirror filter is now an implicit node, so it should be + # invisible when querying the backing chain + device_info = self.vm.qmp('query-block')['return'][0] + assert device_info['qdev'] == '/machine/peripheral/virtio/virtio-backend' + + assert device_info['inserted']['node-name'] == 'source' + + image_info = device_info['inserted']['image'] + assert image_info['filename'] == test_img + assert image_info['backing-image']['filename'] == backing_img + + self.complete_and_wait('mirror') + + def test_explicit_mirror_filter(self): + # Same test as above, but this time we give the mirror filter + # a node-name so it will not be invisible + result = self.vm.qmp('blockdev-add', **self.filterless_chain) + self.assert_qmp(result, 'return', {}) + + # We need this so we can query from above the mirror node + result = self.vm.qmp('device_add', + driver='virtio-blk', + id='virtio', + bus='pci.0', + drive='source') + self.assert_qmp(result, 'return', {}) + + result = self.vm.qmp('blockdev-mirror', + job_id='mirror', + device='source', + target='target', + sync='top', + filter_node_name='mirror-filter') + self.assert_qmp(result, 'return', {}) + + # With a node-name given to it, the mirror filter should now + # be visible + device_info = self.vm.qmp('query-block')['return'][0] + assert device_info['qdev'] == '/machine/peripheral/virtio/virtio-backend' + + assert device_info['inserted']['node-name'] == 'mirror-filter' + + self.complete_and_wait('mirror') + + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index 2c448b4239..ffc779b4d1 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -.......................................................................................... +............................................................................................. ---------------------------------------------------------------------- -Ran 90 tests +Ran 93 tests OK From patchwork Wed Feb 13 22:53:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041685 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FXR1QDJz9rxp for ; Thu, 14 Feb 2019 10:07:03 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36354 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3cL-0005zZ-4N for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 18:07:01 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39661) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Us-0000gx-1l for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:59:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3Ur-0002bF-7Y for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:59:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60376) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3Pd-0006LY-Mb; Wed, 13 Feb 2019 17:53:55 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 68293C049E20; Wed, 13 Feb 2019 22:53:43 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id ED47760856; Wed, 13 Feb 2019 22:53:42 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:10 +0100 Message-Id: <20190213225311.17876-12-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 13 Feb 2019 22:53:43 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 11/12] iotests: Add test for commit in sub directory 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Add a test for committing an overlay in a sub directory to one of the images in its backing chain, using both relative and absolute filenames. Signed-off-by: Max Reitz --- tests/qemu-iotests/020 | 36 ++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/020.out | 10 ++++++++++ 2 files changed, 46 insertions(+) diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020 index 6b972d082f..8cd6a8aae8 100755 --- a/tests/qemu-iotests/020 +++ b/tests/qemu-iotests/020 @@ -31,6 +31,11 @@ _cleanup() _cleanup_test_img rm -f "$TEST_IMG.base" rm -f "$TEST_IMG.orig" + + rm -f "$TEST_DIR/subdir/t.$IMGFMT.base" + rm -f "$TEST_DIR/subdir/t.$IMGFMT.mid" + rm -f "$TEST_DIR/subdir/t.$IMGFMT" + rmdir "$TEST_DIR/subdir" &> /dev/null } trap "_cleanup; exit \$status" 0 1 2 3 15 @@ -134,6 +139,37 @@ $QEMU_IO -c 'writev 0 64k' "$TEST_IMG" | _filter_qemu_io $QEMU_IMG commit "$TEST_IMG" _cleanup + +echo +echo 'Testing commit in sub-directory with relative filenames' +echo + +pushd "$TEST_DIR" > /dev/null + +mkdir subdir + +TEST_IMG="subdir/t.$IMGFMT.base" _make_test_img 1M +TEST_IMG="subdir/t.$IMGFMT.mid" _make_test_img -b "t.$IMGFMT.base" +TEST_IMG="subdir/t.$IMGFMT" _make_test_img -b "t.$IMGFMT.mid" + +# Should work +$QEMU_IMG commit -b "t.$IMGFMT.mid" "subdir/t.$IMGFMT" + +# Might theoretically work, but does not in practice (we have to +# decide between this and the above; and since we always represent +# backing file names as relative to the overlay, we go for the above) +$QEMU_IMG commit -b "subdir/t.$IMGFMT.mid" "subdir/t.$IMGFMT" 2>&1 | \ + _filter_imgfmt + +# This should work as well +$QEMU_IMG commit -b "$TEST_DIR/subdir/t.$IMGFMT.mid" "subdir/t.$IMGFMT" + +popd > /dev/null + +# Now let's try with just absolute filenames +$QEMU_IMG commit -b "$TEST_DIR/subdir/t.$IMGFMT.mid" \ + "$TEST_DIR/subdir/t.$IMGFMT" + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/020.out b/tests/qemu-iotests/020.out index 4b722b2dd0..228c37dded 100644 --- a/tests/qemu-iotests/020.out +++ b/tests/qemu-iotests/020.out @@ -1094,4 +1094,14 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=json:{'driv wrote 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qemu-img: Block job failed: No space left on device + +Testing commit in sub-directory with relative filenames + +Formatting 'subdir/t.IMGFMT.base', fmt=IMGFMT size=1048576 +Formatting 'subdir/t.IMGFMT.mid', fmt=IMGFMT size=1048576 backing_file=t.IMGFMT.base +Formatting 'subdir/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=t.IMGFMT.mid +Image committed. +qemu-img: Did not find 'subdir/t.IMGFMT.mid' in the backing chain of 'subdir/t.IMGFMT' +Image committed. +Image committed. *** done From patchwork Wed Feb 13 22:53:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1041658 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 440FNW6NSxz9sMx for ; Thu, 14 Feb 2019 10:00:10 +1100 (AEDT) Received: from localhost ([127.0.0.1]:36201 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3Vd-0000jz-9U for incoming@patchwork.ozlabs.org; Wed, 13 Feb 2019 18:00:05 -0500 Received: from eggs.gnu.org ([209.51.188.92]:38331) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gu3QG-0005DG-K0 for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gu3QE-0007B7-Hk for qemu-devel@nongnu.org; Wed, 13 Feb 2019 17:54:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59038) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gu3Pf-0006Mn-W5; Wed, 13 Feb 2019 17:53:56 -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 C479937E74; Wed, 13 Feb 2019 22:53:45 +0000 (UTC) Received: from localhost (unknown [10.40.205.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 599D860C64; Wed, 13 Feb 2019 22:53:45 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 13 Feb 2019 23:53:11 +0100 Message-Id: <20190213225311.17876-13-mreitz@redhat.com> In-Reply-To: <20190213225311.17876-1-mreitz@redhat.com> References: <20190213225311.17876-1-mreitz@redhat.com> MIME-Version: 1.0 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.29]); Wed, 13 Feb 2019 22:53:45 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 12/12] iotests: Test committing to overridden backing 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 , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 61 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/040.out | 4 +-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index dc3fe57fbd..155c1d8c23 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -524,5 +524,66 @@ class TestCommitWithFilters(iotests.QMPTestCase): self.assert_qmp(result, 'error/desc', 'Top image file %s not found' % cow3_name) +class TestCommitWithOverriddenBacking(iotests.QMPTestCase): + img_base_a = os.path.join(iotests.test_dir, 'base_a.img') + img_base_b = os.path.join(iotests.test_dir, 'base_b.img') + img_top = os.path.join(iotests.test_dir, 'top.img') + + def setUp(self): + qemu_img('create', '-f', iotests.imgfmt, self.img_base_a, '1M') + qemu_img('create', '-f', iotests.imgfmt, self.img_base_b, '1M') + qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a, \ + self.img_top) + + self.vm = iotests.VM() + self.vm.launch() + + # Use base_b instead of base_a as the backing of top + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'top', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': self.img_top + }, + 'backing': { + 'node-name': 'base', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': self.img_base_b + } + } + }) + self.assert_qmp(result, 'return', {}) + + def tearDown(self): + self.vm.shutdown() + os.remove(self.img_top) + os.remove(self.img_base_a) + os.remove(self.img_base_b) + + def test_commit_to_a(self): + # Try committing to base_a (which should fail, as top's + # backing image is base_b instead) + result = self.vm.qmp('block-commit', + job_id='commit', + device='top', + base=self.img_base_a) + self.assert_qmp(result, 'error/class', 'GenericError') + + def test_commit_to_b(self): + # Try committing to base_b (which should work, since that is + # actually top's backing image) + result = self.vm.qmp('block-commit', + job_id='commit', + device='top', + base=self.img_base_b) + self.assert_qmp(result, 'return', {}) + + self.vm.event_wait('BLOCK_JOB_READY') + self.vm.qmp('block-job-complete', device='commit') + self.vm.event_wait('BLOCK_JOB_COMPLETED') + if __name__ == '__main__': iotests.main(supported_fmts=['qcow2', 'qed']) diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out index 220a5fa82c..bbcbc202a0 100644 --- a/tests/qemu-iotests/040.out +++ b/tests/qemu-iotests/040.out @@ -1,5 +1,5 @@ -............................................... +................................................. ---------------------------------------------------------------------- -Ran 47 tests +Ran 49 tests OK