Re: [Qemu-devel] [PATCH] pci: fixed mismatch of error-handling between pci_qdev_init() and qdev

[SeokYeon Hwang]

> -----Original Message-----
> From: Markus Armbruster [mailto:address@hidden
> Sent: Thursday, November 06, 2014 6:21 PM
> To: SeokYeon Hwang
> Cc: 'Paolo Bonzini'; 'Michael S. Tsirkin'; address@hidden
> Subject: Re: [Qemu-devel] [PATCH] pci: fixed mismatch of error-handling
> between pci_qdev_init() and qdev
> 
> SeokYeon Hwang <address@hidden> writes:
> 
> >> -----Original Message-----
> >> From: Paolo Bonzini [mailto:address@hidden On Behalf Of
> >> Paolo Bonzini
> >> Sent: Wednesday, November 05, 2014 11:55 PM
> >> To: Michael S. Tsirkin
> >> Cc: Markus Armbruster; SeokYeon Hwang; address@hidden
> >> Subject: Re: [Qemu-devel] [PATCH] pci: fixed mismatch of
> >> error-handling between pci_qdev_init() and qdev
> >>
> >>
> >>
> >> On 05/11/2014 14:28, Michael S. Tsirkin wrote:
> >> > > I think bypassing the question by converting to realize makes the
> >> > > most sense...
> >> >
> >> > I'm fine with doing that but Markus's patches wouldn't yet have
> >> > solved the problem by themselves since init is still around, right?
> >> >
> >> > This probably means fixing this bug can't justify merging the
> >> > realize patchset after freeze.
> >>
> >> Yes, I agree.  I meant that the API is not very well defined.  I
> >> would handle everything else on a case-by-case basis, by reviewing
> >> each init function that is converted to realize.
> >>
> >> Since the patch was for an out-of-tree device, it can wait for 2.3
> anyway.
> >>
> >> Paolo
> >
> > I cannot fully understand your conversation.
> 
> You appear to have a PCIDeviceClass method init() returning a positive
> value.  Doesn't work.  Only values <= 0 do.
> 
> Your proposed fix is to make its caller treat a positive value like a
> negative one.
> 
> Paolo points out that init()'s contract is unclear.  His preferred way of
> clarifying it is to convert PCI from init() to realize(), which has a
> sufficiently clear contract.
> 
> Doesn't help you now.  My "pci: Partial conversion to realize" series,
> will help you once it lands, but only if you convert your device.
> 
> You obviously want a solution earlier.  The one you proposed implicitly
> clarifies the PCIDeviceClass init() contract to "zero means success,
> anything else failure".  I don't think that's a good idea, because it
> makes PCIDeviceClass's init() differ from DeviceClass's.  There, non-
> negative value means success, negative means failure (see
> device_realize()).
> 
> Fix your device not to return positive values instead.
> 
> You could additionally fix pci_qdev_init() to treat positive numbers as
> success, for consistency with device_realize(), but that requires auditing
> all existing PCIDeviceClass init() methods.  Waste of your time, because
> they all go away when we convert to realize().
> 
> > But, I think this patch is still worth before all 'init()' convert to
> > 'realize()'.
> > Moreover, It has no side effect at all.
> 
> I don't like it, because it makes PCIDeviceClass's init() inconsistent
> with DeviceClass's.

I understand completely.
Thank you for your kind explanation.