From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38430) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN1Qg-0007vD-T0 for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:11:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VN1Qf-0006GN-6s for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:11:30 -0400 Received: from nodalink.pck.nerim.net ([62.212.105.220]:37411 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN1Qe-0006Fq-EN for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:11:29 -0400 Date: Fri, 20 Sep 2013 16:11:22 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20130920141121.GC6794@irqsave.net> References: <1379678070-14346-1-git-send-email-kwolf@redhat.com> <1379678070-14346-7-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1379678070-14346-7-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 06/17] blockdev: Pass QDict to blockdev_init() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: mreitz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com Le Friday 20 Sep 2013 =E0 13:54:19 (+0200), Kevin Wolf a =E9crit : > Working on a QDict instead of a QemuOpts that accepts anything is more > in line with bdrv_open(). A QDict is what qmp_blockdev_add() already ha= s > anyway, so this saves additional conversions. And last, but not least, > it allows later patches to easily extract legacy options into a > separate, typed QemuOpts for drive_init() (the untyped QemuOpts that > drive_init already has doesn't allow access to numbers, only strings, > and is therefore useless without conversion). >=20 > Signed-off-by: Kevin Wolf > --- > blockdev.c | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) >=20 > diff --git a/blockdev.c b/blockdev.c > index e3cff31..3a1444c 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -302,7 +302,8 @@ static bool check_throttle_config(ThrottleConfig *c= fg, Error **errp) > return true; > } > =20 > -static DriveInfo *blockdev_init(QemuOpts *all_opts, > +/* Takes the ownership of bs_opts */ > +static DriveInfo *blockdev_init(QDict *bs_opts, > BlockInterfaceType block_default_type) > { > const char *buf; > @@ -326,7 +327,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, > int ret; > Error *error =3D NULL; > QemuOpts *opts; > - QDict *bs_opts; > const char *id; > bool has_driver_specific_opts; > BlockDriver *drv =3D NULL; > @@ -334,9 +334,9 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, > translation =3D BIOS_ATA_TRANSLATION_AUTO; > media =3D MEDIA_DISK; > =20 > - /* Check common options by copying from all_opts to opts, all othe= r options > - * are stored in bs_opts. */ > - id =3D qemu_opts_id(all_opts); > + /* Check common options by copying from bs_opts to opts, all other= options > + * stay in bs_opts for processing by bdrv_open(). */ > + id =3D qdict_get_try_str(bs_opts, "id"); > opts =3D qemu_opts_create(&qemu_common_drive_opts, id, 1, &error); > if (error_is_set(&error)) { > qerror_report_err(error); > @@ -344,8 +344,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, > return NULL; > } > =20 > - bs_opts =3D qdict_new(); > - qemu_opts_to_qdict(all_opts, bs_opts); > qemu_opts_absorb_qdict(opts, bs_opts, &error); > if (error_is_set(&error)) { > qerror_report_err(error); > @@ -634,7 +632,6 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts, > dinfo->heads =3D heads; > dinfo->secs =3D secs; > dinfo->trans =3D translation; > - dinfo->opts =3D all_opts; > dinfo->refcount =3D 1; > if (serial !=3D NULL) { > dinfo->serial =3D g_strdup(serial); > @@ -759,6 +756,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInte= rfaceType block_default_type) > { > const char *value; > DriveInfo *dinfo; > + QDict *bs_opts; > =20 > /* Change legacy command line options into QMP ones */ > qemu_opt_rename(all_opts, "iops", "throttling.iops-total"); > @@ -807,14 +805,19 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockIn= terfaceType block_default_type) > qemu_opt_unset(all_opts, "cache"); > } > =20 > + /* Get a QDict for processing the options */ > + bs_opts =3D qdict_new(); > + qemu_opts_to_qdict(all_opts, bs_opts); > + > /* Actual block device init: Functionality shared with blockdev-ad= d */ > - dinfo =3D blockdev_init(all_opts, block_default_type); > + dinfo =3D blockdev_init(bs_opts, block_default_type); > if (dinfo =3D=3D NULL) { > goto fail; > } > =20 > /* Set legacy DriveInfo fields */ > dinfo->enable_auto_del =3D true; > + dinfo->opts =3D all_opts; > =20 > fail: > return dinfo; > @@ -2109,16 +2112,10 @@ void qmp_blockdev_add(BlockdevOptions *options,= Error **errp) > =20 > qdict_flatten(qdict); > =20 > - QemuOpts *opts =3D qemu_opts_from_qdict(&qemu_drive_opts, qdict, &= local_err); > - if (error_is_set(&local_err)) { > - error_propagate(errp, local_err); > + dinfo =3D blockdev_init(qdict, IF_NONE); > + if (!dinfo) { > + error_setg(errp, "Could not open image"); > goto fail; > - } else { > - dinfo =3D blockdev_init(opts, IF_NONE); > - if (!dinfo) { > - error_setg(errp, "Could not open image"); > - goto fail; > - } > } > =20 > fail: > --=20 > 1.8.1.4 >=20 >=20 Reviewed-by: Benoit Canet