Re: [Qemu-devel] [PATCH] qdev: add return value to init() callbacks.

[Markus Armbruster <armbru@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Sorry folks, but it has to be.  One more of these invasive qdev patches.
>
> We have a serious design bug in the qdev interface:  device init
> callbacks can't signal failure because the init() callback has no
> return value.  This patch fixes it.

Yupp.

> We have already one case in-tree where this is needed:
> Try -device virtio-blk-pci (without drive= specified) and watch qemu
> segfault.  This patch fixes it.
>
> With usb+scsi being converted to qdev we'll get more devices where the
> init callback can fail for various reasons.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

I verified that we either

* change a function returning void to return int instead, or

* insert return 0 at the end of a function, or

* change F() to return F(), for some F() that now returns int, or

* insert return -1 instead of continuing to a crash, in
  virtio_blk_init_pci(), or

* handle a failed device init(), in qdev_device_add().

[...]
> diff --git a/hw/vga.c b/hw/vga.c
> index 4a0f197..12c0424 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -2480,7 +2480,7 @@ static void pci_vga_write_config(PCIDevice *d,
>          s->map_addr = 0;
>  }
>  
> -static void pci_vga_initfn(PCIDevice *dev)
> +static int pci_vga_initfn(PCIDevice *dev)
>  {
>       PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev);
>       VGAState *s = &d->vga_state;
> @@ -2511,7 +2511,8 @@ static void pci_vga_initfn(PCIDevice *dev)
>              bios_total_size <<= 1;
>          pci_register_bar(&d->dev, PCI_ROM_SLOT, bios_total_size,
>                           PCI_ADDRESS_SPACE_MEM_PREFETCH, vga_map);
> -    }
> +     }
> +     return 0;
>  }
>  
>  int pci_vga_init(PCIBus *bus,

Looks like slight white-space damage, but isn't; the whole function is
indented weirdly.

[...]

Acked-by: Markus Armbruster <armbru@redhat.com>