From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54701) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tap95-0006xd-IK for qemu-devel@nongnu.org; Tue, 20 Nov 2012 09:50:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tap93-0002LO-3u for qemu-devel@nongnu.org; Tue, 20 Nov 2012 09:49:51 -0500 Received: from cantor2.suse.de ([195.135.220.15]:54324 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tap92-0002LH-Ft for qemu-devel@nongnu.org; Tue, 20 Nov 2012 09:49:49 -0500 Message-ID: <50AB9887.40908@suse.de> Date: Tue, 20 Nov 2012 15:49:43 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1353421834-44687-1-git-send-email-borntraeger@de.ibm.com> In-Reply-To: <1353421834-44687-1-git-send-email-borntraeger@de.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv3] Support default block interfaces per QEMUMachine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger Cc: Kevin Wolf , Peter Maydell , Anthony Liguori , Igor Mitsyanko , qemu-devel , Markus Armbruster , Jens Freimann , Stefan Hajnoczi , Cornelia Huck , Andreas Faerber , Einar Lueck 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 > > Signed-off-by: Christian Borntraeger > CC: Igor Mitsyanko > CC: Markus Armbruster > CC: Kevin Wolf Reviewed-by: Alexander Graf 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);