6:44 p.m.
On 04/05/2016 11:09 AM, Peter Krempa wrote:
Similarly to the DEVICE_DELETED event we will be able to tell when
unplug of certain device types will be rejected by the guest OS. Wire up
the device deletion signalling code to allow handling this.
---
src/qemu/qemu_domain.h | 17 ++++++++++++++++-
src/qemu/qemu_hotplug.c | 30 +++++++++++++++++++++---------
src/qemu/qemu_hotplug.h | 3 ++-
src/qemu/qemu_process.c | 2 +-
4 files changed, 40 insertions(+), 12 deletions(-)
There's a slight conflict here with git master but it should rebase cleanly,
git am --3way couldn't handle it for me for whatever reason
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2b92a90..84b4b16 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -148,6 +148,20 @@ struct qemuDomainJobObj {
typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+/* helper data types for async device unplug */
+typedef enum {
+ QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_NONE = 0,
+ QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_OK,
+ QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_GUEST_REJECTED,
+} qemuDomainUnplugDeviceStatus;
+
+typedef struct _qemuDomainUnplugDevice qemuDomainUnplugDevice;
+typedef qemuDomainUnplugDevice *qemuDomainUnplugDevicePtr;
+struct _qemuDomainUnplugDevice {
+ const char *alias;
+ qemuDomainUnplugDeviceStatus status;
+};
+
typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {
@@ -198,7 +212,8 @@ struct _qemuDomainObjPrivate {
virPerfPtr perf;
- const char *unpluggingDevice; /* alias of the device that is being unplugged */
+ qemuDomainUnplugDevice unplug;
+
FWIW in isolation I'd think 'qemuDomainUnplugDevice' was a function name...
maybe stick with the UnpluggingDevice name. Not a NACK, just an idea if you're
motivated. Also lack Ptr use just looks weird, I'm so used to everything being
a Ptr in libvirt code :)
char **qemuDevices; /* NULL-terminated list of devices aliases
known to QEMU */
bool hookRun; /* true if there was a hook run over this domain */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 134f458..bd0599f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3337,20 +3337,24 @@ qemuDomainMarkDeviceForRemoval(virDomainObjPtr vm,
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
- priv->unpluggingDevice = info->alias;
- else
- priv->unpluggingDevice = NULL;
+ memset(&priv->unplug, 0, sizeof(priv->unplug));
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
+ return;
+
+ priv->unplug.alias = info->alias;
}
static void
qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- priv->unpluggingDevice = NULL;
+ priv->unplug.alias = NULL;
}
/* Returns:
+ * -1 Unplug of the device failed
+ *
* 0 DEVICE_DELETED event is supported and removal of the device did not
* finish in qemuDomainRemoveDeviceWaitTime
*
@@ -3373,17 +3377,23 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
return 1;
until += qemuDomainRemoveDeviceWaitTime;
- while (priv->unpluggingDevice) {
+ while (priv->unplug.alias) {
if ((rc = virDomainObjWaitUntil(vm, until)) == 1)
return 0;
if (rc < 0) {
VIR_WARN("Failed to wait on unplug condition for domain '%s'
"
- "device '%s'", vm->def->name,
priv->unpluggingDevice);
+ "device '%s'", vm->def->name,
priv->unplug.alias);
return 1;
}
}
+ if (priv->unplug.status == QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_GUEST_REJECTED) {
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("unplug of device was rejected by the guest"));
+ return -1;
+ }
+
return 1;
}
Okay, so 0 == didn't succeed in time, 1 == definitely succeeded OR we have no
way of knowing so assume it worked, -1 == definitely failed. That's a lot of
semantics :) But this makes sense, now when the -1 trickles up the stack, it's
when we know for certain that the hotplug failed.
ACK
- Cole
@@ -3395,12 +3405,14 @@
qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
*/
bool
qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
- const char *devAlias)
+ const char *devAlias,
+ qemuDomainUnplugDeviceStatus status)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- if (STREQ_NULLABLE(priv->unpluggingDevice, devAlias)) {
+ if (STREQ_NULLABLE(priv->unplug.alias, devAlias)) {
qemuDomainResetDeviceRemoval(vm);
+ priv->unplug.status = status;
virDomainObjBroadcast(vm);
return true;
}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 4140da3..e6833f0 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -122,6 +122,7 @@ int qemuDomainRemoveDevice(virQEMUDriverPtr driver,
virDomainDeviceDefPtr dev);
bool qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
- const char *devAlias);
+ const char *devAlias,
+ qemuDomainUnplugDeviceStatus status);
#endif /* __QEMU_HOTPLUG_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d9dca74..bbde32c 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1359,7 +1359,7 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
VIR_DEBUG("Device %s removed from domain %p %s",
devAlias, vm, vm->def->name);
- if (qemuDomainSignalDeviceRemoval(vm, devAlias))
+ if (qemuDomainSignalDeviceRemoval(vm, devAlias,
QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_OK))
goto cleanup;
if (VIR_ALLOC(processEvent) < 0)