From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43220) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN1hU-0005sg-6w for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:28:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VN1h6-0003NU-No for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:28:52 -0400 Received: from nodalink.pck.nerim.net ([62.212.105.220]:37419 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN1h6-0003LT-71 for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:28:28 -0400 Date: Fri, 20 Sep 2013 16:28:19 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20130920142819.GD6794@irqsave.net> References: <1379678070-14346-1-git-send-email-kwolf@redhat.com> <1379678070-14346-8-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-8-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 07/17] blockdev: Move parsing of 'media' option to drive_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:20 (+0200), Kevin Wolf a =E9crit : > This moves as much as possible of the processing of the 'media' option > to drive_init so that it can only be accessed using legacy functions, > but never with anything blockdev-add related. >=20 > Signed-off-by: Kevin Wolf > --- > blockdev.c | 73 ++++++++++++++++++++++++++++++++++++++++++------------= -------- > 1 file changed, 50 insertions(+), 23 deletions(-) >=20 > diff --git a/blockdev.c b/blockdev.c > index 3a1444c..5b25d7f 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -302,16 +302,18 @@ static bool check_throttle_config(ThrottleConfig = *cfg, Error **errp) > return true; > } > =20 > +typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; > + > /* Takes the ownership of bs_opts */ > static DriveInfo *blockdev_init(QDict *bs_opts, > - BlockInterfaceType block_default_type) > + BlockInterfaceType block_default_type, > + DriveMediaType media) > { > const char *buf; > const char *file =3D NULL; > const char *serial; > const char *mediastr =3D ""; > BlockInterfaceType type; > - enum { MEDIA_DISK, MEDIA_CDROM } media; > int bus_id, unit_id; > int cyls, heads, secs, translation; > int max_devs; > @@ -332,7 +334,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > BlockDriver *drv =3D NULL; > =20 > translation =3D BIOS_ATA_TRANSLATION_AUTO; > - media =3D MEDIA_DISK; > =20 > /* Check common options by copying from bs_opts to opts, all other= options > * stay in bs_opts for processing by bdrv_open(). */ > @@ -419,19 +420,11 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > } > } > =20 > - if ((buf =3D qemu_opt_get(opts, "media")) !=3D NULL) { > - if (!strcmp(buf, "disk")) { > - media =3D MEDIA_DISK; > - } else if (!strcmp(buf, "cdrom")) { > - if (cyls || secs || heads) { > - error_report("CHS can't be set with media=3D%s", buf); > - return NULL; > - } > - media =3D MEDIA_CDROM; > - } else { > - error_report("'%s' invalid media", buf); > - return NULL; > - } > + if (media =3D=3D MEDIA_CDROM) { > + if (cyls || secs || heads) { > + error_report("CHS can't be set with media=3Dcdrom"); > + return NULL; > + } > } > =20 > if ((buf =3D qemu_opt_get(opts, "discard")) !=3D NULL) { > @@ -752,11 +745,27 @@ static void qemu_opt_rename(QemuOpts *opts, const= char *from, const char *to) > } > } > =20 > +QemuOptsList qemu_legacy_drive_opts =3D { > + .name =3D "drive", > + .head =3D QTAILQ_HEAD_INITIALIZER(qemu_legacy_drive_opts.head), > + .desc =3D { > + { > + .name =3D "media", > + .type =3D QEMU_OPT_STRING, > + .help =3D "media type (disk, cdrom)", > + }, > + { /* end of list */ } > + }, > +}; > + > DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_def= ault_type) > { > const char *value; > - DriveInfo *dinfo; > + DriveInfo *dinfo =3D NULL; > QDict *bs_opts; > + QemuOpts *legacy_opts; > + DriveMediaType media =3D MEDIA_DISK; > + Error *local_err =3D NULL; > =20 > /* Change legacy command line options into QMP ones */ > qemu_opt_rename(all_opts, "iops", "throttling.iops-total"); > @@ -809,8 +818,29 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInt= erfaceType block_default_type) > bs_opts =3D qdict_new(); > qemu_opts_to_qdict(all_opts, bs_opts); > =20 > + legacy_opts =3D qemu_opts_create_nofail(&qemu_legacy_drive_opts); > + qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err); > + if (error_is_set(&local_err)) { > + qerror_report_err(local_err); > + error_free(local_err); > + goto fail; > + } > + > + /* Media type */ > + value =3D qemu_opt_get(legacy_opts, "media"); > + if (value) { > + if (!strcmp(value, "disk")) { > + media =3D MEDIA_DISK; > + } else if (!strcmp(value, "cdrom")) { > + media =3D MEDIA_CDROM; > + } else { > + error_report("'%s' invalid media", value); > + goto fail; > + } > + } > + > /* Actual block device init: Functionality shared with blockdev-ad= d */ > - dinfo =3D blockdev_init(bs_opts, block_default_type); > + dinfo =3D blockdev_init(bs_opts, block_default_type, media); > if (dinfo =3D=3D NULL) { > goto fail; > } > @@ -820,6 +850,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInte= rfaceType block_default_type) > dinfo->opts =3D all_opts; > =20 > fail: > + qemu_opts_del(legacy_opts); > return dinfo; > } > =20 > @@ -2112,7 +2143,7 @@ void qmp_blockdev_add(BlockdevOptions *options, E= rror **errp) > =20 > qdict_flatten(qdict); > =20 > - dinfo =3D blockdev_init(qdict, IF_NONE); > + dinfo =3D blockdev_init(qdict, IF_NONE, MEDIA_DISK); > if (!dinfo) { > error_setg(errp, "Could not open image"); > goto fail; > @@ -2181,10 +2212,6 @@ QemuOptsList qemu_common_drive_opts =3D { > .type =3D QEMU_OPT_STRING, > .help =3D "chs translation (auto, lba. none)", > },{ > - .name =3D "media", > - .type =3D QEMU_OPT_STRING, > - .help =3D "media type (disk, cdrom)", > - },{ > .name =3D "snapshot", > .type =3D QEMU_OPT_BOOL, > .help =3D "enable/disable snapshot mode", > --=20 > 1.8.1.4 >=20 >=20 Reviewed-by: Benoit Canet