[Qemu-devel] [PATCH v3 0/8] More sensible default for -drive interface type

[Qemu-devel] [PATCH v3 0/8] More sensible default for -drive interface type

[Markus Armbruster]

Block backends defined with -drive if=T, T!=none are meant to be
picked up by machine initialization code: a suitable frontend gets
created and wired up automatically.

if=T drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

Many machine types default to if=ide, even though they don't actually
have an IDE controller.  A few default to if=scsi, even though they
lack a SCSI HBA.  Change their default to if=none.

While their, fix handling of index and unit for a few machines with
AHCI controllers.

With the improved default, unintentional definition of orphaned drives
should be unlikely.  Improve the "orphaned drive" warning and turn it
into an error.  Drop a few special cases of this error that are now
redundant.

v3:
* Trivially rebased
* PATCH 6-8: Diagnostic message improved a bit [John Snow]
* PATCH 7: Commit message typo fixed [John Snow]

v2:
* PATCH v1 6/6 dropped; if=scsi eccentricities will be addressed in a
  series of its own
* PATCH 6-8 new

Markus Armbruster (8):
  hw: Default -drive to if=ide explicitly where it works
  hw/arm/cubieboard hw/arm/xlnx-ep108: Fix units_per_default_bus
  hw: Default -drive to if=none instead of ide when ide cannot work
  hw: Default -drive to if=none instead of scsi when scsi cannot work
  hw/arm/highbank: Default -drive to if=ide instead of if=scsi
  blockdev: Improve message for orphaned -drive
  blockdev: Make orphaned -drive fatal
  hw: Drop superfluous special checks for orphaned -drive

 blockdev.c                | 23 +++++++++++++----------
 hw/alpha/dp264.c          |  1 +
 hw/arm/cubieboard.c       |  4 ++++
 hw/arm/highbank.c         |  8 ++++++--
 hw/arm/realview.c         |  1 -
 hw/arm/spitz.c            |  3 +++
 hw/arm/tosa.c             |  1 +
 hw/arm/vexpress.c         |  1 -
 hw/arm/xilinx_zynq.c      |  1 -
 hw/arm/xlnx-ep108.c       |  6 ++++++
 hw/i386/pc.c              |  1 +
 hw/ide/core.c             | 17 -----------------
 hw/mips/mips_fulong2e.c   |  1 +
 hw/mips/mips_jazz.c       |  4 ----
 hw/mips/mips_malta.c      |  1 +
 hw/mips/mips_r4k.c        |  1 +
 hw/ppc/mac_newworld.c     |  1 +
 hw/ppc/mac_oldworld.c     |  1 +
 hw/ppc/prep.c             |  1 +
 hw/sh4/r2d.c              |  1 +
 hw/sparc/sun4m.c          |  5 -----
 hw/sparc64/sun4u.c        |  2 ++
 include/sysemu/blockdev.h | 11 +++++------
 23 files changed, 49 insertions(+), 47 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v3 1/8] hw: Default -drive to if=ide explicitly where it works

[Markus Armbruster]

Block backends defined with -drive if=ide are meant to be picked up by
machine initialization code: a suitable frontend gets created and
wired up automatically.

if=ide drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

-drive parameter "if" is optional, and the default depends on the
machine type.  If a machine type doesn't specify a default, the
default is "ide".

Many machine types default to if=ide, even though they don't actually
have an IDE controller.  A future patch will change these defaults to
something more sensible.  To prepare for it, this patch makes default
"ide" explicit for the machines that actually pick up if=ide drives:

* alpha: clipper
* arm/aarch64: spitz borzoi terrier tosa
* i386/x86_64: generic-pc-machine (with concrete subtypes pc-q35-*
  pc-i440fx-* pc-* isapc xenfv)
* mips64el: fulong2e
* mips/mipsel/mips64el: malta mips
* ppc/ppc64: mac99 g3beige prep
* sh4/sh4eb: r2d
* sparc64: sun4u sun4v

