Re: [Qemu-devel] [PATCHv3] Support default block interfaces per QEMUMachine

[Alexander Graf <agraf@suse.de>]

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);