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

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

[Christian Borntraeger]

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

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

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

[Alexander Graf]

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

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

[Christian Borntraeger]

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

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

[Igor Mitsyanko]

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

-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com

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

[Christian Borntraeger]

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

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

[Markus Armbruster]

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

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

[Stefan Hajnoczi]

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

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

[Christian Borntraeger]

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

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

[Markus Armbruster]

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.

[...]

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

[Markus Armbruster]

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!

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

[Stefan Hajnoczi]

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

[Qemu-devel] [PATCH 0/2] Followup patches regarding block default interface

[Christian Borntraeger]

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

[Qemu-devel] [PATCH 1/2] block: simply default_drive

[Christian Borntraeger]

Markus Armbruster pointed out that there is only one caller
to default_drive with IF_DEFAULT as a type. Lets get rid
of the block_default_type parameter and adopt the caller
to do the right thing (asking the machine struct).

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 vl.c |   20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/vl.c b/vl.c
index 86f0dd8..c3ca0f0 100644
--- a/vl.c
+++ b/vl.c
@@ -899,17 +899,11 @@ static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
     return 0;
 }
 
-static void default_drive(int enable, int snapshot,
-                          BlockInterfaceType block_default_type,
-                          BlockInterfaceType type, int index,
-                          const char *optstr)
+static void default_drive(int enable, int snapshot, BlockInterfaceType type,
+                          int index, const char *optstr)
 {
     QemuOpts *opts;
 
-    if (type == IF_DEFAULT) {
-        type = block_default_type;
-    }
-
     if (!enable || drive_get_by_index(type, index)) {
         return;
     }
@@ -3780,12 +3774,10 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    default_drive(default_cdrom, snapshot, machine->block_default_type,
-                  IF_DEFAULT, 2, CDROM_OPTS);
-    default_drive(default_floppy, snapshot, machine->block_default_type,
-                  IF_FLOPPY, 0, FD_OPTS);
-    default_drive(default_sdcard, snapshot, machine->block_default_type,
-                  IF_SD, 0, SD_OPTS);
+    default_drive(default_cdrom, snapshot, machine->block_default_type, 2,
+                  CDROM_OPTS);
+    default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
+    default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
 
     register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
 
-- 
1.7.9.5

[Qemu-devel] [PATCH 2/2] block: clarify comment about IF_IDE = 0

[Christian Borntraeger]

Lets make the comment regarding IF_IDE more precise
why it must be 0.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 blockdev.h |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/blockdev.h b/blockdev.h
index 1c9ca6a..d73d552 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -19,7 +19,11 @@ 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_IDE must be zero, because we want QEMUMachine member
+     * block_default_type to default-initialize to IF_IDE
+     */
+    IF_IDE = 0,
     IF_NONE,
     IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
     IF_COUNT
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 0/2] Followup patches regarding block default interface

[Stefan Hajnoczi]

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

Re: [Qemu-devel] [PATCH 0/2] Followup patches regarding block default interface

[Markus Armbruster]

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>

Re: [Qemu-devel] [PATCH 0/2] Followup patches regarding block default interface

[Stefan Hajnoczi]

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