10:09 a.m.
Add a new libvirt event VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED that will
handle failure to unplug a device if we are certain that it will not be unplugged
and wire up the ACPI_DEVICE_OST qemu event that is emitted on memory hotunplug
failure so that the event is propagated and the API fails in such case.
Along with that a few refactors were necessary.
Peter Krempa (7):
qemu: hotplug: Properly handle errors in
qemuDomainWaitForDeviceRemoval
qemu: hotplug: Refactor semantics of qemuDomainWaitForDeviceRemoval
qemu: Use domain condition for device removal singalling
qemu: hotplug: Add support for signalling device unplug failure
Add VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event
qemu: monitor: Add support for ACPI_DEVICE_OST event handling
qemu: process: Wire up ACPI OST events to notify users of failed
memory unplug
daemon/remote.c | 36 +++++++++++++
include/libvirt/libvirt-domain.h | 21 ++++++++
src/conf/domain_event.c | 84 ++++++++++++++++++++++++++++++
src/conf/domain_event.h | 7 +++
src/libvirt_private.syms | 2 +
src/qemu/qemu_domain.c | 4 --
src/qemu/qemu_domain.h | 18 ++++++-
src/qemu/qemu_hotplug.c | 110 +++++++++++++++++----------------------
src/qemu/qemu_hotplug.h | 3 +-
src/qemu/qemu_monitor.c | 19 +++++++
src/qemu/qemu_monitor.h | 18 +++++++
src/qemu/qemu_monitor_json.c | 39 ++++++++++++++
src/qemu/qemu_process.c | 71 ++++++++++++++++++++++++-
src/remote/remote_driver.c | 30 +++++++++++
src/remote/remote_protocol.x | 14 ++++-
src/remote_protocol-structs | 6 +++
tools/virsh-domain.c | 18 +++++++
17 files changed, 428 insertions(+), 72 deletions(-)
--
2.8.0
10:09 a.m.
New subject: [libvirt] [PATCH 1/7] qemu: hotplug: Properly handle errors in qemuDomainWaitForDeviceRemoval
Callers ignore if this function returns -1 and continue as though the
DEVICE_DELETED event was not received. Since we can't be sure that the
event was not received we should behave as if the event was not
supported and remove the device definition right away. The error
fortunately won't really happen here.
---
src/qemu/qemu_hotplug.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 48bea6a..6c619e9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3351,8 +3351,8 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
}
/* Returns:
- * -1 on error
- * 0 when DEVICE_DELETED event is unsupported
+ * 0 when DEVICE_DELETED event is unsupported, or we failed to reliably wait
+ * for the event
* 1 when DEVICE_DELETED arrived before the timeout and the caller is
* responsible for finishing the removal
* 2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
@@ -3367,7 +3367,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
return 0;
if (virTimeMillisNow(&until) < 0)
- return -1;
+ return 0;
until += qemuDomainRemoveDeviceWaitTime;
while (priv->unpluggingDevice) {
@@ -3376,9 +3376,9 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
if (errno == ETIMEDOUT) {
return 2;
} else {
- virReportSystemError(errno, "%s",
- _("Unable to wait on unplug condition"));
- return -1;
+ VIR_WARN("Failed to wait on unplug condition for domain '%s'
"
+ "device '%s'", vm->def->name,
priv->unpluggingDevice);
+ return 0;
}
}
}
--
2.8.0
6:14 p.m.
New subject: [libvirt] [PATCH 1/7] qemu: hotplug: Properly handle errors in qemuDomainWaitForDeviceRemoval
On 04/05/2016 11:09 AM, Peter Krempa wrote:
Callers ignore if this function returns -1 and continue as though
the
DEVICE_DELETED event was not received. Since we can't be sure that the
event was not received we should behave as if the event was not
supported and remove the device definition right away. The error
fortunately won't really happen here.
---
src/qemu/qemu_hotplug.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 48bea6a..6c619e9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3351,8 +3351,8 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
}
/* Returns:
- * -1 on error
- * 0 when DEVICE_DELETED event is unsupported
+ * 0 when DEVICE_DELETED event is unsupported, or we failed to reliably wait
+ * for the event
* 1 when DEVICE_DELETED arrived before the timeout and the caller is
* responsible for finishing the removal
* 2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
@@ -3367,7 +3367,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
return 0;
if (virTimeMillisNow(&until) < 0)
- return -1;
+ return 0;
until += qemuDomainRemoveDeviceWaitTime;
while (priv->unpluggingDevice) {
@@ -3376,9 +3376,9 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
if (errno == ETIMEDOUT) {
return 2;
} else {
- virReportSystemError(errno, "%s",
- _("Unable to wait on unplug
condition"));
- return -1;
+ VIR_WARN("Failed to wait on unplug condition for domain
'%s' "
+ "device '%s'", vm->def->name,
priv->unpluggingDevice);
+ return 0;
}
}
}
Makes sense, and I verified every caller is just checking the return value ==
1. ACK
- Cole
10:09 a.m.
New subject: [libvirt] [PATCH 2/7] qemu: hotplug: Refactor semantics of qemuDomainWaitForDeviceRemoval
Neither of the callers cares whether the DEVICE_DELETED event isn't
supported or the event was received. Simplify the code and callers by
unifying the two values and changing the return value constants so that
a temporary variable can be omitted.
---
src/qemu/qemu_hotplug.c | 67 +++++++++++++++----------------------------------
1 file changed, 20 insertions(+), 47 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6c619e9..7317089 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3351,11 +3351,13 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
}
/* Returns:
- * 0 when DEVICE_DELETED event is unsupported, or we failed to reliably wait
- * for the event
- * 1 when DEVICE_DELETED arrived before the timeout and the caller is
- * responsible for finishing the removal
- * 2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
+ * 0 DEVICE_DELETED event is supported and removal of the device did not
+ * finish in qemuDomainRemoveDeviceWaitTime
+ *
+ * 1 when the caller is responsible for finishing the device removal:
+ * - DEVICE_DELETED event is unsupported
+ * - DEVICE_DELETED event arrived before the timeout time
+ * - we failed to reliably wait for the event and thus use fallback behavior
*/
static int
qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
@@ -3364,21 +3366,21 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
unsigned long long until;
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
- return 0;
+ return 1;
if (virTimeMillisNow(&until) < 0)
- return 0;
+ return 1;
until += qemuDomainRemoveDeviceWaitTime;
while (priv->unpluggingDevice) {
if (virCondWaitUntil(&priv->unplugFinished,
&vm->parent.lock, until) < 0) {
if (errno == ETIMEDOUT) {
- return 2;
+ return 0;
} else {
VIR_WARN("Failed to wait on unplug condition for domain '%s'
"
"device '%s'", vm->def->name,
priv->unpluggingDevice);
- return 0;
+ return 1;
}
}
}
@@ -3414,7 +3416,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
- int rc;
if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -3468,11 +3469,8 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1)
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
- else
- ret = 0;
cleanup:
qemuDomainResetDeviceRemoval(vm);
@@ -3486,7 +3484,6 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
- int rc;
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
virReportError(VIR_ERR_OPERATION_FAILED,
@@ -3514,11 +3511,8 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1)
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
ret = qemuDomainRemoveDiskDevice(driver, vm, detach);
- else
- ret = 0;
cleanup:
qemuDomainResetDeviceRemoval(vm);
@@ -3634,7 +3628,6 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
int idx, ret = -1;
virDomainControllerDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
- int rc;
if ((idx = virDomainControllerFind(vm->def,
dev->data.controller->type,
@@ -3702,11 +3695,8 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1)
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
ret = qemuDomainRemoveControllerDevice(driver, vm, detach);
- else
- ret = 0;
cleanup:
qemuDomainResetDeviceRemoval(vm);
@@ -3846,10 +3836,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
if (ret < 0) {
if (virDomainObjIsActive(vm))
virDomainAuditHostdev(vm, detach, "detach", false);
- } else {
- int rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1)
- ret = qemuDomainRemoveHostDevice(driver, vm, detach);
+ } else if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
+ ret = qemuDomainRemoveHostDevice(driver, vm, detach);
}
qemuDomainResetDeviceRemoval(vm);
@@ -3940,7 +3928,6 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
int detachidx, ret = -1;
virDomainNetDefPtr detach = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
- int rc;
if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0)
goto cleanup;
@@ -4017,11 +4004,8 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1)
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
ret = qemuDomainRemoveNetDevice(driver, vm, detach);
- else
- ret = 0;
cleanup:
qemuDomainResetDeviceRemoval(vm);
@@ -4157,7 +4141,6 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
virDomainDefPtr vmdef = vm->def;
virDomainChrDefPtr tmpChr;
char *devstr = NULL;
- int rc;
if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4189,15 +4172,11 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
- rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1) {
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1) {
qemuDomainReleaseDeviceAddress(vm, &tmpChr->info, NULL);
ret = qemuDomainRemoveChrDevice(driver, vm, tmpChr);
- } else {
- ret = 0;
}
-
cleanup:
qemuDomainResetDeviceRemoval(vm);
VIR_FREE(devstr);
@@ -4243,11 +4222,8 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) || rc < 0)
goto cleanup;
- rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1)
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
ret = qemuDomainRemoveRNGDevice(driver, vm, tmpRNG);
- else
- ret = 0;
cleanup:
qemuDomainResetDeviceRemoval(vm);
@@ -4295,11 +4271,8 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
goto cleanup;
- rc = qemuDomainWaitForDeviceRemoval(vm);
- if (rc == 0 || rc == 1)
+ if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
ret = qemuDomainRemoveMemoryDevice(driver, vm, mem);
- else
- ret = 0;
cleanup:
qemuDomainResetDeviceRemoval(vm);
--
2.8.0
6:19 p.m.
New subject: [libvirt] [PATCH 2/7] qemu: hotplug: Refactor semantics of qemuDomainWaitForDeviceRemoval
On 04/05/2016 11:09 AM, Peter Krempa wrote:
Neither of the callers cares whether the DEVICE_DELETED event
isn't
supported or the event was received. Simplify the code and callers by
unifying the two values and changing the return value constants so that
a temporary variable can be omitted.
---
src/qemu/qemu_hotplug.c | 67 +++++++++++++++----------------------------------
1 file changed, 20 insertions(+), 47 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6c619e9..7317089 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3351,11 +3351,13 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm)
}
/* Returns:
- * 0 when DEVICE_DELETED event is unsupported, or we failed to reliably wait
- * for the event
- * 1 when DEVICE_DELETED arrived before the timeout and the caller is
- * responsible for finishing the removal
- * 2 device removal did not finish in qemuDomainRemoveDeviceWaitTime
+ * 0 DEVICE_DELETED event is supported and removal of the device did not
+ * finish in qemuDomainRemoveDeviceWaitTime
+ *
+ * 1 when the caller is responsible for finishing the device removal:
+ * - DEVICE_DELETED event is unsupported
+ * - DEVICE_DELETED event arrived before the timeout time
+ * - we failed to reliably wait for the event and thus use fallback behavior
*/
Makes sense... return 1 basically means 'removal succeeded OR there's no way
we can tell if it succeeded or not, so just update the XML to assume it did'
ACK
- Cole
10:09 a.m.
New subject: [libvirt] [PATCH 3/7] qemu: Use domain condition for device removal singalling
No need to keep two separate conditions. A slight juggling of return
values is needed to accomodate virDomainObjWaitUntil.
---
src/qemu/qemu_domain.c | 4 ----
src/qemu/qemu_domain.h | 1 -
src/qemu/qemu_hotplug.c | 19 +++++++++----------
3 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f38b0f3..8a673f8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -586,9 +586,6 @@ qemuDomainObjPrivateAlloc(void)
goto error;
}
- if (virCondInit(&priv->unplugFinished) < 0)
- goto error;
-
if (!(priv->devs = virChrdevAlloc()))
goto error;
@@ -618,7 +615,6 @@ qemuDomainObjPrivateFree(void *data)
VIR_FREE(priv->lockState);
VIR_FREE(priv->origname);
- virCondDestroy(&priv->unplugFinished);
virStringFreeList(priv->qemuDevices);
virChrdevFree(priv->devs);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 54d7bd7..2b92a90 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -198,7 +198,6 @@ struct _qemuDomainObjPrivate {
virPerfPtr perf;
- virCond unplugFinished; /* signals that unpluggingDevice was unplugged */
const char *unpluggingDevice; /* alias of the device that is being unplugged */
char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7317089..134f458 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3364,6 +3364,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
unsigned long long until;
+ int rc;
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
return 1;
@@ -3373,15 +3374,13 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
until += qemuDomainRemoveDeviceWaitTime;
while (priv->unpluggingDevice) {
- if (virCondWaitUntil(&priv->unplugFinished,
- &vm->parent.lock, until) < 0) {
- if (errno == ETIMEDOUT) {
- return 0;
- } else {
- VIR_WARN("Failed to wait on unplug condition for domain '%s'
"
- "device '%s'", vm->def->name,
priv->unpluggingDevice);
- return 1;
- }
+ 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);
+ return 1;
}
}
@@ -3402,7 +3401,7 @@ qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
if (STREQ_NULLABLE(priv->unpluggingDevice, devAlias)) {
qemuDomainResetDeviceRemoval(vm);
- virCondSignal(&priv->unplugFinished);
+ virDomainObjBroadcast(vm);
return true;
}
return false;
--
2.8.0
6:27 p.m.
New subject: [libvirt] [PATCH 3/7] qemu: Use domain condition for device removal singalling
s/singalling/signaling/
On 04/05/2016 11:09 AM, Peter Krempa wrote:
No need to keep two separate conditions. A slight juggling of return
values is needed to accomodate virDomainObjWaitUntil.
---
src/qemu/qemu_domain.c | 4 ----
src/qemu/qemu_domain.h | 1 -
src/qemu/qemu_hotplug.c | 19 +++++++++----------
3 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f38b0f3..8a673f8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -586,9 +586,6 @@ qemuDomainObjPrivateAlloc(void)
goto error;
}
- if (virCondInit(&priv->unplugFinished) < 0)
- goto error;
-
if (!(priv->devs = virChrdevAlloc()))
goto error;
@@ -618,7 +615,6 @@ qemuDomainObjPrivateFree(void *data)
VIR_FREE(priv->lockState);
VIR_FREE(priv->origname);
- virCondDestroy(&priv->unplugFinished);
virStringFreeList(priv->qemuDevices);
virChrdevFree(priv->devs);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 54d7bd7..2b92a90 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -198,7 +198,6 @@ struct _qemuDomainObjPrivate {
virPerfPtr perf;
- virCond unplugFinished; /* signals that unpluggingDevice was unplugged */
const char *unpluggingDevice; /* alias of the device that is being unplugged */
char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7317089..134f458 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3364,6 +3364,7 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
unsigned long long until;
+ int rc;
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT))
return 1;
@@ -3373,15 +3374,13 @@ qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm)
until += qemuDomainRemoveDeviceWaitTime;
while (priv->unpluggingDevice) {
- if (virCondWaitUntil(&priv->unplugFinished,
- &vm->parent.lock, until) < 0) {
- if (errno == ETIMEDOUT) {
- return 0;
- } else {
- VIR_WARN("Failed to wait on unplug condition for domain
'%s' "
- "device '%s'", vm->def->name,
priv->unpluggingDevice);
- return 1;
- }
+ 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);
+ return 1;
}
}
@@ -3402,7 +3401,7 @@ qemuDomainSignalDeviceRemoval(virDomainObjPtr vm,
if (STREQ_NULLABLE(priv->unpluggingDevice, devAlias)) {
qemuDomainResetDeviceRemoval(vm);
- virCondSignal(&priv->unplugFinished);
+ virDomainObjBroadcast(vm);
return true;
}
return false;
Neat, I didn't know about virDomainObjBroadcast etc. LGTM, ACK
- Cole
10:09 a.m.
New subject: [libvirt] [PATCH 4/7] qemu: hotplug: Add support for signalling device unplug failure
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(-)
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;
+
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;
}
@@ -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)
--
2.8.0
6:44 p.m.
New subject: [libvirt] [PATCH 4/7] qemu: hotplug: Add support for signalling device unplug failure
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)
10:09 a.m.
New subject: [libvirt] [PATCH 5/7] Add VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event
Since we didn't opt to use one single event for device lifecycle for a
VM we are missing one last event if the device removal failed. This
event will be emitted once we asked to eject the device but for some
reason it is not possible.
---
daemon/remote.c | 36 +++++++++++++++++
include/libvirt/libvirt-domain.h | 21 ++++++++++
src/conf/domain_event.c | 84 ++++++++++++++++++++++++++++++++++++++++
src/conf/domain_event.h | 7 ++++
src/libvirt_private.syms | 2 +
src/remote/remote_driver.c | 30 ++++++++++++++
src/remote/remote_protocol.x | 14 ++++++-
src/remote_protocol-structs | 6 +++
tools/virsh-domain.c | 18 +++++++++
9 files changed, 217 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 9db93ff..fde029d 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1136,6 +1136,41 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn,
}
+static int
+remoteRelayDomainEventDeviceRemovalFailed(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *devAlias,
+ void *opaque)
+{
+ daemonClientEventCallbackPtr callback = opaque;
+ remote_domain_event_callback_device_removal_failed_msg data;
+
+ if (callback->callbackID < 0 ||
+ !remoteRelayDomainEventCheckACL(callback->client, conn, dom))
+ return -1;
+
+ VIR_DEBUG("Relaying domain device removal failed event %s %d %s, callback
%d",
+ dom->name, dom->id, devAlias, callback->callbackID);
+
+ /* build return data */
+ memset(&data, 0, sizeof(data));
+
+ if (VIR_STRDUP(data.devAlias, devAlias) < 0)
+ return -1;
+
+ make_nonnull_domain(&data.dom, dom);
+ data.callbackID = callback->callbackID;
+
+ remoteDispatchObjectEventSend(callback->client, remoteProgram,
+
REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED,
+
(xdrproc_t)xdr_remote_domain_event_callback_device_removal_failed_msg,
+ &data);
+
+ return 0;
+}
+
+
+
static virConnectDomainEventGenericCallback domainEventCallbacks[] = {
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot),
@@ -1159,6 +1194,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] =
{
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventMigrationIteration),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventJobCompleted),
+ VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemovalFailed),
};
verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST);
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 728b6eb..8220ab0 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3342,6 +3342,26 @@ typedef void
(*virConnectDomainEventDeviceAddedCallback)(virConnectPtr conn,
const char *devAlias,
void *opaque);
+
+/**
+ * virConnectDomainEventDeviceRemovalFailedCallback:
+ * @conn: connection object
+ * @dom: domain on which the event occurred
+ * @devAlias: device alias
+ * @opaque: application specified data
+ *
+ * This callback occurs when it's certain that removal of a device failed.
+ *
+ * The callback signature to use when registering for an event of type
+ * VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED with
+ * virConnectDomainEventRegisterAny().
+ */
+typedef void (*virConnectDomainEventDeviceRemovalFailedCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *devAlias,
+ void *opaque);
+
+
/**
* virConnectDomainEventMigrationIterationCallback:
* @conn: connection object
@@ -3687,6 +3707,7 @@ typedef enum {
VIR_DOMAIN_EVENT_ID_DEVICE_ADDED = 19, /* virConnectDomainEventDeviceAddedCallback
*/
VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION = 20, /*
virConnectDomainEventMigrationIterationCallback */
VIR_DOMAIN_EVENT_ID_JOB_COMPLETED = 21, /* virConnectDomainEventJobCompletedCallback
*/
+ VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED = 22, /*
virConnectDomainEventDeviceRemovalFailedCallback */
# ifdef VIR_ENUM_SENTINELS
VIR_DOMAIN_EVENT_ID_LAST
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index a9107e5..58823e8 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -58,6 +58,7 @@ static virClassPtr virDomainEventAgentLifecycleClass;
static virClassPtr virDomainEventDeviceAddedClass;
static virClassPtr virDomainEventMigrationIterationClass;
static virClassPtr virDomainEventJobCompletedClass;
+static virClassPtr virDomainEventDeviceRemovalFailedClass;
static void virDomainEventDispose(void *obj);
static void virDomainEventLifecycleDispose(void *obj);
@@ -77,6 +78,7 @@ static void virDomainEventAgentLifecycleDispose(void *obj);
static void virDomainEventDeviceAddedDispose(void *obj);
static void virDomainEventMigrationIterationDispose(void *obj);
static void virDomainEventJobCompletedDispose(void *obj);
+static void virDomainEventDeviceRemovalFailedDispose(void *obj);
static void
virDomainEventDispatchDefaultFunc(virConnectPtr conn,
@@ -256,6 +258,16 @@ struct _virDomainEventJobCompleted {
typedef struct _virDomainEventJobCompleted virDomainEventJobCompleted;
typedef virDomainEventJobCompleted *virDomainEventJobCompletedPtr;
+struct _virDomainEventDeviceRemovalFailed {
+ virDomainEvent parent;
+
+ char *devAlias;
+};
+typedef struct _virDomainEventDeviceRemovalFailed virDomainEventDeviceRemovalFailed;
+typedef virDomainEventDeviceRemovalFailed *virDomainEventDeviceRemovalFailedPtr;
+
+
+
static int
virDomainEventsOnceInit(void)
{
@@ -367,6 +379,12 @@ virDomainEventsOnceInit(void)
sizeof(virDomainEventJobCompleted),
virDomainEventJobCompletedDispose)))
return -1;
+ if (!(virDomainEventDeviceRemovalFailedClass =
+ virClassNew(virDomainEventClass,
+ "virDomainEventDeviceRemovalFailed",
+ sizeof(virDomainEventDeviceRemovalFailed),
+ virDomainEventDeviceRemovalFailedDispose)))
+ return -1;
return 0;
}
@@ -494,6 +512,17 @@ virDomainEventDeviceAddedDispose(void *obj)
VIR_FREE(event->devAlias);
}
+
+static void
+virDomainEventDeviceRemovalFailedDispose(void *obj)
+{
+ virDomainEventDeviceRemovalFailedPtr event = obj;
+ VIR_DEBUG("obj=%p", event);
+
+ VIR_FREE(event->devAlias);
+}
+
+
static void
virDomainEventPMDispose(void *obj)
{
@@ -1340,6 +1369,50 @@ virDomainEventDeviceAddedNewFromDom(virDomainPtr dom,
devAlias);
}
+
+static virObjectEventPtr
+virDomainEventDeviceRemovalFailedNew(int id,
+ const char *name,
+ unsigned char *uuid,
+ const char *devAlias)
+{
+ virDomainEventDeviceRemovalFailedPtr ev;
+
+ if (virDomainEventsInitialize() < 0)
+ return NULL;
+
+ if (!(ev = virDomainEventNew(virDomainEventDeviceAddedClass,
+ VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED,
+ id, name, uuid)))
+ return NULL;
+
+ if (VIR_STRDUP(ev->devAlias, devAlias) < 0)
+ goto error;
+
+ return (virObjectEventPtr)ev;
+
+ error:
+ virObjectUnref(ev);
+ return NULL;
+}
+
+virObjectEventPtr
+virDomainEventDeviceRemovalFailedNewFromObj(virDomainObjPtr obj,
+ const char *devAlias)
+{
+ return virDomainEventDeviceRemovalFailedNew(obj->def->id,
obj->def->name,
+ obj->def->uuid, devAlias);
+}
+
+virObjectEventPtr
+virDomainEventDeviceRemovalFailedNewFromDom(virDomainPtr dom,
+ const char *devAlias)
+{
+ return virDomainEventDeviceRemovalFailedNew(dom->id, dom->name, dom->uuid,
+ devAlias);
+}
+
+
static virObjectEventPtr
virDomainEventAgentLifecycleNew(int id,
const char *name,
@@ -1768,6 +1841,17 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn,
goto cleanup;
}
+ case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED:
+ {
+ virDomainEventDeviceRemovalFailedPtr deviceRemovalFailedEvent;
+
+ deviceRemovalFailedEvent = (virDomainEventDeviceRemovalFailedPtr)event;
+ ((virConnectDomainEventDeviceRemovalFailedCallback)cb)(conn, dom,
+
deviceRemovalFailedEvent->devAlias,
+ cbopaque);
+ goto cleanup;
+ }
+
case VIR_DOMAIN_EVENT_ID_LAST:
break;
}
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 3eb13c8..54fa879 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -191,6 +191,13 @@ virObjectEventPtr
virDomainEventDeviceAddedNewFromDom(virDomainPtr dom,
const char *devAlias);
virObjectEventPtr
+virDomainEventDeviceRemovalFailedNewFromObj(virDomainObjPtr obj,
+ const char *devAlias);
+virObjectEventPtr
+virDomainEventDeviceRemovalFailedNewFromDom(virDomainPtr dom,
+ const char *devAlias);
+
+virObjectEventPtr
virDomainEventTunableNewFromObj(virDomainObjPtr obj,
virTypedParameterPtr params,
int nparams);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 684f06c..aff8622 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -496,6 +496,8 @@ virDomainEventControlErrorNewFromDom;
virDomainEventControlErrorNewFromObj;
virDomainEventDeviceAddedNewFromDom;
virDomainEventDeviceAddedNewFromObj;
+virDomainEventDeviceRemovalFailedNewFromDom;
+virDomainEventDeviceRemovalFailedNewFromObj;
virDomainEventDeviceRemovedNewFromDom;
virDomainEventDeviceRemovedNewFromObj;
virDomainEventDiskChangeNewFromDom;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b03c9ca..da94411 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -322,6 +322,10 @@ static void
remoteDomainBuildEventCallbackDeviceAdded(virNetClientProgramPtr prog,
virNetClientPtr client,
void *evdata, void *opaque);
+static void
+remoteDomainBuildEventCallbackDeviceRemovalFailed(virNetClientProgramPtr prog,
+ virNetClientPtr client,
+ void *evdata, void *opaque);
static void
remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog,
@@ -528,6 +532,10 @@ static virNetClientProgramEvent remoteEvents[] = {
remoteDomainBuildEventCallbackJobCompleted,
sizeof(remote_domain_event_callback_job_completed_msg),
(xdrproc_t)xdr_remote_domain_event_callback_job_completed_msg },
+ { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED,
+ remoteDomainBuildEventCallbackDeviceRemovalFailed,
+ sizeof(remote_domain_event_callback_device_removal_failed_msg),
+ (xdrproc_t)xdr_remote_domain_event_callback_device_removal_failed_msg },
};
static void
@@ -4829,6 +4837,28 @@ remoteDomainBuildEventCallbackDeviceAdded(virNetClientProgramPtr
prog ATTRIBUTE_
remoteEventQueue(priv, event, msg->callbackID);
}
+
+static void
+remoteDomainBuildEventCallbackDeviceRemovalFailed(virNetClientProgramPtr prog
ATTRIBUTE_UNUSED,
+ virNetClientPtr client
ATTRIBUTE_UNUSED,
+ void *evdata, void *opaque)
+{
+ virConnectPtr conn = opaque;
+ remote_domain_event_callback_device_added_msg *msg = evdata;
+ struct private_data *priv = conn->privateData;
+ virDomainPtr dom;
+ virObjectEventPtr event = NULL;
+
+ if (!(dom = get_nonnull_domain(conn, msg->dom)))
+ return;
+
+ event = virDomainEventDeviceRemovalFailedNewFromDom(dom, msg->devAlias);
+
+ virObjectUnref(dom);
+
+ remoteEventQueue(priv, event, msg->callbackID);
+}
+
static void
remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
virNetClientPtr client ATTRIBUTE_UNUSED,
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 8bda792..bab8ef2 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3261,6 +3261,12 @@ struct remote_domain_migrate_start_post_copy_args {
unsigned int flags;
};
+struct remote_domain_event_callback_device_removal_failed_msg {
+ int callbackID;
+ remote_nonnull_domain dom;
+ remote_nonnull_string devAlias;
+};
+
/*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */
@@ -5781,5 +5787,11 @@ enum remote_procedure {
* @generate: both
* @acl: domain:write
*/
- REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 366
+ REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 366,
+
+ /**
+ * @generate: both
+ * @acl: none
+ */
+ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 6dddd52..fe1b8a8 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2730,6 +2730,11 @@ struct remote_domain_migrate_start_post_copy_args {
remote_nonnull_domain dom;
u_int flags;
};
+struct remote_domain_event_callback_device_removal_failed_msg {
+ int callbackID;
+ remote_nonnull_domain dom;
+ remote_nonnull_string devAlias;
+};
enum remote_procedure {
REMOTE_PROC_CONNECT_OPEN = 1,
REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -3097,4 +3102,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_MIGRATE_START_POST_COPY = 364,
REMOTE_PROC_DOMAIN_GET_PERF_EVENTS = 365,
REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 366,
+ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367,
};
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 36d0353..6d4265c 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12301,6 +12301,22 @@ virshEventJobCompletedPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
virshEventPrint(opaque, &buf);
}
+
+static void
+virshEventDeviceRemovalFailedPrint(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainPtr dom,
+ const char *alias,
+ void *opaque)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAsprintf(&buf, _("event 'device-removal-failed' for domain
%s: %s\n"),
+ virDomainGetName(dom),
+ alias);
+ virshEventPrint(opaque, &buf);
+}
+
+
static vshEventCallback vshEventCallbacks[] = {
{ "lifecycle",
VIR_DOMAIN_EVENT_CALLBACK(virshEventLifecyclePrint), },
@@ -12344,6 +12360,8 @@ static vshEventCallback vshEventCallbacks[] = {
VIR_DOMAIN_EVENT_CALLBACK(virshEventMigrationIterationPrint), },
{ "job-completed",
VIR_DOMAIN_EVENT_CALLBACK(virshEventJobCompletedPrint), },
+ { "device-removal-failed",
+ VIR_DOMAIN_EVENT_CALLBACK(virshEventDeviceRemovalFailedPrint), },
};
verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));
--
2.8.0
7:03 p.m.
New subject: [libvirt] [PATCH 5/7] Add VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event
On 04/05/2016 11:09 AM, Peter Krempa wrote:
Since we didn't opt to use one single event for device lifecycle
for a
VM we are missing one last event if the device removal failed. This
event will be emitted once we asked to eject the device but for some
reason it is not possible.
---
daemon/remote.c | 36 +++++++++++++++++
include/libvirt/libvirt-domain.h | 21 ++++++++++
src/conf/domain_event.c | 84 ++++++++++++++++++++++++++++++++++++++++
src/conf/domain_event.h | 7 ++++
src/libvirt_private.syms | 2 +
src/remote/remote_driver.c | 30 ++++++++++++++
src/remote/remote_protocol.x | 14 ++++++-
src/remote_protocol-structs | 6 +++
tools/virsh-domain.c | 18 +++++++++
9 files changed, 217 insertions(+), 1 deletion(-)
ACK, I verified it largely mirrors DeviceAdded signal.
event-test.c follow up patch?
- Cole
6:17 a.m.
New subject: [libvirt] [PATCH 5/7] Add VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event
On Tue, Apr 12, 2016 at 20:03:41 -0400, Cole Robinson wrote:
On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Since we didn't opt to use one single event for device lifecycle for a
> VM we are missing one last event if the device removal failed. This
> event will be emitted once we asked to eject the device but for some
> reason it is not possible.
> ---
> daemon/remote.c | 36 +++++++++++++++++
> include/libvirt/libvirt-domain.h | 21 ++++++++++
> src/conf/domain_event.c | 84 ++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_event.h | 7 ++++
> src/libvirt_private.syms | 2 +
> src/remote/remote_driver.c | 30 ++++++++++++++
> src/remote/remote_protocol.x | 14 ++++++-
> src/remote_protocol-structs | 6 +++
> tools/virsh-domain.c | 18 +++++++++
> 9 files changed, 217 insertions(+), 1 deletion(-)
ACK, I verified it largely mirrors DeviceAdded signal.
I largely inspired myself by DeviceAdded :)
event-test.c follow up patch?
gah I forgot. I'll follow up, looks like I'll refactor it prior to
adding the new stuff.
Thanks.
10:09 a.m.
New subject: [libvirt] [PATCH 6/7] qemu: monitor: Add support for ACPI_DEVICE_OST event handling
The event is emitted on ACPI OSPM Status Indication events.
ACPI standard documentation describes the method as:
This object is an optional control method that is invoked by OSPM to
indicate processing status to the platform. During device ejection,
device hot add, or other event processing, OSPM may need to perform
specific handshaking with the platform. OSPM may also need to indicate
to the platform its inability to complete a requested operation; for
example, when a user presses an ejection button for a device that is
currently in use or is otherwise currently incapable of being ejected.
In this case, the processing of the ACPI Eject Request notification by
OSPM fails. OSPM may indicate this failure to the platform through the
invocation of the _OST control method. As a result of the status
notification indicating ejection failure, the platform may take certain
action including reissuing the notification or perhaps turning on an
appropriate indicator light to signal the failure to the user.
---
src/qemu/qemu_monitor.c | 19 +++++++++++++++++++
src/qemu/qemu_monitor.h | 18 ++++++++++++++++++
src/qemu/qemu_monitor_json.c | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 76 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 10a6713..b7e4fa9 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1549,6 +1549,25 @@ qemuMonitorEmitMigrationPass(qemuMonitorPtr mon,
int
+qemuMonitorEmitAcpiOstInfo(qemuMonitorPtr mon,
+ const char *alias,
+ const char *slotType,
+ const char *slot,
+ unsigned int source,
+ unsigned int status)
+{
+ int ret = -1;
+ VIR_DEBUG("mon=%p, alias='%s', slotType='%s', slot='%s',
source='%u' status=%u",
+ mon, NULLSTR(alias), slotType, slot, source, status);
+
+ QEMU_MONITOR_CALLBACK(mon, ret, domainAcpiOstInfo, mon->vm,
+ alias, slotType, slot, source, status);
+
+ return ret;
+}
+
+
+int
qemuMonitorSetCapabilities(qemuMonitorPtr mon)
{
QEMU_CHECK_MONITOR(mon);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 7906361..bb74917 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -196,6 +196,16 @@ typedef int (*qemuMonitorDomainMigrationPassCallback)(qemuMonitorPtr
mon,
int pass,
void *opaque);
+typedef int (*qemuMonitorDomainAcpiOstInfoCallback)(qemuMonitorPtr mon,
+ virDomainObjPtr vm,
+ const char *alias,
+ const char *slotType,
+ const char *slot,
+ unsigned int source,
+ unsigned int status,
+ void *opaque);
+
+
typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks;
typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr;
struct _qemuMonitorCallbacks {
@@ -226,6 +236,7 @@ struct _qemuMonitorCallbacks {
qemuMonitorDomainSpiceMigratedCallback domainSpiceMigrated;
qemuMonitorDomainMigrationStatusCallback domainMigrationStatus;
qemuMonitorDomainMigrationPassCallback domainMigrationPass;
+ qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo;
};
char *qemuMonitorEscapeArg(const char *in);
@@ -338,6 +349,13 @@ int qemuMonitorEmitMigrationStatus(qemuMonitorPtr mon,
int qemuMonitorEmitMigrationPass(qemuMonitorPtr mon,
int pass);
+int qemuMonitorEmitAcpiOstInfo(qemuMonitorPtr mon,
+ const char *alias,
+ const char *slotType,
+ const char *slot,
+ unsigned int source,
+ unsigned int status);
+
int qemuMonitorStartCPUs(qemuMonitorPtr mon,
virConnectPtr conn);
int qemuMonitorStopCPUs(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e140d0e..78af83e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -88,6 +88,7 @@ static void qemuMonitorJSONHandleSerialChange(qemuMonitorPtr mon,
virJSONValuePt
static void qemuMonitorJSONHandleSpiceMigrated(qemuMonitorPtr mon, virJSONValuePtr
data);
static void qemuMonitorJSONHandleMigrationStatus(qemuMonitorPtr mon, virJSONValuePtr
data);
static void qemuMonitorJSONHandleMigrationPass(qemuMonitorPtr mon, virJSONValuePtr
data);
+static void qemuMonitorJSONHandleAcpiOstInfo(qemuMonitorPtr mon, virJSONValuePtr data);
typedef struct {
const char *type;
@@ -95,6 +96,7 @@ typedef struct {
} qemuEventHandler;
static qemuEventHandler eventHandlers[] = {
+ { "ACPI_DEVICE_OST", qemuMonitorJSONHandleAcpiOstInfo, },
{ "BALLOON_CHANGE", qemuMonitorJSONHandleBalloonChange, },
{ "BLOCK_IO_ERROR", qemuMonitorJSONHandleIOError, },
{ "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCanceled, },
@@ -1026,6 +1028,43 @@ qemuMonitorJSONHandleMigrationPass(qemuMonitorPtr mon,
}
+static void
+qemuMonitorJSONHandleAcpiOstInfo(qemuMonitorPtr mon, virJSONValuePtr data)
+{
+ virJSONValuePtr info;
+ const char *alias;
+ const char *slotType;
+ const char *slot;
+ unsigned int source;
+ unsigned int status;
+
+ if (!(info = virJSONValueObjectGetObject(data, "info")))
+ goto error;
+
+ /* optional */
+ alias = virJSONValueObjectGetString(info, "device");
+
+ if (!(slotType = virJSONValueObjectGetString(info, "slot-type")))
+ goto error;
+
+ if (!(slot = virJSONValueObjectGetString(info, "slot")))
+ goto error;
+
+ if (virJSONValueObjectGetNumberUint(info, "source", &source) < 0)
+ goto error;
+
+ if (virJSONValueObjectGetNumberUint(info, "status", &status) < 0)
+ goto error;
+
+ qemuMonitorEmitAcpiOstInfo(mon, alias, slotType, slot, source, status);
+ return;
+
+ error:
+ VIR_WARN("malformed ACPI_DEVICE_OST event");
+ return;
+}
+
+
int
qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon,
const char *cmd_str,
--
2.8.0
7:10 p.m.
New subject: [libvirt] [PATCH 6/7] qemu: monitor: Add support for ACPI_DEVICE_OST event handling
On 04/05/2016 11:09 AM, Peter Krempa wrote:
The event is emitted on ACPI OSPM Status Indication events.
ACPI standard documentation describes the method as:
This object is an optional control method that is invoked by OSPM to
indicate processing status to the platform. During device ejection,
device hot add, or other event processing, OSPM may need to perform
specific handshaking with the platform. OSPM may also need to indicate
to the platform its inability to complete a requested operation; for
example, when a user presses an ejection button for a device that is
currently in use or is otherwise currently incapable of being ejected.
In this case, the processing of the ACPI Eject Request notification by
OSPM fails. OSPM may indicate this failure to the platform through the
invocation of the _OST control method. As a result of the status
notification indicating ejection failure, the platform may take certain
action including reissuing the notification or perhaps turning on an
appropriate indicator light to signal the failure to the user.
ACK
- Cole
10:09 a.m.
New subject: [libvirt] [PATCH 7/7] qemu: process: Wire up ACPI OST events to notify users of failed memory unplug
Since qemu is now able to notify us that the guest rejected the memory
unplug operation we can relay this to the user and make the API fail
right away.
Additionally document the possible values from the ACPI docs for future
reference.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1320447
---
src/qemu/qemu_process.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bbde32c..effc9b1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1388,6 +1388,74 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon
ATTRIBUTE_UNUSED,
}
+/**
+ *
+ * Meaning of fields reported by the event according to the ACPI standard:
+ * @source:
+ * 0x00 - 0xff: Notification values, as passed at the request time
+ * 0x100: Operating System Shutdown Processing
+ * 0x103: Ejection processing
+ * 0x200: Insertion processing
+ * other values are reserved
+ *
+ * @status:
+ * general values
+ * 0x00: success
+ * 0x01: non-specific failure
+ * 0x02: unrecognized notify code
+ * 0x03 - 0x7f: reserved
+ * other values are specific to the notification type
+ *
+ * for the 0x100 source the following additional codes are standardized
+ * 0x80: OS Shutdown request denied
+ * 0x81: OS Shutdown in progress
+ * 0x82: OS Shutdown completed
+ * 0x83: OS Graceful shutdown not supported
+ * other values are reserved
+ *
+ * Other fields and semantics are specific to the qemu handling of the event.
+ * - @alias may be NULL for successful unplug operations
+ * - @slotType describes the device type a bit more closely, currently the
+ * only known value is 'DIMM'
+ * - @slot describes the specific device
+ *
+ * Note that qemu does not emit the event for all the documented sources or
+ * devices.
+ */
+static int
+qemuProcessHandleAcpiOstInfo(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ virDomainObjPtr vm,
+ const char *alias,
+ const char *slotType,
+ const char *slot,
+ unsigned int source,
+ unsigned int status,
+ void *opaque)
+{
+ virQEMUDriverPtr driver = opaque;
+ virObjectEventPtr event = NULL;
+
+ virObjectLock(vm);
+
+ VIR_DEBUG("ACPI OST info for device %s domain %p %s. "
+ "slotType='%s' slot='%s' source=%u status=%u",
+ NULLSTR(alias), vm, vm->def->name, slotType, slot, source, status);
+
+ /* handle memory unplug failure */
+ if (STREQ(slotType, "DIMM") && alias && status == 1) {
+ qemuDomainSignalDeviceRemoval(vm, alias,
+ QEMU_DOMAIN_UNPLUG_DEVICE_STATUS_GUEST_REJECTED);
+
+ event = virDomainEventDeviceRemovalFailedNewFromObj(vm, alias);
+ }
+
+ virObjectUnlock(vm);
+ qemuDomainEventQueue(driver, event);
+
+ return 0;
+}
+
+
static int
qemuProcessHandleNicRxFilterChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virDomainObjPtr vm,
@@ -1583,6 +1651,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
.domainSpiceMigrated = qemuProcessHandleSpiceMigrated,
.domainMigrationStatus = qemuProcessHandleMigrationStatus,
.domainMigrationPass = qemuProcessHandleMigrationPass,
+ .domainAcpiOstInfo = qemuProcessHandleAcpiOstInfo,
};
static void
--
2.8.0
7:13 p.m.
New subject: [libvirt] [PATCH 7/7] qemu: process: Wire up ACPI OST events to notify users of failed memory unplug
On 04/05/2016 11:09 AM, Peter Krempa wrote:
Since qemu is now able to notify us that the guest rejected the
memory
unplug operation we can relay this to the user and make the API fail
right away.
Additionally document the possible values from the ACPI docs for future
reference.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1320447
ACK, nice series. So do the ACPI bits give us info about any other device
hotplug failures that can be added later? Or is this only for memory hotplug?
- Cole
6:23 a.m.
New subject: [libvirt] [PATCH 7/7] qemu: process: Wire up ACPI OST events to notify users of failed memory unplug
On Tue, Apr 12, 2016 at 20:13:47 -0400, Cole Robinson wrote:
On 04/05/2016 11:09 AM, Peter Krempa wrote:
> Since qemu is now able to notify us that the guest rejected the memory
> unplug operation we can relay this to the user and make the API fail
> right away.
>
> Additionally document the possible values from the ACPI docs for future
> reference.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1320447
ACK, nice series. So do the ACPI bits give us info about any other device
hotplug failures that can be added later? Or is this only for memory hotplug?
AFAIK qemu does not currently emit that event for anything besides
memory devices. It would be really cool and we will be prepared for the
moment if they start emit IT. According to the ACPI specs it should be
possible.
Peter
8:53 a.m.
On Tue, Apr 05, 2016 at 17:09:16 +0200, Peter Krempa wrote:
Add a new libvirt event VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED
that will
handle failure to unplug a device if we are certain that it will not be unplugged
and wire up the ACPI_DEVICE_OST qemu event that is emitted on memory hotunplug
failure so that the event is propagated and the API fails in such case.
Along with that a few refactors were necessary.
Ping? Anybody brave enough to look at this? Thanks.
Peter
9:12 a.m.
On 04/12/2016 09:53 AM, Peter Krempa wrote:
On Tue, Apr 05, 2016 at 17:09:16 +0200, Peter Krempa wrote:
> Add a new libvirt event VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED that will
> handle failure to unplug a device if we are certain that it will not be unplugged
> and wire up the ACPI_DEVICE_OST qemu event that is emitted on memory hotunplug
> failure so that the event is propagated and the API fails in such case.
>
> Along with that a few refactors were necessary.
Ping? Anybody brave enough to look at this? Thanks.
I'll review it today if no one else gets to it, I just started looking at your
disk XML refactoring bits too
- Cole
3181
days inactive
3189
days old
18 comments
2 participants
participants (2)
-
Cole Robinson -
Peter Krempa