[libvirt] [PATCH 21/24] hostdev: Stop early if unmanaged devices have not been detached

[Andrea Bolognani]

On Tue, 2016-03-15 at 14:38 -0400, John Ferlan wrote:
> > As for your question, a device might end up in the inactive
> > list because of:
> > 
> >   * PreparePCIDevices(), step 2, if managed. Just temporarily,
> >     it's moved to the active list in step 5
> > 
> >   * ReattachPCIDevices(), step 4, whether managed or
> >     unmanaged. If it's managed is also removed from the
> >     inactive list in the same step, if it's unmanaged it
> >     will remain in the inactive list
> > 
> >   * PCINodeDeviceDetach()
> > 
> > This is not including rollback paths. So it ends up in the
> > inactive list when it's either *not yet* active, or *no
> > longer* active.
> > 
> > > I don't disagree this is an important step, but it's the "how" we
> > > determine this that I'm questioning.
> > 
> > Hopefully the above helped :)
> 
> Partially, let's consider an initial startup where Prepare is called
> (and I assume we don't call either virHostdevReattachPCIDevice or
> virHostdevPCINodeDeviceDetach yet).
> 
> So how does this code check if the device has been manually detached
> from the host? Step 1 doesn't add to the inactiveList and step 2 only
> adds managed devices.  How would that lookaside element be on the
> inactiveList - what put it there?
> 
> Is there some magic w/ virHostdevPCINodeDeviceDetach that I'm missing?

Oh, I see what you mean now!

The long and short of it is that the new code simply doesn't handle
the situation you're describing :)

I wonder what's the proper way to handle this. The way it worked
before was *way* implicit, and the less implicit stuff we have,
the better.

I guess checking the driver the device is bound to, and adding
it to the inactive list if it's one of the known stub drivers
would do the trick for now. Does that sound reasonable? I'll try
and cobble together an implementation.

Long term, as I mentioned in the past, we should implement a
way for the active and inactive list to be stored to disk and
read back at startup time, ensuring all information are
preserved across daemon restarts.

I'd like to work on that as a follow-up enhancement, though.
For now, creating a solid base to work on (while avoiding
regressions) should be the main focus.

Thanks for catching this!

-- 
Andrea Bolognani
Software Engineer - Virtualization Team