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

[Markus Armbruster]

Gerd Hoffmann <address@hidden> writes:

> On 08/12/09 21:28, Paul Brook wrote:
>>> We have already one case in-tree where this is needed:
>>> Try -device virtio-blk-pci (without drive= specified) and watch qemu
>>> segfault.
>>
>> No. Failure of the init routine should be fatal. i.e. virtio_blk_init_pci
>> should call hw_error.
>
> No.  That policy doesn't belong there.
>
>> If you want to allow graceful failure  (which is pointless for commandline
>> options, but may be desirable for hotplug devices)
>
> Exactly.  And for that reason we must pass up the error to the
> caller. The caller can then decide what to do with it.  For devices
> added via command line options we probably want to exit(1).  For
> hotplugging we probably would not.

exit(1) on typo in the monitor is not nice.

One problem with passing up error codes is diagnostics: when we reach
the point where we can decide on policy, we've lost the context for
precise diagnostics.  net.c tackles that problem by passing the
destination for diagnostics down, to let code report errors with
config_error() while remaining unaware of policy.

>> they you need to also add
>> some way of reporting why device creation failure. fprintf(stderr) is just
>> plain wrong.
>
> Indeed.  When hotplugging via monitor the error message should appear
> on the monitor, not stderr.  It is already an item on my todo list.

Issue also exists elsewhere.  Try monitor command "pci_add auto
model=?".

> I'd prefer to address that in a separate patch though, the patch is
> already big and invasive enough as-is.

It's easy to review for its size precisely because it does just one
simple thing.  Let's keep it that way.