From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MsfkO-0003z8-LS for qemu-devel@nongnu.org; Tue, 29 Sep 2009 12:40:16 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MsfkH-0003rT-Mj for qemu-devel@nongnu.org; Tue, 29 Sep 2009 12:40:15 -0400 Received: from [199.232.76.173] (port=51724 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MsfkH-0003qy-C3 for qemu-devel@nongnu.org; Tue, 29 Sep 2009 12:40:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5355) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MsfkG-0005bt-2v for qemu-devel@nongnu.org; Tue, 29 Sep 2009 12:40:08 -0400 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n8TFqjaa030428 for ; Tue, 29 Sep 2009 11:52:45 -0400 Date: Tue, 29 Sep 2009 17:50:46 +0200 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH 12/13] pci: move unregister from PCIDevice to PCIDeviceInfo Message-ID: <20090929155046.GA13634@redhat.com> References: <1253611767-6483-1-git-send-email-kraxel@redhat.com> <1253611767-6483-13-git-send-email-kraxel@redhat.com> <20090923155844.GB18203@redhat.com> <4ABC75FC.6030408@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4ABC75FC.6030408@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org On Fri, Sep 25, 2009 at 09:49:16AM +0200, Gerd Hoffmann wrote: > Hi, > >>> static int pci_unregister_device(DeviceState *dev) > >>> + msix_uninit(pci_dev); > >> Since devices call msix_add, I think it is cleaner to have them >> uninit it as well in their exit routines. > > Would work too. But this way you can't miss the msix_uninit() call by > accident. What kind of accident? We don't want a garbage collector in here, do we? > It also saves a few lines of code. msix_uninit() carefully > checks whenever msix is actually enabled and it is slow path, so calling > it on non-msix devices isn't a big deal IMHO. > > cheers, > Gerd Yes, it works, but I find it confusing: IMO init and uninit being in separate modules just makes code impossible to figure out. For example, if there's a failure adding the device, device has to call msix_uninit itself, and IMO it's better to have error handling and cleanup in sync. -- MST