From patchwork Thu Aug 9 21:37:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 10562067 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EECB313B4 for ; Thu, 9 Aug 2018 22:13:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DCB972B7F9 for ; Thu, 9 Aug 2018 22:13:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D09972B800; Thu, 9 Aug 2018 22:13:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 259992B7F9 for ; Thu, 9 Aug 2018 22:13:21 +0000 (UTC) Received: from localhost ([::1]:53151 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fntBI-0005Hl-89 for patchwork-qemu-devel@patchwork.kernel.org; Thu, 09 Aug 2018 18:13:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43811) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fnsdU-0004cN-Ft for qemu-devel@nongnu.org; Thu, 09 Aug 2018 17:38:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fnsdS-0004Sx-Pg for qemu-devel@nongnu.org; Thu, 09 Aug 2018 17:38:24 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47664 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fnsdP-0004Nh-1l; Thu, 09 Aug 2018 17:38:19 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 91DC581663CF; Thu, 9 Aug 2018 21:38:18 +0000 (UTC) Received: from localhost (ovpn-204-42.brq.redhat.com [10.40.204.42]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 120822314E; Thu, 9 Aug 2018 21:38:17 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Thu, 9 Aug 2018 23:37:57 +0200 Message-Id: <20180809213801.15098-8-mreitz@redhat.com> In-Reply-To: <20180809213801.15098-1-mreitz@redhat.com> References: <20180809213801.15098-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Thu, 09 Aug 2018 21:38:18 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Thu, 09 Aug 2018 21:38:18 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH 07/11] block: Leave BDS.backing_file constant 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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Parts of the block layer treat BDS.backing_file as if it were whatever the image header says (i.e., if it is a relative path, it is relative to the overlay), other parts treat it like a cache for bs->backing->bs->filename (relative paths are relative to the CWD). Considering bs->backing->bs->filename exists, let us make it mean the former. Among other things, this now allows the user to specify a base when using qemu-img to commit an image file in a directory that is not the CWD (assuming, everything uses relative filenames). Before this patch: $ ./qemu-img create -f qcow2 foo/bot.qcow2 1M $ ./qemu-img create -f qcow2 -b bot.qcow2 foo/mid.qcow2 $ ./qemu-img create -f qcow2 -b mid.qcow2 foo/top.qcow2 $ ./qemu-img commit -b mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find '[...]/foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' After this patch: $ ./qemu-img commit -b mid.qcow2 foo/top.qcow2 Image committed. $ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2 Image committed. With this change, bdrv_find_backing_image() must look at whether the user has overridden a BDS's backing file. If so, it can no longer use bs->backing_file, but must instead compare the given filename against the backing node's filename directly. Along with this, stop updating BDS.backing_format in bdrv_backing_attach() as well. This necessitates a change to the reference output of iotest 191. Signed-off-by: Max Reitz --- include/block/block_int.h | 14 +++++++++----- block.c | 29 ++++++++++++++++++++++------- block/qapi.c | 7 ++++--- qemu-img.c | 12 ++++++++++-- tests/qemu-iotests/191.out | 1 - 5 files changed, 45 insertions(+), 18 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index d3d8b22155..8f2c515ec1 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -737,11 +737,15 @@ struct BlockDriverState { bool walking_aio_notifiers; /* to make removal during iteration safe */ char filename[PATH_MAX]; - char backing_file[PATH_MAX]; /* if non zero, the image is a diff of - this file image */ - /* The backing filename indicated by the image header; if we ever - * open this file, then this is replaced by the resulting BDS's - * filename (i.e. after a bdrv_refresh_filename() run). */ + /* If non-zero, the image is a diff of this image file. Note that + * this the name given in the image header and may therefore not + * be equal to .backing->bs->filename, and relative paths are + * resolved relatively to their overlay. */ + char backing_file[PATH_MAX]; + /* The backing filename indicated by the image header. Contrary + * to backing_file, if we ever open this file, auto_backing_file + * is replaced by the resulting BDS's filename (i.e. after a + * bdrv_refresh_filename() run). */ char auto_backing_file[PATH_MAX]; char backing_format[16]; /* if non-zero and backing_file exists */ diff --git a/block.c b/block.c index 9784ccb385..7fc7dbf364 100644 --- a/block.c +++ b/block.c @@ -78,6 +78,8 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, const BdrvChildRole *child_role, Error **errp); +static bool bdrv_backing_overridden(BlockDriverState *bs); + /* If non-zero, use only whitelisted block drivers */ static int use_bdrv_whitelist; @@ -995,10 +997,6 @@ static void bdrv_backing_attach(BdrvChild *c) bdrv_refresh_filename(backing_hd); parent->open_flags &= ~BDRV_O_NO_BACKING; - pstrcpy(parent->backing_file, sizeof(parent->backing_file), - backing_hd->filename); - pstrcpy(parent->backing_format, sizeof(parent->backing_format), - backing_hd->drv ? backing_hd->drv->format_name : ""); bdrv_op_block_all(backing_hd, parent->backing_blocker); /* Otherwise we won't be able to commit or stream */ @@ -4318,6 +4316,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, char *backing_file_full = NULL; char *filename_tmp = NULL; int is_protocol = 0; + bool filenames_refreshed = false; BlockDriverState *curr_bs = NULL; BlockDriverState *retval = NULL; @@ -4340,9 +4339,25 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *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 */ - if (is_protocol || path_has_protocol(curr_bs->backing_file)) { + if (bdrv_backing_overridden(curr_bs)) { + /* If the backing file was overridden, we can only compare + * directly against the backing node's filename */ + + if (!filenames_refreshed) { + /* This will automatically refresh all of the + * filenames in the rest of the backing chain, so we + * only need to do this once */ + bdrv_refresh_filename(bs_below); + filenames_refreshed = true; + } + + if (strcmp(backing_file, bs_below->filename) == 0) { + retval = bs_below; + break; + } + } else if (is_protocol || path_has_protocol(curr_bs->backing_file)) { + /* If either of the filename paths is actually a protocol, then + * compare unmodified paths; otherwise make paths relative */ char *backing_file_full_ret; if (strcmp(backing_file, curr_bs->backing_file) == 0) { diff --git a/block/qapi.c b/block/qapi.c index cbee819c13..f5288012f5 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -43,7 +43,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, BlockDriverState *bs, Error **errp) { ImageInfo **p_image_info; - BlockDriverState *bs0; + BlockDriverState *bs0, *backing; BlockDeviceInfo *info; if (!bs->drv) { @@ -72,9 +72,10 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, info->node_name = g_strdup(bs->node_name); } - if (bs->backing_file[0]) { + backing = bdrv_filtered_cow_bs(bs); + if (backing) { info->has_backing_file = true; - info->backing_file = g_strdup(bs->backing_file); + info->backing_file = g_strdup(backing->filename); } info->detect_zeroes = bs->detect_zeroes; diff --git a/qemu-img.c b/qemu-img.c index 307e72c9fd..6d405fb6d4 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3292,7 +3292,7 @@ static int img_rebase(int argc, char **argv) /* For safe rebasing we need to compare old and new backing file */ if (!unsafe) { - char backing_name[PATH_MAX]; + char *backing_name; QDict *options = NULL; if (bs->backing_format[0] != '\0') { @@ -3306,16 +3306,24 @@ static int img_rebase(int argc, char **argv) } qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true); } - bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name)); + backing_name = bdrv_get_full_backing_filename(bs, &local_err); + if (local_err) { + error_reportf_err(local_err, + "Could not resolve old backing file name: "); + ret = -1; + goto out; + } blk_old_backing = blk_new_open(backing_name, NULL, options, src_flags, &local_err); if (!blk_old_backing) { error_reportf_err(local_err, "Could not open old backing file '%s': ", backing_name); + g_free(backing_name); ret = -1; goto out; } + g_free(backing_name); if (out_baseimg[0]) { const char *overlay_filename; diff --git a/tests/qemu-iotests/191.out b/tests/qemu-iotests/191.out index 31a0c7d4c4..b87cddc56f 100644 --- a/tests/qemu-iotests/191.out +++ b/tests/qemu-iotests/191.out @@ -604,7 +604,6 @@ wrote 65536/65536 bytes at offset 1048576 "backing-filename": "TEST_DIR/t.IMGFMT.base", "dirty-flag": false }, - "backing-filename-format": "IMGFMT", "virtual-size": 67108864, "filename": "TEST_DIR/t.IMGFMT.ovl3", "cluster-size": 65536,