Message ID | 88ca80e8aca8fb53089dbcc34bdbe72ce8a18e82.1350677361.git.jbaron@redhat.com |
---|---|
State | New |
Headers | show |
On Fri, Oct 19, 2012 at 04:43:26PM -0400, Jason Baron wrote: > From: Jason Baron <jbaron@redhat.com> > > 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. > > Please use 'static inline int get_mach_if(int mach_if)', when accesssing the > new mach_if field. > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Jason Baron <jbaron@redhat.com> 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) > { > 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; > int max_cpus; > unsigned int no_serial:1, > no_parallel:1, > diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c > index eec0fe3..33302f9 100644 > --- a/hw/device-hotplug.c > +++ b/hw/device-hotplug.c > @@ -39,7 +39,7 @@ DriveInfo *add_init_drive(const char *optstr) > if (!opts) > return NULL; > > - dinfo = drive_init(opts, current_machine->use_scsi); > + dinfo = drive_init(opts, current_machine->mach_if); > if (!dinfo) { > qemu_opts_del(opts); > return NULL; > diff --git a/hw/highbank.c b/hw/highbank.c > index 11aa131..35cef06 100644 > --- a/hw/highbank.c > +++ b/hw/highbank.c > @@ -324,7 +324,7 @@ static QEMUMachine highbank_machine = { > .name = "highbank", > .desc = "Calxeda Highbank (ECX-1000)", > .init = highbank_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/leon3.c b/hw/leon3.c > index 7a9729d..cf9dcf8 100644 > --- a/hw/leon3.c > +++ b/hw/leon3.c > @@ -214,7 +214,7 @@ static QEMUMachine leon3_generic_machine = { > .name = "leon3_generic", > .desc = "Leon-3 generic", > .init = leon3_generic_hw_init, > - .use_scsi = 0, > + .mach_if = IF_DEFAULT, > }; > > static void leon3_machine_init(void) > diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c > index db927f1..1c7a725 100644 > --- a/hw/mips_jazz.c > +++ b/hw/mips_jazz.c > @@ -325,14 +325,14 @@ static QEMUMachine mips_magnum_machine = { > .name = "magnum", > .desc = "MIPS Magnum", > .init = mips_magnum_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine mips_pica61_machine = { > .name = "pica61", > .desc = "Acer Pica 61", > .init = mips_pica61_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void mips_jazz_machine_init(void) > diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c > index b45f0ac..b8a03a6 100644 > --- a/hw/pc_sysfw.c > +++ b/hw/pc_sysfw.c > @@ -98,7 +98,7 @@ static void pc_fw_add_pflash_drv(void) > return; > } > > - drive_init(opts, machine->use_scsi); > + drive_init(opts, machine->mach_if); > } > > static void pc_system_flash_init(MemoryRegion *rom_memory, > diff --git a/hw/puv3.c b/hw/puv3.c > index 43f7216..f68bb61 100644 > --- a/hw/puv3.c > +++ b/hw/puv3.c > @@ -120,7 +120,7 @@ static QEMUMachine puv3_machine = { > .desc = "PKUnity Version-3 based on UniCore32", > .init = puv3_init, > .is_default = 1, > - .use_scsi = 0, > + .mach_if = IF_DEFAULT, > }; > > static void puv3_machine_init(void) > diff --git a/hw/realview.c b/hw/realview.c > index 19db4d0..7613f68 100644 > --- a/hw/realview.c > +++ b/hw/realview.c > @@ -382,14 +382,14 @@ static QEMUMachine realview_eb_machine = { > .name = "realview-eb", > .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)", > .init = realview_eb_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine realview_eb_mpcore_machine = { > .name = "realview-eb-mpcore", > .desc = "ARM RealView Emulation Baseboard (ARM11MPCore)", > .init = realview_eb_mpcore_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -403,7 +403,7 @@ static QEMUMachine realview_pbx_a9_machine = { > .name = "realview-pbx-a9", > .desc = "ARM RealView Platform Baseboard Explore for Cortex-A9", > .init = realview_pbx_a9_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/spapr.c b/hw/spapr.c > index 09b8e99..be8129e 100644 > --- a/hw/spapr.c > +++ b/hw/spapr.c > @@ -913,7 +913,7 @@ static QEMUMachine spapr_machine = { > .reset = ppc_spapr_reset, > .max_cpus = MAX_CPUS, > .no_parallel = 1, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void spapr_machine_init(void) > diff --git a/hw/sun4m.c b/hw/sun4m.c > index a04b485..101d552 100644 > --- a/hw/sun4m.c > +++ b/hw/sun4m.c > @@ -1400,7 +1400,7 @@ static QEMUMachine ss5_machine = { > .name = "SS-5", > .desc = "Sun4m platform, SPARCstation 5", > .init = ss5_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .is_default = 1, > }; > > @@ -1408,7 +1408,7 @@ static QEMUMachine ss10_machine = { > .name = "SS-10", > .desc = "Sun4m platform, SPARCstation 10", > .init = ss10_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1416,7 +1416,7 @@ static QEMUMachine ss600mp_machine = { > .name = "SS-600MP", > .desc = "Sun4m platform, SPARCserver 600MP", > .init = ss600mp_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1424,7 +1424,7 @@ static QEMUMachine ss20_machine = { > .name = "SS-20", > .desc = "Sun4m platform, SPARCstation 20", > .init = ss20_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1432,35 +1432,35 @@ static QEMUMachine voyager_machine = { > .name = "Voyager", > .desc = "Sun4m platform, SPARCstation Voyager", > .init = vger_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine ss_lx_machine = { > .name = "LX", > .desc = "Sun4m platform, SPARCstation LX", > .init = ss_lx_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine ss4_machine = { > .name = "SS-4", > .desc = "Sun4m platform, SPARCstation 4", > .init = ss4_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine scls_machine = { > .name = "SPARCClassic", > .desc = "Sun4m platform, SPARCClassic", > .init = scls_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine sbook_machine = { > .name = "SPARCbook", > .desc = "Sun4m platform, SPARCbook", > .init = sbook_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static const struct sun4d_hwdef sun4d_hwdefs[] = { > @@ -1677,7 +1677,7 @@ static QEMUMachine ss1000_machine = { > .name = "SS-1000", > .desc = "Sun4d platform, SPARCserver 1000", > .init = ss1000_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 8, > }; > > @@ -1685,7 +1685,7 @@ static QEMUMachine ss2000_machine = { > .name = "SS-2000", > .desc = "Sun4d platform, SPARCcenter 2000", > .init = ss2000_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 20, > }; > > @@ -1861,7 +1861,7 @@ static QEMUMachine ss2_machine = { > .name = "SS-2", > .desc = "Sun4c platform, SPARCstation 2", > .init = ss2_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void sun4m_register_types(void) > diff --git a/hw/versatilepb.c b/hw/versatilepb.c > index 7b1b025..af5120f 100644 > --- a/hw/versatilepb.c > +++ b/hw/versatilepb.c > @@ -374,14 +374,14 @@ static QEMUMachine versatilepb_machine = { > .name = "versatilepb", > .desc = "ARM Versatile/PB (ARM926EJ-S)", > .init = vpb_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine versatileab_machine = { > .name = "versatileab", > .desc = "ARM Versatile/AB (ARM926EJ-S)", > .init = vab_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void versatile_machine_init(void) > diff --git a/hw/vexpress.c b/hw/vexpress.c > index 3596d1e..3c7c012 100644 > --- a/hw/vexpress.c > +++ b/hw/vexpress.c > @@ -495,7 +495,7 @@ static QEMUMachine vexpress_a9_machine = { > .name = "vexpress-a9", > .desc = "ARM Versatile Express for Cortex-A9", > .init = vexpress_a9_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -503,7 +503,7 @@ static QEMUMachine vexpress_a15_machine = { > .name = "vexpress-a15", > .desc = "ARM Versatile Express for Cortex-A15", > .init = vexpress_a15_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c > index fd46ba2..c70eb69 100644 > --- a/hw/xilinx_zynq.c > +++ b/hw/xilinx_zynq.c > @@ -178,7 +178,7 @@ static QEMUMachine zynq_machine = { > .name = "xilinx-zynq-a9", > .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", > .init = zynq_init, > - .use_scsi = 1, > + .if_default = IF_SCSI, > .max_cpus = 1, > .no_sdcard = 1 > }; > diff --git a/vl.c b/vl.c > index 5b357a3..6b1e546 100644 > --- a/vl.c > +++ b/vl.c > @@ -802,9 +802,9 @@ static int parse_sandbox(QemuOpts *opts, void *opaque) > > static int drive_init_func(QemuOpts *opts, void *opaque) > { > - int *use_scsi = opaque; > + int *mach_if = opaque; > > - return drive_init(opts, *use_scsi) == NULL; > + return drive_init(opts, *mach_if) == NULL; > } > > static int drive_enable_snapshot(QemuOpts *opts, void *opaque) > @@ -815,14 +815,14 @@ static int drive_enable_snapshot(QemuOpts *opts, void *opaque) > return 0; > } > > -static void default_drive(int enable, int snapshot, int use_scsi, > +static void default_drive(int enable, int snapshot, int mach_if, > BlockInterfaceType type, int index, > const char *optstr) > { > QemuOpts *opts; > > if (type == IF_DEFAULT) { > - type = use_scsi ? IF_SCSI : IF_IDE; > + type = get_mach_if(mach_if); > } > > if (!enable || drive_get_by_index(type, index)) { > @@ -833,7 +833,7 @@ static void default_drive(int enable, int snapshot, int use_scsi, > if (snapshot) { > drive_enable_snapshot(opts, NULL); > } > - if (!drive_init(opts, use_scsi)) { > + if (!drive_init(opts, mach_if)) { > exit(1); > } > } > @@ -3547,14 +3547,16 @@ int main(int argc, char **argv, char **envp) > /* open the virtual block devices */ > if (snapshot) > qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0); > - if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, &machine->use_scsi, 1) != 0) > + if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, > + &machine->mach_if, 1) != 0) { > exit(1); > + } > > - default_drive(default_cdrom, snapshot, machine->use_scsi, > + default_drive(default_cdrom, snapshot, machine->mach_if, > IF_DEFAULT, 2, CDROM_OPTS); > - default_drive(default_floppy, snapshot, machine->use_scsi, > + default_drive(default_floppy, snapshot, machine->mach_if, > IF_FLOPPY, 0, FD_OPTS); > - default_drive(default_sdcard, snapshot, machine->use_scsi, > + default_drive(default_sdcard, snapshot, machine->mach_if, > IF_SD, 0, SD_OPTS); > > register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL); > -- > 1.7.1
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 <jbaron@redhat.com> >> >> 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 <pbonzini@redhat.com> >> Signed-off-by: Jason Baron <jbaron@redhat.com> > > 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
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 <jbaron@redhat.com> > >> > >> 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 <pbonzini@redhat.com> > >> Signed-off-by: Jason Baron <jbaron@redhat.com> > > > > 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
Jason Baron <jbaron@redhat.com> writes: > From: Jason Baron <jbaron@redhat.com> > > 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. Even though q35's desire to default to IF_AHCI is driving this patch, generalizing the default interface type makes sense on its own. Even if we IF_AHCI should turn out to need more discussion. > 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. > > Please use 'static inline int get_mach_if(int mach_if)', when accesssing the > new mach_if field. > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Jason Baron <jbaron@redhat.com> > --- > 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 mach_if Suggest to rename mach_if to something more descriptive. Kevin's default_drive_if works for me. > { > 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) BlockInterfaceType mach_if, and return type. > +{ > + 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; > +} > + I'm not sure we should map IF_NONE to IF_IDE. get_mach_if() should return an interface type the board supports. In theory, we could have a board that doesn't define any controllers, but still lets the user define some with -device (say because the board sports a PCI bus). Then IF_NONE would be the only interface type that makes any sense, and therefore the only sensible value of get_mach_if(). If we drop the magic mapping of IF_NONE, only IF_DEFAULT is left, and that one's clearly marked "for use with drive_add() only". No real need for magic mapping then. Could drop get_mach_if() and use mach_if directly. > 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 Unnecessary whitespace change, suggest to drop hunk. > 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; BlockInterfaceType mach_if > int max_cpus; > unsigned int no_serial:1, > no_parallel:1, > diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c > index eec0fe3..33302f9 100644 > --- a/hw/device-hotplug.c > +++ b/hw/device-hotplug.c > @@ -39,7 +39,7 @@ DriveInfo *add_init_drive(const char *optstr) > if (!opts) > return NULL; > > - dinfo = drive_init(opts, current_machine->use_scsi); > + dinfo = drive_init(opts, current_machine->mach_if); > if (!dinfo) { > qemu_opts_del(opts); > return NULL; > diff --git a/hw/highbank.c b/hw/highbank.c > index 11aa131..35cef06 100644 > --- a/hw/highbank.c > +++ b/hw/highbank.c > @@ -324,7 +324,7 @@ static QEMUMachine highbank_machine = { > .name = "highbank", > .desc = "Calxeda Highbank (ECX-1000)", > .init = highbank_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/leon3.c b/hw/leon3.c > index 7a9729d..cf9dcf8 100644 > --- a/hw/leon3.c > +++ b/hw/leon3.c > @@ -214,7 +214,7 @@ static QEMUMachine leon3_generic_machine = { > .name = "leon3_generic", > .desc = "Leon-3 generic", > .init = leon3_generic_hw_init, > - .use_scsi = 0, > + .mach_if = IF_DEFAULT, > }; > > static void leon3_machine_init(void) > diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c > index db927f1..1c7a725 100644 > --- a/hw/mips_jazz.c > +++ b/hw/mips_jazz.c > @@ -325,14 +325,14 @@ static QEMUMachine mips_magnum_machine = { > .name = "magnum", > .desc = "MIPS Magnum", > .init = mips_magnum_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine mips_pica61_machine = { > .name = "pica61", > .desc = "Acer Pica 61", > .init = mips_pica61_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void mips_jazz_machine_init(void) > diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c > index b45f0ac..b8a03a6 100644 > --- a/hw/pc_sysfw.c > +++ b/hw/pc_sysfw.c > @@ -98,7 +98,7 @@ static void pc_fw_add_pflash_drv(void) > return; > } > > - drive_init(opts, machine->use_scsi); > + drive_init(opts, machine->mach_if); > } > > static void pc_system_flash_init(MemoryRegion *rom_memory, > diff --git a/hw/puv3.c b/hw/puv3.c > index 43f7216..f68bb61 100644 > --- a/hw/puv3.c > +++ b/hw/puv3.c > @@ -120,7 +120,7 @@ static QEMUMachine puv3_machine = { > .desc = "PKUnity Version-3 based on UniCore32", > .init = puv3_init, > .is_default = 1, > - .use_scsi = 0, > + .mach_if = IF_DEFAULT, > }; > > static void puv3_machine_init(void) > diff --git a/hw/realview.c b/hw/realview.c > index 19db4d0..7613f68 100644 > --- a/hw/realview.c > +++ b/hw/realview.c > @@ -382,14 +382,14 @@ static QEMUMachine realview_eb_machine = { > .name = "realview-eb", > .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)", > .init = realview_eb_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine realview_eb_mpcore_machine = { > .name = "realview-eb-mpcore", > .desc = "ARM RealView Emulation Baseboard (ARM11MPCore)", > .init = realview_eb_mpcore_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -403,7 +403,7 @@ static QEMUMachine realview_pbx_a9_machine = { > .name = "realview-pbx-a9", > .desc = "ARM RealView Platform Baseboard Explore for Cortex-A9", > .init = realview_pbx_a9_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/spapr.c b/hw/spapr.c > index 09b8e99..be8129e 100644 > --- a/hw/spapr.c > +++ b/hw/spapr.c > @@ -913,7 +913,7 @@ static QEMUMachine spapr_machine = { > .reset = ppc_spapr_reset, > .max_cpus = MAX_CPUS, > .no_parallel = 1, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void spapr_machine_init(void) > diff --git a/hw/sun4m.c b/hw/sun4m.c > index a04b485..101d552 100644 > --- a/hw/sun4m.c > +++ b/hw/sun4m.c > @@ -1400,7 +1400,7 @@ static QEMUMachine ss5_machine = { > .name = "SS-5", > .desc = "Sun4m platform, SPARCstation 5", > .init = ss5_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .is_default = 1, > }; > > @@ -1408,7 +1408,7 @@ static QEMUMachine ss10_machine = { > .name = "SS-10", > .desc = "Sun4m platform, SPARCstation 10", > .init = ss10_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1416,7 +1416,7 @@ static QEMUMachine ss600mp_machine = { > .name = "SS-600MP", > .desc = "Sun4m platform, SPARCserver 600MP", > .init = ss600mp_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1424,7 +1424,7 @@ static QEMUMachine ss20_machine = { > .name = "SS-20", > .desc = "Sun4m platform, SPARCstation 20", > .init = ss20_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1432,35 +1432,35 @@ static QEMUMachine voyager_machine = { > .name = "Voyager", > .desc = "Sun4m platform, SPARCstation Voyager", > .init = vger_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine ss_lx_machine = { > .name = "LX", > .desc = "Sun4m platform, SPARCstation LX", > .init = ss_lx_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine ss4_machine = { > .name = "SS-4", > .desc = "Sun4m platform, SPARCstation 4", > .init = ss4_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine scls_machine = { > .name = "SPARCClassic", > .desc = "Sun4m platform, SPARCClassic", > .init = scls_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine sbook_machine = { > .name = "SPARCbook", > .desc = "Sun4m platform, SPARCbook", > .init = sbook_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static const struct sun4d_hwdef sun4d_hwdefs[] = { > @@ -1677,7 +1677,7 @@ static QEMUMachine ss1000_machine = { > .name = "SS-1000", > .desc = "Sun4d platform, SPARCserver 1000", > .init = ss1000_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 8, > }; > > @@ -1685,7 +1685,7 @@ static QEMUMachine ss2000_machine = { > .name = "SS-2000", > .desc = "Sun4d platform, SPARCcenter 2000", > .init = ss2000_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 20, > }; > > @@ -1861,7 +1861,7 @@ static QEMUMachine ss2_machine = { > .name = "SS-2", > .desc = "Sun4c platform, SPARCstation 2", > .init = ss2_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void sun4m_register_types(void) > diff --git a/hw/versatilepb.c b/hw/versatilepb.c > index 7b1b025..af5120f 100644 > --- a/hw/versatilepb.c > +++ b/hw/versatilepb.c > @@ -374,14 +374,14 @@ static QEMUMachine versatilepb_machine = { > .name = "versatilepb", > .desc = "ARM Versatile/PB (ARM926EJ-S)", > .init = vpb_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static QEMUMachine versatileab_machine = { > .name = "versatileab", > .desc = "ARM Versatile/AB (ARM926EJ-S)", > .init = vab_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > }; > > static void versatile_machine_init(void) > diff --git a/hw/vexpress.c b/hw/vexpress.c > index 3596d1e..3c7c012 100644 > --- a/hw/vexpress.c > +++ b/hw/vexpress.c > @@ -495,7 +495,7 @@ static QEMUMachine vexpress_a9_machine = { > .name = "vexpress-a9", > .desc = "ARM Versatile Express for Cortex-A9", > .init = vexpress_a9_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > @@ -503,7 +503,7 @@ static QEMUMachine vexpress_a15_machine = { > .name = "vexpress-a15", > .desc = "ARM Versatile Express for Cortex-A15", > .init = vexpress_a15_init, > - .use_scsi = 1, > + .mach_if = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c > index fd46ba2..c70eb69 100644 > --- a/hw/xilinx_zynq.c > +++ b/hw/xilinx_zynq.c > @@ -178,7 +178,7 @@ static QEMUMachine zynq_machine = { > .name = "xilinx-zynq-a9", > .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", > .init = zynq_init, > - .use_scsi = 1, > + .if_default = IF_SCSI, > .max_cpus = 1, > .no_sdcard = 1 > }; Didn't check you covered all boards. > diff --git a/vl.c b/vl.c > index 5b357a3..6b1e546 100644 > --- a/vl.c > +++ b/vl.c > @@ -802,9 +802,9 @@ static int parse_sandbox(QemuOpts *opts, void *opaque) > > static int drive_init_func(QemuOpts *opts, void *opaque) > { > - int *use_scsi = opaque; > + int *mach_if = opaque; BlockInterfaceType *mach_if > > - return drive_init(opts, *use_scsi) == NULL; > + return drive_init(opts, *mach_if) == NULL; > } > > static int drive_enable_snapshot(QemuOpts *opts, void *opaque) > @@ -815,14 +815,14 @@ static int drive_enable_snapshot(QemuOpts *opts, void *opaque) > return 0; > } > > -static void default_drive(int enable, int snapshot, int use_scsi, > +static void default_drive(int enable, int snapshot, int mach_if, BlockInterfaceType mach_if > BlockInterfaceType type, int index, > const char *optstr) > { > QemuOpts *opts; > > if (type == IF_DEFAULT) { > - type = use_scsi ? IF_SCSI : IF_IDE; > + type = get_mach_if(mach_if); > } > > if (!enable || drive_get_by_index(type, index)) { > @@ -833,7 +833,7 @@ static void default_drive(int enable, int snapshot, int use_scsi, > if (snapshot) { > drive_enable_snapshot(opts, NULL); > } > - if (!drive_init(opts, use_scsi)) { > + if (!drive_init(opts, mach_if)) { > exit(1); > } > } > @@ -3547,14 +3547,16 @@ int main(int argc, char **argv, char **envp) > /* open the virtual block devices */ > if (snapshot) > qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0); > - if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, &machine->use_scsi, 1) != 0) > + if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, > + &machine->mach_if, 1) != 0) { > exit(1); > + } > > - default_drive(default_cdrom, snapshot, machine->use_scsi, > + default_drive(default_cdrom, snapshot, machine->mach_if, > IF_DEFAULT, 2, CDROM_OPTS); > - default_drive(default_floppy, snapshot, machine->use_scsi, > + default_drive(default_floppy, snapshot, machine->mach_if, > IF_FLOPPY, 0, FD_OPTS); > - default_drive(default_sdcard, snapshot, machine->use_scsi, > + default_drive(default_sdcard, snapshot, machine->mach_if, > IF_SD, 0, SD_OPTS); > > register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
On Wed, Oct 24, 2012 at 03:12:36PM +0200, Markus Armbruster wrote: > Jason Baron <jbaron@redhat.com> writes: > > > From: Jason Baron <jbaron@redhat.com> > > > > 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. > > Even though q35's desire to default to IF_AHCI is driving this patch, > generalizing the default interface type makes sense on its own. Even if > we IF_AHCI should turn out to need more discussion. > > > 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. > > > > Please use 'static inline int get_mach_if(int mach_if)', when accesssing the > > new mach_if field. > > > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Jason Baron <jbaron@redhat.com> > > --- > > 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 mach_if > > Suggest to rename mach_if to something more descriptive. Kevin's > default_drive_if works for me. > 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) > > BlockInterfaceType mach_if, and return type. > > > +{ > > + 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; > > +} > > + > > I'm not sure we should map IF_NONE to IF_IDE. > > get_mach_if() should return an interface type the board supports. In > theory, we could have a board that doesn't define any controllers, but > still lets the user define some with -device (say because the board > sports a PCI bus). Then IF_NONE would be the only interface type that > makes any sense, and therefore the only sensible value of get_mach_if(). > Ok, but no boards use IF_NONE in that sense currently. But planning for the future is good :) > If we drop the magic mapping of IF_NONE, only IF_DEFAULT is left, and > that one's clearly marked "for use with drive_add() only". No real need > for magic mapping then. Could drop get_mach_if() and use mach_if > directly. Meaning explicity set mach_if or default_drive_if to IF_IDE for all machine types that are currently either IF_NONE, IF_DEFAULT, or not explicitly defined? Thanks, -Jason
Jason Baron <jbaron@redhat.com> writes: > From: Jason Baron <jbaron@redhat.com> > > 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. > > Please use 'static inline int get_mach_if(int mach_if)', when accesssing the > new mach_if field. [...] > 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; > int max_cpus; > unsigned int no_serial:1, > no_parallel:1, [...] > diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c > index fd46ba2..c70eb69 100644 > --- a/hw/xilinx_zynq.c > +++ b/hw/xilinx_zynq.c > @@ -178,7 +178,7 @@ static QEMUMachine zynq_machine = { > .name = "xilinx-zynq-a9", > .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", > .init = zynq_init, > - .use_scsi = 1, > + .if_default = IF_SCSI, I doubt this compiles, and if it does, the compiler is mean to you :) > .max_cpus = 1, > .no_sdcard = 1 > }; [...]
Jason Baron <jbaron@redhat.com> writes: > On Wed, Oct 24, 2012 at 03:12:36PM +0200, Markus Armbruster wrote: >> Jason Baron <jbaron@redhat.com> writes: >> >> > From: Jason Baron <jbaron@redhat.com> >> > >> > 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. >> >> Even though q35's desire to default to IF_AHCI is driving this patch, >> generalizing the default interface type makes sense on its own. Even if >> we IF_AHCI should turn out to need more discussion. >> >> > 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. >> > >> > Please use 'static inline int get_mach_if(int mach_if)', when accesssing the >> > new mach_if field. >> > >> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> > Signed-off-by: Jason Baron <jbaron@redhat.com> >> > --- >> > 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 mach_if >> >> Suggest to rename mach_if to something more descriptive. Kevin's >> default_drive_if works for me. >> > > 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) >> >> BlockInterfaceType mach_if, and return type. >> >> > +{ >> > + 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; >> > +} >> > + >> >> I'm not sure we should map IF_NONE to IF_IDE. >> >> get_mach_if() should return an interface type the board supports. In >> theory, we could have a board that doesn't define any controllers, but >> still lets the user define some with -device (say because the board >> sports a PCI bus). Then IF_NONE would be the only interface type that >> makes any sense, and therefore the only sensible value of get_mach_if(). >> > > Ok, but no boards use IF_NONE in that sense currently. But planning for > the future is good :) > >> If we drop the magic mapping of IF_NONE, only IF_DEFAULT is left, and >> that one's clearly marked "for use with drive_add() only". No real need >> for magic mapping then. Could drop get_mach_if() and use mach_if >> directly. > > Meaning explicity set mach_if or default_drive_if to IF_IDE for all > machine types that are currently either IF_NONE, IF_DEFAULT, or not > explicitly defined? Yes. I count 97 machine types. 24 have .use_scsi = 1. Two have explicit .use_scsi = 0, and the remaining 71 have it implicitly. I very much doubt these 73 all sport IDE controllers. Last time I checked[*], 41 machine types supported CD-ROM drives. Makes me estimate we have ~55 machine types that have neither onboard SCSI nor IDE. What happens when .use_scsi = 0 and the board doesn't provide IDE? -drive without if= defines a block backend, but no frontend. Just like if=none, except for the (misleading) default ID. Likewise, when .use_scsi = 1 and the board doesn't provide SCSI. You convert the 24 .use_scsi = 1 to .mach_if = IF_SCSI. Fair enough. But I think it should be changed to a more suitable value for machine types that don't actually provide SCSI. Suspect highbank is an example. Can be done as followup patch; your choice. You convert the two explicit .use_scsi = 0 (leon3_generic and puv3.c) to IF_DEFAULT, effectively IF_IDE. In both cases, the machine doesn't actually provide an IDE controller as far as I can tell. Again, changing it to a more suitable value would make sense. Followup patch, or drop the .use_scsi = 0 in a preparatory patch, or convert to "no initializer" to fold this case into the next one, or whatever else works for you. You leave the 71 without a .use_scsi initializer alone, which results in IF_NONE, effectively IF_IDE. Fair enough. Again, it should be changed to a more suitable value for the machine types that don't provide IDE. If you drop the magic mapping, IF_NONE means IF_NONE, which may be a more suitable value for the ones that don't provide IDE. So you'd have to add .mach_if = IF_IDE only to the minority that does provide IDE, and leave the remaining ~55 ones alone. Not so bad, isn't it? [*] http://lists.nongnu.org/archive/html/qemu-devel/2012-08/msg02993.html
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) { 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; int max_cpus; unsigned int no_serial:1, no_parallel:1, diff --git a/hw/device-hotplug.c b/hw/device-hotplug.c index eec0fe3..33302f9 100644 --- a/hw/device-hotplug.c +++ b/hw/device-hotplug.c @@ -39,7 +39,7 @@ DriveInfo *add_init_drive(const char *optstr) if (!opts) return NULL; - dinfo = drive_init(opts, current_machine->use_scsi); + dinfo = drive_init(opts, current_machine->mach_if); if (!dinfo) { qemu_opts_del(opts); return NULL; diff --git a/hw/highbank.c b/hw/highbank.c index 11aa131..35cef06 100644 --- a/hw/highbank.c +++ b/hw/highbank.c @@ -324,7 +324,7 @@ static QEMUMachine highbank_machine = { .name = "highbank", .desc = "Calxeda Highbank (ECX-1000)", .init = highbank_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; diff --git a/hw/leon3.c b/hw/leon3.c index 7a9729d..cf9dcf8 100644 --- a/hw/leon3.c +++ b/hw/leon3.c @@ -214,7 +214,7 @@ static QEMUMachine leon3_generic_machine = { .name = "leon3_generic", .desc = "Leon-3 generic", .init = leon3_generic_hw_init, - .use_scsi = 0, + .mach_if = IF_DEFAULT, }; static void leon3_machine_init(void) diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c index db927f1..1c7a725 100644 --- a/hw/mips_jazz.c +++ b/hw/mips_jazz.c @@ -325,14 +325,14 @@ static QEMUMachine mips_magnum_machine = { .name = "magnum", .desc = "MIPS Magnum", .init = mips_magnum_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static QEMUMachine mips_pica61_machine = { .name = "pica61", .desc = "Acer Pica 61", .init = mips_pica61_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static void mips_jazz_machine_init(void) diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c index b45f0ac..b8a03a6 100644 --- a/hw/pc_sysfw.c +++ b/hw/pc_sysfw.c @@ -98,7 +98,7 @@ static void pc_fw_add_pflash_drv(void) return; } - drive_init(opts, machine->use_scsi); + drive_init(opts, machine->mach_if); } static void pc_system_flash_init(MemoryRegion *rom_memory, diff --git a/hw/puv3.c b/hw/puv3.c index 43f7216..f68bb61 100644 --- a/hw/puv3.c +++ b/hw/puv3.c @@ -120,7 +120,7 @@ static QEMUMachine puv3_machine = { .desc = "PKUnity Version-3 based on UniCore32", .init = puv3_init, .is_default = 1, - .use_scsi = 0, + .mach_if = IF_DEFAULT, }; static void puv3_machine_init(void) diff --git a/hw/realview.c b/hw/realview.c index 19db4d0..7613f68 100644 --- a/hw/realview.c +++ b/hw/realview.c @@ -382,14 +382,14 @@ static QEMUMachine realview_eb_machine = { .name = "realview-eb", .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)", .init = realview_eb_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static QEMUMachine realview_eb_mpcore_machine = { .name = "realview-eb-mpcore", .desc = "ARM RealView Emulation Baseboard (ARM11MPCore)", .init = realview_eb_mpcore_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; @@ -403,7 +403,7 @@ static QEMUMachine realview_pbx_a9_machine = { .name = "realview-pbx-a9", .desc = "ARM RealView Platform Baseboard Explore for Cortex-A9", .init = realview_pbx_a9_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; diff --git a/hw/spapr.c b/hw/spapr.c index 09b8e99..be8129e 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -913,7 +913,7 @@ static QEMUMachine spapr_machine = { .reset = ppc_spapr_reset, .max_cpus = MAX_CPUS, .no_parallel = 1, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static void spapr_machine_init(void) diff --git a/hw/sun4m.c b/hw/sun4m.c index a04b485..101d552 100644 --- a/hw/sun4m.c +++ b/hw/sun4m.c @@ -1400,7 +1400,7 @@ static QEMUMachine ss5_machine = { .name = "SS-5", .desc = "Sun4m platform, SPARCstation 5", .init = ss5_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .is_default = 1, }; @@ -1408,7 +1408,7 @@ static QEMUMachine ss10_machine = { .name = "SS-10", .desc = "Sun4m platform, SPARCstation 10", .init = ss10_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; @@ -1416,7 +1416,7 @@ static QEMUMachine ss600mp_machine = { .name = "SS-600MP", .desc = "Sun4m platform, SPARCserver 600MP", .init = ss600mp_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; @@ -1424,7 +1424,7 @@ static QEMUMachine ss20_machine = { .name = "SS-20", .desc = "Sun4m platform, SPARCstation 20", .init = ss20_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; @@ -1432,35 +1432,35 @@ static QEMUMachine voyager_machine = { .name = "Voyager", .desc = "Sun4m platform, SPARCstation Voyager", .init = vger_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static QEMUMachine ss_lx_machine = { .name = "LX", .desc = "Sun4m platform, SPARCstation LX", .init = ss_lx_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static QEMUMachine ss4_machine = { .name = "SS-4", .desc = "Sun4m platform, SPARCstation 4", .init = ss4_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static QEMUMachine scls_machine = { .name = "SPARCClassic", .desc = "Sun4m platform, SPARCClassic", .init = scls_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static QEMUMachine sbook_machine = { .name = "SPARCbook", .desc = "Sun4m platform, SPARCbook", .init = sbook_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static const struct sun4d_hwdef sun4d_hwdefs[] = { @@ -1677,7 +1677,7 @@ static QEMUMachine ss1000_machine = { .name = "SS-1000", .desc = "Sun4d platform, SPARCserver 1000", .init = ss1000_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 8, }; @@ -1685,7 +1685,7 @@ static QEMUMachine ss2000_machine = { .name = "SS-2000", .desc = "Sun4d platform, SPARCcenter 2000", .init = ss2000_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 20, }; @@ -1861,7 +1861,7 @@ static QEMUMachine ss2_machine = { .name = "SS-2", .desc = "Sun4c platform, SPARCstation 2", .init = ss2_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static void sun4m_register_types(void) diff --git a/hw/versatilepb.c b/hw/versatilepb.c index 7b1b025..af5120f 100644 --- a/hw/versatilepb.c +++ b/hw/versatilepb.c @@ -374,14 +374,14 @@ static QEMUMachine versatilepb_machine = { .name = "versatilepb", .desc = "ARM Versatile/PB (ARM926EJ-S)", .init = vpb_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static QEMUMachine versatileab_machine = { .name = "versatileab", .desc = "ARM Versatile/AB (ARM926EJ-S)", .init = vab_init, - .use_scsi = 1, + .mach_if = IF_SCSI, }; static void versatile_machine_init(void) diff --git a/hw/vexpress.c b/hw/vexpress.c index 3596d1e..3c7c012 100644 --- a/hw/vexpress.c +++ b/hw/vexpress.c @@ -495,7 +495,7 @@ static QEMUMachine vexpress_a9_machine = { .name = "vexpress-a9", .desc = "ARM Versatile Express for Cortex-A9", .init = vexpress_a9_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; @@ -503,7 +503,7 @@ static QEMUMachine vexpress_a15_machine = { .name = "vexpress-a15", .desc = "ARM Versatile Express for Cortex-A15", .init = vexpress_a15_init, - .use_scsi = 1, + .mach_if = IF_SCSI, .max_cpus = 4, }; diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c index fd46ba2..c70eb69 100644 --- a/hw/xilinx_zynq.c +++ b/hw/xilinx_zynq.c @@ -178,7 +178,7 @@ static QEMUMachine zynq_machine = { .name = "xilinx-zynq-a9", .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", .init = zynq_init, - .use_scsi = 1, + .if_default = IF_SCSI, .max_cpus = 1, .no_sdcard = 1 }; diff --git a/vl.c b/vl.c index 5b357a3..6b1e546 100644 --- a/vl.c +++ b/vl.c @@ -802,9 +802,9 @@ static int parse_sandbox(QemuOpts *opts, void *opaque) static int drive_init_func(QemuOpts *opts, void *opaque) { - int *use_scsi = opaque; + int *mach_if = opaque; - return drive_init(opts, *use_scsi) == NULL; + return drive_init(opts, *mach_if) == NULL; } static int drive_enable_snapshot(QemuOpts *opts, void *opaque) @@ -815,14 +815,14 @@ static int drive_enable_snapshot(QemuOpts *opts, void *opaque) return 0; } -static void default_drive(int enable, int snapshot, int use_scsi, +static void default_drive(int enable, int snapshot, int mach_if, BlockInterfaceType type, int index, const char *optstr) { QemuOpts *opts; if (type == IF_DEFAULT) { - type = use_scsi ? IF_SCSI : IF_IDE; + type = get_mach_if(mach_if); } if (!enable || drive_get_by_index(type, index)) { @@ -833,7 +833,7 @@ static void default_drive(int enable, int snapshot, int use_scsi, if (snapshot) { drive_enable_snapshot(opts, NULL); } - if (!drive_init(opts, use_scsi)) { + if (!drive_init(opts, mach_if)) { exit(1); } } @@ -3547,14 +3547,16 @@ int main(int argc, char **argv, char **envp) /* open the virtual block devices */ if (snapshot) qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0); - if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, &machine->use_scsi, 1) != 0) + if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, + &machine->mach_if, 1) != 0) { exit(1); + } - default_drive(default_cdrom, snapshot, machine->use_scsi, + default_drive(default_cdrom, snapshot, machine->mach_if, IF_DEFAULT, 2, CDROM_OPTS); - default_drive(default_floppy, snapshot, machine->use_scsi, + default_drive(default_floppy, snapshot, machine->mach_if, IF_FLOPPY, 0, FD_OPTS); - default_drive(default_sdcard, snapshot, machine->use_scsi, + default_drive(default_sdcard, snapshot, machine->mach_if, IF_SD, 0, SD_OPTS); register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);