Note that ppc64 machine powernv already sets an "ide" default
explicitly.  Its IDE controller isn't implemented, yet.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/alpha/dp264.c        | 1 +
 hw/arm/spitz.c          | 3 +++
 hw/arm/tosa.c           | 1 +
 hw/i386/pc.c            | 1 +
 hw/mips/mips_fulong2e.c | 1 +
 hw/mips/mips_malta.c    | 1 +
 hw/mips/mips_r4k.c      | 1 +
 hw/ppc/mac_newworld.c   | 1 +
 hw/ppc/mac_oldworld.c   | 1 +
 hw/ppc/prep.c           | 1 +
 hw/sh4/r2d.c            | 1 +
 hw/sparc64/sun4u.c      | 2 ++
 12 files changed, 15 insertions(+)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index d6431fd..85405da 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -177,6 +177,7 @@ static void clipper_machine_init(MachineClass *mc)
 {
     mc->desc = "Alpha DP264/CLIPPER";
     mc->init = clipper_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 4;
     mc->is_default = 1;
 }
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 949a15a..fe2d5a7 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -998,6 +998,7 @@ static void spitzpda_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sharp SL-C3000 (Spitz) PDA (PXA270)";
     mc->init = spitz_init;
+    mc->block_default_type = IF_IDE;
 }
 
 static const TypeInfo spitzpda_type = {
@@ -1012,6 +1013,7 @@ static void borzoipda_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sharp SL-C3100 (Borzoi) PDA (PXA270)";
     mc->init = borzoi_init;
+    mc->block_default_type = IF_IDE;
 }
 
 static const TypeInfo borzoipda_type = {
@@ -1026,6 +1028,7 @@ static void terrierpda_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sharp SL-C3200 (Terrier) PDA (PXA270)";
     mc->init = terrier_init;
+    mc->block_default_type = IF_IDE;
 }
 
 static const TypeInfo terrierpda_type = {
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index c3db996..9f58a23 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -263,6 +263,7 @@ static void tosapda_machine_init(MachineClass *mc)
 {
     mc->desc = "Sharp SL-6000 (Tosa) PDA (PXA255)";
     mc->init = tosa_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("tosa", tosapda_machine_init)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e3fcd51..a555c35 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2339,6 +2339,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->query_hotpluggable_cpus = pc_query_hotpluggable_cpus;
     mc->default_boot_order = "cad";
     mc->hot_add_cpu = pc_hot_add_cpu;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 255;
     mc->reset = pc_machine_reset;
     hc->pre_plug = pc_machine_device_pre_plug_cb;
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 9a4dae4..e636c3a 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -387,6 +387,7 @@ static void mips_fulong2e_machine_init(MachineClass *mc)
 {
     mc->desc = "Fulong 2e mini pc";
     mc->init = mips_fulong2e_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("fulong2e", mips_fulong2e_machine_init)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 75877de..5dd177e 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1264,6 +1264,7 @@ static void mips_malta_machine_init(MachineClass *mc)
 {
     mc->desc = "MIPS Malta Core LV";
     mc->init = mips_malta_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 16;
     mc->is_default = 1;
 }
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 27548c4..748586e 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -306,6 +306,7 @@ static void mips_machine_init(MachineClass *mc)
 {
     mc->desc = "mips r4k platform";
     mc->init = mips_r4k_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("mips", mips_machine_init)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 2bfdb64..716aea6 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -518,6 +518,7 @@ static void core99_machine_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Mac99 based PowerMAC";
     mc->init = ppc_core99_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = MAX_CPUS;
     mc->default_boot_order = "cd";
     mc->kvm_type = core99_kvm_type;
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 56282c5..5df94e2 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -368,6 +368,7 @@ static void heathrow_machine_init(MachineClass *mc)
 {
     mc->desc = "Heathrow based PowerMAC";
     mc->init = ppc_heathrow_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = MAX_CPUS;
 #ifndef TARGET_PPC64
     mc->is_default = 1;
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index ca7959c..961230c 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -684,6 +684,7 @@ static void prep_machine_init(MachineClass *mc)
 {
     mc->desc = "PowerPC PREP platform";
     mc->init = ppc_prep_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = MAX_CPUS;
     mc->default_boot_order = "cad";
 }
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index db373c7..6d06968 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -362,6 +362,7 @@ static void r2d_machine_init(MachineClass *mc)
 {
     mc->desc = "r2d-plus board";
     mc->init = r2d_init;
+    mc->block_default_type = IF_IDE;
 }
 
 DEFINE_MACHINE("r2d", r2d_machine_init)
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d1a6bca..d347b66 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -579,6 +579,7 @@ static void sun4u_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sun4u platform";
     mc->init = sun4u_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 1; /* XXX for now */
     mc->is_default = 1;
     mc->default_boot_order = "c";
@@ -596,6 +597,7 @@ static void sun4v_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Sun4v platform";
     mc->init = sun4v_init;
+    mc->block_default_type = IF_IDE;
     mc->max_cpus = 1; /* XXX for now */
     mc->default_boot_order = "c";
 }
-- 
2.7.4

[Qemu-devel] [PATCH v3 2/8] hw/arm/cubieboard hw/arm/xlnx-ep108: Fix units_per_default_bus

[Markus Armbruster]

Machine types cubieboard, xlnx-ep108, xlnx-zcu102 have an onboard AHCI
controller, but neglect to set their MachineClass member
units_per_default_bus = 1.  This permits -drive if=ide,unit=1, which
makes no sense for AHCI.  It also screws up index=N for odd N, because
it gets desugared to unit=1,bus=N/2

Doesn't really matter, because these machine types fail to honor
-drive if=ide.  Add the missing units_per_default_bus = 1 anyway,
along with a TODO comment on what needs to be done for -drive if=ide.

Also set block_default_type = IF_IDE explicitly.  It's currently the
default, but the next commit will change it to something more
sensible, and we want to keep the IF_IDE default for these three
machines.  See also the previous commit.

Cc: Beniamino Galvani <b.galvani@gmail.com>
Cc: Alistair Francis <alistair.francis@xilinx.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Alistair Francis <alistair.francis@xilinx.com>
---
 hw/arm/cubieboard.c | 4 ++++
 hw/arm/xlnx-ep108.c | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index dd19ba3..b98e1c4 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -71,6 +71,8 @@ static void cubieboard_init(MachineState *machine)
     memory_region_add_subregion(get_system_memory(), AW_A10_SDRAM_BASE,
                                 &s->sdram);
 
+    /* TODO create and connect IDE devices for ide_drive_get() */
+
     cubieboard_binfo.ram_size = machine->ram_size;
     cubieboard_binfo.kernel_filename = machine->kernel_filename;
     cubieboard_binfo.kernel_cmdline = machine->kernel_cmdline;
@@ -82,6 +84,8 @@ static void cubieboard_machine_init(MachineClass *mc)
 {
     mc->desc = "cubietech cubieboard";
     mc->init = cubieboard_init;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
 }
 
 DEFINE_MACHINE("cubieboard", cubieboard_machine_init)
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 4ec590a..860780a 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -106,6 +106,8 @@ static void xlnx_ep108_init(MachineState *machine)
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi[i]), 1, cs_line);
     }
 
+    /* TODO create and connect IDE devices for ide_drive_get() */
+
     xlnx_ep108_binfo.ram_size = ram_size;
     xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
     xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
@@ -118,6 +120,8 @@ static void xlnx_ep108_machine_init(MachineClass *mc)
 {
     mc->desc = "Xilinx ZynqMP EP108 board";
     mc->init = xlnx_ep108_init;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
 }
 
 DEFINE_MACHINE("xlnx-ep108", xlnx_ep108_machine_init)
@@ -126,6 +130,8 @@ static void xlnx_zcu102_machine_init(MachineClass *mc)
 {
     mc->desc = "Xilinx ZynqMP ZCU102 board";
     mc->init = xlnx_ep108_init;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
 }
 
 DEFINE_MACHINE("xlnx-zcu102", xlnx_zcu102_machine_init)
-- 
2.7.4

[Qemu-devel] [PATCH v3 3/8] hw: Default -drive to if=none instead of ide when ide cannot work

[Markus Armbruster]

Block backends defined with -drive if=ide are meant to be picked up by
machine initialization code: a suitable frontend gets created and
wired up automatically.

if=ide drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

-drive parameter "if" is optional, and the default depends on the
machine type.  If a machine type doesn't specify a default, the
default is "ide".

Many machine types implicitly default to if=ide that way, even though
they don't actually have an IDE controller.  This makes no sense.

Change the implicit default to if=none.  Affected machines:

* all targets: none
* aarch64/arm: akita ast2500 canon cheetah collie connex imx25
  integratorcp kzm lm3s6965evb lm3s811evb mainstone musicpal n800 n810
  netduino2 nuri palmetto realview romulus sabrelite smdkc210 sx1 sx1
  verdex z2
* cris: axis-dev88
* i386/x86_64: xenpv
* lm32: lm32-evr lm32-uclinux milkymist
* m68k: an5206 dummy mcf5208evb
* microblaze/microblazeel: petalogix-ml605 petalogix-s3adsp1800
* mips/mips64/mips64el/mipsel: mipssim
* moxie: moxiesim
* or32: or32-sim
* ppc/ppc64/ppcemb: bamboo ref405ep taihu virtex-ml507
* ppc/ppc64: mpc8544ds ppce500
* sh4/sh4eb: shix
* sparc: leon3_generic
* sparc64: niagara
* tricore: tricore_testboard
* unicore32: puv3
* xtensa/xtensaeb: kc705 lx200 lx60 ml605 sim

None of these machines have an IDE controller, let alone code to
honor if=ide.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: xen-devel@lists.xensource.com
Cc: Michael Walle <michael@walle.cc>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Anthony Green <green@moxielogic.com>
Cc: Jia Liu <proljc@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Fabien Chouteau <chouteau@adacore.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-By: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 include/sysemu/blockdev.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 16432f3..351a039 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -19,12 +19,11 @@ void blockdev_auto_del(BlockBackend *blk);
 typedef enum {
     IF_DEFAULT = -1,            /* for use with drive_add() only */
     /*
-     * IF_IDE must be zero, because we want MachineClass member
-     * block_default_type to default-initialize to IF_IDE
+     * IF_NONE must be zero, because we want MachineClass member
+     * block_default_type to default-initialize to IF_NONE
      */
-    IF_IDE = 0,
-    IF_NONE,
-    IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+    IF_NONE = 0,
+    IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
     IF_COUNT
 } BlockInterfaceType;
 
-- 
2.7.4

[PATCH v3 3/8] hw: Default -drive to if=none instead of ide when ide cannot work

[Markus Armbruster]

Block backends defined with -drive if=ide are meant to be picked up by
machine initialization code: a suitable frontend gets created and
wired up automatically.

if=ide drives not picked up that way can still be used with -device as
if they had if=none, but that's unclean and best avoided.  Unused ones
produce an "Orphaned drive without device" warning.

-drive parameter "if" is optional, and the default depends on the
machine type.  If a machine type doesn't specify a default, the
default is "ide".

Many machine types implicitly default to if=ide that way, even though
they don't actually have an IDE controller.  This makes no sense.

Change the implicit default to if=none.  Affected machines:

* all targets: none
* aarch64/arm: akita ast2500 canon cheetah collie connex imx25
  integratorcp kzm lm3s6965evb lm3s811evb mainstone musicpal n800 n810
  netduino2 nuri palmetto realview romulus sabrelite smdkc210 sx1 sx1
  verdex z2
* cris: axis-dev88
* i386/x86_64: xenpv
* lm32: lm32-evr lm32-uclinux milkymist
* m68k: an5206 dummy mcf5208evb
* microblaze/microblazeel: petalogix-ml605 petalogix-s3adsp1800
* mips/mips64/mips64el/mipsel: mipssim
* moxie: moxiesim
* or32: or32-sim
* ppc/ppc64/ppcemb: bamboo ref405ep taihu virtex-ml507
* ppc/ppc64: mpc8544ds ppce500
* sh4/sh4eb: shix
* sparc: leon3_generic
* sparc64: niagara
* tricore: tricore_testboard
* unicore32: puv3
* xtensa/xtensaeb: kc705 lx200 lx60 ml605 sim

None of these machines have an IDE controller, let alone code to
honor if=ide.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: xen-devel@lists.xensource.com
Cc: Michael Walle <michael@walle.cc>
Cc: Laurent Vivier <laurent@vivier.eu>
Cc: Anthony Green <green@moxielogic.com>
Cc: Jia Liu <proljc@gmail.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Fabien Chouteau <chouteau@adacore.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Artyom Tarasenko <atar4qemu@gmail.com>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-By: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 include/sysemu/blockdev.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 16432f3..351a039 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -19,12 +19,11 @@ void blockdev_auto_del(BlockBackend *blk);
 typedef enum {
     IF_DEFAULT = -1,            /* for use with drive_add() only */
     /*
-     * IF_IDE must be zero, because we want MachineClass member
-     * block_default_type to default-initialize to IF_IDE
+     * IF_NONE must be zero, because we want MachineClass member
+     * block_default_type to default-initialize to IF_NONE
      */
-    IF_IDE = 0,
-    IF_NONE,
-    IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+    IF_NONE = 0,
+    IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
     IF_COUNT
 } BlockInterfaceType;
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Qemu-devel] [PATCH v3 4/8] hw: Default -drive to if=none instead of scsi when scsi cannot work

