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@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);
+}
+
+
 static int
 qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
                            virDomainObjPtr vm,
@@ -4132,6 +4143,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
         qemuDomainRemoveSCSIVHostDevice(driver, vm, hostdev);
         break;
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+        qemuDomainRemoveMediatedDevice(driver, vm, hostdev);
         break;
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
         break;
@@ -5059,6 +5071,32 @@ qemuDomainDetachSCSIVHostDevice(virQEMUDriverPtr driver,
     return ret;
 }
 
+
+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;
+
+    return ret;
+}
+
+
 static int
 qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
                                virDomainObjPtr vm,
@@ -5082,6 +5120,9 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
         ret = qemuDomainDetachSCSIVHostDevice(driver, vm, detach);
         break;
+    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+        ret = qemuDomainDetachMediatedDevice(driver, vm, detach);
+        break;
     default:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("hot unplug is not supported for hostdev subsys type '%s'"),
@@ -5111,6 +5152,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
     virDomainHostdevSubsysUSBPtr usbsrc = &subsys->u.usb;
     virDomainHostdevSubsysPCIPtr pcisrc = &subsys->u.pci;
     virDomainHostdevSubsysSCSIPtr scsisrc = &subsys->u.scsi;
+    virDomainHostdevSubsysMediatedDevPtr mdevsrc = &subsys->u.mdev;
     virDomainHostdevDefPtr detach = NULL;
     int idx;
 
@@ -5159,6 +5201,11 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
             }
             break;
         }
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+            virReportError(VIR_ERR_DEVICE_MISSING,
+                           _("mediated device '%s' not found"),
+                           mdevsrc->uuidstr);
+            break;
         case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
             break;
         default:
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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.

> +}
> +
> +
>  static int
>  qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
>                             virDomainObjPtr vm,
> @@ -4132,6 +4143,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
>          qemuDomainRemoveSCSIVHostDevice(driver, vm, hostdev);
>          break;
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> +        qemuDomainRemoveMediatedDevice(driver, vm, hostdev);
>          break;
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
>          break;
> @@ -5059,6 +5071,32 @@ qemuDomainDetachSCSIVHostDevice(virQEMUDriverPtr driver,
>      return ret;
>  }
>  
> +
> +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.

> +
> +    return ret;
> +}
> +
> +
>  static int
>  qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
>                                 virDomainObjPtr vm,
> @@ -5082,6 +5120,9 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
>          ret = qemuDomainDetachSCSIVHostDevice(driver, vm, detach);
>          break;
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> +        ret = qemuDomainDetachMediatedDevice(driver, vm, detach);
> +        break;
>      default:
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("hot unplug is not supported for hostdev subsys type '%s'"),
> @@ -5111,6 +5152,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
>      virDomainHostdevSubsysUSBPtr usbsrc = &subsys->u.usb;
>      virDomainHostdevSubsysPCIPtr pcisrc = &subsys->u.pci;
>      virDomainHostdevSubsysSCSIPtr scsisrc = &subsys->u.scsi;
> +    virDomainHostdevSubsysMediatedDevPtr mdevsrc = &subsys->u.mdev;
>      virDomainHostdevDefPtr detach = NULL;
>      int idx;
>  
> @@ -5159,6 +5201,11 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
>              }
>              break;
>          }
> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> +            virReportError(VIR_ERR_DEVICE_MISSING,
> +                           _("mediated device '%s' not found"),
> +                           mdevsrc->uuidstr);
> +            break;
>          case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
>              break;
>          default:
> -- 
> 2.14.3
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Mar 28, 2018 at 11:47:47 +0200, Erik Skultety wrote:
> 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@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.

Okay, I did not notice that this was registered under the hostdev
removal function rather than the device removal function.

> > > +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.

Fair enough, both points come from the fact that I've assumed that these
are generic device deletion functions as we have for other devices and
not hostdev-specific ones, called from the hostdev removal function.

The confusion originates from the fact that the hostdev-specific workers
use the same prefix as other device deletion fucntions rather than
having something like "hostdev" in the name.

ACK to this patch if you fix the very long line as suggested.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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

[...]

> @@ -5059,6 +5071,32 @@ qemuDomainDetachSCSIVHostDevice(virQEMUDriverPtr driver,
>      return ret;
>  }
>  
> +
> +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"));

This line is very long and can be optimized by moving the formatting
string on the previous line.

> +        return -1;
> +    }
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list