From patchwork Mon Dec 17 22:43:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 1014805 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=2001:4830:134:3::11; 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 [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43Jbnv6m9Wz9s4s for ; Tue, 18 Dec 2018 09:45:07 +1100 (AEDT) Received: from localhost ([::1]:50226 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gZ1dJ-00073O-CN for incoming@patchwork.ozlabs.org; Mon, 17 Dec 2018 17:45:05 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33155) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gZ1cK-0006ow-CD for qemu-devel@nongnu.org; Mon, 17 Dec 2018 17:44:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gZ1cH-0005xQ-U4 for qemu-devel@nongnu.org; Mon, 17 Dec 2018 17:44:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36968) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gZ1cA-0005rU-Ls; Mon, 17 Dec 2018 17:43:54 -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 BE9CC394D5E; Mon, 17 Dec 2018 22:43:53 +0000 (UTC) Received: from localhost (ovpn-204-73.brq.redhat.com [10.40.204.73]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 169775D6A6; Mon, 17 Dec 2018 22:43:52 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 17 Dec 2018 23:43:18 +0100 Message-Id: <20181217224348.14922-2-mreitz@redhat.com> In-Reply-To: <20181217224348.14922-1-mreitz@redhat.com> References: <20181217224348.14922-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.38]); Mon, 17 Dec 2018 22:43:53 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v12 01/31] block: Use bdrv_refresh_filename() to pull X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Alberto Garcia , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Before this patch, bdrv_refresh_filename() is used in a pushing manner: Whenever the BDS graph is modified, the parents of the modified edges are supposed to be updated (recursively upwards). However, that is nonviable, considering that we want child changes not to concern parents. Also, in the long run we want a pull model anyway: Here, we would have a bdrv_filename() function which returns a BDS's filename, freshly constructed. This patch is an intermediate step. It adds bdrv_refresh_filename() calls before every place a BDS.filename value is used. The only exceptions are protocol drivers that use their own filename, which clearly would not profit from refreshing that filename before. Also, bdrv_get_encrypted_filename() is removed along the way (as a user of BDS.filename), since it is completely unused. In turn, all of the calls to bdrv_refresh_filename() before this patch are removed, because we no longer have to call this function on graph changes. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia --- include/block/block.h | 1 - block.c | 31 +++++++++++++++---------------- block/qapi.c | 4 ++++ block/raw-format.c | 1 + block/replication.c | 2 -- block/vhdx-log.c | 1 + block/vmdk.c | 6 ++++++ blockdev.c | 8 ++++++++ qemu-img.c | 11 +++++++++-- 9 files changed, 44 insertions(+), 21 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index f70a843b72..1e54c8f505 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -483,7 +483,6 @@ void bdrv_round_to_clusters(BlockDriverState *bs, int64_t *cluster_offset, int64_t *cluster_bytes); -const char *bdrv_get_encrypted_filename(BlockDriverState *bs); void bdrv_get_backing_filename(BlockDriverState *bs, char *filename, int filename_size); void bdrv_get_full_backing_filename(BlockDriverState *bs, diff --git a/block.c b/block.c index 4f5ff2cc12..2f29c56536 100644 --- a/block.c +++ b/block.c @@ -323,8 +323,11 @@ void bdrv_get_full_backing_filename_from_filename(const char *backed, void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz, Error **errp) { - char *backed = bs->exact_filename[0] ? bs->exact_filename : bs->filename; + char *backed; + bdrv_refresh_filename(bs); + + backed = bs->exact_filename[0] ? bs->exact_filename : bs->filename; bdrv_get_full_backing_filename_from_filename(backed, bs->backing_file, dest, sz, errp); } @@ -1004,6 +1007,8 @@ static void bdrv_backing_attach(BdrvChild *c) "node is used as backing hd of '%s'", bdrv_get_device_or_node_name(parent)); + bdrv_refresh_filename(backing_hd); + parent->open_flags &= ~BDRV_O_NO_BACKING; pstrcpy(parent->backing_file, sizeof(parent->backing_file), backing_hd->filename); @@ -1413,6 +1418,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file, } if (file != NULL) { + bdrv_refresh_filename(blk_bs(file)); filename = blk_bs(file)->filename; } else { /* @@ -2303,8 +2309,6 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, bdrv_unref(backing_hd); } - bdrv_refresh_filename(bs); - out: bdrv_refresh_limits(bs, NULL); } @@ -2833,8 +2837,6 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, g_free(child_key_dot); } - bdrv_refresh_filename(bs); - /* Check if any unknown options were used */ if (qdict_size(options) != 0) { const QDictEntry *entry = qdict_first(options); @@ -3279,6 +3281,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, if (local_err != NULL) { error_propagate(errp, local_err); } else { + bdrv_refresh_filename(reopen_state->bs); error_setg(errp, "failed while preparing to reopen image '%s'", reopen_state->bs->filename); } @@ -3874,7 +3877,10 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base, /* success - we can delete the intermediate states, and link top->base */ /* TODO Check graph modification op blockers (BLK_PERM_GRAPH_MOD) once * we've figured out how they should work. */ - backing_file_str = backing_file_str ? backing_file_str : base->filename; + if (!backing_file_str) { + bdrv_refresh_filename(base); + backing_file_str = base->filename; + } QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) { /* Check whether we are allowed to switch c from top to base */ @@ -4274,16 +4280,6 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs) return bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP; } -const char *bdrv_get_encrypted_filename(BlockDriverState *bs) -{ - if (bs->backing && bs->backing->bs->encrypted) - return bs->backing_file; - else if (bs->encrypted) - return bs->filename; - else - return NULL; -} - void bdrv_get_backing_filename(BlockDriverState *bs, char *filename, int filename_size) { @@ -4403,6 +4399,9 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, is_protocol = path_has_protocol(backing_file); + /* This will recursively refresh everything in the backing chain */ + bdrv_refresh_filename(bs); + for (curr_bs = bs; curr_bs->backing; curr_bs = curr_bs->backing->bs) { /* If either of the filename paths is actually a protocol, then diff --git a/block/qapi.c b/block/qapi.c index c66f949db8..743136b84e 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -51,6 +51,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, return NULL; } + bdrv_refresh_filename(bs); + info = g_malloc0(sizeof(*info)); info->file = g_strdup(bs->filename); info->ro = bs->read_only; @@ -264,6 +266,8 @@ void bdrv_query_image_info(BlockDriverState *bs, goto out; } + bdrv_refresh_filename(bs); + info = g_new0(ImageInfo, 1); info->filename = g_strdup(bs->filename); info->format = g_strdup(bdrv_get_format_name(bs)); diff --git a/block/raw-format.c b/block/raw-format.c index 6f6dc99b2c..d07bcdae62 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -436,6 +436,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, bs->file->bs->supported_zero_flags); if (bs->probed && !bdrv_is_read_only(bs)) { + bdrv_refresh_filename(bs->file->bs); fprintf(stderr, "WARNING: Image format was not specified for '%s' and probing " "guessed raw.\n" diff --git a/block/replication.c b/block/replication.c index e70dd95001..9b332002ee 100644 --- a/block/replication.c +++ b/block/replication.c @@ -616,8 +616,6 @@ static void replication_done(void *opaque, int ret) if (ret == 0) { s->stage = BLOCK_REPLICATION_DONE; - /* refresh top bs's filename */ - bdrv_refresh_filename(bs); s->active_disk = NULL; s->secondary_disk = NULL; s->hidden_disk = NULL; diff --git a/block/vhdx-log.c b/block/vhdx-log.c index ecd64266c5..3149ff08d8 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -803,6 +803,7 @@ int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s, bool *flushed, if (logs.valid) { if (bs->read_only) { + bdrv_refresh_filename(bs); ret = -EPERM; error_setg(errp, "VHDX image file '%s' opened read-only, but " diff --git a/block/vmdk.c b/block/vmdk.c index 2c9e86d98f..bb6033d409 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -479,6 +479,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent, extent->l1_table, l1_size); if (ret < 0) { + bdrv_refresh_filename(extent->file->bs); error_setg_errno(errp, -ret, "Could not read l1 table from extent '%s'", extent->file->bs->filename); @@ -499,6 +500,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent, extent->l1_backup_table, l1_size); if (ret < 0) { + bdrv_refresh_filename(extent->file->bs); error_setg_errno(errp, -ret, "Could not read l1 backup table from extent '%s'", extent->file->bs->filename); @@ -530,6 +532,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs, ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); if (ret < 0) { + bdrv_refresh_filename(file->bs); error_setg_errno(errp, -ret, "Could not read header from file '%s'", file->bs->filename); @@ -607,6 +610,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); if (ret < 0) { + bdrv_refresh_filename(file->bs); error_setg_errno(errp, -ret, "Could not read header from file '%s'", file->bs->filename); @@ -861,6 +865,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, if (!path_is_absolute(fname) && !path_has_protocol(fname) && !desc_file_path[0]) { + bdrv_refresh_filename(bs->file->bs); error_setg(errp, "Cannot use relative extent paths with VMDK " "descriptor file '%s'", bs->file->bs->filename); return -EINVAL; @@ -2241,6 +2246,7 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent) { ImageInfo *info = g_new0(ImageInfo, 1); + bdrv_refresh_filename(extent->file->bs); *info = (ImageInfo){ .filename = g_strdup(extent->file->bs->filename), .format = g_strdup(extent->type), diff --git a/blockdev.c b/blockdev.c index e6c8349409..a26ca00803 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1629,6 +1629,7 @@ static void external_snapshot_prepare(BlkActionState *common, error_setg_errno(errp, -size, "bdrv_getlength failed"); goto out; } + bdrv_refresh_filename(state->old_bs); bdrv_img_create(new_image_file, format, state->old_bs->filename, state->old_bs->drv->format_name, @@ -3191,6 +3192,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, goto out; } assert(bdrv_get_aio_context(base_bs) == aio_context); + bdrv_refresh_filename(base_bs); base_name = base_bs->filename; } @@ -3310,6 +3312,10 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, goto out; } } else if (has_top && top) { + /* This strcmp() is just a shortcut, there is no need to + * refresh @bs's filename. If it mismatches, + * bdrv_find_backing_image() will do the refresh and may still + * return @bs. */ if (strcmp(bs->filename, top) != 0) { top_bs = bdrv_find_backing_image(bs, top); } @@ -3470,6 +3476,7 @@ 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, size, flags, false, &local_err); @@ -3845,6 +3852,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) break; case NEW_IMAGE_MODE_ABSOLUTE_PATHS: /* create new image with backing file */ + bdrv_refresh_filename(source); bdrv_img_create(arg->target, format, source->filename, source->drv->format_name, diff --git a/qemu-img.c b/qemu-img.c index ad04f59565..da14ec0bb5 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2790,6 +2790,7 @@ static int get_block_status(BlockDriverState *bs, int64_t offset, BlockDriverState *file; bool has_offset; int64_t map; + char *filename = NULL; /* As an optimization, we could cache the current range of unallocated * clusters in each file of the chain, and avoid querying the same @@ -2817,6 +2818,11 @@ static int get_block_status(BlockDriverState *bs, int64_t offset, has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID); + if (file && has_offset) { + bdrv_refresh_filename(file); + filename = file->filename; + } + *e = (MapEntry) { .start = offset, .length = bytes, @@ -2825,8 +2831,8 @@ static int get_block_status(BlockDriverState *bs, int64_t offset, .offset = map, .has_offset = has_offset, .depth = depth, - .has_filename = file && has_offset, - .filename = file && has_offset ? file->filename : NULL, + .has_filename = filename, + .filename = filename, }; return 0; @@ -3327,6 +3333,7 @@ static int img_rebase(int argc, char **argv) qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true); } + bdrv_refresh_filename(bs); overlay_filename = bs->exact_filename[0] ? bs->exact_filename : bs->filename; out_real_path = g_malloc(PATH_MAX);