| Message ID | 1353421834-44687-1-git-send-email-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 11/20/2012 03:30 PM, Christian Borntraeger wrote: > There are QEMUMachines that have neither IF_IDE nor IF_SCSI as a > default/standard interface to their block devices / drives. Therefore, > this patch introduces a new field default_block_type per QEMUMachine > struct. The prior use_scsi field becomes thereby obsolete and is > replaced through .default_block_type = IF_SCSI. > > This patch also changes the default for s390x to IF_VIRTIO and > removes an early hack that converts IF_IDE drives. > Other parties have already claimed interest (e.g. IF_SD for exynos) > > To create a sane default, for machines that dont specify a > default_block_type, this patch makes IF_IDE = 0 and IF_NONE = 1. > I checked all users of IF_NONE (blockdev.c and ww/device-hotplug.c) > as well as IF_IDE and it seems that it is ok to change the defines - > in other words, I found no obvious (to me) assumption in the code > regarding IF_NONE==0. IF_NONE is only set if there is an > explicit if=none. Without if=* the interface becomes IF_DEFAULT. > > I would suggest to have some additional care, e.g. by letting > this patch sit some days in the block tree. > > Based on an initial patch from Einar Lueck<elelueck@de.ibm.com> > > Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com> > CC: Igor Mitsyanko<i.mitsyanko@samsung.com> > CC: Markus Armbruster<armbru@redhat.com> > CC: Kevin Wolf<kwolf@redhat.com> Reviewed-by: Alexander Graf <agraf@suse.de> However, I would consider this 1.4 material :). Alex > --- > blockdev.c | 4 ++-- > blockdev.h | 5 +++-- > hw/boards.h | 3 ++- > hw/device-hotplug.c | 2 +- > hw/highbank.c | 2 +- > hw/leon3.c | 1 - > hw/mips_jazz.c | 4 ++-- > hw/pc_sysfw.c | 2 +- > hw/puv3.c | 1 - > hw/realview.c | 7 ++++--- > hw/s390-virtio.c | 17 ++--------------- > hw/spapr.c | 2 +- > hw/sun4m.c | 24 ++++++++++++------------ > hw/versatilepb.c | 4 ++-- > hw/vexpress.c | 4 ++-- > hw/xilinx_zynq.c | 2 +- > vl.c | 21 ++++++++++++--------- > 17 files changed, 48 insertions(+), 57 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index e73fd6e..4a4d99f 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, BlockInterfaceType block_default_type) > { > 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 = block_default_type; > } > > max_devs = if_max_devs[type]; > diff --git a/blockdev.h b/blockdev.h > index 5f27b64..1c9ca6a 100644 > --- a/blockdev.h > +++ b/blockdev.h > @@ -19,8 +19,9 @@ void blockdev_auto_del(BlockDriverState *bs); > > typedef enum { > IF_DEFAULT = -1, /* for use with drive_add() only */ > + IF_IDE, /* machines without block_default_type get 0 */ > IF_NONE, > - IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > + IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > IF_COUNT > } BlockInterfaceType; > > @@ -51,7 +52,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs); > QemuOpts *drive_def(const char *optstr); > QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, > const char *optstr); > -DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi); > +DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type); > > /* device-hotplug */ > > diff --git a/hw/boards.h b/hw/boards.h > index 813d0e5..c66fa16 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -3,6 +3,7 @@ > #ifndef HW_BOARDS_H > #define HW_BOARDS_H > > +#include "blockdev.h" > #include "qdev.h" > > typedef struct QEMUMachineInitArgs { > @@ -24,7 +25,7 @@ typedef struct QEMUMachine { > const char *desc; > QEMUMachineInitFunc *init; > QEMUMachineResetFunc *reset; > - int use_scsi; > + BlockInterfaceType block_default_type; > 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..a06848a 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->block_default_type); > if (!dinfo) { > qemu_opts_del(opts); > return NULL; > diff --git a/hw/highbank.c b/hw/highbank.c > index afbb005..e8e5bf0 100644 > --- a/hw/highbank.c > +++ b/hw/highbank.c > @@ -326,7 +326,7 @@ static QEMUMachine highbank_machine = { > .name = "highbank", > .desc = "Calxeda Highbank (ECX-1000)", > .init = highbank_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/leon3.c b/hw/leon3.c > index 7742738..ef83dff 100644 > --- a/hw/leon3.c > +++ b/hw/leon3.c > @@ -212,7 +212,6 @@ static QEMUMachine leon3_generic_machine = { > .name = "leon3_generic", > .desc = "Leon-3 generic", > .init = leon3_generic_hw_init, > - .use_scsi = 0, > }; > > static void leon3_machine_init(void) > diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c > index 0847427..ea1416a 100644 > --- a/hw/mips_jazz.c > +++ b/hw/mips_jazz.c > @@ -324,14 +324,14 @@ static QEMUMachine mips_magnum_machine = { > .name = "magnum", > .desc = "MIPS Magnum", > .init = mips_magnum_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine mips_pica61_machine = { > .name = "pica61", > .desc = "Acer Pica 61", > .init = mips_pica61_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void mips_jazz_machine_init(void) > diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c > index 9d7c5f4..8a7080e 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->block_default_type); > } > > static void pc_system_flash_init(MemoryRegion *rom_memory, > diff --git a/hw/puv3.c b/hw/puv3.c > index 764799c..3d77349 100644 > --- a/hw/puv3.c > +++ b/hw/puv3.c > @@ -122,7 +122,6 @@ static QEMUMachine puv3_machine = { > .desc = "PKUnity Version-3 based on UniCore32", > .init = puv3_init, > .is_default = 1, > - .use_scsi = 0, > }; > > static void puv3_machine_init(void) > diff --git a/hw/realview.c b/hw/realview.c > index e789c15..34448f0 100644 > --- a/hw/realview.c > +++ b/hw/realview.c > @@ -364,14 +364,14 @@ static QEMUMachine realview_eb_machine = { > .name = "realview-eb", > .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)", > .init = realview_eb_init, > - .use_scsi = 1, > + .block_default_type = 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, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -379,13 +379,14 @@ static QEMUMachine realview_pb_a8_machine = { > .name = "realview-pb-a8", > .desc = "ARM RealView Platform Baseboard for Cortex-A8", > .init = realview_pb_a8_init, > + .block_default_type = IF_SCSI, > }; > > 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, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index 685cb54..64bb5a2 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -315,21 +315,6 @@ static void s390_init(QEMUMachineInitArgs *args) > qdev_set_nic_properties(dev, nd); > qdev_init_nofail(dev); > } > - > - /* Create VirtIO disk drives */ > - for(i = 0; i< MAX_BLK_DEVS; i++) { > - DriveInfo *dinfo; > - DeviceState *dev; > - > - dinfo = drive_get(IF_IDE, 0, i); > - if (!dinfo) { > - continue; > - } > - > - dev = qdev_create((BusState *)s390_bus, "virtio-blk-s390"); > - qdev_prop_set_drive_nofail(dev, "drive", dinfo->bdrv); > - qdev_init_nofail(dev); > - } > } > > static QEMUMachine s390_machine = { > @@ -337,6 +322,7 @@ static QEMUMachine s390_machine = { > .alias = "s390", > .desc = "VirtIO based S390 machine", > .init = s390_init, > + .block_default_type = IF_VIRTIO, > .no_cdrom = 1, > .no_floppy = 1, > .no_serial = 1, > @@ -353,3 +339,4 @@ static void s390_machine_init(void) > } > > machine_init(s390_machine_init); > + > diff --git a/hw/spapr.c b/hw/spapr.c > index ad3f0ea..d955f02 100644 > --- a/hw/spapr.c > +++ b/hw/spapr.c > @@ -924,9 +924,9 @@ static QEMUMachine spapr_machine = { > .desc = "pSeries Logical Partition (PAPR compliant)", > .init = ppc_spapr_init, > .reset = ppc_spapr_reset, > + .block_default_type = IF_SCSI, > .max_cpus = MAX_CPUS, > .no_parallel = 1, > - .use_scsi = 1, > }; > > static void spapr_machine_init(void) > diff --git a/hw/sun4m.c b/hw/sun4m.c > index 1a78676..52cf82b 100644 > --- a/hw/sun4m.c > +++ b/hw/sun4m.c > @@ -1426,7 +1426,7 @@ static QEMUMachine ss5_machine = { > .name = "SS-5", > .desc = "Sun4m platform, SPARCstation 5", > .init = ss5_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .is_default = 1, > }; > > @@ -1434,7 +1434,7 @@ static QEMUMachine ss10_machine = { > .name = "SS-10", > .desc = "Sun4m platform, SPARCstation 10", > .init = ss10_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1442,7 +1442,7 @@ static QEMUMachine ss600mp_machine = { > .name = "SS-600MP", > .desc = "Sun4m platform, SPARCserver 600MP", > .init = ss600mp_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1450,7 +1450,7 @@ static QEMUMachine ss20_machine = { > .name = "SS-20", > .desc = "Sun4m platform, SPARCstation 20", > .init = ss20_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1458,35 +1458,35 @@ static QEMUMachine voyager_machine = { > .name = "Voyager", > .desc = "Sun4m platform, SPARCstation Voyager", > .init = vger_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine ss_lx_machine = { > .name = "LX", > .desc = "Sun4m platform, SPARCstation LX", > .init = ss_lx_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine ss4_machine = { > .name = "SS-4", > .desc = "Sun4m platform, SPARCstation 4", > .init = ss4_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine scls_machine = { > .name = "SPARCClassic", > .desc = "Sun4m platform, SPARCClassic", > .init = scls_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine sbook_machine = { > .name = "SPARCbook", > .desc = "Sun4m platform, SPARCbook", > .init = sbook_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static const struct sun4d_hwdef sun4d_hwdefs[] = { > @@ -1709,7 +1709,7 @@ static QEMUMachine ss1000_machine = { > .name = "SS-1000", > .desc = "Sun4d platform, SPARCserver 1000", > .init = ss1000_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 8, > }; > > @@ -1717,7 +1717,7 @@ static QEMUMachine ss2000_machine = { > .name = "SS-2000", > .desc = "Sun4d platform, SPARCcenter 2000", > .init = ss2000_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 20, > }; > > @@ -1896,7 +1896,7 @@ static QEMUMachine ss2_machine = { > .name = "SS-2", > .desc = "Sun4c platform, SPARCstation 2", > .init = ss2_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void sun4m_register_types(void) > diff --git a/hw/versatilepb.c b/hw/versatilepb.c > index 25e652b..4892c1d 100644 > --- a/hw/versatilepb.c > +++ b/hw/versatilepb.c > @@ -358,14 +358,14 @@ static QEMUMachine versatilepb_machine = { > .name = "versatilepb", > .desc = "ARM Versatile/PB (ARM926EJ-S)", > .init = vpb_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine versatileab_machine = { > .name = "versatileab", > .desc = "ARM Versatile/AB (ARM926EJ-S)", > .init = vab_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void versatile_machine_init(void) > diff --git a/hw/vexpress.c b/hw/vexpress.c > index d93f057..e89694c 100644 > --- a/hw/vexpress.c > +++ b/hw/vexpress.c > @@ -477,7 +477,7 @@ static QEMUMachine vexpress_a9_machine = { > .name = "vexpress-a9", > .desc = "ARM Versatile Express for Cortex-A9", > .init = vexpress_a9_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -485,7 +485,7 @@ static QEMUMachine vexpress_a15_machine = { > .name = "vexpress-a15", > .desc = "ARM Versatile Express for Cortex-A15", > .init = vexpress_a15_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c > index 1f12a3d..a4a71c2 100644 > --- a/hw/xilinx_zynq.c > +++ b/hw/xilinx_zynq.c > @@ -200,7 +200,7 @@ static QEMUMachine zynq_machine = { > .name = "xilinx-zynq-a9", > .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", > .init = zynq_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 1, > .no_sdcard = 1 > }; > diff --git a/vl.c b/vl.c > index c8e9c78..86f0dd8 100644 > --- a/vl.c > +++ b/vl.c > @@ -886,9 +886,9 @@ static int cleanup_add_fd(QemuOpts *opts, void *opaque) > > static int drive_init_func(QemuOpts *opts, void *opaque) > { > - int *use_scsi = opaque; > + BlockInterfaceType *block_default_type = opaque; > > - return drive_init(opts, *use_scsi) == NULL; > + return drive_init(opts, *block_default_type) == NULL; > } > > static int drive_enable_snapshot(QemuOpts *opts, void *opaque) > @@ -899,14 +899,15 @@ 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, > + BlockInterfaceType block_default_type, > BlockInterfaceType type, int index, > const char *optstr) > { > QemuOpts *opts; > > if (type == IF_DEFAULT) { > - type = use_scsi ? IF_SCSI : IF_IDE; > + type = block_default_type; > } > > if (!enable || drive_get_by_index(type, index)) { > @@ -917,7 +918,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, type)) { > exit(1); > } > } > @@ -3774,14 +3775,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->block_default_type, 1) != 0) { > exit(1); > + } > > - default_drive(default_cdrom, snapshot, machine->use_scsi, > + default_drive(default_cdrom, snapshot, machine->block_default_type, > IF_DEFAULT, 2, CDROM_OPTS); > - default_drive(default_floppy, snapshot, machine->use_scsi, > + default_drive(default_floppy, snapshot, machine->block_default_type, > IF_FLOPPY, 0, FD_OPTS); > - default_drive(default_sdcard, snapshot, machine->use_scsi, > + default_drive(default_sdcard, snapshot, machine->block_default_type, > IF_SD, 0, SD_OPTS); > > register_savevm_live(NULL, "ram", 0, 4,&savevm_ram_handlers, NULL);
On 20/11/12 15:49, Alexander Graf wrote: > On 11/20/2012 03:30 PM, Christian Borntraeger wrote: >> There are QEMUMachines that have neither IF_IDE nor IF_SCSI as a >> default/standard interface to their block devices / drives. Therefore, >> this patch introduces a new field default_block_type per QEMUMachine >> struct. The prior use_scsi field becomes thereby obsolete and is >> replaced through .default_block_type = IF_SCSI. >> >> This patch also changes the default for s390x to IF_VIRTIO and >> removes an early hack that converts IF_IDE drives. >> Other parties have already claimed interest (e.g. IF_SD for exynos) >> >> To create a sane default, for machines that dont specify a >> default_block_type, this patch makes IF_IDE = 0 and IF_NONE = 1. >> I checked all users of IF_NONE (blockdev.c and ww/device-hotplug.c) >> as well as IF_IDE and it seems that it is ok to change the defines - >> in other words, I found no obvious (to me) assumption in the code >> regarding IF_NONE==0. IF_NONE is only set if there is an >> explicit if=none. Without if=* the interface becomes IF_DEFAULT. >> >> I would suggest to have some additional care, e.g. by letting >> this patch sit some days in the block tree. >> >> Based on an initial patch from Einar Lueck<elelueck@de.ibm.com> >> >> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com> >> CC: Igor Mitsyanko<i.mitsyanko@samsung.com> >> CC: Markus Armbruster<armbru@redhat.com> >> CC: Kevin Wolf<kwolf@redhat.com> > > Reviewed-by: Alexander Graf <agraf@suse.de> > > However, I would consider this 1.4 material :). Agreed. Kevin, Stefan, any chance to queue that up in your tree for 1.4? Christian
This version of patch doesn't change anything for majority of machines, its just that later we can choose ourself which default block interface to use. So, nothing to review from Exynos point of view, but patch is OK, except realview_pb_a8 part (see bellow): Acked-by: Igor Mitsyanko <i.mitsyanko@samsung.com> On 11/20/2012 06:30 PM, Christian Borntraeger wrote: > > @@ -379,13 +379,14 @@ static QEMUMachine realview_pb_a8_machine = { > .name = "realview-pb-a8", > .desc = "ARM RealView Platform Baseboard for Cortex-A8", > .init = realview_pb_a8_init, > + .block_default_type = IF_SCSI, > }; > I think that's an unintentional change? > 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, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index 685cb54..64bb5a2 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -315,21 +315,6 @@ static void s390_init(QEMUMachineInitArgs *args) > qdev_set_nic_properties(dev, nd); > qdev_init_nofail(dev); > } > - > - /* Create VirtIO disk drives */ > - for(i = 0; i < MAX_BLK_DEVS; i++) { > - DriveInfo *dinfo; > - DeviceState *dev; > - > - dinfo = drive_get(IF_IDE, 0, i); > - if (!dinfo) { > - continue; > - } > - > - dev = qdev_create((BusState *)s390_bus, "virtio-blk-s390"); > - qdev_prop_set_drive_nofail(dev, "drive", dinfo->bdrv); > - qdev_init_nofail(dev); > - } > } > > static QEMUMachine s390_machine = { > @@ -337,6 +322,7 @@ static QEMUMachine s390_machine = { > .alias = "s390", > .desc = "VirtIO based S390 machine", > .init = s390_init, > + .block_default_type = IF_VIRTIO, > .no_cdrom = 1, > .no_floppy = 1, > .no_serial = 1, > @@ -353,3 +339,4 @@ static void s390_machine_init(void) > } > > machine_init(s390_machine_init); > + > diff --git a/hw/spapr.c b/hw/spapr.c > index ad3f0ea..d955f02 100644 > --- a/hw/spapr.c > +++ b/hw/spapr.c > @@ -924,9 +924,9 @@ static QEMUMachine spapr_machine = { > .desc = "pSeries Logical Partition (PAPR compliant)", > .init = ppc_spapr_init, > .reset = ppc_spapr_reset, > + .block_default_type = IF_SCSI, > .max_cpus = MAX_CPUS, > .no_parallel = 1, > - .use_scsi = 1, > }; > > static void spapr_machine_init(void) > diff --git a/hw/sun4m.c b/hw/sun4m.c > index 1a78676..52cf82b 100644 > --- a/hw/sun4m.c > +++ b/hw/sun4m.c > @@ -1426,7 +1426,7 @@ static QEMUMachine ss5_machine = { > .name = "SS-5", > .desc = "Sun4m platform, SPARCstation 5", > .init = ss5_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .is_default = 1, > }; > > @@ -1434,7 +1434,7 @@ static QEMUMachine ss10_machine = { > .name = "SS-10", > .desc = "Sun4m platform, SPARCstation 10", > .init = ss10_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1442,7 +1442,7 @@ static QEMUMachine ss600mp_machine = { > .name = "SS-600MP", > .desc = "Sun4m platform, SPARCserver 600MP", > .init = ss600mp_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1450,7 +1450,7 @@ static QEMUMachine ss20_machine = { > .name = "SS-20", > .desc = "Sun4m platform, SPARCstation 20", > .init = ss20_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1458,35 +1458,35 @@ static QEMUMachine voyager_machine = { > .name = "Voyager", > .desc = "Sun4m platform, SPARCstation Voyager", > .init = vger_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine ss_lx_machine = { > .name = "LX", > .desc = "Sun4m platform, SPARCstation LX", > .init = ss_lx_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine ss4_machine = { > .name = "SS-4", > .desc = "Sun4m platform, SPARCstation 4", > .init = ss4_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine scls_machine = { > .name = "SPARCClassic", > .desc = "Sun4m platform, SPARCClassic", > .init = scls_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine sbook_machine = { > .name = "SPARCbook", > .desc = "Sun4m platform, SPARCbook", > .init = sbook_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static const struct sun4d_hwdef sun4d_hwdefs[] = { > @@ -1709,7 +1709,7 @@ static QEMUMachine ss1000_machine = { > .name = "SS-1000", > .desc = "Sun4d platform, SPARCserver 1000", > .init = ss1000_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 8, > }; > > @@ -1717,7 +1717,7 @@ static QEMUMachine ss2000_machine = { > .name = "SS-2000", > .desc = "Sun4d platform, SPARCcenter 2000", > .init = ss2000_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 20, > }; > > @@ -1896,7 +1896,7 @@ static QEMUMachine ss2_machine = { > .name = "SS-2", > .desc = "Sun4c platform, SPARCstation 2", > .init = ss2_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void sun4m_register_types(void) > diff --git a/hw/versatilepb.c b/hw/versatilepb.c > index 25e652b..4892c1d 100644 > --- a/hw/versatilepb.c > +++ b/hw/versatilepb.c > @@ -358,14 +358,14 @@ static QEMUMachine versatilepb_machine = { > .name = "versatilepb", > .desc = "ARM Versatile/PB (ARM926EJ-S)", > .init = vpb_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine versatileab_machine = { > .name = "versatileab", > .desc = "ARM Versatile/AB (ARM926EJ-S)", > .init = vab_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void versatile_machine_init(void) > diff --git a/hw/vexpress.c b/hw/vexpress.c > index d93f057..e89694c 100644 > --- a/hw/vexpress.c > +++ b/hw/vexpress.c > @@ -477,7 +477,7 @@ static QEMUMachine vexpress_a9_machine = { > .name = "vexpress-a9", > .desc = "ARM Versatile Express for Cortex-A9", > .init = vexpress_a9_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -485,7 +485,7 @@ static QEMUMachine vexpress_a15_machine = { > .name = "vexpress-a15", > .desc = "ARM Versatile Express for Cortex-A15", > .init = vexpress_a15_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c > index 1f12a3d..a4a71c2 100644 > --- a/hw/xilinx_zynq.c > +++ b/hw/xilinx_zynq.c > @@ -200,7 +200,7 @@ static QEMUMachine zynq_machine = { > .name = "xilinx-zynq-a9", > .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", > .init = zynq_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 1, > .no_sdcard = 1 > }; > diff --git a/vl.c b/vl.c > index c8e9c78..86f0dd8 100644 > --- a/vl.c > +++ b/vl.c > @@ -886,9 +886,9 @@ static int cleanup_add_fd(QemuOpts *opts, void *opaque) > > static int drive_init_func(QemuOpts *opts, void *opaque) > { > - int *use_scsi = opaque; > + BlockInterfaceType *block_default_type = opaque; > > - return drive_init(opts, *use_scsi) == NULL; > + return drive_init(opts, *block_default_type) == NULL; > } > > static int drive_enable_snapshot(QemuOpts *opts, void *opaque) > @@ -899,14 +899,15 @@ 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, > + BlockInterfaceType block_default_type, > BlockInterfaceType type, int index, > const char *optstr) > { > QemuOpts *opts; > > if (type == IF_DEFAULT) { > - type = use_scsi ? IF_SCSI : IF_IDE; > + type = block_default_type; > } > > if (!enable || drive_get_by_index(type, index)) { > @@ -917,7 +918,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, type)) { > exit(1); > } > } > @@ -3774,14 +3775,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->block_default_type, 1) != 0) { > exit(1); > + } > > - default_drive(default_cdrom, snapshot, machine->use_scsi, > + default_drive(default_cdrom, snapshot, machine->block_default_type, > IF_DEFAULT, 2, CDROM_OPTS); > - default_drive(default_floppy, snapshot, machine->use_scsi, > + default_drive(default_floppy, snapshot, machine->block_default_type, > IF_FLOPPY, 0, FD_OPTS); > - default_drive(default_sdcard, snapshot, machine->use_scsi, > + default_drive(default_sdcard, snapshot, machine->block_default_type, > IF_SD, 0, SD_OPTS); > > register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
On 21/11/12 13:07, Igor Mitsyanko wrote: > This version of patch doesn't change anything for majority of machines, its just that later we can > choose ourself which default block interface to use. So, nothing to review from Exynos point of view, but patch > is OK, except realview_pb_a8 part (see bellow): > > Acked-by: Igor Mitsyanko <i.mitsyanko@samsung.com> > > > On 11/20/2012 06:30 PM, Christian Borntraeger wrote: > >> @@ -379,13 +379,14 @@ static QEMUMachine realview_pb_a8_machine = { >> .name = "realview-pb-a8", >> .desc = "ARM RealView Platform Baseboard for Cortex-A8", >> .init = realview_pb_a8_init, >> + .block_default_type = IF_SCSI, >> }; >> > > > I think that's an unintentional change? Indeed. Kevin, Stefan, in case of no other findings, can you fix that hunk yourself or do you want a new patch? Christian
Christian Borntraeger <borntraeger@de.ibm.com> writes: > There are QEMUMachines that have neither IF_IDE nor IF_SCSI as a > default/standard interface to their block devices / drives. Therefore, > this patch introduces a new field default_block_type per QEMUMachine > struct. The prior use_scsi field becomes thereby obsolete and is > replaced through .default_block_type = IF_SCSI. > > This patch also changes the default for s390x to IF_VIRTIO and > removes an early hack that converts IF_IDE drives. Recommend to do this in a separate patch. > Other parties have already claimed interest (e.g. IF_SD for exynos) > > To create a sane default, for machines that dont specify a > default_block_type, this patch makes IF_IDE = 0 and IF_NONE = 1. > I checked all users of IF_NONE (blockdev.c and ww/device-hotplug.c) Typo: hw/ > as well as IF_IDE and it seems that it is ok to change the defines - > in other words, I found no obvious (to me) assumption in the code > regarding IF_NONE==0. Checking for IF_NONE and IF_IDE is not enough; we have to check that BlockInterfaceType values are never used in a way that's broken by this change, such as C89 initializers and implicit comparisons to zero. I just did that, and couldn't find anything. Feel free to update the commit message accordingly. > IF_NONE is only set if there is an > explicit if=none. Without if=* the interface becomes IF_DEFAULT. > > I would suggest to have some additional care, e.g. by letting > this patch sit some days in the block tree. Procedural remarks like this one can go below the --- line, so they won't get committed. > Based on an initial patch from Einar Lueck <elelueck@de.ibm.com> > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > CC: Igor Mitsyanko <i.mitsyanko@samsung.com> > CC: Markus Armbruster <armbru@redhat.com> > CC: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 4 ++-- > blockdev.h | 5 +++-- > hw/boards.h | 3 ++- > hw/device-hotplug.c | 2 +- > hw/highbank.c | 2 +- > hw/leon3.c | 1 - > hw/mips_jazz.c | 4 ++-- > hw/pc_sysfw.c | 2 +- > hw/puv3.c | 1 - > hw/realview.c | 7 ++++--- > hw/s390-virtio.c | 17 ++--------------- > hw/spapr.c | 2 +- > hw/sun4m.c | 24 ++++++++++++------------ > hw/versatilepb.c | 4 ++-- > hw/vexpress.c | 4 ++-- > hw/xilinx_zynq.c | 2 +- > vl.c | 21 ++++++++++++--------- > 17 files changed, 48 insertions(+), 57 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index e73fd6e..4a4d99f 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, BlockInterfaceType block_default_type) Call it default_block_if_type? > { > 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 = block_default_type; > } > > max_devs = if_max_devs[type]; > diff --git a/blockdev.h b/blockdev.h > index 5f27b64..1c9ca6a 100644 > --- a/blockdev.h > +++ b/blockdev.h > @@ -19,8 +19,9 @@ void blockdev_auto_del(BlockDriverState *bs); > > typedef enum { > IF_DEFAULT = -1, /* for use with drive_add() only */ > + IF_IDE, /* machines without block_default_type get 0 */ Comment could be clearer. Perhaps: /* * IF_IDE must be zero, because we want QEMUMachine member * block_default_type to default-initialize to IF_IDE */ IF_IDE = 0, > IF_NONE, > - IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > + IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > IF_COUNT > } BlockInterfaceType; > > @@ -51,7 +52,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs); > QemuOpts *drive_def(const char *optstr); > QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, > const char *optstr); > -DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi); > +DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type); > > /* device-hotplug */ > > diff --git a/hw/boards.h b/hw/boards.h > index 813d0e5..c66fa16 100644 > --- a/hw/boards.h > +++ b/hw/boards.h > @@ -3,6 +3,7 @@ > #ifndef HW_BOARDS_H > #define HW_BOARDS_H > > +#include "blockdev.h" > #include "qdev.h" > > typedef struct QEMUMachineInitArgs { > @@ -24,7 +25,7 @@ typedef struct QEMUMachine { > const char *desc; > QEMUMachineInitFunc *init; > QEMUMachineResetFunc *reset; > - int use_scsi; > + BlockInterfaceType block_default_type; Call it default_block_if_type? > 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..a06848a 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->block_default_type); > if (!dinfo) { > qemu_opts_del(opts); > return NULL; > diff --git a/hw/highbank.c b/hw/highbank.c > index afbb005..e8e5bf0 100644 > --- a/hw/highbank.c > +++ b/hw/highbank.c > @@ -326,7 +326,7 @@ static QEMUMachine highbank_machine = { > .name = "highbank", > .desc = "Calxeda Highbank (ECX-1000)", > .init = highbank_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/leon3.c b/hw/leon3.c > index 7742738..ef83dff 100644 > --- a/hw/leon3.c > +++ b/hw/leon3.c > @@ -212,7 +212,6 @@ static QEMUMachine leon3_generic_machine = { > .name = "leon3_generic", > .desc = "Leon-3 generic", > .init = leon3_generic_hw_init, > - .use_scsi = 0, > }; > > static void leon3_machine_init(void) > diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c > index 0847427..ea1416a 100644 > --- a/hw/mips_jazz.c > +++ b/hw/mips_jazz.c > @@ -324,14 +324,14 @@ static QEMUMachine mips_magnum_machine = { > .name = "magnum", > .desc = "MIPS Magnum", > .init = mips_magnum_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine mips_pica61_machine = { > .name = "pica61", > .desc = "Acer Pica 61", > .init = mips_pica61_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void mips_jazz_machine_init(void) > diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c > index 9d7c5f4..8a7080e 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->block_default_type); > } > > static void pc_system_flash_init(MemoryRegion *rom_memory, > diff --git a/hw/puv3.c b/hw/puv3.c > index 764799c..3d77349 100644 > --- a/hw/puv3.c > +++ b/hw/puv3.c > @@ -122,7 +122,6 @@ static QEMUMachine puv3_machine = { > .desc = "PKUnity Version-3 based on UniCore32", > .init = puv3_init, > .is_default = 1, > - .use_scsi = 0, > }; > > static void puv3_machine_init(void) > diff --git a/hw/realview.c b/hw/realview.c > index e789c15..34448f0 100644 > --- a/hw/realview.c > +++ b/hw/realview.c > @@ -364,14 +364,14 @@ static QEMUMachine realview_eb_machine = { > .name = "realview-eb", > .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)", > .init = realview_eb_init, > - .use_scsi = 1, > + .block_default_type = 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, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -379,13 +379,14 @@ static QEMUMachine realview_pb_a8_machine = { > .name = "realview-pb-a8", > .desc = "ARM RealView Platform Baseboard for Cortex-A8", > .init = realview_pb_a8_init, > + .block_default_type = IF_SCSI, > }; > Igor already spotted this one. > 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, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c > index 685cb54..64bb5a2 100644 > --- a/hw/s390-virtio.c > +++ b/hw/s390-virtio.c > @@ -315,21 +315,6 @@ static void s390_init(QEMUMachineInitArgs *args) > qdev_set_nic_properties(dev, nd); > qdev_init_nofail(dev); > } > - > - /* Create VirtIO disk drives */ > - for(i = 0; i < MAX_BLK_DEVS; i++) { > - DriveInfo *dinfo; > - DeviceState *dev; > - > - dinfo = drive_get(IF_IDE, 0, i); > - if (!dinfo) { > - continue; > - } > - > - dev = qdev_create((BusState *)s390_bus, "virtio-blk-s390"); > - qdev_prop_set_drive_nofail(dev, "drive", dinfo->bdrv); > - qdev_init_nofail(dev); > - } > } > Recommend to do this in a separate patch... > static QEMUMachine s390_machine = { > @@ -337,6 +322,7 @@ static QEMUMachine s390_machine = { > .alias = "s390", > .desc = "VirtIO based S390 machine", > .init = s390_init, > + .block_default_type = IF_VIRTIO, > .no_cdrom = 1, > .no_floppy = 1, > .no_serial = 1, ... along with this hunk. > @@ -353,3 +339,4 @@ static void s390_machine_init(void) > } > > machine_init(s390_machine_init); > + > diff --git a/hw/spapr.c b/hw/spapr.c > index ad3f0ea..d955f02 100644 > --- a/hw/spapr.c > +++ b/hw/spapr.c > @@ -924,9 +924,9 @@ static QEMUMachine spapr_machine = { > .desc = "pSeries Logical Partition (PAPR compliant)", > .init = ppc_spapr_init, > .reset = ppc_spapr_reset, > + .block_default_type = IF_SCSI, > .max_cpus = MAX_CPUS, > .no_parallel = 1, > - .use_scsi = 1, > }; > > static void spapr_machine_init(void) > diff --git a/hw/sun4m.c b/hw/sun4m.c > index 1a78676..52cf82b 100644 > --- a/hw/sun4m.c > +++ b/hw/sun4m.c > @@ -1426,7 +1426,7 @@ static QEMUMachine ss5_machine = { > .name = "SS-5", > .desc = "Sun4m platform, SPARCstation 5", > .init = ss5_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .is_default = 1, > }; > > @@ -1434,7 +1434,7 @@ static QEMUMachine ss10_machine = { > .name = "SS-10", > .desc = "Sun4m platform, SPARCstation 10", > .init = ss10_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1442,7 +1442,7 @@ static QEMUMachine ss600mp_machine = { > .name = "SS-600MP", > .desc = "Sun4m platform, SPARCserver 600MP", > .init = ss600mp_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1450,7 +1450,7 @@ static QEMUMachine ss20_machine = { > .name = "SS-20", > .desc = "Sun4m platform, SPARCstation 20", > .init = ss20_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -1458,35 +1458,35 @@ static QEMUMachine voyager_machine = { > .name = "Voyager", > .desc = "Sun4m platform, SPARCstation Voyager", > .init = vger_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine ss_lx_machine = { > .name = "LX", > .desc = "Sun4m platform, SPARCstation LX", > .init = ss_lx_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine ss4_machine = { > .name = "SS-4", > .desc = "Sun4m platform, SPARCstation 4", > .init = ss4_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine scls_machine = { > .name = "SPARCClassic", > .desc = "Sun4m platform, SPARCClassic", > .init = scls_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine sbook_machine = { > .name = "SPARCbook", > .desc = "Sun4m platform, SPARCbook", > .init = sbook_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static const struct sun4d_hwdef sun4d_hwdefs[] = { > @@ -1709,7 +1709,7 @@ static QEMUMachine ss1000_machine = { > .name = "SS-1000", > .desc = "Sun4d platform, SPARCserver 1000", > .init = ss1000_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 8, > }; > > @@ -1717,7 +1717,7 @@ static QEMUMachine ss2000_machine = { > .name = "SS-2000", > .desc = "Sun4d platform, SPARCcenter 2000", > .init = ss2000_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 20, > }; > > @@ -1896,7 +1896,7 @@ static QEMUMachine ss2_machine = { > .name = "SS-2", > .desc = "Sun4c platform, SPARCstation 2", > .init = ss2_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void sun4m_register_types(void) > diff --git a/hw/versatilepb.c b/hw/versatilepb.c > index 25e652b..4892c1d 100644 > --- a/hw/versatilepb.c > +++ b/hw/versatilepb.c > @@ -358,14 +358,14 @@ static QEMUMachine versatilepb_machine = { > .name = "versatilepb", > .desc = "ARM Versatile/PB (ARM926EJ-S)", > .init = vpb_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static QEMUMachine versatileab_machine = { > .name = "versatileab", > .desc = "ARM Versatile/AB (ARM926EJ-S)", > .init = vab_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > }; > > static void versatile_machine_init(void) > diff --git a/hw/vexpress.c b/hw/vexpress.c > index d93f057..e89694c 100644 > --- a/hw/vexpress.c > +++ b/hw/vexpress.c > @@ -477,7 +477,7 @@ static QEMUMachine vexpress_a9_machine = { > .name = "vexpress-a9", > .desc = "ARM Versatile Express for Cortex-A9", > .init = vexpress_a9_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > @@ -485,7 +485,7 @@ static QEMUMachine vexpress_a15_machine = { > .name = "vexpress-a15", > .desc = "ARM Versatile Express for Cortex-A15", > .init = vexpress_a15_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 4, > }; > > diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c > index 1f12a3d..a4a71c2 100644 > --- a/hw/xilinx_zynq.c > +++ b/hw/xilinx_zynq.c > @@ -200,7 +200,7 @@ static QEMUMachine zynq_machine = { > .name = "xilinx-zynq-a9", > .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", > .init = zynq_init, > - .use_scsi = 1, > + .block_default_type = IF_SCSI, > .max_cpus = 1, > .no_sdcard = 1 > }; > diff --git a/vl.c b/vl.c > index c8e9c78..86f0dd8 100644 > --- a/vl.c > +++ b/vl.c > @@ -886,9 +886,9 @@ static int cleanup_add_fd(QemuOpts *opts, void *opaque) > > static int drive_init_func(QemuOpts *opts, void *opaque) > { > - int *use_scsi = opaque; > + BlockInterfaceType *block_default_type = opaque; > > - return drive_init(opts, *use_scsi) == NULL; > + return drive_init(opts, *block_default_type) == NULL; > } > > static int drive_enable_snapshot(QemuOpts *opts, void *opaque) > @@ -899,14 +899,15 @@ 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, > + BlockInterfaceType block_default_type, > BlockInterfaceType type, int index, > const char *optstr) > { > QemuOpts *opts; > > if (type == IF_DEFAULT) { > - type = use_scsi ? IF_SCSI : IF_IDE; > + type = block_default_type; > } > > if (!enable || drive_get_by_index(type, index)) { There's just one caller that passes IF_DEFAULT. We could change it to pass machine->block_default_type instead, and drop parameter block_default_type. Follow-up patch. > @@ -917,7 +918,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, type)) { > exit(1); > } > } > @@ -3774,14 +3775,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->block_default_type, 1) != 0) { > exit(1); > + } > > - default_drive(default_cdrom, snapshot, machine->use_scsi, > + default_drive(default_cdrom, snapshot, machine->block_default_type, > IF_DEFAULT, 2, CDROM_OPTS); > - default_drive(default_floppy, snapshot, machine->use_scsi, > + default_drive(default_floppy, snapshot, machine->block_default_type, > IF_FLOPPY, 0, FD_OPTS); > - default_drive(default_sdcard, snapshot, machine->use_scsi, > + default_drive(default_sdcard, snapshot, machine->block_default_type, > IF_SD, 0, SD_OPTS); > > register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
On Tue, Nov 20, 2012 at 03:30:34PM +0100, Christian Borntraeger wrote: > There are QEMUMachines that have neither IF_IDE nor IF_SCSI as a > default/standard interface to their block devices / drives. Therefore, > this patch introduces a new field default_block_type per QEMUMachine > struct. The prior use_scsi field becomes thereby obsolete and is > replaced through .default_block_type = IF_SCSI. > > This patch also changes the default for s390x to IF_VIRTIO and > removes an early hack that converts IF_IDE drives. > Other parties have already claimed interest (e.g. IF_SD for exynos) > > To create a sane default, for machines that dont specify a > default_block_type, this patch makes IF_IDE = 0 and IF_NONE = 1. > I checked all users of IF_NONE (blockdev.c and ww/device-hotplug.c) > as well as IF_IDE and it seems that it is ok to change the defines - > in other words, I found no obvious (to me) assumption in the code > regarding IF_NONE==0. IF_NONE is only set if there is an > explicit if=none. Without if=* the interface becomes IF_DEFAULT. > > I would suggest to have some additional care, e.g. by letting > this patch sit some days in the block tree. > > Based on an initial patch from Einar Lueck <elelueck@de.ibm.com> > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > CC: Igor Mitsyanko <i.mitsyanko@samsung.com> > CC: Markus Armbruster <armbru@redhat.com> > CC: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 4 ++-- > blockdev.h | 5 +++-- > hw/boards.h | 3 ++- > hw/device-hotplug.c | 2 +- > hw/highbank.c | 2 +- > hw/leon3.c | 1 - > hw/mips_jazz.c | 4 ++-- > hw/pc_sysfw.c | 2 +- > hw/puv3.c | 1 - > hw/realview.c | 7 ++++--- > hw/s390-virtio.c | 17 ++--------------- > hw/spapr.c | 2 +- > hw/sun4m.c | 24 ++++++++++++------------ > hw/versatilepb.c | 4 ++-- > hw/vexpress.c | 4 ++-- > hw/xilinx_zynq.c | 2 +- > vl.c | 21 ++++++++++++--------- > 17 files changed, 48 insertions(+), 57 deletions(-) Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next This will be part of QEMU 1.4. Stefan
On 22/11/12 13:02, Markus Armbruster wrote: Thanks for the review, Stefan already applied the patch, though. Is there anything that you really want to have a followup patch besides this one? > There's just one caller that passes IF_DEFAULT. We could change it to > pass machine->block_default_type instead, and drop parameter > block_default_type. Follow-up patch. I will have a look on that. Christian
Markus Armbruster <armbru@redhat.com> writes: > Christian Borntraeger <borntraeger@de.ibm.com> writes: > >> There are QEMUMachines that have neither IF_IDE nor IF_SCSI as a >> default/standard interface to their block devices / drives. Therefore, >> this patch introduces a new field default_block_type per QEMUMachine >> struct. The prior use_scsi field becomes thereby obsolete and is >> replaced through .default_block_type = IF_SCSI. >> >> This patch also changes the default for s390x to IF_VIRTIO and >> removes an early hack that converts IF_IDE drives. > > Recommend to do this in a separate patch. > >> Other parties have already claimed interest (e.g. IF_SD for exynos) >> >> To create a sane default, for machines that dont specify a >> default_block_type, this patch makes IF_IDE = 0 and IF_NONE = 1. >> I checked all users of IF_NONE (blockdev.c and ww/device-hotplug.c) > > Typo: hw/ > >> as well as IF_IDE and it seems that it is ok to change the defines - >> in other words, I found no obvious (to me) assumption in the code >> regarding IF_NONE==0. > > Checking for IF_NONE and IF_IDE is not enough; we have to check that > BlockInterfaceType values are never used in a way that's broken by this > change, such as C89 initializers and implicit comparisons to zero. > > I just did that, and couldn't find anything. Almost missed this one: it changes this error message of monitor command drive_add: (qemu) drive_add 0 if=ide Can't hot-add drive to type 1 to Can't hot-add drive to type 0 I don't think we care. Yes, the error message sucks. [...]
Christian Borntraeger <borntraeger@de.ibm.com> writes: > On 22/11/12 13:02, Markus Armbruster wrote: > > Thanks for the review, Stefan already applied the patch, though. > Is there anything that you really want to have a followup patch > besides this one? "Really want" is too strong, but improving the comment on the definition of IF_IDE would be nice. >> There's just one caller that passes IF_DEFAULT. We could change it to >> pass machine->block_default_type instead, and drop parameter >> block_default_type. Follow-up patch. > > I will have a look on that. Thanks!
On Thu, Nov 22, 2012 at 2:22 PM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 22/11/12 13:02, Markus Armbruster wrote: > > Thanks for the review, Stefan already applied the patch, though. > Is there anything that you really want to have a followup patch > besides this one? Follow-up patches are probably the best step to handle any additional comments. However, if you prefer we can replace the patch in block-next since that tree won't see any movement until the 1.4 window opens up. Stefan
here are two followup patches. Christian Borntraeger (2): block: simply default_drive block: clarify comment about IF_IDE = 0 blockdev.h | 6 +++++- vl.c | 20 ++++++-------------- 2 files changed, 11 insertions(+), 15 deletions(-)
On Thu, Nov 22, 2012 at 9:02 PM, Christian Borntraeger <borntraeger@de.ibm.com> wrote: > here are two followup patches. > > > Christian Borntraeger (2): > block: simply default_drive > block: clarify comment about IF_IDE = 0 > > blockdev.h | 6 +++++- > vl.c | 20 ++++++-------------- > 2 files changed, 11 insertions(+), 15 deletions(-) Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Looks good but I'd like to give Markus a chance to review this. Stefan
Christian Borntraeger <borntraeger@de.ibm.com> writes: > here are two followup patches. Thanks! Stefan, PATCH 1/2 could be squashed into "Support default block interfaces per QEMUMachine". Your choice. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On Thu, Nov 22, 2012 at 09:02:54PM +0100, Christian Borntraeger wrote: > here are two followup patches. > > > Christian Borntraeger (2): > block: simply default_drive > block: clarify comment about IF_IDE = 0 > > blockdev.h | 6 +++++- > vl.c | 20 ++++++-------------- > 2 files changed, 11 insertions(+), 15 deletions(-) > > -- > 1.7.9.5 > > Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan
diff --git a/blockdev.c b/blockdev.c index e73fd6e..4a4d99f 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, BlockInterfaceType block_default_type) { 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 = block_default_type; } max_devs = if_max_devs[type]; diff --git a/blockdev.h b/blockdev.h index 5f27b64..1c9ca6a 100644 --- a/blockdev.h +++ b/blockdev.h @@ -19,8 +19,9 @@ void blockdev_auto_del(BlockDriverState *bs); typedef enum { IF_DEFAULT = -1, /* for use with drive_add() only */ + IF_IDE, /* machines without block_default_type get 0 */ IF_NONE, - IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, + IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, IF_COUNT } BlockInterfaceType; @@ -51,7 +52,7 @@ DriveInfo *drive_get_by_blockdev(BlockDriverState *bs); QemuOpts *drive_def(const char *optstr); QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, const char *optstr); -DriveInfo *drive_init(QemuOpts *arg, int default_to_scsi); +DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type); /* device-hotplug */ diff --git a/hw/boards.h b/hw/boards.h index 813d0e5..c66fa16 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -3,6 +3,7 @@ #ifndef HW_BOARDS_H #define HW_BOARDS_H +#include "blockdev.h" #include "qdev.h" typedef struct QEMUMachineInitArgs { @@ -24,7 +25,7 @@ typedef struct QEMUMachine { const char *desc; QEMUMachineInitFunc *init; QEMUMachineResetFunc *reset; - int use_scsi; + BlockInterfaceType block_default_type; 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..a06848a 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->block_default_type); if (!dinfo) { qemu_opts_del(opts); return NULL; diff --git a/hw/highbank.c b/hw/highbank.c index afbb005..e8e5bf0 100644 --- a/hw/highbank.c +++ b/hw/highbank.c @@ -326,7 +326,7 @@ static QEMUMachine highbank_machine = { .name = "highbank", .desc = "Calxeda Highbank (ECX-1000)", .init = highbank_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 4, }; diff --git a/hw/leon3.c b/hw/leon3.c index 7742738..ef83dff 100644 --- a/hw/leon3.c +++ b/hw/leon3.c @@ -212,7 +212,6 @@ static QEMUMachine leon3_generic_machine = { .name = "leon3_generic", .desc = "Leon-3 generic", .init = leon3_generic_hw_init, - .use_scsi = 0, }; static void leon3_machine_init(void) diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c index 0847427..ea1416a 100644 --- a/hw/mips_jazz.c +++ b/hw/mips_jazz.c @@ -324,14 +324,14 @@ static QEMUMachine mips_magnum_machine = { .name = "magnum", .desc = "MIPS Magnum", .init = mips_magnum_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static QEMUMachine mips_pica61_machine = { .name = "pica61", .desc = "Acer Pica 61", .init = mips_pica61_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static void mips_jazz_machine_init(void) diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c index 9d7c5f4..8a7080e 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->block_default_type); } static void pc_system_flash_init(MemoryRegion *rom_memory, diff --git a/hw/puv3.c b/hw/puv3.c index 764799c..3d77349 100644 --- a/hw/puv3.c +++ b/hw/puv3.c @@ -122,7 +122,6 @@ static QEMUMachine puv3_machine = { .desc = "PKUnity Version-3 based on UniCore32", .init = puv3_init, .is_default = 1, - .use_scsi = 0, }; static void puv3_machine_init(void) diff --git a/hw/realview.c b/hw/realview.c index e789c15..34448f0 100644 --- a/hw/realview.c +++ b/hw/realview.c @@ -364,14 +364,14 @@ static QEMUMachine realview_eb_machine = { .name = "realview-eb", .desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)", .init = realview_eb_init, - .use_scsi = 1, + .block_default_type = 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, + .block_default_type = IF_SCSI, .max_cpus = 4, }; @@ -379,13 +379,14 @@ static QEMUMachine realview_pb_a8_machine = { .name = "realview-pb-a8", .desc = "ARM RealView Platform Baseboard for Cortex-A8", .init = realview_pb_a8_init, + .block_default_type = IF_SCSI, }; 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, + .block_default_type = IF_SCSI, .max_cpus = 4, }; diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c index 685cb54..64bb5a2 100644 --- a/hw/s390-virtio.c +++ b/hw/s390-virtio.c @@ -315,21 +315,6 @@ static void s390_init(QEMUMachineInitArgs *args) qdev_set_nic_properties(dev, nd); qdev_init_nofail(dev); } - - /* Create VirtIO disk drives */ - for(i = 0; i < MAX_BLK_DEVS; i++) { - DriveInfo *dinfo; - DeviceState *dev; - - dinfo = drive_get(IF_IDE, 0, i); - if (!dinfo) { - continue; - } - - dev = qdev_create((BusState *)s390_bus, "virtio-blk-s390"); - qdev_prop_set_drive_nofail(dev, "drive", dinfo->bdrv); - qdev_init_nofail(dev); - } } static QEMUMachine s390_machine = { @@ -337,6 +322,7 @@ static QEMUMachine s390_machine = { .alias = "s390", .desc = "VirtIO based S390 machine", .init = s390_init, + .block_default_type = IF_VIRTIO, .no_cdrom = 1, .no_floppy = 1, .no_serial = 1, @@ -353,3 +339,4 @@ static void s390_machine_init(void) } machine_init(s390_machine_init); + diff --git a/hw/spapr.c b/hw/spapr.c index ad3f0ea..d955f02 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -924,9 +924,9 @@ static QEMUMachine spapr_machine = { .desc = "pSeries Logical Partition (PAPR compliant)", .init = ppc_spapr_init, .reset = ppc_spapr_reset, + .block_default_type = IF_SCSI, .max_cpus = MAX_CPUS, .no_parallel = 1, - .use_scsi = 1, }; static void spapr_machine_init(void) diff --git a/hw/sun4m.c b/hw/sun4m.c index 1a78676..52cf82b 100644 --- a/hw/sun4m.c +++ b/hw/sun4m.c @@ -1426,7 +1426,7 @@ static QEMUMachine ss5_machine = { .name = "SS-5", .desc = "Sun4m platform, SPARCstation 5", .init = ss5_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .is_default = 1, }; @@ -1434,7 +1434,7 @@ static QEMUMachine ss10_machine = { .name = "SS-10", .desc = "Sun4m platform, SPARCstation 10", .init = ss10_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 4, }; @@ -1442,7 +1442,7 @@ static QEMUMachine ss600mp_machine = { .name = "SS-600MP", .desc = "Sun4m platform, SPARCserver 600MP", .init = ss600mp_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 4, }; @@ -1450,7 +1450,7 @@ static QEMUMachine ss20_machine = { .name = "SS-20", .desc = "Sun4m platform, SPARCstation 20", .init = ss20_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 4, }; @@ -1458,35 +1458,35 @@ static QEMUMachine voyager_machine = { .name = "Voyager", .desc = "Sun4m platform, SPARCstation Voyager", .init = vger_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static QEMUMachine ss_lx_machine = { .name = "LX", .desc = "Sun4m platform, SPARCstation LX", .init = ss_lx_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static QEMUMachine ss4_machine = { .name = "SS-4", .desc = "Sun4m platform, SPARCstation 4", .init = ss4_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static QEMUMachine scls_machine = { .name = "SPARCClassic", .desc = "Sun4m platform, SPARCClassic", .init = scls_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static QEMUMachine sbook_machine = { .name = "SPARCbook", .desc = "Sun4m platform, SPARCbook", .init = sbook_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static const struct sun4d_hwdef sun4d_hwdefs[] = { @@ -1709,7 +1709,7 @@ static QEMUMachine ss1000_machine = { .name = "SS-1000", .desc = "Sun4d platform, SPARCserver 1000", .init = ss1000_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 8, }; @@ -1717,7 +1717,7 @@ static QEMUMachine ss2000_machine = { .name = "SS-2000", .desc = "Sun4d platform, SPARCcenter 2000", .init = ss2000_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 20, }; @@ -1896,7 +1896,7 @@ static QEMUMachine ss2_machine = { .name = "SS-2", .desc = "Sun4c platform, SPARCstation 2", .init = ss2_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static void sun4m_register_types(void) diff --git a/hw/versatilepb.c b/hw/versatilepb.c index 25e652b..4892c1d 100644 --- a/hw/versatilepb.c +++ b/hw/versatilepb.c @@ -358,14 +358,14 @@ static QEMUMachine versatilepb_machine = { .name = "versatilepb", .desc = "ARM Versatile/PB (ARM926EJ-S)", .init = vpb_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static QEMUMachine versatileab_machine = { .name = "versatileab", .desc = "ARM Versatile/AB (ARM926EJ-S)", .init = vab_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, }; static void versatile_machine_init(void) diff --git a/hw/vexpress.c b/hw/vexpress.c index d93f057..e89694c 100644 --- a/hw/vexpress.c +++ b/hw/vexpress.c @@ -477,7 +477,7 @@ static QEMUMachine vexpress_a9_machine = { .name = "vexpress-a9", .desc = "ARM Versatile Express for Cortex-A9", .init = vexpress_a9_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 4, }; @@ -485,7 +485,7 @@ static QEMUMachine vexpress_a15_machine = { .name = "vexpress-a15", .desc = "ARM Versatile Express for Cortex-A15", .init = vexpress_a15_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 4, }; diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c index 1f12a3d..a4a71c2 100644 --- a/hw/xilinx_zynq.c +++ b/hw/xilinx_zynq.c @@ -200,7 +200,7 @@ static QEMUMachine zynq_machine = { .name = "xilinx-zynq-a9", .desc = "Xilinx Zynq Platform Baseboard for Cortex-A9", .init = zynq_init, - .use_scsi = 1, + .block_default_type = IF_SCSI, .max_cpus = 1, .no_sdcard = 1 }; diff --git a/vl.c b/vl.c index c8e9c78..86f0dd8 100644 --- a/vl.c +++ b/vl.c @@ -886,9 +886,9 @@ static int cleanup_add_fd(QemuOpts *opts, void *opaque) static int drive_init_func(QemuOpts *opts, void *opaque) { - int *use_scsi = opaque; + BlockInterfaceType *block_default_type = opaque; - return drive_init(opts, *use_scsi) == NULL; + return drive_init(opts, *block_default_type) == NULL; } static int drive_enable_snapshot(QemuOpts *opts, void *opaque) @@ -899,14 +899,15 @@ 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, + BlockInterfaceType block_default_type, BlockInterfaceType type, int index, const char *optstr) { QemuOpts *opts; if (type == IF_DEFAULT) { - type = use_scsi ? IF_SCSI : IF_IDE; + type = block_default_type; } if (!enable || drive_get_by_index(type, index)) { @@ -917,7 +918,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, type)) { exit(1); } } @@ -3774,14 +3775,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->block_default_type, 1) != 0) { exit(1); + } - default_drive(default_cdrom, snapshot, machine->use_scsi, + default_drive(default_cdrom, snapshot, machine->block_default_type, IF_DEFAULT, 2, CDROM_OPTS); - default_drive(default_floppy, snapshot, machine->use_scsi, + default_drive(default_floppy, snapshot, machine->block_default_type, IF_FLOPPY, 0, FD_OPTS); - default_drive(default_sdcard, snapshot, machine->use_scsi, + default_drive(default_sdcard, snapshot, machine->block_default_type, IF_SD, 0, SD_OPTS); register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
There are QEMUMachines that have neither IF_IDE nor IF_SCSI as a default/standard interface to their block devices / drives. Therefore, this patch introduces a new field default_block_type per QEMUMachine struct. The prior use_scsi field becomes thereby obsolete and is replaced through .default_block_type = IF_SCSI. This patch also changes the default for s390x to IF_VIRTIO and removes an early hack that converts IF_IDE drives. Other parties have already claimed interest (e.g. IF_SD for exynos) To create a sane default, for machines that dont specify a default_block_type, this patch makes IF_IDE = 0 and IF_NONE = 1. I checked all users of IF_NONE (blockdev.c and ww/device-hotplug.c) as well as IF_IDE and it seems that it is ok to change the defines - in other words, I found no obvious (to me) assumption in the code regarding IF_NONE==0. IF_NONE is only set if there is an explicit if=none. Without if=* the interface becomes IF_DEFAULT. I would suggest to have some additional care, e.g. by letting this patch sit some days in the block tree. Based on an initial patch from Einar Lueck <elelueck@de.ibm.com> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> CC: Igor Mitsyanko <i.mitsyanko@samsung.com> CC: Markus Armbruster <armbru@redhat.com> CC: Kevin Wolf <kwolf@redhat.com> --- blockdev.c | 4 ++-- blockdev.h | 5 +++-- hw/boards.h | 3 ++- hw/device-hotplug.c | 2 +- hw/highbank.c | 2 +- hw/leon3.c | 1 - hw/mips_jazz.c | 4 ++-- hw/pc_sysfw.c | 2 +- hw/puv3.c | 1 - hw/realview.c | 7 ++++--- hw/s390-virtio.c | 17 ++--------------- hw/spapr.c | 2 +- hw/sun4m.c | 24 ++++++++++++------------ hw/versatilepb.c | 4 ++-- hw/vexpress.c | 4 ++-- hw/xilinx_zynq.c | 2 +- vl.c | 21 ++++++++++++--------- 17 files changed, 48 insertions(+), 57 deletions(-)