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

[Erik Skultety]

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 at redhat.com>
> > ---
> >  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.

Erik