--- Notes: Version 2: - new patch src/qemu/qemu_hotplug.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 788ad47..f2dddc8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2475,6 +2475,19 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, } +static void +qemuDomainRemoveChrDevice(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + virDomainChrDefPtr chr) +{ + VIR_DEBUG("Removing character device %s from domain %p %s", + chr->info.alias, vm, vm->def->name); + + qemuDomainChrRemove(vm->def, chr); + virDomainChrDefFree(chr); +} + + int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr detach) @@ -3172,8 +3185,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, } qemuDomainObjExitMonitor(driver, vm); - qemuDomainChrRemove(vmdef, tmpChr); - virDomainChrDefFree(tmpChr); + qemuDomainRemoveChrDevice(driver, vm, tmpChr); ret = 0; cleanup: -- 1.8.3.2

--- Notes: Already ACKed in version 1 Version 2: - no change daemon/remote.c | 32 +++++++++++++ include/libvirt/libvirt.h.in | 18 ++++++++ python/libvirt-override-virConnect.py | 9 ++++ python/libvirt-override.c | 52 ++++++++++++++++++++- src/conf/domain_event.c | 85 +++++++++++++++++++++++++++-------- src/conf/domain_event.h | 5 +++ src/libvirt_private.syms | 2 + src/remote/remote_driver.c | 32 +++++++++++++ src/remote/remote_protocol.x | 13 +++++- src/remote_protocol-structs | 5 +++ 10 files changed, 233 insertions(+), 20 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 5847e60..a10a308 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -600,6 +600,37 @@ static int remoteRelayDomainEventPMSuspendDisk(virConnectPtr conn ATTRIBUTE_UNUS return 0; } +static int +remoteRelayDomainEventDeviceRemoved(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *devAlias, + void *opaque) +{ + virNetServerClientPtr client = opaque; + remote_domain_event_device_removed_msg data; + + if (!client) + return -1; + + VIR_DEBUG("Relaying domain device removed event %s %d %s", + dom->name, dom->id, devAlias); + + /* build return data */ + memset(&data, 0, sizeof(data)); + + if (VIR_STRDUP(data.devAlias, devAlias) < 0) + return -1; + + make_nonnull_domain(&data.dom, dom); + + remoteDispatchDomainEventSend(client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED, + (xdrproc_t)xdr_remote_domain_event_device_removed_msg, + &data); + + return 0; +} + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), @@ -617,6 +648,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspend), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBalloonChange), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspendDisk), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceRemoved), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 32c36f1..25cbe48 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4854,6 +4854,23 @@ typedef void (*virConnectDomainEventPMSuspendDiskCallback)(virConnectPtr conn, int reason, void *opaque); +/** + * virConnectDomainEventDeviceRemovedCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @devAlias: device alias + * @opaque: application specified data + * + * This callback occurs when a device is removed from the domain. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *devAlias, + void *opaque); + /** * VIR_DOMAIN_EVENT_CALLBACK: @@ -4880,6 +4897,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_PMSUSPEND = 12, /* virConnectDomainEventPMSuspendCallback */ VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE = 13, /* virConnectDomainEventBalloonChangeCallback */ VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK = 14, /* virConnectDomainEventPMSuspendDiskCallback */ + VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED = 15, /* virConnectDomainEventDeviceRemovedCallback */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 5495b70..f2b4b23 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -179,6 +179,15 @@ cb(self, virDomain(self, _obj=dom), reason, opaque) return 0 + def _dispatchDomainEventDeviceRemovedCallback(self, dom, devAlias, cbData): + """Dispatches event to python user domain device removed event callbacks + """ + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(self, virDomain(self, _obj=dom), devAlias, opaque) + return 0 + def domainEventDeregisterAny(self, callbackID): """Removes a Domain Event Callback. De-registering for a domain callback will disable delivery of this event type """ diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 01c941e..2e94de9 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -6225,6 +6225,51 @@ libvirt_virConnectDomainEventPMSuspendDiskCallback(virConnectPtr conn ATTRIBUTE_ return ret; } +static int +libvirt_virConnectDomainEventDeviceRemovedCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *devAlias, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject*)opaque; + PyObject *pyobj_dom; + PyObject *pyobj_ret; + PyObject *pyobj_conn; + PyObject *dictKey; + int ret = -1; + + LIBVIRT_ENSURE_THREAD_STATE; + /* Create a python instance of this virDomainPtr */ + virDomainRef(dom); + + pyobj_dom = libvirt_virDomainPtrWrap(dom); + Py_INCREF(pyobj_cbData); + + dictKey = libvirt_constcharPtrWrap("conn"); + pyobj_conn = PyDict_GetItem(pyobj_cbData, dictKey); + Py_DECREF(dictKey); + + /* Call the Callback Dispatcher */ + pyobj_ret = PyObject_CallMethod(pyobj_conn, + (char*)"_dispatchDomainEventDeviceRemovedCallback", + (char*)"OsO", + pyobj_dom, devAlias, pyobj_cbData); + + Py_DECREF(pyobj_cbData); + Py_DECREF(pyobj_dom); + + if (!pyobj_ret) { + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + ret = 0; + } + + LIBVIRT_RELEASE_THREAD_STATE; + return ret; +} + static PyObject * libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, PyObject * args) @@ -6254,7 +6299,7 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, else dom = PyvirDomain_Get(pyobj_dom); - switch (eventID) { + switch ((virDomainEventID) eventID) { case VIR_DOMAIN_EVENT_ID_LIFECYCLE: cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventLifecycleCallback); break; @@ -6300,6 +6345,11 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, case VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK: cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventPMSuspendDiskCallback); break; + case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED: + cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventDeviceRemovedCallback); + + case VIR_DOMAIN_EVENT_ID_LAST: + break; } if (!cb) { diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index fde24be..640463c 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -122,6 +122,9 @@ struct _virDomainEvent { /* In unit of 1024 bytes */ unsigned long long actual; } balloonChange; + struct { + char *devAlias; + } deviceRemoved; } data; }; @@ -1157,6 +1160,44 @@ virDomainEventPtr virDomainEventBalloonChangeNewFromObj(virDomainObjPtr obj, return ev; } +static virDomainEventPtr +virDomainEventDeviceRemovedNew(int id, + const char *name, + unsigned char *uuid, + const char *devAlias) +{ + virDomainEventPtr ev = + virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED, + id, name, uuid); + + if (ev) { + if (VIR_STRDUP(ev->data.deviceRemoved.devAlias, devAlias) < 0) + goto error; + } + + return ev; + +error: + virDomainEventFree(ev); + return NULL; +} + +virDomainEventPtr +virDomainEventDeviceRemovedNewFromObj(virDomainObjPtr obj, + const char *devAlias) +{ + return virDomainEventDeviceRemovedNew(obj->def->id, obj->def->name, + obj->def->uuid, devAlias); +} + +virDomainEventPtr +virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, + const char *devAlias) +{ + return virDomainEventDeviceRemovedNew(dom->id, dom->name, dom->uuid, + devAlias); +} + /** * virDomainEventQueuePush: * @evtQueue: the dom event queue @@ -1204,30 +1245,30 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, return; dom->id = event->dom.id; - switch (event->eventID) { + switch ((virDomainEventID) event->eventID) { case VIR_DOMAIN_EVENT_ID_LIFECYCLE: ((virConnectDomainEventCallback)cb)(conn, dom, event->data.lifecycle.type, event->data.lifecycle.detail, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_REBOOT: (cb)(conn, dom, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_RTC_CHANGE: ((virConnectDomainEventRTCChangeCallback)cb)(conn, dom, event->data.rtcChange.offset, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_WATCHDOG: ((virConnectDomainEventWatchdogCallback)cb)(conn, dom, event->data.watchdog.action, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_IO_ERROR: ((virConnectDomainEventIOErrorCallback)cb)(conn, dom, @@ -1235,7 +1276,7 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, event->data.ioError.devAlias, event->data.ioError.action, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON: ((virConnectDomainEventIOErrorReasonCallback)cb)(conn, dom, @@ -1244,7 +1285,7 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, event->data.ioError.action, event->data.ioError.reason, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_GRAPHICS: ((virConnectDomainEventGraphicsCallback)cb)(conn, dom, @@ -1254,12 +1295,12 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, event->data.graphics.authScheme, event->data.graphics.subject, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_CONTROL_ERROR: (cb)(conn, dom, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_BLOCK_JOB: ((virConnectDomainEventBlockJobCallback)cb)(conn, dom, @@ -1267,7 +1308,7 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, event->data.blockJob.type, event->data.blockJob.status, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_DISK_CHANGE: ((virConnectDomainEventDiskChangeCallback)cb)(conn, dom, @@ -1276,38 +1317,46 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, event->data.diskChange.devAlias, event->data.diskChange.reason, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_TRAY_CHANGE: ((virConnectDomainEventTrayChangeCallback)cb)(conn, dom, event->data.trayChange.devAlias, event->data.trayChange.reason, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_PMWAKEUP: ((virConnectDomainEventPMWakeupCallback)cb)(conn, dom, 0, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_PMSUSPEND: ((virConnectDomainEventPMSuspendCallback)cb)(conn, dom, 0, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE: ((virConnectDomainEventBalloonChangeCallback)cb)(conn, dom, event->data.balloonChange.actual, cbopaque); - break; + goto cleanup; case VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK: ((virConnectDomainEventPMSuspendDiskCallback)cb)(conn, dom, 0, cbopaque); - break; + goto cleanup; - default: - VIR_WARN("Unexpected event ID %d", event->eventID); + case VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED: + ((virConnectDomainEventDeviceRemovedCallback)cb)(conn, dom, + event->data.deviceRemoved.devAlias, + cbopaque); + goto cleanup; + + case VIR_DOMAIN_EVENT_ID_LAST: break; } + VIR_WARN("Unexpected event ID %d", event->eventID); + +cleanup: virDomainFree(dom); } diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index 5f64a47..f6b957d 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -132,6 +132,11 @@ virDomainEventPtr virDomainEventBalloonChangeNewFromObj(virDomainObjPtr obj, uns virDomainEventPtr virDomainEventPMSuspendDiskNewFromObj(virDomainObjPtr obj); virDomainEventPtr virDomainEventPMSuspendDiskNewFromDom(virDomainPtr dom); +virDomainEventPtr virDomainEventDeviceRemovedNewFromObj(virDomainObjPtr obj, + const char *devAlias); +virDomainEventPtr virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, + const char *devAlias); + void virDomainEventFree(virDomainEventPtr event); void virDomainEventStateFree(virDomainEventStatePtr state); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fc4e750..d4bf8e3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -407,6 +407,8 @@ virDomainEventBlockJobNewFromDom; virDomainEventBlockJobNewFromObj; virDomainEventControlErrorNewFromDom; virDomainEventControlErrorNewFromObj; +virDomainEventDeviceRemovedNewFromDom; +virDomainEventDeviceRemovedNewFromObj; virDomainEventDiskChangeNewFromDom; virDomainEventDiskChangeNewFromObj; virDomainEventFree; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 0298aab..3d58e31 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -268,6 +268,11 @@ remoteDomainBuildEventPMSuspendDisk(virNetClientProgramPtr prog, virNetClientPtr client, void *evdata, void *opaque); +static void +remoteDomainBuildEventDeviceRemoved(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); + static virNetClientProgramEvent remoteDomainEvents[] = { { REMOTE_PROC_DOMAIN_EVENT_RTC_CHANGE, remoteDomainBuildEventRTCChange, @@ -329,6 +334,10 @@ static virNetClientProgramEvent remoteDomainEvents[] = { remoteDomainBuildEventPMSuspendDisk, sizeof(remote_domain_event_pmsuspend_disk_msg), (xdrproc_t)xdr_remote_domain_event_pmsuspend_disk_msg }, + { REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED, + remoteDomainBuildEventDeviceRemoved, + sizeof(remote_domain_event_device_removed_msg), + (xdrproc_t)xdr_remote_domain_event_device_removed_msg }, }; enum virDrvOpenRemoteFlags { @@ -4695,6 +4704,29 @@ remoteDomainBuildEventPMSuspendDisk(virNetClientProgramPtr prog ATTRIBUTE_UNUSED } +static void +remoteDomainBuildEventDeviceRemoved(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + struct private_data *priv = conn->privateData; + remote_domain_event_device_removed_msg *msg = evdata; + virDomainPtr dom; + virDomainEventPtr event = NULL; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) + return; + + event = virDomainEventDeviceRemovedNewFromDom(dom, msg->devAlias); + + virDomainFree(dom); + + remoteDomainEventQueue(priv, event); +} + + static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteSecretOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d1c77f5..84d696b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2814,6 +2814,11 @@ struct remote_domain_migrate_confirm3_params_args { int cancelled; }; +struct remote_domain_event_device_removed_msg { + remote_nonnull_domain dom; + remote_nonnull_string devAlias; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -4958,5 +4963,11 @@ enum remote_procedure { * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG */ - REMOTE_PROC_DOMAIN_SET_MEMORY_STATS_PERIOD = 308 + REMOTE_PROC_DOMAIN_SET_MEMORY_STATS_PERIOD = 308, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 309 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 86e7650..cfc84dd 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2298,6 +2298,10 @@ struct remote_domain_migrate_confirm3_params_args { u_int flags; int cancelled; }; +struct remote_domain_event_device_removed_msg { + remote_nonnull_domain dom; + remote_nonnull_string devAlias; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -2607,4 +2611,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_FINISH3_PARAMS = 306, REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307, REMOTE_PROC_DOMAIN_SET_MEMORY_STATS_PERIOD = 308, + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 309, }; -- 1.8.3.2

