From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57648) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQMKc-0002tb-MC for qemu-devel@nongnu.org; Mon, 22 Oct 2012 14:02:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQMKb-0007d2-7y for qemu-devel@nongnu.org; Mon, 22 Oct 2012 14:02:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38684) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQMKa-0007cr-VX for qemu-devel@nongnu.org; Mon, 22 Oct 2012 14:02:29 -0400 Date: Mon, 22 Oct 2012 14:02:23 -0400 From: Jason Baron Message-ID: <20121022180223.GA23972@redhat.com> References: <88ca80e8aca8fb53089dbcc34bdbe72ce8a18e82.1350677361.git.jbaron@redhat.com> <20121022104731.GA28828@redhat.com> <50852D65.2090201@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50852D65.2090201@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 01/26] blockdev: Introduce a default machine blockdev interface field, QEMUMachine->mach_if List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: aliguori@us.ibm.com, juzhang@redhat.com, "Michael S. Tsirkin" , jan.kiszka@siemens.com, qemu-devel@nongnu.org, agraf@suse.de, blauwirbel@gmail.com, yamahata@valinux.co.jp, alex.williamson@redhat.com, kevin@koconnor.net, avi@redhat.com, mkletzan@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, afaerber@suse.de, armbru@redhat.com, kraxel@redhat.com On Mon, Oct 22, 2012 at 01:26:29PM +0200, Kevin Wolf wrote: > Am 22.10.2012 12:47, schrieb Michael S. Tsirkin: > > On Fri, Oct 19, 2012 at 04:43:26PM -0400, Jason Baron wrote: > >> From: Jason Baron > >> > >> The current QEMUMachine definition has a 'use_scsi' field to indicate if a > >> machine type should use scsi by default. However, Q35 wants to use ahci by > >> default. Thus, introdue a new field in the QEMUMachine defintion, mach_if. > >> > >> This field should be initialized by the machine type to the default interface > >> type which it wants to use (IF_SCSI, IF_AHCI, etc.). If no mach_if is defined, > >> or it is set to 'IF_DEFAULT' or 'IF_NONE', we currently assume IF_IDE. > > Is this default mechanism necessary? Can't we make sure that each > machine does define its preferred interface, and doesn't define it as > IF_DEFAULT (which would be the same as an explicit IF_IDE anyway)? > IF_NONE is currently defined as 0, so I wanted to make sure that any machine types that didn't explicity define the 'mach_if' field would be mapped to IF_IDE. If you don't like having 'IF_DEFAULT' there, I can simply drop 2 'IF_DEFAULT' settings that I had. Would that be ok with you? > Also, 'mach_if' isn't a very descriptive name. Something like > 'default_drive_if' would be better. > ok, will update. > >> Please use 'static inline int get_mach_if(int mach_if)', when accesssing the > >> new mach_if field. > >> > >> Reviewed-by: Paolo Bonzini > >> Signed-off-by: Jason Baron > > > > Kevin, could you review/ack this patch pls? > > > >> --- > >> blockdev.c | 4 ++-- > >> blockdev.h | 19 +++++++++++++++++++ > >> hw/boards.h | 2 +- > >> hw/device-hotplug.c | 2 +- > >> hw/highbank.c | 2 +- > >> hw/leon3.c | 2 +- > >> hw/mips_jazz.c | 4 ++-- > >> hw/pc_sysfw.c | 2 +- > >> hw/puv3.c | 2 +- > >> hw/realview.c | 6 +++--- > >> hw/spapr.c | 2 +- > >> hw/sun4m.c | 24 ++++++++++++------------ > >> hw/versatilepb.c | 4 ++-- > >> hw/vexpress.c | 4 ++-- > >> hw/xilinx_zynq.c | 2 +- > >> vl.c | 20 +++++++++++--------- > >> 16 files changed, 61 insertions(+), 40 deletions(-) > >> > >> diff --git a/blockdev.c b/blockdev.c > >> index 99828ad..c9a49c8 100644 > >> --- a/blockdev.c > >> +++ b/blockdev.c > >> @@ -275,7 +275,7 @@ static bool do_check_io_limits(BlockIOLimit *io_limits) > >> return true; > >> } > >> > >> -DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > >> +DriveInfo *drive_init(QemuOpts *opts, int mach_if) > > BlockInterfaceType, not int. > ok. > >> { > >> const char *buf; > >> const char *file = NULL; > >> @@ -325,7 +325,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) > >> return NULL; > >> } > >> } else { > >> - type = default_to_scsi ? IF_SCSI : IF_IDE; > >> + type = get_mach_if(mach_if); > >> } > >> > >> max_devs = if_max_devs[type]; > >> diff --git a/blockdev.h b/blockdev.h > >> index 5f27b64..8b126ad 100644 > >> --- a/blockdev.h > >> +++ b/blockdev.h > >> @@ -40,6 +40,22 @@ struct DriveInfo { > >> int refcount; > >> }; > >> > >> +/* > >> + * Each qemu machine type defines a mach_if field for its default > >> + * interface type. If its unspecified, we set it to IF_IDE. > >> + */ > >> +static inline int get_mach_if(int mach_if) > >> +{ > >> + assert(mach_if < IF_COUNT); > >> + assert(mach_if >= IF_DEFAULT); > >> + > >> + if ((mach_if == IF_NONE) || (mach_if == IF_DEFAULT)) { > >> + return IF_IDE; > >> + } > >> + > >> + return mach_if; > >> +} > >> + > >> DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit); > >> DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); > >> int drive_get_max_bus(BlockInterfaceType type); > >> @@ -61,4 +77,7 @@ void qmp_change_blockdev(const char *device, const char *filename, > >> bool has_format, const char *format, Error **errp); > >> void do_commit(Monitor *mon, const QDict *qdict); > >> int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data); > >> + > >> + > >> + > >> #endif > >> diff --git a/hw/boards.h b/hw/boards.h > >> index a2e0a54..969fd67 100644 > >> --- a/hw/boards.h > >> +++ b/hw/boards.h > >> @@ -20,7 +20,7 @@ typedef struct QEMUMachine { > >> const char *desc; > >> QEMUMachineInitFunc *init; > >> QEMUMachineResetFunc *reset; > >> - int use_scsi; > >> + int mach_if; > > Same here. > > Kevin ok. Thanks, -Jason