From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46901) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmJZf-0007XQ-6h for qemu-devel@nongnu.org; Thu, 06 Nov 2014 04:41:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XmJZY-0005vH-K0 for qemu-devel@nongnu.org; Thu, 06 Nov 2014 04:41:51 -0500 Received: from mailout1.samsung.com ([203.254.224.24]:45432) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmJZY-0005oA-3k for qemu-devel@nongnu.org; Thu, 06 Nov 2014 04:41:44 -0500 Received: from epcpsbgm1.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NEM00KX52XE5220@mailout1.samsung.com> for qemu-devel@nongnu.org; Thu, 06 Nov 2014 18:41:38 +0900 (KST) From: SeokYeon Hwang References: <1415182311-13799-1-git-send-email-syeon.hwang@samsung.com> <20141105124609.GA4354@redhat.com> <87lhnpn5y1.fsf@blackfin.pond.sub.org> <545A23A7.7090401@redhat.com> <20141105132815.GA4884@redhat.com> <545A3A61.50001@redhat.com> <045901cff969$02203060$06609120$@samsung.com> <87wq784rdr.fsf@blackfin.pond.sub.org> In-reply-to: <87wq784rdr.fsf@blackfin.pond.sub.org> Date: Thu, 06 Nov 2014 18:41:41 +0900 Message-id: <066901cff9a5$de98a720$9bc9f560$@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-transfer-encoding: 7bit Content-language: ko Subject: Re: [Qemu-devel] [PATCH] pci: fixed mismatch of error-handling between pci_qdev_init() and qdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Markus Armbruster' Cc: 'Paolo Bonzini' , qemu-devel@nongnu.org, "'Michael S. Tsirkin'" > -----Original Message----- > From: Markus Armbruster [mailto:armbru@redhat.com] > Sent: Thursday, November 06, 2014 6:21 PM > To: SeokYeon Hwang > Cc: 'Paolo Bonzini'; 'Michael S. Tsirkin'; qemu-devel@nongnu.org > Subject: Re: [Qemu-devel] [PATCH] pci: fixed mismatch of error-handling > between pci_qdev_init() and qdev > > SeokYeon Hwang writes: > > >> -----Original Message----- > >> From: Paolo Bonzini [mailto:paolo.bonzini@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 understand completely. Thank you for your kind explanation.