[Markus Armbruster]

Block backends defined with -drive if=scsi are meant to be picked up
by machine initialization code: a suitable frontend gets created and
wired up automatically.

if=scsi drives not picked up that way can still be used with -device
as if they had if=none, but that's unclean and best avoided.  Unused
ones produce an "Orphaned drive without device" warning.

A few machine types default to if=scsi, even though they don't
actually have a SCSI HBA.  This makes no sense.  Change their default
to if=none.  Affected machines:

* aarch64/arm: realview-pbx-a9 vexpress-a9 vexpress-a15 xilinx-zynq-a9

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Alistair Francis <alistair.francis@xilinx.com>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
---
 hw/arm/realview.c    | 1 -
 hw/arm/vexpress.c    | 1 -
 hw/arm/xilinx_zynq.c | 1 -
 3 files changed, 3 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 8eafcca..8c11c7a 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -443,7 +443,6 @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "ARM RealView Platform Baseboard Explore for Cortex-A9";
     mc->init = realview_pbx_a9_init;
-    mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
 }
 
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index e057568..c6b1e67 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -752,7 +752,6 @@ static void vexpress_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "ARM Versatile Express";
     mc->init = vexpress_common_init;
-    mc->block_default_type = IF_SCSI;
     mc->max_cpus = 4;
 }
 
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 7dac20d..3985356 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -323,7 +323,6 @@ static void zynq_machine_init(MachineClass *mc)
 {
     mc->desc = "Xilinx Zynq Platform Baseboard for Cortex-A9";
     mc->init = zynq_init;
-    mc->block_default_type = IF_SCSI;
     mc->max_cpus = 1;
     mc->no_sdcard = 1;
 }
