From f67503e5bd8997ea7ec3f4bfa0af0e06321771a6 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 18 Feb 2014 18:33:05 +0100 Subject: [PATCH] block: Change BDS parameter of bdrv_open() to ** Make bdrv_open() take a pointer to a BDS pointer, similarly to bdrv_file_open(). If a pointer to a NULL pointer is given, bdrv_open() will create a new BDS with an empty name; if the BDS pointer is not NULL, that existing BDS will be reused (in the same way as bdrv_open() already did). Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block.c | 64 ++++++++++++++++++++++++++++++++------------------- block/blkdebug.c | 1 + block/blkverify.c | 2 ++ block/qcow2.c | 14 +++++++---- block/vmdk.c | 5 ++-- block/vvfat.c | 6 ++--- blockdev.c | 20 ++++++++-------- hw/block/xen_disk.c | 2 +- include/block/block.h | 2 +- qemu-img.c | 10 ++++---- qemu-io.c | 2 +- qemu-nbd.c | 2 +- 12 files changed, 73 insertions(+), 57 deletions(-) diff --git a/block.c b/block.c index 034e1ab..c39bd3d 100644 --- a/block.c +++ b/block.c @@ -1046,7 +1046,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, } if (!drv->bdrv_file_open) { - ret = bdrv_open(bs, filename, options, flags, drv, &local_err); + ret = bdrv_open(&bs, filename, options, flags, drv, &local_err); options = NULL; } else { ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err); @@ -1115,8 +1115,6 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) sizeof(backing_filename)); } - bs->backing_hd = bdrv_new(""); - if (bs->backing_format[0] != '\0') { back_drv = bdrv_find_format(bs->backing_format); } @@ -1125,11 +1123,11 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) back_flags = bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_COPY_ON_READ); - ret = bdrv_open(bs->backing_hd, + assert(bs->backing_hd == NULL); + ret = bdrv_open(&bs->backing_hd, *backing_filename ? backing_filename : NULL, options, back_flags, back_drv, &local_err); if (ret < 0) { - bdrv_unref(bs->backing_hd); bs->backing_hd = NULL; bs->open_flags |= BDRV_O_NO_BACKING; error_setg(errp, "Could not open backing file: %s", @@ -1166,6 +1164,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp) * BlockdevRef. * * The BlockdevRef will be removed from the options QDict. + * + * To conform with the behavior of bdrv_open(), *pbs has to be NULL. */ int bdrv_open_image(BlockDriverState **pbs, const char *filename, QDict *options, const char *bdref_key, int flags, @@ -1176,6 +1176,9 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, char *bdref_key_dot; const char *reference; + assert(pbs); + assert(*pbs == NULL); + bdref_key_dot = g_strdup_printf("%s.", bdref_key); qdict_extract_subqdict(options, &image_options, bdref_key_dot); g_free(bdref_key_dot); @@ -1196,8 +1199,6 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, /* If a filename is given and the block driver should be detected automatically (instead of using none), use bdrv_open() in order to do that auto-detection. */ - BlockDriverState *bs; - if (reference) { error_setg(errp, "Cannot reference an existing block device while " "giving a filename"); @@ -1205,13 +1206,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, goto done; } - bs = bdrv_new(""); - ret = bdrv_open(bs, filename, image_options, flags, NULL, errp); - if (ret < 0) { - bdrv_unref(bs); - } else { - *pbs = bs; - } + ret = bdrv_open(pbs, filename, image_options, flags, NULL, errp); } else { ret = bdrv_file_open(pbs, filename, reference, image_options, flags, errp); @@ -1229,17 +1224,28 @@ done: * empty set of options. The reference to the QDict belongs to the block layer * after the call (even on failure), so if the caller intends to reuse the * dictionary, it needs to use QINCREF() before calling bdrv_open. + * + * If *pbs is NULL, a new BDS will be created with a pointer to it stored there. + * If it is not NULL, the referenced BDS will be reused. */ -int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, +int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options, int flags, BlockDriver *drv, Error **errp) { int ret; /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */ char tmp_filename[PATH_MAX + 1]; - BlockDriverState *file = NULL; + BlockDriverState *file = NULL, *bs; const char *drvname; Error *local_err = NULL; + assert(pbs); + + if (*pbs) { + bs = *pbs; + } else { + bs = bdrv_new(""); + } + /* NULL means an empty set of options */ if (options == NULL) { options = qdict_new(); @@ -1260,12 +1266,11 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, instead of opening 'filename' directly */ /* Get the required size from the image */ - bs1 = bdrv_new(""); QINCREF(options); - ret = bdrv_open(bs1, filename, options, BDRV_O_NO_BACKING, + bs1 = NULL; + ret = bdrv_open(&bs1, filename, options, BDRV_O_NO_BACKING, drv, &local_err); if (ret < 0) { - bdrv_unref(bs1); goto fail; } total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK; @@ -1322,6 +1327,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, flags |= BDRV_O_ALLOW_RDWR; } + assert(file == NULL); ret = bdrv_open_image(&file, filename, options, "file", bdrv_open_flags(bs, flags | BDRV_O_UNMAP), true, true, &local_err); @@ -1393,6 +1399,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, bdrv_dev_change_media_cb(bs, true); } + *pbs = bs; return 0; unlink_and_fail: @@ -1406,13 +1413,24 @@ fail: QDECREF(bs->options); QDECREF(options); bs->options = NULL; + if (!*pbs) { + /* If *pbs is NULL, a new BDS has been created in this function and + needs to be freed now. Otherwise, it does not need to be closed, + since it has not really been opened yet. */ + bdrv_unref(bs); + } if (local_err) { error_propagate(errp, local_err); } return ret; close_and_fail: - bdrv_close(bs); + /* See fail path, but now the BDS has to be always closed */ + if (*pbs) { + bdrv_close(bs); + } else { + bdrv_unref(bs); + } QDECREF(options); if (local_err) { error_propagate(errp, local_err); @@ -5290,9 +5308,8 @@ void bdrv_img_create(const char *filename, const char *fmt, back_flags = flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING); - bs = bdrv_new(""); - - ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags, + bs = NULL; + ret = bdrv_open(&bs, backing_file->value.s, NULL, back_flags, backing_drv, &local_err); if (ret < 0) { error_setg_errno(errp, -ret, "Could not open '%s': %s", @@ -5300,7 +5317,6 @@ void bdrv_img_create(const char *filename, const char *fmt, error_get_pretty(local_err)); error_free(local_err); local_err = NULL; - bdrv_unref(bs); goto out; } bdrv_get_geometry(bs, &size); diff --git a/block/blkdebug.c b/block/blkdebug.c index ee10013..46bd086 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -410,6 +410,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, s->state = 1; /* Open the backing file */ + assert(bs->file == NULL); ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options, "image", flags, true, false, &local_err); if (ret < 0) { diff --git a/block/blkverify.c b/block/blkverify.c index 1563c88..7d8a32e 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -135,6 +135,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, } /* Open the raw file */ + assert(bs->file == NULL); ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-raw"), options, "raw", flags, true, false, &local_err); if (ret < 0) { @@ -143,6 +144,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags, } /* Open the test file */ + assert(s->test_file == NULL); ret = bdrv_open_image(&s->test_file, qemu_opt_get(opts, "x-image"), options, "test", flags, false, false, &local_err); if (ret < 0) { diff --git a/block/qcow2.c b/block/qcow2.c index b1dbdb1..309ea12 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1543,7 +1543,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, goto out; } - bdrv_close(bs); + bdrv_unref(bs); + bs = NULL; /* * And now open the image and make it consistent first (i.e. increase the @@ -1552,7 +1553,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, */ BlockDriver* drv = bdrv_find_format("qcow2"); assert(drv != NULL); - ret = bdrv_open(bs, filename, NULL, + ret = bdrv_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err); if (ret < 0) { error_propagate(errp, local_err); @@ -1599,10 +1600,11 @@ static int qcow2_create2(const char *filename, int64_t total_size, } } - bdrv_close(bs); + bdrv_unref(bs); + bs = NULL; /* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */ - ret = bdrv_open(bs, filename, NULL, + ret = bdrv_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING, drv, &local_err); if (local_err) { @@ -1612,7 +1614,9 @@ static int qcow2_create2(const char *filename, int64_t total_size, ret = 0; out: - bdrv_unref(bs); + if (bs) { + bdrv_unref(bs); + } return ret; } diff --git a/block/vmdk.c b/block/vmdk.c index ff6f5ee..0622db5 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1755,10 +1755,9 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, goto exit; } if (backing_file) { - BlockDriverState *bs = bdrv_new(""); - ret = bdrv_open(bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp); + BlockDriverState *bs = NULL; + ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp); if (ret != 0) { - bdrv_unref(bs); goto exit; } if (strcmp(bs->drv->format_name, "vmdk")) { diff --git a/block/vvfat.c b/block/vvfat.c index a19e4ca..0a9f886 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2936,15 +2936,13 @@ static int enable_write_target(BDRVVVFATState *s) goto err; } - s->qcow = bdrv_new(""); - - ret = bdrv_open(s->qcow, s->qcow_filename, NULL, + s->qcow = NULL; + ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow, &local_err); if (ret < 0) { qerror_report_err(local_err); error_free(local_err); - bdrv_unref(s->qcow); goto err; } diff --git a/blockdev.c b/blockdev.c index 3cc8cda..056ec4c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -504,7 +504,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, bdrv_flags |= ro ? 0 : BDRV_O_RDWR; QINCREF(bs_opts); - ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error); + ret = bdrv_open(&dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error); if (ret < 0) { error_setg(errp, "could not open disk image %s: %s", @@ -1330,12 +1330,12 @@ static void external_snapshot_prepare(BlkTransactionState *common, qstring_from_str(snapshot_node_name)); } - /* We will manually add the backing_hd field to the bs later */ - state->new_bs = bdrv_new(""); /* TODO Inherit bs->options or only take explicit options with an * extended QMP command? */ - ret = bdrv_open(state->new_bs, new_image_file, options, + assert(state->new_bs == NULL); + ret = bdrv_open(&state->new_bs, new_image_file, options, flags | BDRV_O_NO_BACKING, drv, &local_err); + /* We will manually add the backing_hd field to the bs later */ if (ret != 0) { error_propagate(errp, local_err); } @@ -1582,7 +1582,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename, Error *local_err = NULL; int ret; - ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv, &local_err); + ret = bdrv_open(&bs, filename, NULL, bdrv_flags, drv, &local_err); if (ret < 0) { error_propagate(errp, local_err); return; @@ -2018,10 +2018,9 @@ void qmp_drive_backup(const char *device, const char *target, return; } - target_bs = bdrv_new(""); - ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err); + target_bs = NULL; + ret = bdrv_open(&target_bs, target, NULL, flags, drv, &local_err); if (ret < 0) { - bdrv_unref(target_bs); error_propagate(errp, local_err); return; } @@ -2162,11 +2161,10 @@ void qmp_drive_mirror(const char *device, const char *target, /* Mirroring takes care of copy-on-write using the source's backing * file. */ - target_bs = bdrv_new(""); - ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv, + target_bs = NULL; + ret = bdrv_open(&target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv, &local_err); if (ret < 0) { - bdrv_unref(target_bs); error_propagate(errp, local_err); return; } diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 098f6c6..14e6d0b 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -813,7 +813,7 @@ static int blk_connect(struct XenDevice *xendev) Error *local_err = NULL; BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto, readonly); - if (bdrv_open(blkdev->bs, + if (bdrv_open(&blkdev->bs, blkdev->filename, NULL, qflags, drv, &local_err) != 0) { xen_be_printf(&blkdev->xendev, 0, "error: %s\n", diff --git a/include/block/block.h b/include/block/block.h index 963a61f..980869d 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -190,7 +190,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, QDict *options, const char *bdref_key, int flags, bool force_raw, bool allow_none, Error **errp); int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp); -int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, +int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options, int flags, BlockDriver *drv, Error **errp); BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, BlockDriverState *bs, int flags); diff --git a/qemu-img.c b/qemu-img.c index 0927b09..59b1013 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -289,7 +289,7 @@ static BlockDriverState *bdrv_new_open(const char *filename, drv = NULL; } - ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err); + ret = bdrv_open(&bs, filename, NULL, flags, drv, &local_err); if (ret < 0) { error_report("Could not open '%s': %s", filename, error_get_pretty(local_err)); @@ -310,9 +310,7 @@ static BlockDriverState *bdrv_new_open(const char *filename, } return bs; fail: - if (bs) { - bdrv_unref(bs); - } + bdrv_unref(bs); return NULL; } @@ -2314,7 +2312,7 @@ static int img_rebase(int argc, char **argv) bs_old_backing = bdrv_new("old_backing"); bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name)); - ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS, + ret = bdrv_open(&bs_old_backing, backing_name, NULL, BDRV_O_FLAGS, old_backing_drv, &local_err); if (ret) { error_report("Could not open old backing file '%s': %s", @@ -2324,7 +2322,7 @@ static int img_rebase(int argc, char **argv) } if (out_baseimg[0]) { bs_new_backing = bdrv_new("new_backing"); - ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS, + ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS, new_backing_drv, &local_err); if (ret) { error_report("Could not open new backing file '%s': %s", diff --git a/qemu-io.c b/qemu-io.c index 7f459d8..8da8f6e 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -68,7 +68,7 @@ static int openfile(char *name, int flags, int growable, QDict *opts) } else { qemuio_bs = bdrv_new("hda"); - if (bdrv_open(qemuio_bs, name, opts, flags, NULL, &local_err) < 0) { + if (bdrv_open(&qemuio_bs, name, opts, flags, NULL, &local_err) < 0) { fprintf(stderr, "%s: can't open device %s: %s\n", progname, name, error_get_pretty(local_err)); error_free(local_err); diff --git a/qemu-nbd.c b/qemu-nbd.c index 136e8c9..0cf123c 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -597,7 +597,7 @@ int main(int argc, char **argv) bs = bdrv_new("hda"); srcpath = argv[optind]; - ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err); + ret = bdrv_open(&bs, srcpath, NULL, flags, drv, &local_err); if (ret < 0) { errno = -ret; err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind], -- 2.7.4