From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37094) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQG9W-0000cp-IF for qemu-devel@nongnu.org; Mon, 22 Oct 2012 07:26:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQG9V-0007Y1-8u for qemu-devel@nongnu.org; Mon, 22 Oct 2012 07:26:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17817) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQG9V-0007Xx-0f for qemu-devel@nongnu.org; Mon, 22 Oct 2012 07:26:37 -0400 Message-ID: <50852D65.2090201@redhat.com> Date: Mon, 22 Oct 2012 13:26:29 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <88ca80e8aca8fb53089dbcc34bdbe72ce8a18e82.1350677361.git.jbaron@redhat.com> <20121022104731.GA28828@redhat.com> In-Reply-To: <20121022104731.GA28828@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: "Michael S. Tsirkin" Cc: aliguori@us.ibm.com, juzhang@redhat.com, jan.kiszka@siemens.com, Jason Baron , 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 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)? Also, 'mach_if' isn't a very descriptive name. Something like 'default_drive_if' would be better. >> 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. >> { >> 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