-- 
2.7.4

[Qemu-devel] [PATCH v3 5/8] hw/arm/highbank: Default -drive to if=ide instead of if=scsi

[Markus Armbruster]

These machines have no onboard SCSI HBA, and no way to plug one.
-drive if=scsi therefore cannot work.  They do have an onboard IDE
controller (sysbus-ahci), but fail to honor if=ide.

Change their default to if=ide, and add a TODO comment on what needs
to be done to actually honor -drive if=ide.

Cc: Rob Herring <robh@kernel.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/arm/highbank.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 80e5fd4..0a4508c 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -363,6 +363,8 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
         sysbus_connect_irq(SYS_BUS_DEVICE(dev), 2, pic[82]);
     }
 
+    /* TODO create and connect IDE devices for ide_drive_get() */
+
     highbank_binfo.ram_size = ram_size;
     highbank_binfo.kernel_filename = kernel_filename;
     highbank_binfo.kernel_cmdline = kernel_cmdline;
@@ -405,7 +407,8 @@ static void highbank_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Calxeda Highbank (ECX-1000)";
     mc->init = highbank_init;
-    mc->block_default_type = IF_SCSI;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
     mc->max_cpus = 4;
 }
 
@@ -421,7 +424,8 @@ static void midway_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Calxeda Midway (ECX-2000)";
     mc->init = midway_init;
