block: Use bdrv_refresh_filename() to pull

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 <mreitz@redhat.com>
Message-id: 20190201192935.18394-2-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
Max Reitz 2019-02-01 20:29:05 +01:00
parent 83c68e149a
commit f30c66ba6e
9 changed files with 44 additions and 21 deletions

31
block.c
View File

@ -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, void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz,
Error **errp) 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, bdrv_get_full_backing_filename_from_filename(backed, bs->backing_file,
dest, sz, errp); dest, sz, errp);
} }
@ -1004,6 +1007,8 @@ static void bdrv_backing_attach(BdrvChild *c)
"node is used as backing hd of '%s'", "node is used as backing hd of '%s'",
bdrv_get_device_or_node_name(parent)); bdrv_get_device_or_node_name(parent));
bdrv_refresh_filename(backing_hd);
parent->open_flags &= ~BDRV_O_NO_BACKING; parent->open_flags &= ~BDRV_O_NO_BACKING;
pstrcpy(parent->backing_file, sizeof(parent->backing_file), pstrcpy(parent->backing_file, sizeof(parent->backing_file),
backing_hd->filename); backing_hd->filename);
@ -1413,6 +1418,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file,
} }
if (file != NULL) { if (file != NULL) {
bdrv_refresh_filename(blk_bs(file));
filename = blk_bs(file)->filename; filename = blk_bs(file)->filename;
} else { } else {
/* /*
@ -2334,8 +2340,6 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
bdrv_unref(backing_hd); bdrv_unref(backing_hd);
} }
bdrv_refresh_filename(bs);
out: out:
bdrv_refresh_limits(bs, NULL); bdrv_refresh_limits(bs, NULL);
} }
@ -2864,8 +2868,6 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
g_free(child_key_dot); g_free(child_key_dot);
} }
bdrv_refresh_filename(bs);
/* Check if any unknown options were used */ /* Check if any unknown options were used */
if (qdict_size(options) != 0) { if (qdict_size(options) != 0) {
const QDictEntry *entry = qdict_first(options); const QDictEntry *entry = qdict_first(options);
@ -3310,6 +3312,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
if (local_err != NULL) { if (local_err != NULL) {
error_propagate(errp, local_err); error_propagate(errp, local_err);
} else { } else {
bdrv_refresh_filename(reopen_state->bs);
error_setg(errp, "failed while preparing to reopen image '%s'", error_setg(errp, "failed while preparing to reopen image '%s'",
reopen_state->bs->filename); reopen_state->bs->filename);
} }
@ -3937,7 +3940,10 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
/* success - we can delete the intermediate states, and link top->base */ /* success - we can delete the intermediate states, and link top->base */
/* TODO Check graph modification op blockers (BLK_PERM_GRAPH_MOD) once /* TODO Check graph modification op blockers (BLK_PERM_GRAPH_MOD) once
* we've figured out how they should work. */ * 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) { QLIST_FOREACH_SAFE(c, &top->parents, next_parent, next) {
/* Check whether we are allowed to switch c from top to base */ /* Check whether we are allowed to switch c from top to base */
@ -4485,16 +4491,6 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs)
return bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP; 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, void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size) char *filename, int filename_size)
{ {
@ -4615,6 +4611,9 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
is_protocol = path_has_protocol(backing_file); 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) { for (curr_bs = bs; curr_bs->backing; curr_bs = curr_bs->backing->bs) {
/* If either of the filename paths is actually a protocol, then /* If either of the filename paths is actually a protocol, then

View File

@ -51,6 +51,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
return NULL; return NULL;
} }
bdrv_refresh_filename(bs);
info = g_malloc0(sizeof(*info)); info = g_malloc0(sizeof(*info));
info->file = g_strdup(bs->filename); info->file = g_strdup(bs->filename);
info->ro = bs->read_only; info->ro = bs->read_only;
@ -264,6 +266,8 @@ void bdrv_query_image_info(BlockDriverState *bs,
goto out; goto out;
} }
bdrv_refresh_filename(bs);
info = g_new0(ImageInfo, 1); info = g_new0(ImageInfo, 1);
info->filename = g_strdup(bs->filename); info->filename = g_strdup(bs->filename);
info->format = g_strdup(bdrv_get_format_name(bs)); info->format = g_strdup(bdrv_get_format_name(bs));

View File

@ -436,6 +436,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
bs->file->bs->supported_zero_flags); bs->file->bs->supported_zero_flags);
if (bs->probed && !bdrv_is_read_only(bs)) { if (bs->probed && !bdrv_is_read_only(bs)) {
bdrv_refresh_filename(bs->file->bs);
fprintf(stderr, fprintf(stderr,
"WARNING: Image format was not specified for '%s' and probing " "WARNING: Image format was not specified for '%s' and probing "
"guessed raw.\n" "guessed raw.\n"

View File

@ -616,8 +616,6 @@ static void replication_done(void *opaque, int ret)
if (ret == 0) { if (ret == 0) {
s->stage = BLOCK_REPLICATION_DONE; s->stage = BLOCK_REPLICATION_DONE;
/* refresh top bs's filename */
bdrv_refresh_filename(bs);
s->active_disk = NULL; s->active_disk = NULL;
s->secondary_disk = NULL; s->secondary_disk = NULL;
s->hidden_disk = NULL; s->hidden_disk = NULL;

View File

@ -803,6 +803,7 @@ int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s, bool *flushed,
if (logs.valid) { if (logs.valid) {
if (bs->read_only) { if (bs->read_only) {
bdrv_refresh_filename(bs);
ret = -EPERM; ret = -EPERM;
error_setg(errp, error_setg(errp,
"VHDX image file '%s' opened read-only, but " "VHDX image file '%s' opened read-only, but "

View File

@ -479,6 +479,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,
extent->l1_table, extent->l1_table,
l1_size); l1_size);
if (ret < 0) { if (ret < 0) {
bdrv_refresh_filename(extent->file->bs);
error_setg_errno(errp, -ret, error_setg_errno(errp, -ret,
"Could not read l1 table from extent '%s'", "Could not read l1 table from extent '%s'",
extent->file->bs->filename); extent->file->bs->filename);
@ -499,6 +500,7 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,
extent->l1_backup_table, extent->l1_backup_table,
l1_size); l1_size);
if (ret < 0) { if (ret < 0) {
bdrv_refresh_filename(extent->file->bs);
error_setg_errno(errp, -ret, error_setg_errno(errp, -ret,
"Could not read l1 backup table from extent '%s'", "Could not read l1 backup table from extent '%s'",
extent->file->bs->filename); 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)); ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
if (ret < 0) { if (ret < 0) {
bdrv_refresh_filename(file->bs);
error_setg_errno(errp, -ret, error_setg_errno(errp, -ret,
"Could not read header from file '%s'", "Could not read header from file '%s'",
file->bs->filename); file->bs->filename);
@ -607,6 +610,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
if (ret < 0) { if (ret < 0) {
bdrv_refresh_filename(file->bs);
error_setg_errno(errp, -ret, error_setg_errno(errp, -ret,
"Could not read header from file '%s'", "Could not read header from file '%s'",
file->bs->filename); 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) && if (!path_is_absolute(fname) && !path_has_protocol(fname) &&
!desc_file_path[0]) !desc_file_path[0])
{ {
bdrv_refresh_filename(bs->file->bs);
error_setg(errp, "Cannot use relative extent paths with VMDK " error_setg(errp, "Cannot use relative extent paths with VMDK "
"descriptor file '%s'", bs->file->bs->filename); "descriptor file '%s'", bs->file->bs->filename);
return -EINVAL; return -EINVAL;
@ -2470,6 +2475,7 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent)
{ {
ImageInfo *info = g_new0(ImageInfo, 1); ImageInfo *info = g_new0(ImageInfo, 1);
bdrv_refresh_filename(extent->file->bs);
*info = (ImageInfo){ *info = (ImageInfo){
.filename = g_strdup(extent->file->bs->filename), .filename = g_strdup(extent->file->bs->filename),
.format = g_strdup(extent->type), .format = g_strdup(extent->type),

View File

@ -1627,6 +1627,7 @@ static void external_snapshot_prepare(BlkActionState *common,
error_setg_errno(errp, -size, "bdrv_getlength failed"); error_setg_errno(errp, -size, "bdrv_getlength failed");
goto out; goto out;
} }
bdrv_refresh_filename(state->old_bs);
bdrv_img_create(new_image_file, format, bdrv_img_create(new_image_file, format,
state->old_bs->filename, state->old_bs->filename,
state->old_bs->drv->format_name, state->old_bs->drv->format_name,
@ -3230,6 +3231,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
goto out; goto out;
} }
assert(bdrv_get_aio_context(base_bs) == aio_context); assert(bdrv_get_aio_context(base_bs) == aio_context);
bdrv_refresh_filename(base_bs);
base_name = base_bs->filename; base_name = base_bs->filename;
} }
@ -3349,6 +3351,10 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,
goto out; goto out;
} }
} else if (has_top && top) { } 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) { if (strcmp(bs->filename, top) != 0) {
top_bs = bdrv_find_backing_image(bs, top); top_bs = bdrv_find_backing_image(bs, top);
} }
@ -3509,6 +3515,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
if (backup->mode != NEW_IMAGE_MODE_EXISTING) { if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
assert(backup->format); assert(backup->format);
if (source) { if (source) {
bdrv_refresh_filename(source);
bdrv_img_create(backup->target, backup->format, source->filename, bdrv_img_create(backup->target, backup->format, source->filename,
source->drv->format_name, NULL, source->drv->format_name, NULL,
size, flags, false, &local_err); size, flags, false, &local_err);
@ -3889,6 +3896,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
break; break;
case NEW_IMAGE_MODE_ABSOLUTE_PATHS: case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
/* create new image with backing file */ /* create new image with backing file */
bdrv_refresh_filename(source);
bdrv_img_create(arg->target, format, bdrv_img_create(arg->target, format,
source->filename, source->filename,
source->drv->format_name, source->drv->format_name,

View File

@ -485,7 +485,6 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
int64_t *cluster_offset, int64_t *cluster_offset,
int64_t *cluster_bytes); int64_t *cluster_bytes);
const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
void bdrv_get_backing_filename(BlockDriverState *bs, void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size); char *filename, int filename_size);
void bdrv_get_full_backing_filename(BlockDriverState *bs, void bdrv_get_full_backing_filename(BlockDriverState *bs,

View File

@ -2790,6 +2790,7 @@ static int get_block_status(BlockDriverState *bs, int64_t offset,
BlockDriverState *file; BlockDriverState *file;
bool has_offset; bool has_offset;
int64_t map; int64_t map;
char *filename = NULL;
/* As an optimization, we could cache the current range of unallocated /* As an optimization, we could cache the current range of unallocated
* clusters in each file of the chain, and avoid querying the same * 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); has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID);
if (file && has_offset) {
bdrv_refresh_filename(file);
filename = file->filename;
}
*e = (MapEntry) { *e = (MapEntry) {
.start = offset, .start = offset,
.length = bytes, .length = bytes,
@ -2825,8 +2831,8 @@ static int get_block_status(BlockDriverState *bs, int64_t offset,
.offset = map, .offset = map,
.has_offset = has_offset, .has_offset = has_offset,
.depth = depth, .depth = depth,
.has_filename = file && has_offset, .has_filename = filename,
.filename = file && has_offset ? file->filename : NULL, .filename = filename,
}; };
return 0; return 0;
@ -3334,6 +3340,7 @@ static int img_rebase(int argc, char **argv)
qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true); qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true);
} }
bdrv_refresh_filename(bs);
overlay_filename = bs->exact_filename[0] ? bs->exact_filename overlay_filename = bs->exact_filename[0] ? bs->exact_filename
: bs->filename; : bs->filename;
out_real_path = g_malloc(PATH_MAX); out_real_path = g_malloc(PATH_MAX);