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