-    mc->block_default_type = IF_SCSI;
+    mc->block_default_type = IF_IDE;
+    mc->units_per_default_bus = 1;
     mc->max_cpus = 4;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH v3 6/8] blockdev: Improve message for orphaned -drive

[Markus Armbruster]

We warn when a -drive isn't supported by the machine type (commit
a66c9dc):

    $ qemu-system-x86_64 -S -display none -drive if=mtd
    Warning: Orphaned drive without device: id=mtd0,file=,if=mtd,bus=0,unit=0

Improve this to point to the offending bit of configuration:

    qemu-system-x86_64: -drive if=mtd: warning: machine type does not support if=mtd,bus=0,unit=0

Especially nice when it's hidden behind -readconfig foo.cfg:

    qemu-system-x86_64:foo.cfg:140: warning: machine type does not support if=mtd,bus=0,unit=0

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index db82ac9..eb75f35 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -231,6 +231,7 @@ bool drive_check_orphaned(void)
 {
     BlockBackend *blk;
     DriveInfo *dinfo;
+    Location loc;
     bool rs = false;
 
     for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
@@ -239,10 +240,12 @@ bool drive_check_orphaned(void)
         /* Unless this is a default drive, this may be an oversight. */
         if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
             dinfo->type != IF_NONE) {
-            fprintf(stderr, "Warning: Orphaned drive without device: "
-                    "id=%s,file=%s,if=%s,bus=%d,unit=%d\n",
-                    blk_name(blk), blk_bs(blk) ? blk_bs(blk)->filename : "",
-                    if_name[dinfo->type], dinfo->bus, dinfo->unit);
+            loc_push_none(&loc);
+            qemu_opts_loc_restore(dinfo->opts);
+            error_report("warning: machine type does not support"
+                         " if=%s,bus=%d,unit=%d",
+                         if_name[dinfo->type], dinfo->bus, dinfo->unit);
+            loc_pop(&loc);
             rs = true;
         }
     }
