From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52154) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN2Fi-0000Jy-Jr for qemu-devel@nongnu.org; Fri, 20 Sep 2013 11:04:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VN2Fd-0006Lq-HV for qemu-devel@nongnu.org; Fri, 20 Sep 2013 11:04:14 -0400 Received: from nodalink.pck.nerim.net ([62.212.105.220]:37451 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN2Fc-0006Lk-T0 for qemu-devel@nongnu.org; Fri, 20 Sep 2013 11:04:09 -0400 Date: Fri, 20 Sep 2013 17:04:00 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20130920150359.GF6794@irqsave.net> References: <1379678070-14346-1-git-send-email-kwolf@redhat.com> <1379678070-14346-10-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-10-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 09/17] blockdev: Moving parsing of geometry options 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:22 (+0200), Kevin Wolf a =E9crit : > This moves all of the geometry options (cyls/heads/secs/trans) to > drive_init so that they can only be accessed using legacy functions, bu= t > never with anything blockdev-add related. >=20 > Signed-off-by: Kevin Wolf > --- > blockdev.c | 136 +++++++++++++++++++++++++++++++----------------------= -------- > 1 file changed, 69 insertions(+), 67 deletions(-) >=20 > diff --git a/blockdev.c b/blockdev.c > index dc3f01a..ba8b6b4 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -314,7 +314,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > const char *serial; > const char *mediastr =3D ""; > int bus_id, unit_id; > - int cyls, heads, secs, translation; > int max_devs; > int index; > int ro =3D 0; > @@ -332,8 +331,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > bool has_driver_specific_opts; > BlockDriver *drv =3D NULL; > =20 > - translation =3D BIOS_ATA_TRANSLATION_AUTO; > - > /* 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"); > @@ -362,10 +359,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > unit_id =3D qemu_opt_get_number(opts, "unit", -1); > index =3D qemu_opt_get_number(opts, "index", -1); > =20 > - cyls =3D qemu_opt_get_number(opts, "cyls", 0); > - heads =3D qemu_opt_get_number(opts, "heads", 0); > - secs =3D qemu_opt_get_number(opts, "secs", 0); > - > snapshot =3D qemu_opt_get_bool(opts, "snapshot", 0); > ro =3D qemu_opt_get_bool(opts, "read-only", 0); > copy_on_read =3D qemu_opt_get_bool(opts, "copy-on-read", false); > @@ -375,46 +368,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > =20 > max_devs =3D if_max_devs[type]; > =20 > - if (cyls || heads || secs) { > - if (cyls < 1) { > - error_report("invalid physical cyls number"); > - return NULL; > - } > - if (heads < 1) { > - error_report("invalid physical heads number"); > - return NULL; > - } > - if (secs < 1) { > - error_report("invalid physical secs number"); > - return NULL; > - } > - } > - > - if ((buf =3D qemu_opt_get(opts, "trans")) !=3D NULL) { > - if (!cyls) { > - error_report("'%s' trans must be used with cyls, heads and= secs", > - buf); > - return NULL; > - } > - if (!strcmp(buf, "none")) > - translation =3D BIOS_ATA_TRANSLATION_NONE; > - else if (!strcmp(buf, "lba")) > - translation =3D BIOS_ATA_TRANSLATION_LBA; > - else if (!strcmp(buf, "auto")) > - translation =3D BIOS_ATA_TRANSLATION_AUTO; > - else { > - error_report("'%s' invalid translation type", 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; > - } > - } > - > if ((buf =3D qemu_opt_get(opts, "discard")) !=3D NULL) { > if (bdrv_parse_discard_flags(buf, &bdrv_flags) !=3D 0) { > error_report("invalid discard option"); > @@ -609,10 +562,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > dinfo->type =3D type; > dinfo->bus =3D bus_id; > dinfo->unit =3D unit_id; > - dinfo->cyls =3D cyls; > - dinfo->heads =3D heads; > - dinfo->secs =3D secs; > - dinfo->trans =3D translation; > dinfo->refcount =3D 1; > if (serial !=3D NULL) { > dinfo->serial =3D g_strdup(serial); > @@ -745,6 +694,22 @@ QemuOptsList qemu_legacy_drive_opts =3D { > .name =3D "if", > .type =3D QEMU_OPT_STRING, > .help =3D "interface (ide, scsi, sd, mtd, floppy, pflash, = virtio)", > + },{ > + .name =3D "cyls", > + .type =3D QEMU_OPT_NUMBER, > + .help =3D "number of cylinders (ide disk geometry)", > + },{ > + .name =3D "heads", > + .type =3D QEMU_OPT_NUMBER, > + .help =3D "number of heads (ide disk geometry)", > + },{ > + .name =3D "secs", > + .type =3D QEMU_OPT_NUMBER, > + .help =3D "number of sectors (ide disk geometry)", > + },{ > + .name =3D "trans", > + .type =3D QEMU_OPT_STRING, > + .help =3D "chs translation (auto, lba. none)", > }, > { /* end of list */ } > }, > @@ -758,6 +723,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInte= rfaceType block_default_type) > QemuOpts *legacy_opts; > DriveMediaType media =3D MEDIA_DISK; > BlockInterfaceType type; > + int cyls, heads, secs, translation; > Error *local_err =3D NULL; > =20 > /* Change legacy command line options into QMP ones */ > @@ -847,6 +813,53 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInt= erfaceType block_default_type) > type =3D block_default_type; > } > =20 > + /* Geometry */ > + cyls =3D qemu_opt_get_number(legacy_opts, "cyls", 0); > + heads =3D qemu_opt_get_number(legacy_opts, "heads", 0); > + secs =3D qemu_opt_get_number(legacy_opts, "secs", 0); > + > + if (cyls || heads || secs) { > + if (cyls < 1) { > + error_report("invalid physical cyls number"); > + return NULL; > + } > + if (heads < 1) { > + error_report("invalid physical heads number"); > + return NULL; > + } > + if (secs < 1) { > + error_report("invalid physical secs number"); > + return NULL; > + } > + } > + > + translation =3D BIOS_ATA_TRANSLATION_AUTO; > + value =3D qemu_opt_get(legacy_opts, "trans"); > + if (value !=3D NULL) { > + if (!cyls) { > + error_report("'%s' trans must be used with cyls, heads and= secs", > + value); > + return NULL; > + } > + if (!strcmp(value, "none")) { > + translation =3D BIOS_ATA_TRANSLATION_NONE; > + } else if (!strcmp(value, "lba")) { > + translation =3D BIOS_ATA_TRANSLATION_LBA; > + } else if (!strcmp(value, "auto")) { > + translation =3D BIOS_ATA_TRANSLATION_AUTO; > + } else { > + error_report("'%s' invalid translation type", value); > + return NULL; > + } > + } > + > + if (media =3D=3D MEDIA_CDROM) { > + if (cyls || secs || heads) { > + error_report("CHS can't be set with media=3Dcdrom"); > + return NULL; > + } > + } > + > /* Actual block device init: Functionality shared with blockdev-ad= d */ > dinfo =3D blockdev_init(bs_opts, type, media); > if (dinfo =3D=3D NULL) { > @@ -857,6 +870,11 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInt= erfaceType block_default_type) > dinfo->enable_auto_del =3D true; > dinfo->opts =3D all_opts; > =20 > + dinfo->cyls =3D cyls; > + dinfo->heads =3D heads; > + dinfo->secs =3D secs; > + dinfo->trans =3D translation; > + > fail: > qemu_opts_del(legacy_opts); > return dinfo; > @@ -2200,22 +2218,6 @@ QemuOptsList qemu_common_drive_opts =3D { > .type =3D QEMU_OPT_NUMBER, > .help =3D "index number", > },{ > - .name =3D "cyls", > - .type =3D QEMU_OPT_NUMBER, > - .help =3D "number of cylinders (ide disk geometry)", > - },{ > - .name =3D "heads", > - .type =3D QEMU_OPT_NUMBER, > - .help =3D "number of heads (ide disk geometry)", > - },{ > - .name =3D "secs", > - .type =3D QEMU_OPT_NUMBER, > - .help =3D "number of sectors (ide disk geometry)", > - },{ > - .name =3D "trans", > - .type =3D QEMU_OPT_STRING, > - .help =3D "chs translation (auto, lba. none)", > - },{ > .name =3D "snapshot", > .type =3D QEMU_OPT_BOOL, > .help =3D "enable/disable snapshot mode", > --=20 > 1.8.1.4 >=20 >=20 Aside from the "goto fail;" impacting multiple patch: Reviewed-for-C-bugs-by: Benoit Canet