From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56721) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCqwK-0001rW-5L for qemu-devel@nongnu.org; Mon, 10 Feb 2014 08:30:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCqwE-0008AZ-Vx for qemu-devel@nongnu.org; Mon, 10 Feb 2014 08:30:24 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:49561) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCqwE-0008AQ-9n for qemu-devel@nongnu.org; Mon, 10 Feb 2014 08:30:18 -0500 Date: Mon, 10 Feb 2014 14:30:17 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140210133016.GG3088@irqsave.net> References: <1391881159-13585-1-git-send-email-mreitz@redhat.com> <1391881159-13585-3-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1391881159-13585-3-git-send-email-mreitz@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 2/8] block: Add reference parameter to bdrv_open() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , Jeff Cody , qemu-devel@nongnu.org, Stefan Hajnoczi Le Saturday 08 Feb 2014 =E0 18:39:13 (+0100), Max Reitz a =E9crit : > Allow bdrv_open() to handle references to existing block devices just a= s > bdrv_file_open() is already capable of. >=20 > Signed-off-by: Max Reitz > --- > 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(-) >=20 > 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, > } > =20 > if (!drv->bdrv_file_open) { > - ret =3D bdrv_open(&bs, filename, options, flags, drv, &local_e= rr); > + ret =3D bdrv_open(&bs, filename, NULL, options, flags, drv, &l= ocal_err); > options =3D NULL; > } else { > ret =3D 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) > =20 > assert(bs->backing_hd =3D=3D NULL); > ret =3D bdrv_open(&bs->backing_hd, > - *backing_filename ? backing_filename : NULL, optio= ns, > + *backing_filename ? backing_filename : NULL, NULL,= options, > back_flags, back_drv, &local_err); > if (ret < 0) { > bs->backing_hd =3D NULL; > @@ -1199,7 +1199,7 @@ int bdrv_open_image(BlockDriverState **pbs, const= char *filename, > goto done; > } > =20 > - ret =3D bdrv_open(pbs, filename, image_options, flags, NULL, e= rrp); > + ret =3D bdrv_open(pbs, filename, NULL, image_options, flags, N= ULL, errp); > } else { > ret =3D 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 sto= red there. > * If it is not NULL, the referenced BDS will be reused. > */ > -int bdrv_open(BlockDriverState **pbs, const char *filename, QDict *opt= ions, > - 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 woul= d help ? Aside from that: Reviewed-by: Benoit Canet > { > 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, > =20 > assert(pbs); > =20 > + if (reference) { > + bool options_non_empty =3D options ? qdict_size(options) : fal= se; > + QDECREF(options); > + > + if (*pbs) { > + error_setg(errp, "Cannot reuse an existing BDS when refere= ncing " > + "another block device"); > + return -EINVAL; > + } > + > + if (filename || options_non_empty) { > + error_setg(errp, "Cannot reference an existing block devic= e with " > + "additional options or a new filename"); > + return -EINVAL; > + } > + > + bs =3D bdrv_find(reference); > + if (!bs) { > + error_setg(errp, "Cannot find block device '%s'", referenc= e); > + return -ENODEV; > + } > + bdrv_ref(bs); > + *pbs =3D bs; > + return 0; > + } > + > if (*pbs) { > bs =3D *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 =3D NULL; > - ret =3D bdrv_open(&bs1, filename, options, BDRV_O_NO_BACKING, > + ret =3D bdrv_open(&bs1, filename, NULL, options, BDRV_O_NO_BAC= KING, > 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_BA= CKING); > =20 > bs =3D NULL; > - ret =3D bdrv_open(&bs, backing_file->value.s, NULL, back_f= lags, > + ret =3D 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, in= t64_t total_size, > */ > BlockDriver* drv =3D bdrv_find_format("qcow2"); > assert(drv !=3D NULL); > - ret =3D bdrv_open(&bs, filename, NULL, > + ret =3D bdrv_open(&bs, filename, NULL, NULL, > BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_e= rr); > if (ret < 0) { > error_propagate(errp, local_err); > @@ -1604,7 +1604,7 @@ static int qcow2_create2(const char *filename, in= t64_t total_size, > bs =3D NULL; > =20 > /* Reopen the image without BDRV_O_NO_FLUSH to flush it before ret= urning */ > - ret =3D bdrv_open(&bs, filename, NULL, > + ret =3D 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, QEMU= OptionParameter *options, > } > if (backing_file) { > BlockDriverState *bs =3D NULL; > - ret =3D bdrv_open(&bs, backing_file, NULL, BDRV_O_NO_BACKING, = NULL, errp); > + ret =3D bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_NO_BAC= KING, NULL, > + errp); > if (ret !=3D 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) > } > =20 > s->qcow =3D NULL; > - ret =3D bdrv_open(&s->qcow, s->qcow_filename, NULL, > + ret =3D 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, Q= Dict *bs_opts, > bdrv_flags |=3D ro ? 0 : BDRV_O_RDWR; > =20 > QINCREF(bs_opts); > - ret =3D bdrv_open(&dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &e= rror); > + ret =3D bdrv_open(&dinfo->bdrv, file, NULL, bs_opts, bdrv_flags, d= rv, &error); > =20 > if (ret < 0) { > error_setg(errp, "could not open disk image %s: %s", > @@ -1304,7 +1304,7 @@ static void external_snapshot_prepare(BlkTransact= ionState *common, > /* TODO Inherit bs->options or only take explicit options with an > * extended QMP command? */ > assert(state->new_bs =3D=3D NULL); > - ret =3D bdrv_open(&state->new_bs, new_image_file, options, > + ret =3D 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 !=3D 0) { > @@ -1555,7 +1555,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverSt= ate *bs, const char *filename, > Error *local_err =3D NULL; > int ret; > =20 > - ret =3D bdrv_open(&bs, filename, NULL, bdrv_flags, drv, &local_err= ); > + ret =3D bdrv_open(&bs, filename, NULL, NULL, bdrv_flags, drv, &loc= al_err); > if (ret < 0) { > error_propagate(errp, local_err); > return; > @@ -1992,7 +1992,7 @@ void qmp_drive_backup(const char *device, const c= har *target, > } > =20 > target_bs =3D NULL; > - ret =3D bdrv_open(&target_bs, target, NULL, flags, drv, &local_err= ); > + ret =3D bdrv_open(&target_bs, target, NULL, NULL, flags, drv, &loc= al_err); > if (ret < 0) { > error_propagate(errp, local_err); > return; > @@ -2135,8 +2135,8 @@ void qmp_drive_mirror(const char *device, const c= har *target, > * file. > */ > target_bs =3D NULL; > - ret =3D bdrv_open(&target_bs, target, NULL, flags | BDRV_O_NO_BACK= ING, drv, > - &local_err); > + ret =3D bdrv_open(&target_bs, target, NULL, NULL, flags | BDRV_O_N= O_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 =3D NULL; > BlockDriver *drv =3D bdrv_find_whitelisted_format(blkdev->= fileproto, > readonly); > - if (bdrv_open(&blkdev->bs, > - blkdev->filename, NULL, qflags, drv, &local_= err) !=3D 0) > + if (bdrv_open(&blkdev->bs, blkdev->filename, NULL, NULL, q= flags, > + drv, &local_err) !=3D 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 c= har *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 *opt= ions, > - 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 =3D NULL; > } > =20 > - ret =3D bdrv_open(&bs, filename, NULL, flags, drv, &local_err); > + ret =3D bdrv_open(&bs, filename, NULL, NULL, flags, drv, &local_er= r); > 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) > =20 > bs_old_backing =3D bdrv_new("old_backing"); > bdrv_get_backing_filename(bs, backing_name, sizeof(backing_nam= e)); > - ret =3D bdrv_open(&bs_old_backing, backing_name, NULL, BDRV_O_= FLAGS, > + ret =3D bdrv_open(&bs_old_backing, backing_name, NULL, NULL, B= DRV_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 =3D bdrv_new("new_backing"); > - ret =3D bdrv_open(&bs_new_backing, out_baseimg, NULL, BDRV= _O_FLAGS, > - new_backing_drv, &local_err); > + ret =3D 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 growab= le, QDict *opts) > } else { > qemuio_bs =3D bdrv_new("hda"); > =20 > - if (bdrv_open(&qemuio_bs, name, opts, flags, NULL, &local_err)= < 0) { > + if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, NULL, &loca= l_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) > =20 > bs =3D bdrv_new("hda"); > srcpath =3D argv[optind]; > - ret =3D bdrv_open(&bs, srcpath, NULL, flags, drv, &local_err); > + ret =3D bdrv_open(&bs, srcpath, NULL, NULL, flags, drv, &local_err= ); > if (ret < 0) { > errno =3D -ret; > err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind]= , > --=20 > 1.8.5.4 >=20 >=20