From: "Benoît Canet" <benoit.canet@irqsave.net>
To: Max Reitz <mreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Jeff Cody <jcody@redhat.com>,
qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/8] block: Add reference parameter to bdrv_open()
Date: Mon, 10 Feb 2014 14:30:17 +0100 [thread overview]
Message-ID: <20140210133016.GG3088@irqsave.net> (raw)
In-Reply-To: <1391881159-13585-3-git-send-email-mreitz@redhat.com>
Le Saturday 08 Feb 2014 à 18:39:13 (+0100), Max Reitz a écrit :
> Allow bdrv_open() to handle references to existing block devices just as
> bdrv_file_open() is already capable of.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 41 ++++++++++++++++++++++++++++++++++-------
> block/qcow2.c | 4 ++--
> block/vmdk.c | 3 ++-
> block/vvfat.c | 2 +-
> blockdev.c | 12 ++++++------
> hw/block/xen_disk.c | 4 ++--
> include/block/block.h | 5 +++--
> qemu-img.c | 8 ++++----
> qemu-io.c | 4 +++-
> qemu-nbd.c | 2 +-
> 10 files changed, 58 insertions(+), 27 deletions(-)
>
> diff --git a/block.c b/block.c
> index 40a585a..3a32c37 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, NULL, options, flags, drv, &local_err);
> options = NULL;
> } else {
> ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
> @@ -1119,7 +1119,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
>
> assert(bs->backing_hd == NULL);
> ret = bdrv_open(&bs->backing_hd,
> - *backing_filename ? backing_filename : NULL, options,
> + *backing_filename ? backing_filename : NULL, NULL, options,
> back_flags, back_drv, &local_err);
> if (ret < 0) {
> bs->backing_hd = NULL;
> @@ -1199,7 +1199,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
> goto done;
> }
>
> - ret = bdrv_open(pbs, filename, image_options, flags, NULL, errp);
> + ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
> } else {
> ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
> errp);
> @@ -1221,8 +1221,9 @@ done:
> * 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 **pbs, const char *filename, QDict *options,
> - int flags, BlockDriver *drv, Error **errp)
> +int bdrv_open(BlockDriverState **pbs, const char *filename,
> + const char *reference, QDict *options, int flags,
> + BlockDriver *drv, Error **errp)
Maybe a little reference to the reference parameters in the comments would help
?
Aside from that:
Reviewed-by: Benoit Canet <benoit@irqsave.net>
> {
> int ret;
> /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> @@ -1233,6 +1234,32 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
>
> assert(pbs);
>
> + if (reference) {
> + bool options_non_empty = options ? qdict_size(options) : false;
> + QDECREF(options);
> +
> + if (*pbs) {
> + error_setg(errp, "Cannot reuse an existing BDS when referencing "
> + "another block device");
> + return -EINVAL;
> + }
> +
> + if (filename || options_non_empty) {
> + error_setg(errp, "Cannot reference an existing block device with "
> + "additional options or a new filename");
> + return -EINVAL;
> + }
> +
> + bs = bdrv_find(reference);
> + if (!bs) {
> + error_setg(errp, "Cannot find block device '%s'", reference);
> + return -ENODEV;
> + }
> + bdrv_ref(bs);
> + *pbs = bs;
> + return 0;
> + }
> +
> if (*pbs) {
> bs = *pbs;
> } else {
> @@ -1260,7 +1287,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *options,
> /* Get the required size from the image */
> QINCREF(options);
> bs1 = NULL;
> - ret = bdrv_open(&bs1, filename, options, BDRV_O_NO_BACKING,
> + ret = bdrv_open(&bs1, filename, NULL, options, BDRV_O_NO_BACKING,
> drv, &local_err);
> if (ret < 0) {
> goto fail;
> @@ -5305,7 +5332,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
> flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
>
> bs = NULL;
> - ret = bdrv_open(&bs, backing_file->value.s, NULL, back_flags,
> + ret = bdrv_open(&bs, backing_file->value.s, NULL, NULL, back_flags,
> backing_drv, &local_err);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Could not open '%s': %s",
> diff --git a/block/qcow2.c b/block/qcow2.c
> index c8e8ba7..6996276 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1553,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, NULL,
> BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
> if (ret < 0) {
> error_propagate(errp, local_err);
> @@ -1604,7 +1604,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
> 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, NULL,
> BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
> drv, &local_err);
> if (error_is_set(&local_err)) {
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 52cd12a..e007793 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1756,7 +1756,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
> }
> if (backing_file) {
> BlockDriverState *bs = NULL;
> - ret = bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, NULL, errp);
> + ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_NO_BACKING, NULL,
> + errp);
> if (ret != 0) {
> goto exit;
> }
> diff --git a/block/vvfat.c b/block/vvfat.c
> index ae7bc6f..d7a830e 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -2937,7 +2937,7 @@ static int enable_write_target(BDRVVVFATState *s)
> }
>
> s->qcow = NULL;
> - ret = bdrv_open(&s->qcow, s->qcow_filename, NULL,
> + ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, NULL,
> BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
> &local_err);
> if (ret < 0) {
> diff --git a/blockdev.c b/blockdev.c
> index 6dd351c..7b7e349 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, NULL, bs_opts, bdrv_flags, drv, &error);
>
> if (ret < 0) {
> error_setg(errp, "could not open disk image %s: %s",
> @@ -1304,7 +1304,7 @@ static void external_snapshot_prepare(BlkTransactionState *common,
> /* TODO Inherit bs->options or only take explicit options with an
> * extended QMP command? */
> assert(state->new_bs == NULL);
> - ret = bdrv_open(&state->new_bs, new_image_file, options,
> + ret = bdrv_open(&state->new_bs, new_image_file, NULL, options,
> flags | BDRV_O_NO_BACKING, drv, &local_err);
> /* We will manually add the backing_hd field to the bs later */
> if (ret != 0) {
> @@ -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, NULL, bdrv_flags, drv, &local_err);
> if (ret < 0) {
> error_propagate(errp, local_err);
> return;
> @@ -1992,7 +1992,7 @@ void qmp_drive_backup(const char *device, const char *target,
> }
>
> target_bs = NULL;
> - ret = bdrv_open(&target_bs, target, NULL, flags, drv, &local_err);
> + ret = bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &local_err);
> if (ret < 0) {
> error_propagate(errp, local_err);
> return;
> @@ -2135,8 +2135,8 @@ void qmp_drive_mirror(const char *device, const char *target,
> * file.
> */
> target_bs = NULL;
> - ret = bdrv_open(&target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
> - &local_err);
> + ret = bdrv_open(&target_bs, target, NULL, NULL, flags | BDRV_O_NO_BACKING,
> + drv, &local_err);
> if (ret < 0) {
> error_propagate(errp, local_err);
> return;
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 14e6d0b..61e6ff3 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -813,8 +813,8 @@ 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,
> - blkdev->filename, NULL, qflags, drv, &local_err) != 0)
> + if (bdrv_open(&blkdev->bs, blkdev->filename, NULL, NULL, qflags,
> + drv, &local_err) != 0)
> {
> xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
> error_get_pretty(local_err));
> diff --git a/include/block/block.h b/include/block/block.h
> index 980869d..a421041 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -190,8 +190,9 @@ 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 **pbs, const char *filename, QDict *options,
> - int flags, BlockDriver *drv, Error **errp);
> +int bdrv_open(BlockDriverState **pbs, const char *filename,
> + const char *reference, QDict *options, int flags,
> + BlockDriver *drv, Error **errp);
> BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
> BlockDriverState *bs, int flags);
> int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
> diff --git a/qemu-img.c b/qemu-img.c
> index 897aa56..b834d52 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, NULL, flags, drv, &local_err);
> if (ret < 0) {
> error_report("Could not open '%s': %s", filename,
> error_get_pretty(local_err));
> @@ -2312,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, NULL, BDRV_O_FLAGS,
> old_backing_drv, &local_err);
> if (ret) {
> error_report("Could not open old backing file '%s': %s",
> @@ -2322,8 +2322,8 @@ 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,
> - new_backing_drv, &local_err);
> + ret = bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL,
> + BDRV_O_FLAGS, new_backing_drv, &local_err);
> if (ret) {
> error_report("Could not open new backing file '%s': %s",
> out_baseimg, error_get_pretty(local_err));
> diff --git a/qemu-io.c b/qemu-io.c
> index 8da8f6e..61d54c0 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -68,7 +68,9 @@ 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, NULL, 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 0cf123c..711162c 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, 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
>
>
next prev parent reply other threads:[~2014-02-10 13:30 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-08 17:39 [Qemu-devel] [PATCH v2 0/8] block: Integrate bdrv_file_open() into bdrv_open() Max Reitz
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 1/8] block: Change BDS parameter of bdrv_open() to ** Max Reitz
2014-02-10 12:42 ` Kevin Wolf
2014-02-12 0:10 ` Max Reitz
2014-02-10 13:17 ` Benoît Canet
2014-02-12 0:15 ` Max Reitz
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 2/8] block: Add reference parameter to bdrv_open() Max Reitz
2014-02-10 13:30 ` Benoît Canet [this message]
2014-02-12 0:17 ` Max Reitz
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 3/8] block: Make bdrv_file_open() static Max Reitz
2014-02-10 13:40 ` Benoît Canet
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 4/8] block: Reuse reference handling from bdrv_open() Max Reitz
2014-02-10 13:42 ` Benoît Canet
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 5/8] block: Remove bdrv_new() from bdrv_file_open() Max Reitz
2014-02-10 13:49 ` Benoît Canet
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 6/8] block: Handle bs->options in bdrv_open() only Max Reitz
2014-02-10 16:23 ` Benoît Canet
2014-02-10 16:23 ` Benoît Canet
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 7/8] block: Reuse success path from bdrv_open() Max Reitz
2014-02-10 14:56 ` Kevin Wolf
2014-02-12 0:26 ` Max Reitz
2014-02-10 16:28 ` Benoît Canet
2014-02-08 17:39 ` [Qemu-devel] [PATCH v2 8/8] block: Remove bdrv_open_image()'s force_raw option Max Reitz
2014-02-10 16:31 ` Benoît Canet
2014-02-10 15:01 ` [Qemu-devel] [PATCH v2 0/8] block: Integrate bdrv_file_open() into bdrv_open() Kevin Wolf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140210133016.GG3088@irqsave.net \
--to=benoit.canet@irqsave.net \
--cc=jcody@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).