-- 
2.7.4

[Qemu-devel] [PATCH v3 7/8] blockdev: Make orphaned -drive fatal

[Markus Armbruster]

Block backends defined with "-drive if=T" with T other than "none" are
meant to be picked up by machine initialization code: a suitable
frontend gets created and wired up automatically.

If machine initialization code doesn't comply, the block backend
remains unused.  This triggers a warning since commit a66c9dc, v2.2.0.
Drives created by default are exempted; use -nodefaults to get rid of
them.

Turn this warning into an error.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 blockdev.c                | 14 +++++++-------
 include/sysemu/blockdev.h |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index eb75f35..bbf9d4d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -227,30 +227,30 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
     return NULL;
 }
 
-bool drive_check_orphaned(void)
+void drive_check_orphaned(void)
 {
     BlockBackend *blk;
     DriveInfo *dinfo;
     Location loc;
-    bool rs = false;
+    bool orphans = false;
 
     for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
         dinfo = blk_legacy_dinfo(blk);
-        /* If dinfo->bdrv->dev is NULL, it has no device attached. */
-        /* Unless this is a default drive, this may be an oversight. */
         if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
             dinfo->type != IF_NONE) {
             loc_push_none(&loc);
             qemu_opts_loc_restore(dinfo->opts);
-            error_report("warning: machine type does not support"
+            error_report("machine type does not support"
                          " if=%s,bus=%d,unit=%d",
                          if_name[dinfo->type], dinfo->bus, dinfo->unit);
             loc_pop(&loc);
-            rs = true;
+            orphans = true;
         }
     }
 
-    return rs;
+    if (orphans) {
+        exit(1);
+    }
 }
 
 DriveInfo *drive_get_by_index(BlockInterfaceType type, int index)
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 351a039..ac22f2a 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -48,7 +48,7 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo);
 void override_max_devs(BlockInterfaceType type, int max_devs);
 
 DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
-bool drive_check_orphaned(void);
+void drive_check_orphaned(void);
 DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
 int drive_get_max_bus(BlockInterfaceType type);
 int drive_get_max_devs(BlockInterfaceType type);
-- 
2.7.4

Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/8] blockdev: Make orphaned -drive fatal

[John Snow]

On 02/15/2017 05:05 AM, Markus Armbruster wrote:
> Block backends defined with "-drive if=T" with T other than "none" are
> meant to be picked up by machine initialization code: a suitable
> frontend gets created and wired up automatically.
> 
> If machine initialization code doesn't comply, the block backend
> remains unused.  This triggers a warning since commit a66c9dc, v2.2.0.
> Drives created by default are exempted; use -nodefaults to get rid of
> them.
> 
> Turn this warning into an error.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  blockdev.c                | 14 +++++++-------
>  include/sysemu/blockdev.h |  2 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index eb75f35..bbf9d4d 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -227,30 +227,30 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit)
>      return NULL;
>  }
>  
> -bool drive_check_orphaned(void)
> +void drive_check_orphaned(void)
>  {
>      BlockBackend *blk;
>      DriveInfo *dinfo;
>      Location loc;
> -    bool rs = false;
> +    bool orphans = false;
>  
>      for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>          dinfo = blk_legacy_dinfo(blk);
> -        /* If dinfo->bdrv->dev is NULL, it has no device attached. */
> -        /* Unless this is a default drive, this may be an oversight. */
>          if (!blk_get_attached_dev(blk) && !dinfo->is_default &&
>              dinfo->type != IF_NONE) {
>              loc_push_none(&loc);
>              qemu_opts_loc_restore(dinfo->opts);
> -            error_report("warning: machine type does not support"
> +            error_report("machine type does not support"
>                           " if=%s,bus=%d,unit=%d",
>                           if_name[dinfo->type], dinfo->bus, dinfo->unit);
>              loc_pop(&loc);
> -            rs = true;
> +            orphans = true;
>          }
>      }
>  
> -    return rs;
> +    if (orphans) {
> +        exit(1);
> +    }
>  }
>  
>  DriveInfo *drive_get_by_index(BlockInterfaceType type, int index)
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 351a039..ac22f2a 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -48,7 +48,7 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo);
>  void override_max_devs(BlockInterfaceType type, int max_devs);
>  
>  DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
> -bool drive_check_orphaned(void);
> +void drive_check_orphaned(void);
>  DriveInfo *drive_get_by_index(BlockInterfaceType type, int index);
>  int drive_get_max_bus(BlockInterfaceType type);
>  int drive_get_max_devs(BlockInterfaceType type);
> 

