3:57 a.m.
Libvirt shouldn't forbid the operation as unsupported. In fact, from VFIO point
of view, mdevs have supported hot plug since the beginning. Then it's up to the
3rd party vendor driver whether it can cope with this feature reliably or not.
Erik Skultety (4):
qemu: hotplug: Provide a string of a subsystem type instead of an int
qemu: hotplug: Introduce hot plug support for mediated devices
qemu: hotplug: Introduce hot unplug for mediated devices
news: Update release news with mediated devices hot {plug,unplug}
docs/news.xml | 14 +++++
src/qemu/qemu_hotplug.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 149 insertions(+), 1 deletion(-)
--
2.14.3
3:57 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Provide a string of a subsystem type instead of an int
If one tries to detach a non-existent device, the error they get is:
"Unexpected hostdev type <int>". Let's use ToString conversion, since
the XML parser would have complained already if the type to be unplugged
was unknown to libvirt.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 49af4d4ff..6ec401e21 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5077,7 +5077,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected hostdev type %d"), subsys->type);
+ _("unexpected hostdev type '%s'"),
+ virDomainHostdevSubsysTypeToString(subsys->type));
break;
}
return -1;
--
2.14.3
4:17 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Provide a string of a subsystem type instead of an int
On Tue, Mar 27, 2018 at 10:57:13 +0200, Erik Skultety wrote:
If one tries to detach a non-existent device, the error they get is:
"Unexpected hostdev type <int>". Let's use ToString conversion,
since
the XML parser would have complained already if the type to be unplugged
was unknown to libvirt.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 49af4d4ff..6ec401e21 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5077,7 +5077,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected hostdev type %d"), subsys->type);
+ _("unexpected hostdev type '%s'"),
+ virDomainHostdevSubsysTypeToString(subsys->type));
If the type is out of range of the enum this will return NULL. So you
need to use NULLSTR()
ACK with that tweak
4:24 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Provide a string of a subsystem type instead of an int
On Tue, Mar 27, 2018 at 11:17:51AM +0200, Peter Krempa wrote:
On Tue, Mar 27, 2018 at 10:57:13 +0200, Erik Skultety wrote:
> If one tries to detach a non-existent device, the error they get is:
> "Unexpected hostdev type <int>". Let's use ToString conversion,
since
> the XML parser would have complained already if the type to be unplugged
> was unknown to libvirt.
>
> Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> ---
> src/qemu/qemu_hotplug.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 49af4d4ff..6ec401e21 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -5077,7 +5077,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
> break;
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("unexpected hostdev type %d"),
subsys->type);
> + _("unexpected hostdev type '%s'"),
> + virDomainHostdevSubsysTypeToString(subsys->type));
If the type is out of range of the enum this will return NULL. So you
need to use NULLSTR()
ACK with that tweak
Actually any default: or _LAST: case should use virReportEnumRangeError().
The virDomainHostdevSubsysTypeToString() methods should only be used in
explicitly matched enums constants.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:42 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Provide a string of a subsystem type instead of an int
On Tue, Mar 27, 2018 at 10:24:54AM +0100, Daniel P. Berrangé wrote:
On Tue, Mar 27, 2018 at 11:17:51AM +0200, Peter Krempa wrote:
> On Tue, Mar 27, 2018 at 10:57:13 +0200, Erik Skultety wrote:
> > If one tries to detach a non-existent device, the error they get is:
> > "Unexpected hostdev type <int>". Let's use ToString
conversion, since
> > the XML parser would have complained already if the type to be unplugged
> > was unknown to libvirt.
> >
> > Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> > ---
> > src/qemu/qemu_hotplug.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > index 49af4d4ff..6ec401e21 100644
> > --- a/src/qemu/qemu_hotplug.c
> > +++ b/src/qemu/qemu_hotplug.c
> > @@ -5077,7 +5077,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
> > break;
> > default:
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("unexpected hostdev type %d"),
subsys->type);
> > + _("unexpected hostdev type
'%s'"),
> > +
virDomainHostdevSubsysTypeToString(subsys->type));
>
> If the type is out of range of the enum this will return NULL. So you
> need to use NULLSTR()
>
> ACK with that tweak
Actually any default: or _LAST: case should use virReportEnumRangeError().
The virDomainHostdevSubsysTypeToString() methods should only be used in
explicitly matched enums constants.
I understand having this kind of safe guard, but we have a specific code path
here where the XML parser has already taken care of non-existent types and
we're left with only types which do not support or do not care about hotplug,
therefore an error like "Unexpected value <number> blah" doesn't IMHO
make
sense and doesn't tell you anything, we should reformulate the whole
'unexpected' part of the error, we expected it just fine, it's just
there's no
such device in the domain and even if it was, we don't know whether we can do
hotplug on it, that's what you can read from the code as you follow the detach
procedure.
Erik
8:46 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Provide a string of a subsystem type instead of an int
On Tue, Mar 27, 2018 at 03:42:08PM +0200, Erik Skultety wrote:
On Tue, Mar 27, 2018 at 10:24:54AM +0100, Daniel P. Berrangé wrote:
> On Tue, Mar 27, 2018 at 11:17:51AM +0200, Peter Krempa wrote:
> > On Tue, Mar 27, 2018 at 10:57:13 +0200, Erik Skultety wrote:
> > > If one tries to detach a non-existent device, the error they get is:
> > > "Unexpected hostdev type <int>". Let's use ToString
conversion, since
> > > the XML parser would have complained already if the type to be unplugged
> > > was unknown to libvirt.
> > >
> > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> > > ---
> > > src/qemu/qemu_hotplug.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > > index 49af4d4ff..6ec401e21 100644
> > > --- a/src/qemu/qemu_hotplug.c
> > > +++ b/src/qemu/qemu_hotplug.c
> > > @@ -5077,7 +5077,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr
driver,
> > > break;
> > > default:
> > > virReportError(VIR_ERR_INTERNAL_ERROR,
> > > - _("unexpected hostdev type %d"),
subsys->type);
> > > + _("unexpected hostdev type
'%s'"),
> > > +
virDomainHostdevSubsysTypeToString(subsys->type));
> >
> > If the type is out of range of the enum this will return NULL. So you
> > need to use NULLSTR()
> >
> > ACK with that tweak
>
> Actually any default: or _LAST: case should use virReportEnumRangeError().
> The virDomainHostdevSubsysTypeToString() methods should only be used in
> explicitly matched enums constants.
I understand having this kind of safe guard, but we have a specific code path
here where the XML parser has already taken care of non-existent types and
we're left with only types which do not support or do not care about hotplug,
therefore an error like "Unexpected value <number> blah" doesn't IMHO
make
sense and doesn't tell you anything, we should reformulate the whole
'unexpected' part of the error, we expected it just fine, it's just
there's no
such device in the domain and even if it was, we don't know whether we can do
hotplug on it, that's what you can read from the code as you follow the detach
procedure.
The point is to be robust against mistakes in the XML parser, or code that
runs after it. We've had cases in the code where we parsed the wrong enum
into a field, so we would have ended up with unexpected values. Or something
could mistakenly overwrite the type value after parsing but before calling
this function.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:57 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: hotplug: Provide a string of a subsystem type instead of an int
On Tue, Mar 27, 2018 at 02:46:49PM +0100, Daniel P. Berrangé wrote:
On Tue, Mar 27, 2018 at 03:42:08PM +0200, Erik Skultety wrote:
> On Tue, Mar 27, 2018 at 10:24:54AM +0100, Daniel P. Berrangé wrote:
> > On Tue, Mar 27, 2018 at 11:17:51AM +0200, Peter Krempa wrote:
> > > On Tue, Mar 27, 2018 at 10:57:13 +0200, Erik Skultety wrote:
> > > > If one tries to detach a non-existent device, the error they get is:
> > > > "Unexpected hostdev type <int>". Let's use
ToString conversion, since
> > > > the XML parser would have complained already if the type to be
unplugged
> > > > was unknown to libvirt.
> > > >
> > > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> > > > ---
> > > > src/qemu/qemu_hotplug.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> > > > index 49af4d4ff..6ec401e21 100644
> > > > --- a/src/qemu/qemu_hotplug.c
> > > > +++ b/src/qemu/qemu_hotplug.c
> > > > @@ -5077,7 +5077,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr
driver,
> > > > break;
> > > > default:
> > > > virReportError(VIR_ERR_INTERNAL_ERROR,
> > > > - _("unexpected hostdev type
%d"), subsys->type);
> > > > + _("unexpected hostdev type
'%s'"),
> > > > +
virDomainHostdevSubsysTypeToString(subsys->type));
> > >
> > > If the type is out of range of the enum this will return NULL. So you
> > > need to use NULLSTR()
> > >
> > > ACK with that tweak
> >
> > Actually any default: or _LAST: case should use virReportEnumRangeError().
> > The virDomainHostdevSubsysTypeToString() methods should only be used in
> > explicitly matched enums constants.
>
> I understand having this kind of safe guard, but we have a specific code path
> here where the XML parser has already taken care of non-existent types and
> we're left with only types which do not support or do not care about hotplug,
> therefore an error like "Unexpected value <number> blah" doesn't
IMHO make
> sense and doesn't tell you anything, we should reformulate the whole
> 'unexpected' part of the error, we expected it just fine, it's just
there's no
> such device in the domain and even if it was, we don't know whether we can do
> hotplug on it, that's what you can read from the code as you follow the detach
> procedure.
The point is to be robust against mistakes in the XML parser, or code that
runs after it. We've had cases in the code where we parsed the wrong enum
into a field, so we would have ended up with unexpected values. Or something
could mistakenly overwrite the type value after parsing but before calling
this function.
Fair enough, I'm going to drop this patch for the time being and create a
bite-sized task to add virReportEnumRangeError to all the places where it's
missing.
Erik
3:57 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: hotplug: Introduce hot plug support for mediated devices
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>
---
src/qemu/qemu_hotplug.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6ec401e21..4abc7393b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2567,6 +2567,88 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver,
}
+static int
+qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainHostdevDefPtr hostdev)
+{
+ int ret = -1;
+ char *devstr = NULL;
+ bool added = false;
+ bool teardowncgroup = false;
+ bool teardownlabel = false;
+ bool teardowndevice = false;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV,
+ { .hostdev = hostdev } };
+
+ if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ return -1;
+
+ if (qemuHostdevPrepareMediatedDevices(driver,
+ vm->def->name,
+ &hostdev,
+ 1) < 0)
+ goto cleanup;
+ added = true;
+
+ if (qemuDomainNamespaceSetupHostdev(vm, hostdev) < 0)
+ goto cleanup;
+ teardowndevice = true;
+
+ if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
+ goto cleanup;
+ teardowncgroup = true;
+
+ if (qemuSecuritySetHostdevLabel(driver, vm, hostdev) < 0)
+ goto cleanup;
+ teardownlabel = true;
+
+ if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
+ goto cleanup;
+
+ if (!(devstr = qemuBuildHostdevMediatedDevStr(vm->def, hostdev,
+ priv->qemuCaps)))
+ goto cleanup;
+
+ if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0)
+ goto cleanup;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ ret = qemuMonitorAddDevice(priv->mon, devstr);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
+ if (ret < 0)
+ goto cleanup;
+
+ VIR_APPEND_ELEMENT_INPLACE(vm->def->hostdevs, vm->def->nhostdevs,
hostdev);
+ ret = 0;
+ cleanup:
+ if (ret < 0) {
+ if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
+ VIR_WARN("Unable to remove host device cgroup ACL on hotplug
fail");
+ if (teardownlabel &&
+ qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0)
+ VIR_WARN("Unable to restore host device labelling on hotplug
fail");
+ if (teardowndevice &&
+ qemuDomainNamespaceTeardownHostdev(vm, hostdev) < 0)
+ VIR_WARN("Unable to remove host device from /dev");
+ if (added)
+ qemuHostdevReAttachMediatedDevices(driver,
+ vm->def->name,
+ vm->def->hostdevs,
+ vm->def->nhostdevs);
+ qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
+ }
+ VIR_FREE(devstr);
+ return ret;
+}
+
+
int
qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -2602,6 +2684,10 @@ qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
if (qemuDomainAttachSCSIVHostDevice(driver, vm, hostdev) < 0)
goto error;
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+ if (qemuDomainAttachMediatedDevice(driver, vm, hostdev) < 0)
+ goto error;
+ break;
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.14.3
4:33 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: hotplug: Introduce hot plug support for mediated devices
On Tue, Mar 27, 2018 at 10:57:14 +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.
I think the feature was missing since d77e1a9642fe1efe9aa5f737a640354.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6ec401e21..4abc7393b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2567,6 +2567,88 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver,
}
+static int
+qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainHostdevDefPtr hostdev)
+{
+ int ret = -1;
+ char *devstr = NULL;
+ bool added = false;
+ bool teardowncgroup = false;
+ bool teardownlabel = false;
+ bool teardowndevice = false;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV,
+ { .hostdev = hostdev } };
+
+ if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0)
+ return -1;
+
+ if (qemuHostdevPrepareMediatedDevices(driver,
+ vm->def->name,
+ &hostdev,
+ 1) < 0)
+ goto cleanup;
+ added = true;
+
+ if (qemuDomainNamespaceSetupHostdev(vm, hostdev) < 0)
+ goto cleanup;
+ teardowndevice = true;
+
+ if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
+ goto cleanup;
+ teardowncgroup = true;
+
+ if (qemuSecuritySetHostdevLabel(driver, vm, hostdev) < 0)
+ goto cleanup;
+ teardownlabel = true;
+
+ if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
+ goto cleanup;
+
+ if (!(devstr = qemuBuildHostdevMediatedDevStr(vm->def, hostdev,
+ priv->qemuCaps)))
+ goto cleanup;
Usually we'd call this prior to setting up everything, bug given that
this does not do any validation it's okay.
+
+ if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0)
Spaces around + sign.
+ goto cleanup;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ ret = qemuMonitorAddDevice(priv->mon, devstr);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
+ if (ret < 0)
+ goto cleanup;
+
+ VIR_APPEND_ELEMENT_INPLACE(vm->def->hostdevs, vm->def->nhostdevs,
hostdev);
+ ret = 0;
+ cleanup:
+ if (ret < 0) {
+ if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
+ VIR_WARN("Unable to remove host device cgroup ACL on hotplug
fail");
+ if (teardownlabel &&
+ qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0)
+ VIR_WARN("Unable to restore host device labelling on hotplug
fail");
+ if (teardowndevice &&
+ qemuDomainNamespaceTeardownHostdev(vm, hostdev) < 0)
+ VIR_WARN("Unable to remove host device from /dev");
+ if (added)
+ qemuHostdevReAttachMediatedDevices(driver,
+ vm->def->name,
+ vm->def->hostdevs,
+ vm->def->nhostdevs);
All of them?!?
+ qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL);
+ }
+ VIR_FREE(devstr);
+ return ret;
+}
+
+
int
qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -2602,6 +2684,10 @@ qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
if (qemuDomainAttachSCSIVHostDevice(driver, vm, hostdev) < 0)
goto error;
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
+ if (qemuDomainAttachMediatedDevice(driver, vm, hostdev) < 0)
+ goto error;
+ break;
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.14.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
3:57 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: hotplug: Introduce hot unplug for mediated devices
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>
---
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
4:41 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: hotplug: Introduce hot unplug for mediated devices
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>
---
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:47 a.m.
New subject: [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>
> ---
> 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
4:59 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: hotplug: Introduce hot unplug for mediated devices
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(a)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.
4:53 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: hotplug: Introduce hot unplug for mediated devices
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>
---
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;
+ }
3:57 a.m.
New subject: [libvirt] [PATCH 4/4] news: Update release news with mediated devices hot {plug, unplug}
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
docs/news.xml | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml
index 108889574..6075cc9ca 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -92,6 +92,20 @@
previously started domains to 256, or restart them.
</description>
</change>
+ <change>
+ <summary>
+ qemu: Support hot plug and hot unplug with mediated devices
+ </summary>
+ <description>
+ Libvirt now allows mediated devices to be hot plugged and hot
+ unplugged from a guest rather than reporting an error that this isn't
+ supported. In fact, this has been supported with mediated devices
+ since their first appearance in 4.10 kernel. Be ware though that this
+ still doesn't guarantee any functionality with respect to the guest
+ driver which might not cope with a device being hot plugged or
+ unplugged.
+ </description>
+ </change>
</section>
<section title="Bug fixes">
</section>
--
2.14.3
4:52 a.m.
New subject: [libvirt] [PATCH 4/4] news: Update release news with mediated devices hot {plug, unplug}
On Tue, Mar 27, 2018 at 10:57:16 +0200, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
docs/news.xml | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml
index 108889574..6075cc9ca 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -92,6 +92,20 @@
previously started domains to 256, or restart them.
</description>
</change>
+ <change>
+ <summary>
+ qemu: Support hot plug and hot unplug with mediated devices
s/with/of/
+ </summary>
+ <description>
+ Libvirt now allows mediated devices to be hot plugged and hot
+ unplugged from a guest rather than reporting an error that this isn't
The end of the sentence is kind of obvious.
+ supported. In fact, this has been supported with mediated
devices
I'd say 'In fact, kernel supported this ...' so that it's obvious which
component you are speaking of.
+ since their first appearance in 4.10 kernel. Be ware
though that this
+ still doesn't guarantee any functionality with respect to the guest
+ driver which might not cope with a device being hot plugged or
+ unplugged.
As with any hotplug. The drivers in the guest can ignore the device, or
disallow unplug if it is not supported.
If you want to point out that drivers currently don't support it in some
cases, please be more direct.
2461
days inactive
2462
days old
15 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Erik Skultety -
Peter Krempa