Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6 0/2] Preparation for PCI devices convert to realize()

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> Cao jin <address@hidden> writes:
>
>> Hi John
>>
>> On 12/05/2015 01:55 AM, John Snow wrote:
>>>
>>>
>>> On 12/04/2015 02:47 AM, Cao jin wrote:
>>>> Hi,
>>>>      As you know, there are many PCI devices still using .init() as its
>>>>      initialization function, I am planning to do the "convert to 
>>>> realize()"
>>>>      work, and PCI bridge devices are chosen first.
>>>>      The supporting functions should be modified first. msi_init() a 
>>>> supporting
>>>>      function for PCI devices.
>>>>
>>>>      Maybe it should be put in 2.6, as title indicated
>>>>
>>>> Cao jin (2):
>>>>    Add param Error** to msi_init()
>>>>    Modify callers of msi_init()
>>>>
>>>>   hw/audio/intel-hda.c               |  7 ++++++-
>>>>   hw/ide/ich.c                       |  2 +-
>>>>   hw/net/vmxnet3.c                   |  3 ++-
>>>>   hw/pci-bridge/ioh3420.c            |  6 +++++-
>>>>   hw/pci-bridge/pci_bridge_dev.c     |  6 +++++-
>>>>   hw/pci-bridge/xio3130_downstream.c |  7 ++++++-
>>>>   hw/pci-bridge/xio3130_upstream.c   |  7 ++++++-
>>>>   hw/pci/msi.c                       | 17 +++++++++++++----
>>>>   hw/scsi/megasas.c                  |  2 +-
>>>>   hw/scsi/vmw_pvscsi.c               |  3 ++-
>>>>   hw/usb/hcd-xhci.c                  |  5 ++++-
>>>>   hw/vfio/pci.c                      |  3 ++-
>>>>   include/hw/pci/msi.h               |  4 ++--
>>>>   13 files changed, 55 insertions(+), 17 deletions(-)
>>>>
>>>
>>> You'll need to squash these patches as the first patch will break git
>>> bisect.
>>>
>>
>> Ok, will squash it. And I have another question： what`s the benefit
>> of converting to realize? Because AFAICT, doing this make the error
>> reporting machanism seems clean & clear, all device-init errors are
>> passed above along the call chain. I mean, besides, are there any
>> other benefits?
>
> Let's compare behavior on error:
>
> * realize methods pass an error object to their caller
>
>   The caller decides how to handle the error.
>
>   Many callers simply pass it on.  Typically, errors from realize are
>   passed up the call chain through qdev_device_add().
>
>   Eventually, the caller that handles the error is reached.  If the
>   error needs to be reported to the user, it decides how.  Examples:
>
>   - device_init_func(), which does the actual work for -device, reports
>     the human-readable message to stderr.
>
>   - hmp_device_add(), which does the work for HMP device_add, reports
>     the human-readable message to to the appropriate monitor.
>
>   - handle_qmp_command(), which handles all QMP commands including QMP
>     device_add, sends a QMP error reply down the QMP connection.
>
>   - petalogix_ml605_init(), which creates the "petalogix-ml605" board,
>     aborts when realizing the CPU fails.
>
> * init methods report to stderr and return -1
>
>   Fine when the error should be reported to stderr.

Correction: init methods should use error_report(), which reports either
to stderr or the current monitor.

>                                                      This is fine for
>   some of the above examples, and wrong on others.  In particular, a
>   device needs to provide a realize method to be fully work with
>   device_add, unless its init method cannot fail.

This is the case all the same.