From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47890) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN1zo-0002HA-Ub for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:47:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VN1zi-0001CI-UR for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:47:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:28013) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VN1zi-0001CE-MT for qemu-devel@nongnu.org; Fri, 20 Sep 2013 10:47:42 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8KElfs6000301 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 20 Sep 2013 10:47:41 -0400 Message-ID: <523C600A.5000805@redhat.com> Date: Fri, 20 Sep 2013 16:47:38 +0200 From: Max Reitz MIME-Version: 1.0 References: <1379678070-14346-1-git-send-email-kwolf@redhat.com> <1379678070-14346-9-git-send-email-kwolf@redhat.com> In-Reply-To: <1379678070-14346-9-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 08/17] blockdev: Move parsing of 'if' option to drive_init List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com On 2013-09-20 13:54, Kevin Wolf wrote: > It's always IF_NONE for blockdev-add. > > Signed-off-by: Kevin Wolf > --- > blockdev.c | 40 ++++++++++++++++++++++------------------ > 1 file changed, 22 insertions(+), 18 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 5b25d7f..dc3f01a 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -306,14 +306,13 @@ 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 type, > DriveMediaType media) > { > const char *buf; > const char *file = NULL; > const char *serial; > const char *mediastr = ""; > - BlockInterfaceType type; > int bus_id, unit_id; > int cyls, heads, secs, translation; > int max_devs; > @@ -374,17 +373,6 @@ static DriveInfo *blockdev_init(QDict *bs_opts, > file = qemu_opt_get(opts, "file"); > serial = qemu_opt_get(opts, "serial"); > > - if ((buf = qemu_opt_get(opts, "if")) != NULL) { > - for (type = 0; type < IF_COUNT && strcmp(buf, if_name[type]); type++) > - ; > - if (type == IF_COUNT) { > - error_report("unsupported bus type '%s'", buf); > - return NULL; > - } > - } else { > - type = block_default_type; > - } > - > max_devs = if_max_devs[type]; > > if (cyls || heads || secs) { > @@ -753,6 +741,10 @@ QemuOptsList qemu_legacy_drive_opts = { > .name = "media", > .type = QEMU_OPT_STRING, > .help = "media type (disk, cdrom)", > + },{ > + .name = "if", > + .type = QEMU_OPT_STRING, > + .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", > }, > { /* end of list */ } > }, > @@ -765,6 +757,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) > QDict *bs_opts; > QemuOpts *legacy_opts; > DriveMediaType media = MEDIA_DISK; > + BlockInterfaceType type; > Error *local_err = NULL; > > /* Change legacy command line options into QMP ones */ > @@ -839,8 +832,23 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) > } > } > > + /* Controller type */ > + value = qemu_opt_get(legacy_opts, "if"); > + if (value) { > + for (type = 0; > + type < IF_COUNT && strcmp(value, if_name[type]); > + type++) { > + } > + if (type == IF_COUNT) { > + error_report("unsupported bus type '%s'", value); > + return NULL; I'd suggest "goto fail;" instead. Max > + } > + } else { > + type = block_default_type; > + } > + > /* Actual block device init: Functionality shared with blockdev-add */ > - dinfo = blockdev_init(bs_opts, block_default_type, media); > + dinfo = blockdev_init(bs_opts, type, media); > if (dinfo == NULL) { > goto fail; > } > @@ -2188,10 +2196,6 @@ QemuOptsList qemu_common_drive_opts = { > .type = QEMU_OPT_NUMBER, > .help = "unit number (i.e. lun for scsi)", > },{ > - .name = "if", > - .type = QEMU_OPT_STRING, > - .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", > - },{ > .name = "index", > .type = QEMU_OPT_NUMBER, > .help = "index number",