Re: [Qemu-devel] [PATCH v2 for-2.3 1/5] hw: Mark devices picking up block backends actively FIXME

[Andreas Färber]

Am 25.03.2015 um 15:16 schrieb Markus Armbruster:
> Drives defined with if!=none are for board initialization to wire up.
> Board code calls drive_get() or similar to find them, and creates
> devices with their qdev drive properties set accordingly.
> 
> Except a few devices go on a fishing expedition for a suitable backend
> instead of exposing a drive property for board code to set: they call
> driver_get() or drive_get_next() in their realize() or init() method
> to implicitly connect to the "next" backend with a certain interface
> type.
> 
> Picking up backends that way works when the devices are created by
> board code.  But it's inappropriate for -device or device_add.  Not
> only is this inconsistent with how the other block device models work
> (they connect to a backend explicitly identified by a "drive"
> property), it breaks when the "next" backend has been picked up by the
> board already.
> 
> Example:
> 
>     $ qemu-system-arm -S -M connex -pflash flash.img -device ssi-sd
>     Aborted (core dumped)
> 
> Mark them with suitable FIXME comments.
> 
> Cc: Andrzej Zaborowski <address@hidden>
> Cc: Peter Crosthwaite <address@hidden>
> Cc: "Andreas Färber" <address@hidden>
> Cc: Michael Walle <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  hw/arm/spitz.c            | 1 +
>  hw/block/m25p80.c         | 1 +
>  hw/isa/pc87312.c          | 2 ++
>  hw/sd/milkymist-memcard.c | 1 +
>  hw/sd/pl181.c             | 1 +
>  hw/sd/sdhci.c             | 1 +
>  hw/sd/ssi-sd.c            | 1 +
>  7 files changed, 8 insertions(+)
[...]
> diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
> index 40a1106..2849e8d 100644
> --- a/hw/isa/pc87312.c
> +++ b/hw/isa/pc87312.c
> @@ -319,11 +319,13 @@ static void pc87312_realize(DeviceState *dev, Error 
> **errp)
>          d = DEVICE(isa);
>          qdev_prop_set_uint32(d, "iobase", get_fdc_iobase(s));
>          qdev_prop_set_uint32(d, "irq", 6);
> +        /* FIXME use a qdev drive property instead of drive_get() */
>          drive = drive_get(IF_FLOPPY, 0, 0);
>          if (drive != NULL) {
>              qdev_prop_set_drive_nofail(d, "driveA",
>                                         blk_by_legacy_dinfo(drive));
>          }
> +        /* FIXME use a qdev drive property instead of drive_get() */
>          drive = drive_get(IF_FLOPPY, 0, 1);
>          if (drive != NULL) {
>              qdev_prop_set_drive_nofail(d, "driveB",

As can be seen, there are drive properties driveA and driveB already on
the destination device. How do you imagine this to be fixed? Add alias
properties on the container device? (CC'ing Stefan)

Regards,
Andreas