Re: [libvirt] [PATCH 3/4] qemu: hotplug: Introduce hot unplug for mediated devices

On Tue, Mar 27, 2018 at 11:41:45AM +0200, Peter Krempa wrote: > On Tue, Mar 27, 2018 at 10:57:15 +0200, Erik Skultety wrote: > > Mediated devices support hot-{plug,unplug} since their introduction in > > kernel 4.10, however this feature has been missing in libvirt since > > commit ec783d7c introduced a hostdev type for mdevs. > > > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; > > --- > > src/qemu/qemu_hotplug.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > > index 4abc7393b..ff77b47bc 100644 > > --- a/src/qemu/qemu_hotplug.c > > +++ b/src/qemu/qemu_hotplug.c > > @@ -4030,6 +4030,17 @@ qemuDomainRemoveSCSIVHostDevice(virQEMUDriverPtr driver, > > qemuHostdevReAttachSCSIVHostDevices(driver, vm->def->name, &hostdev, 1); > > } > > > > + > > +static void > > +qemuDomainRemoveMediatedDevice(virQEMUDriverPtr driver, > > + virDomainObjPtr vm, > > + virDomainHostdevDefPtr hostdev) > > +{ > > + qemuHostdevReAttachMediatedDevices(driver, vm->def->name, &hostdev, 1); > > + qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); > > Looks like you are missing teardown of the cgroups, and namespace > membership here. I'm not, this is handled from qemuDomainRemoveHostDevice which is called from qemuDomainDetachThisHostDevice right after qemuDomainDetachMediatedDevice was called.

> > +static int > > +qemuDomainDetachMediatedDevice(virQEMUDriverPtr driver, > > + virDomainObjPtr vm, > > + virDomainHostdevDefPtr detach) > > +{ > > + int ret = -1; > > + qemuDomainObjPrivatePtr priv = vm->privateData; > > + > > + if (!detach->info->alias) { > > + virReportError(VIR_ERR_OPERATION_FAILED, > > + "%s", _("device cannot be detached without a device alias")); > > + return -1; > > + } > > + > > + qemuDomainMarkDeviceForRemoval(vm, detach->info); > > + > > + qemuDomainObjEnterMonitor(driver, vm); > > + ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); > > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > > + ret = -1; > > You need to wait for removal here and delete it inplace if the call > returns soon enough.. Also call to qemuDomainResetDeviceRemoval is > missing. All of this is already done as part of qemuDomainDetachThisHostDevice, point 1 being done right at the end of the function mentioned above (qemuDomainWaitForDeviceRemoval), point 2 being satisfied by the last call in the same function.