On Thu, Nov 06, 2014 at 10:20:32AM +0100, Markus Armbruster wrote: > SeokYeon Hwang <syeon.hw...@samsung.com> writes: > > >> -----Original Message----- > >> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo > >> Bonzini > >> Sent: Wednesday, November 05, 2014 11:55 PM > >> To: Michael S. Tsirkin > >> Cc: Markus Armbruster; SeokYeon Hwang; qemu-devel@nongnu.org > >> 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 agree with Markus here. A positive return value should not indicate an error. -- MST
