[libvirt] [PATCH 5/6] qemu: Reduce memlock limit after detaching hostdev

[Andrea Bolognani]

On Thu, 2015-12-10 at 12:11 -0500, John Ferlan wrote:
> > > Since the add was in qemuDomainAttachHostPCIDevice, why is the remove
> > > not in qemuDomainDetachHostPCIDevice?  Doing it here would mean it's
> > > call for any host device removal - whether or not it was ever added.
> > 
> > Because qemuDomainDetachHostPCIDevice() is where we ask for the device
> > to be removed from the guest, but we have to wait for it to actually
> > be removed (and for the guest definition to be updated) before changing
> > the memory locking limit.
> > 
> > I guess I could move this code to qemuDomainDetachThisHostDevice(), but
> > keeping it close to where the guest definition is updated looks cleaner
> > to me.
> 
> Yeah - this all got confusing as I was paging back and forth...
> "DetachDevice" and "RemoveDevice"... Then I reviewed another change for
> shmem and got even more confused.

I know what you mean: the code could sure use some moving around to
make it a bit more symmetric. And what about the names?

  qemuDomainDetachDiskDevice()
  qemuDomainDetachDeviceDiskLive()

Ehm...

  qemuDomainDetachHostPCIDevice()
  qemuDomainDetachHostDevice()
  qemuDomainDetachThisHostDevice()

Oh boy :)

> > I also fail to see how a device that was never added could be removed,
> > could you please elaborate?
> 
> Since qemuDomainRemoveHostDevice could be called for any host device
> removal and not specifically the HostPCIDevice, I was pointing out by
> calling qemuDomainAdjustMaxMemLock there you could be calling it even
> when a HostPCIDevice wasn't being removed.

Oh, I get it now.

Calling qemuDomainAdjustMaxMemLock() more than necessary shouldn't
really cause any harm, but adding a check for

  hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI

around the call is even better.

Would that address your concerns?

> It was clear to me when I wrote it ;-)

Again, I know exactly what you mean ;)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team