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)
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list