6,7:

Reviewed-by: John Snow <jsnow@redhat.com>

[Qemu-devel] [PATCH v3 8/8] hw: Drop superfluous special checks for orphaned -drive

[Markus Armbruster]

We've traditionally rejected orphans here and there, but not
systematically.  For instance, the sun4m machines have an onboard SCSI
HBA (bus=0), and have always rejected bus>0.  Other machines with an
onboard SCSI HBA don't.

Commit a66c9dc made all orphans trigger a warning, and the previous
commit turned this into an error.  The checks "here and there" are now
redundant.  Drop them.

Note that the one in mips_jazz.c was wrong: it rejected bus > MAX_FD,
but MAX_FD is the number of floppy drives per bus.

Error messages change from

    $ qemu-system-x86_64 -drive if=ide,bus=2
    qemu-system-x86_64: Too many IDE buses defined (3 > 2)
    $ qemu-system-mips64 -M magnum,accel=qtest -drive if=floppy,bus=2,id=fd1
    qemu: too many floppy drives
    $ qemu-system-sparc -M LX -drive if=scsi,bus=1
    qemu: too many SCSI bus

to

    $ qemu-system-x86_64 -drive if=ide,bus=2
    qemu-system-x86_64: -drive if=ide,bus=2: machine type does not support if=ide,bus=2,unit=0
    $ qemu-system-mips64 -M magnum,accel=qtest -drive if=floppy,bus=2,id=fd1
    qemu-system-mips64: -drive if=floppy,bus=2,id=fd1: machine type does not support if=floppy,bus=2,unit=0
    $ qemu-system-sparc -M LX -drive if=scsi,bus=1
    qemu-system-sparc: -drive if=scsi,bus=1: machine type does not support if=scsi,bus=1,unit=0

