Re: [Qemu-devel] [PATCH 12/13] pci: move unregister from PCIDevice to PCIDeviceInfo

[Michael S. Tsirkin]

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