From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37844) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XTUxm-0007VV-Bf for qemu-devel@nongnu.org; Mon, 15 Sep 2014 08:01:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XTUxh-0006Mb-8s for qemu-devel@nongnu.org; Mon, 15 Sep 2014 08:00:58 -0400 Received: from dew.nodalink.com ([95.130.14.197]:55196) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XTUxg-0006M1-U1 for qemu-devel@nongnu.org; Mon, 15 Sep 2014 08:00:53 -0400 Date: Mon, 15 Sep 2014 12:00:51 +0000 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140915120051.GC22086@nodalink.com> References: <1410620427-20089-1-git-send-email-armbru@redhat.com> <1410620427-20089-2-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1410620427-20089-2-git-send-email-armbru@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 01/23] block: Split bdrv_new_root() off bdrv_new() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, benoit.canet@nodalink.com On Sat, Sep 13, 2014 at 05:00:05PM +0200, Markus Armbruster wrote: > Creating an anonymous BDS can't fail. Make that obvious. >=20 > Signed-off-by: Markus Armbruster > --- > block.c | 28 +++++++++++++++++++--------- > block/iscsi.c | 2 +- > block/vvfat.c | 2 +- > blockdev.c | 2 +- > hw/block/xen_disk.c | 2 +- > include/block/block.h | 3 ++- > qemu-img.c | 6 +++--- > qemu-io.c | 2 +- > qemu-nbd.c | 2 +- > 9 files changed, 30 insertions(+), 19 deletions(-) >=20 > diff --git a/block.c b/block.c > index 02ea90f..4fe3b62 100644 > --- a/block.c > +++ b/block.c > @@ -336,10 +336,11 @@ void bdrv_register(BlockDriver *bdrv) > } > =20 > /* create a new block device (by default it is empty) */ I wonder if this comment is stale. What is the emptiness of a block device anyway ? > -BlockDriverState *bdrv_new(const char *device_name, Error **errp) > +BlockDriverState *bdrv_new_root(const char *device_name, Error **errp) > { > BlockDriverState *bs; > - int i; > + > + assert(*device_name); > =20 > if (bdrv_find(device_name)) { > error_setg(errp, "Device with id '%s' already exists", > @@ -353,12 +354,21 @@ BlockDriverState *bdrv_new(const char *device_nam= e, Error **errp) > return NULL; > } > =20 > + bs =3D bdrv_new(); > + > + pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); > + QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); > + > + return bs; > +} > + > +BlockDriverState *bdrv_new(void) > +{ > + BlockDriverState *bs; > + int i; > + > bs =3D g_new0(BlockDriverState, 1); > QLIST_INIT(&bs->dirty_bitmaps); > - pstrcpy(bs->device_name, sizeof(bs->device_name), device_name); > - if (device_name[0] !=3D '\0') { > - QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); > - } > for (i =3D 0; i < BLOCK_OP_TYPE_MAX; i++) { > QLIST_INIT(&bs->op_blockers[i]); > } > @@ -1219,7 +1229,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, = QDict *options, Error **errp) > goto free_exit; > } > =20 > - backing_hd =3D bdrv_new("", errp); > + backing_hd =3D bdrv_new(); > =20 > if (bs->backing_format[0] !=3D '\0') { > back_drv =3D bdrv_find_format(bs->backing_format); > @@ -1348,7 +1358,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *b= s, int flags, Error **errp) > qdict_put(snapshot_options, "file.filename", > qstring_from_str(tmp_filename)); > =20 > - bs_snapshot =3D bdrv_new("", &error_abort); > + bs_snapshot =3D bdrv_new(); > =20 > ret =3D bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options, > flags, bdrv_qcow2, &local_err); > @@ -1419,7 +1429,7 @@ int bdrv_open(BlockDriverState **pbs, const char = *filename, > if (*pbs) { > bs =3D *pbs; > } else { > - bs =3D bdrv_new("", &error_abort); > + bs =3D bdrv_new(); > } > =20 > /* NULL means an empty set of options */ > diff --git a/block/iscsi.c b/block/iscsi.c > index 3e19202..af3d0f6 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1528,7 +1528,7 @@ static int iscsi_create(const char *filename, Qem= uOpts *opts, Error **errp) > IscsiLun *iscsilun =3D NULL; > QDict *bs_options; > =20 > - bs =3D bdrv_new("", &error_abort); > + bs =3D bdrv_new(); > =20 > /* Read out options */ > total_size =3D > diff --git a/block/vvfat.c b/block/vvfat.c > index 731e591..6c9fde0 100644 > --- a/block/vvfat.c > +++ b/block/vvfat.c > @@ -2939,7 +2939,7 @@ static int enable_write_target(BDRVVVFATState *s,= Error **errp) > unlink(s->qcow_filename); > #endif > =20 > - bdrv_set_backing_hd(s->bs, bdrv_new("", &error_abort)); > + bdrv_set_backing_hd(s->bs, bdrv_new()); > s->bs->backing_hd->drv =3D &vvfat_write_target; > s->bs->backing_hd->opaque =3D g_new(void *, 1); > *(void**)s->bs->backing_hd->opaque =3D s; > diff --git a/blockdev.c b/blockdev.c > index 450f95c..c9463e3 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -463,7 +463,7 @@ static DriveInfo *blockdev_init(const char *file, Q= Dict *bs_opts, > } > =20 > /* init */ > - bs =3D bdrv_new(qemu_opts_id(opts), errp); > + bs =3D bdrv_new_root(qemu_opts_id(opts), errp); > if (!bs) { > goto early_err; > } > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index 2dcef07..71f0953 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -856,7 +856,7 @@ static int blk_connect(struct XenDevice *xendev) > =20 > /* setup via xenbus -> create new block driver instance */ > xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus set= up)\n"); > - blkdev->bs =3D bdrv_new(blkdev->dev, NULL); > + blkdev->bs =3D bdrv_new_root(blkdev->dev, NULL); > if (!blkdev->bs) { > return -1; > } > diff --git a/include/block/block.h b/include/block/block.h > index 8f4ad16..6cabc98 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -203,7 +203,8 @@ BlockDriver *bdrv_find_whitelisted_format(const cha= r *format_name, > int bdrv_create(BlockDriver *drv, const char* filename, > QemuOpts *opts, Error **errp); > int bdrv_create_file(const char *filename, QemuOpts *opts, Error **err= p); > -BlockDriverState *bdrv_new(const char *device_name, Error **errp); > +BlockDriverState *bdrv_new_root(const char *device_name, Error **errp)= ; > +BlockDriverState *bdrv_new(void); > void bdrv_make_anon(BlockDriverState *bs); > void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old); > void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); > diff --git a/qemu-img.c b/qemu-img.c > index 91d1ac3..58d59d0 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -296,7 +296,7 @@ static BlockDriverState *bdrv_new_open(const char *= id, > Error *local_err =3D NULL; > int ret; > =20 > - bs =3D bdrv_new(id, &error_abort); > + bs =3D bdrv_new_root(id, &error_abort); > =20 > if (fmt) { > drv =3D bdrv_find_format(fmt); > @@ -2425,7 +2425,7 @@ static int img_rebase(int argc, char **argv) > if (!unsafe) { > char backing_name[1024]; > =20 > - bs_old_backing =3D bdrv_new("old_backing", &error_abort); > + bs_old_backing =3D bdrv_new_root("old_backing", &error_abort); > bdrv_get_backing_filename(bs, backing_name, sizeof(backing_nam= e)); > ret =3D bdrv_open(&bs_old_backing, backing_name, NULL, NULL, s= rc_flags, > old_backing_drv, &local_err); > @@ -2436,7 +2436,7 @@ static int img_rebase(int argc, char **argv) > goto out; > } > if (out_baseimg[0]) { > - bs_new_backing =3D bdrv_new("new_backing", &error_abort); > + bs_new_backing =3D bdrv_new_root("new_backing", &error_abo= rt); > ret =3D bdrv_open(&bs_new_backing, out_baseimg, NULL, NULL= , src_flags, > new_backing_drv, &local_err); > if (ret) { > diff --git a/qemu-io.c b/qemu-io.c > index d2ab694..24ca59c 100644 > --- a/qemu-io.c > +++ b/qemu-io.c > @@ -58,7 +58,7 @@ static int openfile(char *name, int flags, int growab= le, QDict *opts) > return 1; > } > =20 > - qemuio_bs =3D bdrv_new("hda", &error_abort); > + qemuio_bs =3D bdrv_new_root("hda", &error_abort); > =20 > if (growable) { > flags |=3D BDRV_O_PROTOCOL; > diff --git a/qemu-nbd.c b/qemu-nbd.c > index f3cf8a2..24b8454 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -687,7 +687,7 @@ int main(int argc, char **argv) > drv =3D NULL; > } > =20 > - bs =3D bdrv_new("hda", &error_abort); > + bs =3D bdrv_new_root("hda", &error_abort); > =20 > srcpath =3D argv[optind]; > ret =3D bdrv_open(&bs, srcpath, NULL, NULL, flags, drv, &local_err= ); > --=20 > 1.9.3 >=20 Reviewed-by: Beno=EEt Canet