--- Notes: Version 2: - document limited waiting to make most async removal synchronous src/libvirt.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index 0cdac0d..1f44a28 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -10889,6 +10889,22 @@ error: * block copy operation on the device being detached; in that case, * use virDomainBlockJobAbort() to stop the block copy first. * + * Beware that depending on the hypervisor and device type, detaching a device + * from a running domain may be asynchronous. That is, calling + * virDomainDetachDeviceFlags may just request device removal while the device + * is actually removed later (in cooperation with a guest OS). Previously, + * this fact was ignored and the device could have been removed from domain + * configuration before it was actually removed by the hypervisor causing + * various failures on subsequent operations. To check whether the device was + * successfully removed, either recheck domain configuration using + * virDomainGetXMLDesc() or add handler for VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED + * event. In case the device is already gone when virDomainDetachDeviceFlags + * returns, the event is delivered before this API call ends. To help existing + * clients work better in most cases, this API will try to transform an + * asynchronous device removal that finishes shortly after the request into + * a synchronous removal. In other words, this API may wait a bit for the + * removal to complete in case it was not synchronous. + * * Returns 0 in case of success, -1 in case of failure. */ int -- 1.8.3.2

--- Notes: Already ACKed in version 1 Version 2: - no change src/conf/domain_conf.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ src/libvirt_private.syms | 1 + 3 files changed, 46 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 44be81e..57cd9b1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18314,3 +18314,44 @@ virDomainDiskDefGenSecurityLabelDef(const char *model) return seclabel; } + + +typedef struct { + const char *devAlias; + virDomainDeviceDefPtr dev; +} virDomainDefFindDeviceCallbackData; + +static int +virDomainDefFindDeviceCallback(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info, + void *opaque) +{ + virDomainDefFindDeviceCallbackData *data = opaque; + + if (STREQ_NULLABLE(info->alias, data->devAlias)) { + *data->dev = *dev; + return -1; + } + return 0; +} + +int +virDomainDefFindDevice(virDomainDefPtr def, + const char *devAlias, + virDomainDeviceDefPtr dev) +{ + virDomainDefFindDeviceCallbackData data = { devAlias, dev }; + + dev->type = VIR_DOMAIN_DEVICE_NONE; + virDomainDeviceInfoIterateInternal(def, virDomainDefFindDeviceCallback, + true, &data); + + if (dev->type == VIR_DOMAIN_DEVICE_NONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("no device found with alias %s"), devAlias); + return -1; + } + + return 0; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b14afd9..25dad16 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2688,4 +2688,8 @@ virDomainDefMaybeAddController(virDomainDefPtr def, char *virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps); +int virDomainDefFindDevice(virDomainDefPtr def, + const char *devAlias, + virDomainDeviceDefPtr dev); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d4bf8e3..b1c4032 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -153,6 +153,7 @@ virDomainDefClearDeviceAliases; virDomainDefClearPCIAddresses; virDomainDefCompatibleDevice; virDomainDefCopy; +virDomainDefFindDevice; virDomainDefFormat; virDomainDefFormatInternal; virDomainDefFree; -- 1.8.3.2

