[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PATCH v5 5/6] block: Drop BlockDriverState.filename
|
From: |
Max Reitz |
|
Subject: |
[Qemu-block] [PATCH v5 5/6] block: Drop BlockDriverState.filename |
|
Date: |
Mon, 19 Oct 2015 20:49:05 +0200 |
That field is now only used during initialization of BlockDriverStates
(opening images) and for error or warning messages. Performance is not
that much of an issue here, so we can drop the field and replace its use
by a call to bdrv_filename(). By doing so we can ensure the result
always to be recent, whereas the contents of BlockDriverState.filename
may have been obsoleted by manipulations of single BlockDriverStates or
of the BDS graph.
The users of the BDS filename field were changed as follows:
- copying the filename into another buffer is trivially replaced by
using bdrv_filename() instead of the copy function
- strdup() on the filename is replaced by a call to
bdrv_filename(filename, NULL, 0)
- bdrv_filename(bs, bs->filename, sizeof(bs->filename)) is replaced by
bdrv_refresh_filename(bs)
(these were introduced by the patch before the previous patch)
- anywhere else bdrv_filename(..., NULL, 0) is used, any access to
BlockDriverState.filename is then replaced by the buffer returned, and
it is freed when it is no longer needed
Signed-off-by: Max Reitz <address@hidden>
---
block.c | 46 ++++++++++++++++++++++++++++++----------------
block/blkverify.c | 3 +--
block/commit.c | 4 +++-
block/mirror.c | 16 ++++++++++++----
block/qapi.c | 4 ++--
block/quorum.c | 3 +--
block/raw_bsd.c | 4 +++-
block/vhdx-log.c | 5 ++++-
block/vmdk.c | 22 ++++++++++++++++------
block/vpc.c | 7 +++++--
blockdev.c | 25 +++++++++++++++++++------
include/block/block_int.h | 1 -
12 files changed, 96 insertions(+), 44 deletions(-)
diff --git a/block.c b/block.c
index 1061d80..a17e95f 100644
--- a/block.c
+++ b/block.c
@@ -226,10 +226,10 @@ 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_filename(bs, NULL, 0);
bdrv_get_full_backing_filename_from_filename(backed, bs->backing_file,
dest, sz, errp);
+ g_free(backed);
}
void bdrv_register(BlockDriver *bdrv)
@@ -817,6 +817,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild
*file,
QDict *options, int flags, BlockDriver *drv, Error **errp)
{
int ret, open_flags;
+ char *filename_buffer = NULL;
const char *filename;
const char *node_name = NULL;
QemuOpts *opts;
@@ -827,7 +828,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild
*file,
assert(options != NULL && bs->options != options);
if (file != NULL) {
- filename = file->bs->filename;
+ filename_buffer = bdrv_filename(file->bs, NULL, 0);
+ filename = filename_buffer;
} else {
filename = qdict_get_try_str(options, "filename");
}
@@ -835,7 +837,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild
*file,
if (drv->bdrv_needs_filename && !filename) {
error_setg(errp, "The '%s' block driver requires a file name",
drv->format_name);
- return -EINVAL;
+ ret = -EINVAL;
+ goto fail_filename;
}
trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
@@ -883,11 +886,10 @@ static int bdrv_open_common(BlockDriverState *bs,
BdrvChild *file,
}
if (filename != NULL) {
- pstrcpy(bs->filename, sizeof(bs->filename), filename);
+ pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), filename);
} else {
- bs->filename[0] = '\0';
+ bs->exact_filename[0] = '\0';
}
- pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), bs->filename);
bs->drv = drv;
bs->opaque = g_malloc0(drv->instance_size);
@@ -947,6 +949,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild
*file,
assert((bs->request_alignment != 0) || bdrv_is_sg(bs));
qemu_opts_del(opts);
+ g_free(filename_buffer);
return 0;
free_and_fail:
@@ -956,6 +959,8 @@ free_and_fail:
bs->drv = NULL;
fail_opts:
qemu_opts_del(opts);
+fail_filename:
+ g_free(filename_buffer);
return ret;
}
@@ -1155,7 +1160,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
BlockDriverState *backing_hd)
}
bs->backing = bdrv_attach_child(bs, backing_hd, &child_backing);
bs->open_flags &= ~BDRV_O_NO_BACKING;
- pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
+ bdrv_filename(backing_hd, bs->backing_file, sizeof(bs->backing_file));
pstrcpy(bs->backing_format, sizeof(bs->backing_format),
backing_hd->drv ? backing_hd->drv->format_name : "");
@@ -1527,7 +1532,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs,
const char *filename,
}
}
- bdrv_filename(bs, bs->filename, sizeof(bs->filename));
+ bdrv_refresh_filename(bs);
/* Check if any unknown options were used */
if (options && (qdict_size(options) != 0)) {
@@ -1807,8 +1812,10 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
if (local_err != NULL) {
error_propagate(errp, local_err);
} else {
+ char *filename = bdrv_filename(reopen_state->bs, NULL, 0);
error_setg(errp, "failed while preparing to reopen image '%s'",
- reopen_state->bs->filename);
+ filename);
+ g_free(filename);
}
goto error;
}
@@ -2369,6 +2376,7 @@ int bdrv_drop_intermediate(BlockDriverState *active,
BlockDriverState *top,
BlockDriverState *base, const char
*backing_file_str)
{
BlockDriverState *new_top_bs = NULL;
+ char *base_filename = NULL;
int ret = -EIO;
if (!top->drv || !base->drv) {
@@ -2395,7 +2403,10 @@ int bdrv_drop_intermediate(BlockDriverState *active,
BlockDriverState *top,
}
/* success - we can delete the intermediate states, and link top->base */
- backing_file_str = backing_file_str ? backing_file_str : base->filename;
+ if (!backing_file_str) {
+ base_filename = bdrv_filename(base, NULL, 0);
+ backing_file_str = base_filename;
+ }
ret = bdrv_change_backing_file(new_top_bs, backing_file_str,
base->drv ? base->drv->format_name : "");
if (ret) {
@@ -2405,6 +2416,7 @@ int bdrv_drop_intermediate(BlockDriverState *active,
BlockDriverState *top,
ret = 0;
exit:
+ g_free(base_filename);
return ret;
}
@@ -2818,7 +2830,7 @@ char *bdrv_get_encrypted_filename(BlockDriverState *bs)
if (bs->backing && bs->backing->bs->encrypted) {
return g_strdup(bs->backing_file);
} else if (bs->encrypted) {
- return g_strdup(bs->filename);
+ return bdrv_filename(bs, NULL, 0);
} else {
return NULL;
}
@@ -2918,7 +2930,7 @@ int bdrv_is_snapshot(BlockDriverState *bs)
}
/* backing_file can either be relative, or absolute, or a protocol. If it is
- * relative, it must be relative to the chain. So, passing in bs->filename
+ * relative, it must be relative to the chain. So, passing in the filename
* from a BDS as backing_file should not be done, as that may be relative to
* the CWD rather than the chain. */
BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
@@ -2953,18 +2965,20 @@ BlockDriverState
*bdrv_find_backing_image(BlockDriverState *bs,
} else {
/* If not an absolute filename path, make it relative to the
current
* image's filename path */
- path_combine(filename_tmp, PATH_MAX, curr_bs->filename,
- backing_file);
+ char *curr_filename = bdrv_filename(curr_bs, NULL, 0);
+ path_combine(filename_tmp, PATH_MAX, curr_filename, backing_file);
/* We are going to compare absolute pathnames */
if (!realpath(filename_tmp, filename_full)) {
+ g_free(curr_filename);
continue;
}
/* We need to make sure the backing filename we are comparing
against
* is relative to the current image filename (or absolute) */
- path_combine(filename_tmp, PATH_MAX, curr_bs->filename,
+ path_combine(filename_tmp, PATH_MAX, curr_filename,
curr_bs->backing_file);
+ g_free(curr_filename);
if (!realpath(filename_tmp, backing_file_full)) {
continue;
diff --git a/block/blkverify.c b/block/blkverify.c
index 3af37f8..c5f8e8d 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -312,8 +312,7 @@ static void blkverify_refresh_filename(BlockDriverState *bs)
BDRVBlkverifyState *s = bs->opaque;
/* bs->file->bs has already been refreshed */
- bdrv_filename(s->test_file->bs, s->test_file->bs->filename,
- sizeof(s->test_file->bs->filename));
+ bdrv_refresh_filename(s->test_file->bs);
if (bs->file->bs->full_open_options
&& s->test_file->bs->full_open_options)
diff --git a/block/commit.c b/block/commit.c
index fdebe87..3055ce1 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -228,7 +228,9 @@ void commit_start(BlockDriverState *bs, BlockDriverState
*base,
overlay_bs = bdrv_find_overlay(bs, top);
if (overlay_bs == NULL) {
- error_setg(errp, "Could not find overlay image for %s:",
top->filename);
+ char *top_filename = bdrv_filename(top, NULL, 0);
+ error_setg(errp, "Could not find overlay image for %s:", top_filename);
+ g_free(top_filename);
return;
}
diff --git a/block/mirror.c b/block/mirror.c
index b1252a1..4554d36 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -797,25 +797,33 @@ void commit_active_start(BlockDriverState *bs,
BlockDriverState *base,
length = bdrv_getlength(bs);
if (length < 0) {
- error_setg_errno(errp, -length,
- "Unable to determine length of %s", bs->filename);
+ char *filename = bdrv_filename(bs, NULL, 0);
+ error_setg_errno(errp, -length, "Unable to determine length of %s",
+ filename);
+ g_free(filename);
goto error_restore_flags;
}
base_length = bdrv_getlength(base);
if (base_length < 0) {
+ char *base_filename = bdrv_filename(base, NULL, 0);
error_setg_errno(errp, -base_length,
- "Unable to determine length of %s", base->filename);
+ "Unable to determine length of %s", base_filename);
+ g_free(base_filename);
goto error_restore_flags;
}
if (length > base_length) {
ret = bdrv_truncate(base, length);
if (ret < 0) {
+ char *filename = bdrv_filename(bs, NULL, 0);
+ char *base_filename = bdrv_filename(base, NULL, 0);
error_setg_errno(errp, -ret,
"Top image %s is larger than base image %s, and "
"resize of base image failed",
- bs->filename, base->filename);
+ filename, base_filename);
+ g_free(filename);
+ g_free(base_filename);
goto error_restore_flags;
}
}
diff --git a/block/qapi.c b/block/qapi.c
index ec0f513..f43436b 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -38,7 +38,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs,
Error **errp)
BlockDriverState *bs0;
BlockDeviceInfo *info = g_malloc0(sizeof(*info));
- info->file = g_strdup(bs->filename);
+ info->file = bdrv_filename(bs, NULL, 0);
info->ro = bs->read_only;
info->drv = g_strdup(bs->drv->format_name);
info->encrypted = bs->encrypted;
@@ -218,7 +218,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
}
info = g_new0(ImageInfo, 1);
- info->filename = g_strdup(bs->filename);
+ info->filename = bdrv_filename(bs, NULL, 0);
info->format = g_strdup(bdrv_get_format_name(bs));
info->virtual_size = size;
info->actual_size = bdrv_get_allocated_file_size(bs);
diff --git a/block/quorum.c b/block/quorum.c
index 16ec188..b9ba028 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1005,8 +1005,7 @@ static void quorum_refresh_filename(BlockDriverState *bs)
int i;
for (i = 0; i < s->num_children; i++) {
- bdrv_filename(s->children[i]->bs, s->children[i]->bs->filename,
- sizeof(s->children[i]->bs->filename));
+ bdrv_refresh_filename(s->children[i]->bs);
if (!s->children[i]->bs->full_open_options) {
return;
}
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 0aded31..df01006 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -205,6 +205,7 @@ static int raw_open(BlockDriverState *bs, QDict *options,
int flags,
bs->sg = bs->file->bs->sg;
if (bs->probed && !bdrv_is_read_only(bs)) {
+ char *filename = bdrv_filename(bs->file->bs, NULL, 0);
fprintf(stderr,
"WARNING: Image format was not specified for '%s' and probing "
"guessed raw.\n"
@@ -212,7 +213,8 @@ static int raw_open(BlockDriverState *bs, QDict *options,
int flags,
"raw images, write operations on block 0 will be restricted.\n"
" Specify the 'raw' format explicitly to remove the "
"restrictions.\n",
- bs->file->bs->filename);
+ filename);
+ g_free(filename);
}
return 0;
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index 47ae4b1..c98b4d8 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -783,13 +783,16 @@ int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState
*s, bool *flushed,
if (logs.valid) {
if (bs->read_only) {
+ char *filename = bdrv_filename(bs, NULL, 0);
+
ret = -EPERM;
error_setg_errno(errp, EPERM,
"VHDX image file '%s' opened read-only, but "
"contains a log that needs to be replayed. To "
"replay the log, execute:\n qemu-img check -r "
"all '%s'",
- bs->filename, bs->filename);
+ filename, filename);
+ g_free(filename);
goto exit;
}
/* now flush the log */
diff --git a/block/vmdk.c b/block/vmdk.c
index 0effb7d..3a6e660 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -445,9 +445,11 @@ static int vmdk_init_tables(BlockDriverState *bs,
VmdkExtent *extent,
extent->l1_table,
l1_size);
if (ret < 0) {
+ char *extent_filename = bdrv_filename(extent->file->bs, NULL, 0);
error_setg_errno(errp, -ret,
"Could not read l1 table from extent '%s'",
- extent->file->bs->filename);
+ extent_filename);
+ g_free(extent_filename);
goto fail_l1;
}
for (i = 0; i < extent->l1_size; i++) {
@@ -465,9 +467,11 @@ static int vmdk_init_tables(BlockDriverState *bs,
VmdkExtent *extent,
extent->l1_backup_table,
l1_size);
if (ret < 0) {
+ char *extent_filename = bdrv_filename(extent->file->bs, NULL, 0);
error_setg_errno(errp, -ret,
"Could not read l1 backup table from extent '%s'",
- extent->file->bs->filename);
+ extent_filename);
+ g_free(extent_filename);
goto fail_l1b;
}
for (i = 0; i < extent->l1_size; i++) {
@@ -496,9 +500,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
ret = bdrv_pread(file->bs, sizeof(magic), &header, sizeof(header));
if (ret < 0) {
+ char *filename = bdrv_filename(file->bs, NULL, 0);
error_setg_errno(errp, -ret,
"Could not read header from file '%s'",
- file->bs->filename);
+ filename);
+ g_free(filename);
return ret;
}
ret = vmdk_add_extent(bs, file, false,
@@ -573,9 +579,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
ret = bdrv_pread(file->bs, sizeof(magic), &header, sizeof(header));
if (ret < 0) {
+ char *filename = bdrv_filename(file->bs, NULL, 0);
error_setg_errno(errp, -ret,
"Could not read header from file '%s'",
- file->bs->filename);
+ filename);
+ g_free(filename);
return -EINVAL;
}
if (header.capacity == 0) {
@@ -819,8 +827,10 @@ static int vmdk_parse_extents(const char *desc,
BlockDriverState *bs,
if (!path_is_absolute(fname) && !path_has_protocol(fname) &&
!desc_file_path[0])
{
+ char *filename = bdrv_filename(bs->file->bs, NULL, 0);
error_setg(errp, "Cannot use relative extent paths with VMDK "
- "descriptor file '%s'", bs->file->bs->filename);
+ "descriptor file '%s'", filename);
+ g_free(filename);
return -EINVAL;
}
@@ -2092,7 +2102,7 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent *extent)
ImageInfo *info = g_new0(ImageInfo, 1);
*info = (ImageInfo){
- .filename = g_strdup(extent->file->bs->filename),
+ .filename = bdrv_filename(extent->file->bs, NULL, 0),
.format = g_strdup(extent->type),
.virtual_size = extent->sectors * BDRV_SECTOR_SIZE,
.compressed = extent->compressed,
diff --git a/block/vpc.c b/block/vpc.c
index 299d373..a3c4c17 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -204,9 +204,12 @@ static int vpc_open(BlockDriverState *bs, QDict *options,
int flags,
checksum = be32_to_cpu(footer->checksum);
footer->checksum = 0;
- if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum)
+ if (vpc_checksum(s->footer_buf, HEADER_SIZE) != checksum) {
+ char *filename = bdrv_filename(bs, NULL, 0);
fprintf(stderr, "block-vpc: The header checksum of '%s' is "
- "incorrect.\n", bs->filename);
+ "incorrect.\n", filename);
+ g_free(filename);
+ }
/* Write 'checksum' back to footer, or else will leave it with zero. */
footer->checksum = cpu_to_be32(checksum);
diff --git a/blockdev.c b/blockdev.c
index b79b0a6..5887e09 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -229,10 +229,13 @@ bool drive_check_orphaned(void)
/* Unless this is a default drive, this may be an oversight. */
if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
dinfo->type != IF_NONE) {
+ char *filename = blk_bs(blk) ? bdrv_filename(blk_bs(blk), NULL, 0)
+ : NULL;
fprintf(stderr, "Warning: Orphaned drive without device: "
"id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
- blk_name(blk), blk_bs(blk) ? blk_bs(blk)->filename : "",
- if_name[dinfo->type], dinfo->bus, dinfo->unit);
+ blk_name(blk), filename ?: "", if_name[dinfo->type],
+ dinfo->bus, dinfo->unit);
+ g_free(filename);
rs = true;
}
}
@@ -1596,10 +1599,12 @@ static void
external_snapshot_prepare(BlkTransactionState *common,
/* create new image w/backing file */
if (mode != NEW_IMAGE_MODE_EXISTING) {
+ char *old_filename = bdrv_filename(state->old_bs, NULL, 0);
bdrv_img_create(new_image_file, format,
- state->old_bs->filename,
+ old_filename,
state->old_bs->drv->format_name,
NULL, -1, flags, &local_err, false);
+ g_free(old_filename);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -2650,9 +2655,11 @@ void qmp_block_commit(const char *device,
top_bs = bs;
if (has_top && top) {
- if (strcmp(bs->filename, top) != 0) {
+ char *filename = bdrv_filename(bs, NULL, 0);
+ if (strcmp(filename, top) != 0) {
top_bs = bdrv_find_backing_image(bs, top);
}
+ g_free(filename);
}
if (top_bs == NULL) {
@@ -2791,9 +2798,11 @@ void qmp_drive_backup(const char *device, const char
*target,
if (mode != NEW_IMAGE_MODE_EXISTING) {
assert(format);
if (source) {
- bdrv_img_create(target, format, source->filename,
+ char *source_filename = bdrv_filename(source, NULL, 0);
+ bdrv_img_create(target, format, source_filename,
source->drv->format_name, NULL,
size, flags, &local_err, false);
+ g_free(source_filename);
} else {
bdrv_img_create(target, format, NULL, NULL, NULL,
size, flags, &local_err, false);
@@ -3043,15 +3052,19 @@ void qmp_drive_mirror(const char *device, const char
*target,
bdrv_img_create(target, format,
NULL, NULL, NULL, size, flags, &local_err, false);
} else {
+ char *source_filename;
+
switch (mode) {
case NEW_IMAGE_MODE_EXISTING:
break;
case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
/* create new image with backing file */
+ source_filename = bdrv_filename(source, NULL, 0);
bdrv_img_create(target, format,
- source->filename,
+ source_filename,
source->drv->format_name,
NULL, size, flags, &local_err, false);
+ g_free(source_filename);
break;
default:
abort();
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e472a03..65dd5bd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -371,7 +371,6 @@ struct BlockDriverState {
* regarding this BDS's context */
QLIST_HEAD(, BdrvAioNotifier) aio_notifiers;
- char filename[PATH_MAX];
char backing_file[PATH_MAX]; /* if non zero, the image is a diff of
this file image */
char backing_format[16]; /* if non-zero and backing_file exists */
--
2.6.1
- Re: [Qemu-block] [PATCH v5 1/6] block: Change bdrv_get_encrypted_filename(), (continued)