Re: [PATCH 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error

[Philippe Mathieu-Daudé]

On 6/5/20 4:56 PM, Markus Armbruster wrote:
> On error, pxa2xx_mmci_init() reports to stderr and returns NULL.
> Callers don't check for errors.  Machines akita, borzoi, mainstone,
> spitz, terrier, tosa, and z2 crash shortly after, like this:
> 
>     $ qemu-system-aarch64 -M akita -drive if=sd,readonly=on
>     qemu-system-aarch64: failed to init SD card: Cannot use read-only drive 
> as SD card
>     Segmentation fault (core dumped)
> 
> Machines connex and verdex reach the check for orphaned drives first:
> 
>     $ aarch64-softmmu/qemu-system-aarch64 -M connex -drive if=sd,readonly=on 
> -accel qtest
>     qemu-system-aarch64: failed to init SD card: Cannot use read-only drive 
> as SD card
>     qemu-system-aarch64: -drive if=sd,readonly=on: machine type does not 
> support if=sd,bus=0,unit=0
> 
> Make pxa2xx_mmci_init() fail cleanly right away.
> 
> Cc: Andrzej Zaborowski <balrogg@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/sd/pxa2xx_mmci.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
> index 3407617afc..68bed24480 100644
> --- a/hw/sd/pxa2xx_mmci.c
> +++ b/hw/sd/pxa2xx_mmci.c
> @@ -18,7 +18,6 @@
>  #include "hw/arm/pxa.h"
>  #include "hw/sd/sd.h"
>  #include "hw/qdev-properties.h"
> -#include "qemu/error-report.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
>  #include "trace.h"
> @@ -483,7 +482,6 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
>      DeviceState *dev, *carddev;
>      SysBusDevice *sbd;
>      PXA2xxMMCIState *s;
> -    Error *err = NULL;
>  
>      dev = qdev_new(TYPE_PXA2XX_MMCI);
>      s = PXA2XX_MMCI(dev);
> @@ -496,16 +494,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
>  
>      /* Create and plug in the sd card */
>      carddev = qdev_new(TYPE_SD_CARD);
> -    qdev_prop_set_drive_err(carddev, "drive", blk, &err);
> -    if (err) {
> -        error_reportf_err(err, "failed to init SD card: ");
> -        return NULL;
> -    }
> -    qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"), &err);
> -    if (err) {
> -        error_reportf_err(err, "failed to init SD card: ");
> -        return NULL;
> -    }
> +    qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
> +    qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"),
> +                           &error_fatal);
>  
>      return s;
>  }
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>