[libvirt] [PATCHv2 01/12] pci: eliminate unused driver arg from virPCIDeviceDetach

[Daniel P. Berrange]

On Tue, Jun 25, 2013 at 12:35:03PM -0400, Laine Stump wrote:
> On 06/25/2013 05:50 AM, Daniel P. Berrange wrote:
> > On Mon, Jun 24, 2013 at 11:05:27PM -0400, Laine Stump wrote:
> >> The driver arg to virPCIDeviceDetach is no longer used (the name of
> >> the stub driver is now set in the virPCIDevice object, and
> >> virPCIDeviceDetach retrieves it from there). Remove it.
> > What happens when libvirtd is restarted ? Are we correctly preserving
> > the original value across restart, or are we capable of auto-detecting
> > the correct value again ?
> 
> That's a good question, for two reasons: 1) I didn't know the answer for
> sure when you asked it, and 2) I went back over the code, learned the
> answer, and it was a good answer :-)
> 
> Originally the stubDriverName in the virPCIDevice object was only used
> for a short period - during qemu's "device Preparation" it will create a
> virPCIDeviceList from the virDomainHostdevDef list, then later iterate
> through the virPCIDeviceList to detach all the devices from their host
> drivers; during this detach loop, the original viDomainHostdevDef isn't
> available, so an indicator of which stub to bind to must be stashed in
> the virPCIDevice object when it is created.
> 
> Later it became useful to be able to rely on the stubDriverName always
> being correct (at least in the activePciHostdevs list; it doesn't matter
> for inactivePciHostdevs). Fortunately that list is recreated whenever
> libvirt restarts by calling qemuUpdateActivePciHostdevs() for every
> active domain. That function also updates stubDriverName for each active
> hostdev based on the config (which must be the actual driver in use,
> otherwise we would have encountered an error when the device was
> originally being attached).
> 
> So, the answer is yes, we auto-set the correct value in the virPCIDevice
> object when libvirtd restarts.

ACK, to the patch then.

> However, this entire topic brings up a question that I've had for awhile
> - what happens to the "inactivePciHostdevs" list when libvirt restarts?
> This is the list of devices that have been detached from the host by
> libvirt's virNodeDevice API, but are not currently in use by any domain.
> This list isn't stored in any status file, and is not reconstructed when
> libvirtd restarts (the information to do that doesn't exist). I *think*
> the answer is that it doesn't matter, because libvirt will just use the
> device anyway, then put it on the inactive list when it's done with it.
> But that suggests the question "Then why have the inactive list?" I
> don't know the answer to that one, but it does seem like either we
> should be reconstructing inactivePciHostdevs when libvirtd restarts, or
> we should just give up and not use (or maintain) it.

Yes, that is odd. Would have to look back to see why we introduced
this list in the first place. If we can kill it so much the better

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|