--- Notes: Version 2: - DEVICE_DELETED event handler always removes the device - wait for up to 5 seconds after device_del returns to make async removal synchronous in normal cases src/qemu/qemu_domain.c | 4 ++ src/qemu/qemu_domain.h | 3 + src/qemu/qemu_hotplug.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_hotplug.h | 7 +++ src/qemu/qemu_process.c | 32 ++++++++++ 5 files changed, 199 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 06efe14..257f4db 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -220,6 +220,9 @@ qemuDomainObjPrivateAlloc(void) goto error; } + if (virCondInit(&priv->unplugFinished) < 0) + goto error; + if (!(priv->devs = virChrdevAlloc())) goto error; @@ -248,6 +251,7 @@ qemuDomainObjPrivateFree(void *data) VIR_FREE(priv->lockState); VIR_FREE(priv->origname); + virCondDestroy(&priv->unplugFinished); virChrdevFree(priv->devs); /* This should never be non-NULL if we get here, but just in case... */ diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 750841f..2f3b141e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -168,6 +168,9 @@ struct _qemuDomainObjPrivate { size_t ncleanupCallbacks_max; virCgroupPtr cgroup; + + virCond unplugFinished; /* signals that unpluggingDevice was unplugged */ + const char *unpluggingDevice; /* alias of the device that is being unplugged */ }; typedef enum { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f2dddc8..b1ddd92 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -48,6 +48,7 @@ #include "device_conf.h" #include "virstoragefile.h" #include "virstring.h" +#include "virtime.h" #define VIR_FROM_THIS VIR_FROM_QEMU #define CHANGE_MEDIA_RETRIES 10 @@ -2488,6 +2489,122 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, } +void +qemuDomainRemoveDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) +{ + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_DISK: + qemuDomainRemoveDiskDevice(driver, vm, dev->data.disk); + break; + case VIR_DOMAIN_DEVICE_CONTROLLER: + qemuDomainRemoveControllerDevice(driver, vm, dev->data.controller); + break; + case VIR_DOMAIN_DEVICE_NET: + qemuDomainRemoveNetDevice(driver, vm, dev->data.net); + break; + case VIR_DOMAIN_DEVICE_HOSTDEV: + qemuDomainRemoveHostDevice(driver, vm, dev->data.hostdev); + break; + + case VIR_DOMAIN_DEVICE_CHR: + qemuDomainRemoveChrDevice(driver, vm, dev->data.chr); + break; + + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("don't know how to remove a %s device"), + virDomainDeviceTypeToString(dev->type)); + break; + } +} + + +/* Wait up to 5 seconds for device removal to finish. */ +#define QEMU_REMOVAL_WAIT_TIME (1000ull * 5) + +static void +qemuDomainMarkDeviceForRemoval(virDomainObjPtr vm, + virDomainDeviceInfoPtr info) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT)) + priv->unpluggingDevice = info->alias; + else + priv->unpluggingDevice = NULL; +} + +static void +qemuDomainResetDeviceRemoval(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + priv->unpluggingDevice = NULL; +} + +/* Returns: + * -1 on error + * 0 when DEVICE_DELETED event is unsupported + * 1 when device removal finished + * 2 device removal did not finish in QEMU_REMOVAL_WAIT_TIME + */ +static int +qemuDomainWaitForDeviceRemoval(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + unsigned long long until; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT)) + return 0; + + if (virTimeMillisNow(&until) < 0) + return -1; + until += QEMU_REMOVAL_WAIT_TIME; + + while (priv->unpluggingDevice) { + if (virCondWaitUntil(&priv->unplugFinished, + &vm->parent.lock, until) < 0) { + if (errno == ETIMEDOUT) { + return 2; + } else { + virReportSystemError(errno, "%s", + _("Unable to wait on unplug condition")); + return -1; + } + } + } + + return 1; +} + +void +qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, + const char *devAlias) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (STREQ_NULLABLE(priv->unpluggingDevice, devAlias)) { + qemuDomainResetDeviceRemoval(vm); + virCondSignal(&priv->unplugFinished); + } +} + + int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr detach) @@ -2526,6 +2643,8 @@ int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) goto cleanup; + qemuDomainMarkDeviceForRemoval(vm, &detach->info); + qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { @@ -2547,10 +2666,12 @@ int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); - qemuDomainRemoveDiskDevice(driver, vm, detach); + if (!qemuDomainWaitForDeviceRemoval(vm)) + qemuDomainRemoveDiskDevice(driver, vm, detach); ret = 0; cleanup: + qemuDomainResetDeviceRemoval(vm); VIR_FREE(drivestr); return ret; } @@ -2583,6 +2704,8 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, QEMU_DRIVE_HOST_PREFIX, detach->info.alias) < 0) goto cleanup; + qemuDomainMarkDeviceForRemoval(vm, &detach->info); + qemuDomainObjEnterMonitor(driver, vm); if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { qemuDomainObjExitMonitor(driver, vm); @@ -2595,10 +2718,12 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); - qemuDomainRemoveDiskDevice(driver, vm, detach); + if (!qemuDomainWaitForDeviceRemoval(vm)) + qemuDomainRemoveDiskDevice(driver, vm, detach); ret = 0; cleanup: + qemuDomainResetDeviceRemoval(vm); VIR_FREE(drivestr); return ret; } @@ -2698,6 +2823,8 @@ int qemuDomainDetachPciControllerDevice(virQEMUDriverPtr driver, goto cleanup; } + qemuDomainMarkDeviceForRemoval(vm, &detach->info); + qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { @@ -2713,11 +2840,13 @@ int qemuDomainDetachPciControllerDevice(virQEMUDriverPtr driver, } qemuDomainObjExitMonitor(driver, vm); - qemuDomainRemoveControllerDevice(driver, vm, detach); + if (!qemuDomainWaitForDeviceRemoval(vm)) + qemuDomainRemoveControllerDevice(driver, vm, detach); ret = 0; cleanup: + qemuDomainResetDeviceRemoval(vm); return ret; } @@ -2745,6 +2874,8 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver, return -1; } + qemuDomainMarkDeviceForRemoval(vm, detach->info); + qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); @@ -2776,6 +2907,8 @@ qemuDomainDetachHostUsbDevice(virQEMUDriverPtr driver, return -1; } + qemuDomainMarkDeviceForRemoval(vm, detach->info); + qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); qemuDomainObjExitMonitor(driver, vm); @@ -2811,6 +2944,8 @@ qemuDomainDetachHostScsiDevice(virQEMUDriverPtr driver, if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, detach, priv->qemuCaps))) goto cleanup; + qemuDomainMarkDeviceForRemoval(vm, detach->info); + qemuDomainObjEnterMonitor(driver, vm); if ((ret = qemuMonitorDelDevice(priv->mon, detach->info->alias)) == 0) { if ((ret = qemuMonitorDriveDel(priv->mon, drvstr)) < 0) { @@ -2859,9 +2994,11 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, if (ret < 0) virDomainAuditHostdev(vm, detach, "detach", false); - else + else if (!qemuDomainWaitForDeviceRemoval(vm)) qemuDomainRemoveHostDevice(driver, vm, detach); + qemuDomainResetDeviceRemoval(vm); + return ret; } @@ -2992,6 +3129,8 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, if (virAsprintf(&hostnet_name, "host%s", detach->info.alias) < 0) goto cleanup; + qemuDomainMarkDeviceForRemoval(vm, &detach->info); + qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { @@ -3024,9 +3163,13 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, } qemuDomainObjExitMonitor(driver, vm); - qemuDomainRemoveNetDevice(driver, vm, detach); + if (!qemuDomainWaitForDeviceRemoval(vm)) + qemuDomainRemoveNetDevice(driver, vm, detach); + ret = 0; + cleanup: + qemuDomainResetDeviceRemoval(vm); VIR_FREE(hostnet_name); return ret; } @@ -3173,6 +3316,8 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, return ret; } + qemuDomainMarkDeviceForRemoval(vm, &tmpChr->info); + qemuDomainObjEnterMonitor(driver, vm); if (devstr && qemuMonitorDelDevice(priv->mon, tmpChr->info.alias) < 0) { qemuDomainObjExitMonitor(driver, vm); @@ -3185,10 +3330,12 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, } qemuDomainObjExitMonitor(driver, vm); - qemuDomainRemoveChrDevice(driver, vm, tmpChr); + if (!qemuDomainWaitForDeviceRemoval(vm)) + qemuDomainRemoveChrDevice(driver, vm, tmpChr); ret = 0; cleanup: + qemuDomainResetDeviceRemoval(vm); VIR_FREE(devstr); VIR_FREE(charAlias); return ret; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index e9951fb..6fa1f33 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -119,4 +119,11 @@ virDomainChrDefPtr qemuDomainChrRemove(virDomainDefPtr vmdef, virDomainChrDefPtr chr); +void qemuDomainRemoveDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev); + +void qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, + const char *devAlias); + #endif /* __QEMU_HOTPLUG_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3d5e8f6..79307a9 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1313,6 +1313,37 @@ cleanup: } +static int +qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *devAlias) +{ + virQEMUDriverPtr driver = qemu_driver; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainDeviceDef dev; + + virObjectLock(vm); + + VIR_DEBUG("Device %s removed from domain %p %s", + devAlias, vm, vm->def->name); + + qemuDomainSignalDeviceRemoval(vm, devAlias); + + if (virDomainDefFindDevice(vm->def, devAlias, &dev) < 0) + goto cleanup; + + qemuDomainRemoveDevice(driver, vm, &dev); + + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) + VIR_WARN("unable to save domain status with balloon change"); + +cleanup: + virObjectUnlock(vm); + virObjectUnref(cfg); + return 0; +} + + static qemuMonitorCallbacks monitorCallbacks = { .destroy = qemuProcessHandleMonitorDestroy, .eofNotify = qemuProcessHandleMonitorEOF, @@ -1333,6 +1364,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainBalloonChange = qemuProcessHandleBalloonChange, .domainPMSuspendDisk = qemuProcessHandlePMSuspendDisk, .domainGuestPanic = qemuProcessHandleGuestPanic, + .domainDeviceDeleted = qemuProcessHandleDeviceDeleted, }; static int -- 1.8.3.2

--- Notes: Version 2: - update qemuhotplugtest to initialize domainEventState src/qemu/qemu_hotplug.c | 30 ++++++++++++++++++++++++++++-- tests/qemuhotplugtest.c | 3 +++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b1ddd92..a6d9a03 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2234,6 +2234,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainDiskDefPtr disk) { virDomainDeviceDef dev; + virDomainEventPtr event; size_t i; VIR_DEBUG("Removing disk %s from domain %p %s", @@ -2241,6 +2242,10 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainAuditDisk(vm, disk->src, NULL, "detach", true); + event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias); + if (event) + qemuDomainEventQueue(driver, event); + for (i = 0; i < vm->def->ndisks; i++) { if (vm->def->disks[i] == disk) { virDomainDiskRemove(vm->def, i); @@ -2269,15 +2274,20 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, static void -qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainControllerDefPtr controller) { + virDomainEventPtr event; size_t i; VIR_DEBUG("Removing controller %s from domain %p %s", controller->info.alias, vm, vm->def->name); + event = virDomainEventDeviceRemovedNewFromObj(vm, controller->info.alias); + if (event) + qemuDomainEventQueue(driver, event); + for (i = 0; i < vm->def->ncontrollers; i++) { if (vm->def->controllers[i] == controller) { virDomainControllerRemove(vm->def, i); @@ -2297,6 +2307,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virNetDevVPortProfilePtr vport; + virDomainEventPtr event; size_t i; VIR_DEBUG("Removing network interface %s from domain %p %s", @@ -2304,6 +2315,10 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, virDomainAuditNet(vm, net, NULL, "detach", true); + event = virDomainEventDeviceRemovedNewFromObj(vm, net->info.alias); + if (event) + qemuDomainEventQueue(driver, event); + for (i = 0; i < vm->def->nnets; i++) { if (vm->def->nets[i] == net) { virDomainNetRemove(vm->def, i); @@ -2420,11 +2435,16 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev) { virDomainNetDefPtr net = NULL; + virDomainEventPtr event; size_t i; VIR_DEBUG("Removing host device %s from domain %p %s", hostdev->info->alias, vm, vm->def->name); + event = virDomainEventDeviceRemovedNewFromObj(vm, hostdev->info->alias); + if (event) + qemuDomainEventQueue(driver, event); + if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET) { net = hostdev->parent.data.net; @@ -2477,13 +2497,19 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, static void -qemuDomainRemoveChrDevice(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainChrDefPtr chr) { + virDomainEventPtr event; + VIR_DEBUG("Removing character device %s from domain %p %s", chr->info.alias, vm, vm->def->name); + event = virDomainEventDeviceRemovedNewFromObj(vm, chr->info.alias); + if (event) + qemuDomainEventQueue(driver, event); + qemuDomainChrRemove(vm->def, chr); virDomainChrDefFree(chr); } diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d4971c2..ef3670e 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -270,6 +270,9 @@ mymain(void) VIR_FREE(driver.config->spiceListen); VIR_FREE(driver.config->vncListen); + if (!(driver.domainEventState = virDomainEventStateNew())) + return EXIT_FAILURE; + /* some dummy values from 'config file' */ if (VIR_STRDUP_QUIET(driver.config->spicePassword, "123456") < 0) return EXIT_FAILURE; -- 1.8.3.2

On Thu, Jul 18, 2013 at 11:44:20 +0100, Daniel Berrange wrote: No, all methods are specific to a single device type and since we want to emit the event for all device types, we have to emit it in each of these methods. Jirka