Le Saturday 08 Feb 2014 à 18:39:12 (+0100), Max Reitz a écrit :
> 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 <mre...@redhat.com>
> ---
> 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, 72 insertions(+), 58 deletions(-)
>
> diff --git a/block.c b/block.c
> index 636aa11..40a585a 100644
> --- a/block.c
> +++ b/block.c
> @@ -1040,7 +1040,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);
> @@ -1109,8 +1109,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);
> }
> @@ -1119,11 +1117,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",
> @@ -1160,6 +1158,10 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
> *options, Error **errp)
> * BlockdevRef.
> *
> * The BlockdevRef will be removed from the options QDict.
> + *
> + * As with 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_image(BlockDriverState **pbs, const char *filename,
> QDict *options, const char *bdref_key, int flags,
> @@ -1190,8 +1192,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");
> @@ -1199,13 +1199,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);
> @@ -1223,22 +1217,32 @@ 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();
> }
> -
nitpicking: spurious line removal.
> bs->options = options;
> options = qdict_clone_shallow(options);
>
> @@ -1254,12 +1258,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;
> @@ -1316,6 +1319,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);
is_temporary can already be set to one the goto after the bdrv_open_image should
be unlink_and_fail yet I don't know if the fix belong to this patch.
> @@ -1387,6 +1391,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:
> @@ -1400,13 +1405,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 (error_is_set(&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 (error_is_set(&local_err)) {
> error_propagate(errp, local_err);
> @@ -5288,9 +5304,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",
> @@ -5298,7 +5313,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 8eb0db0..053fa4c 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 cfcbcf4..86585e7 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 0b4335c..c8e8ba7 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 (error_is_set(&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 e809e2e..52cd12a 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 664941c..ae7bc6f 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 36ceece..6dd351c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -510,7 +510,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",
> @@ -1301,12 +1301,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);
> }
> @@ -1555,7 +1555,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;
> @@ -1991,10 +1991,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;
> }
> @@ -2135,11 +2134,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 c989850..897aa56 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],
> --
> 1.8.5.4
>
>