Cc: John Snow <jsnow@redhat.com>
Cc: "Hervé Poussineau" <hpoussin@reactos.org>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ide/core.c       | 17 -----------------
 hw/mips/mips_jazz.c |  4 ----
 hw/sparc/sun4m.c    |  5 -----
 3 files changed, 26 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 43709e5..cfa5de6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2840,23 +2840,6 @@ const VMStateDescription vmstate_ide_bus = {
 void ide_drive_get(DriveInfo **hd, int n)
 {
     int i;
-    int highest_bus = drive_get_max_bus(IF_IDE) + 1;
-    int max_devs = drive_get_max_devs(IF_IDE);
-    int n_buses = max_devs ? (n / max_devs) : n;
-
-    /*
-     * Note: The number of actual buses available is not known.
-     * We compute this based on the size of the DriveInfo* array, n.
-     * If it is less than max_devs * <num_real_buses>,
-     * We will stop looking for drives prematurely instead of overfilling
-     * the array.
-     */
-
-    if (highest_bus > n_buses) {
-        error_report("Too many IDE buses defined (%d > %d)",
-                     highest_bus, n_buses);
-        exit(1);
-    }
 
     for (i = 0; i < n; i++) {
         hd[i] = drive_get_by_index(IF_IDE, i);
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 73f6c9f..1cef581 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -291,10 +291,6 @@ static void mips_jazz_init(MachineState *machine,
              qdev_get_gpio_in(rc4030, 5), &esp_reset, &dma_enable);
 
     /* Floppy */
-    if (drive_get_max_bus(IF_FLOPPY) >= MAX_FD) {
-        fprintf(stderr, "qemu: too many floppy drives\n");
-        exit(1);
-    }
     for (n = 0; n < MAX_FD; n++) {
         fds[n] = drive_get(IF_FLOPPY, 0, n);
     }
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index f5b6efd..61416a6 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -989,11 +989,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     slavio_misc_init(hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base,
                      slavio_irq[30], fdc_tc);
 
-    if (drive_get_max_bus(IF_SCSI) > 0) {
-        fprintf(stderr, "qemu: too many SCSI bus\n");
-        exit(1);
-    }
-
     esp_init(hwdef->esp_base, 2,
              espdma_memory_read, espdma_memory_write,
              espdma, espdma_irq, &esp_reset, &dma_enable);
-- 
2.7.4

Re: [Qemu-devel] [PATCH v3 8/8] hw: Drop superfluous special checks for orphaned -drive

[John Snow]

On 02/15/2017 05:05 AM, Markus Armbruster wrote:
> We've traditionally rejected orphans here and there, but not
> systematically.  For instance, the sun4m machines have an onboard SCSI
> HBA (bus=0), and have always rejected bus>0.  Other machines with an
> onboard SCSI HBA don't.
> 
> Commit a66c9dc made all orphans trigger a warning, and the previous
> commit turned this into an error.  The checks "here and there" are now
> redundant.  Drop them.
> 
> Note that the one in mips_jazz.c was wrong: it rejected bus > MAX_FD,
> but MAX_FD is the number of floppy drives per bus.
> 
> Error messages change from
> 
>     $ qemu-system-x86_64 -drive if=ide,bus=2
>     qemu-system-x86_64: Too many IDE buses defined (3 > 2)
>     $ qemu-system-mips64 -M magnum,accel=qtest -drive if=floppy,bus=2,id=fd1
>     qemu: too many floppy drives
>     $ qemu-system-sparc -M LX -drive if=scsi,bus=1
>     qemu: too many SCSI bus
> 
> to
> 
>     $ qemu-system-x86_64 -drive if=ide,bus=2
>     qemu-system-x86_64: -drive if=ide,bus=2: machine type does not support if=ide,bus=2,unit=0
>     $ qemu-system-mips64 -M magnum,accel=qtest -drive if=floppy,bus=2,id=fd1
>     qemu-system-mips64: -drive if=floppy,bus=2,id=fd1: machine type does not support if=floppy,bus=2,unit=0
>     $ qemu-system-sparc -M LX -drive if=scsi,bus=1
>     qemu-system-sparc: -drive if=scsi,bus=1: machine type does not support if=scsi,bus=1,unit=0
> 
> Cc: John Snow <jsnow@redhat.com>
> Cc: "Hervé Poussineau" <hpoussin@reactos.org>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/ide/core.c       | 17 -----------------
>  hw/mips/mips_jazz.c |  4 ----
>  hw/sparc/sun4m.c    |  5 -----
>  3 files changed, 26 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 43709e5..cfa5de6 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2840,23 +2840,6 @@ const VMStateDescription vmstate_ide_bus = {
>  void ide_drive_get(DriveInfo **hd, int n)
>  {
>      int i;
> -    int highest_bus = drive_get_max_bus(IF_IDE) + 1;
> -    int max_devs = drive_get_max_devs(IF_IDE);
> -    int n_buses = max_devs ? (n / max_devs) : n;
> -
> -    /*
> -     * Note: The number of actual buses available is not known.
> -     * We compute this based on the size of the DriveInfo* array, n.
> -     * If it is less than max_devs * <num_real_buses>,
> -     * We will stop looking for drives prematurely instead of overfilling
> -     * the array.
> -     */
> -
> -    if (highest_bus > n_buses) {
> -        error_report("Too many IDE buses defined (%d > %d)",
> -                     highest_bus, n_buses);
> -        exit(1);
> -    }
>  
>      for (i = 0; i < n; i++) {
>          hd[i] = drive_get_by_index(IF_IDE, i);
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 73f6c9f..1cef581 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -291,10 +291,6 @@ static void mips_jazz_init(MachineState *machine,
>               qdev_get_gpio_in(rc4030, 5), &esp_reset, &dma_enable);
>  
>      /* Floppy */
> -    if (drive_get_max_bus(IF_FLOPPY) >= MAX_FD) {
> -        fprintf(stderr, "qemu: too many floppy drives\n");
> -        exit(1);
> -    }
>      for (n = 0; n < MAX_FD; n++) {
>          fds[n] = drive_get(IF_FLOPPY, 0, n);
>      }
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index f5b6efd..61416a6 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -989,11 +989,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      slavio_misc_init(hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base,
>                       slavio_irq[30], fdc_tc);
>  
> -    if (drive_get_max_bus(IF_SCSI) > 0) {
> -        fprintf(stderr, "qemu: too many SCSI bus\n");
> -        exit(1);
> -    }
> -
>      esp_init(hwdef->esp_base, 2,
>               espdma_memory_read, espdma_memory_write,
>               espdma, espdma_irq, &esp_reset, &dma_enable);
> 

http://i.imgur.com/v1Lvzb1.jpg

Reviewed-by: John Snow <jsnow@redhat.com>