--- 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 b87255a..bd7417f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4851,6 +4851,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: @@ -4877,6 +4894,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 0ab7632..49f07c1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -405,6 +405,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 81ecef1..b4bb926 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 2e9dc1d..d42e5af 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2808,6 +2808,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. */ @@ -4944,6 +4949,12 @@ enum remote_procedure { * @generate: none * @acl: domain:migrate */ - REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307 + REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 308 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e38d24a..8d064fe 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2293,6 +2293,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, @@ -2601,4 +2605,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3_PARAMS = 305, REMOTE_PROC_DOMAIN_MIGRATE_FINISH3_PARAMS = 306, REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3_PARAMS = 307, + REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 308, }; -- 1.8.3.2

--- examples/domain-events/events-c/event-test.c | 23 +++++++++++++++++++++- examples/domain-events/events-python/event-test.py | 4 ++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index e9b3881..fe4eb56 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -428,6 +428,17 @@ static int myDomainEventPMSuspendDiskCallback(virConnectPtr conn ATTRIBUTE_UNUSE return 0; } +static int +myDomainEventDeviceRemovedCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *devAlias, + void *opaque ATTRIBUTE_UNUSED) +{ + printf("%s EVENT: Domain %s(%d) device removed: %s\n", + __func__, virDomainGetName(dom), virDomainGetID(dom), devAlias); + return 0; +} + static void myFreeFunc(void *opaque) { char *str = opaque; @@ -467,6 +478,7 @@ int main(int argc, char **argv) int callback12ret = -1; int callback13ret = -1; int callback14ret = -1; + int callback15ret = -1; struct sigaction action_stop; memset(&action_stop, 0, sizeof(action_stop)); @@ -575,6 +587,12 @@ int main(int argc, char **argv) VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK, VIR_DOMAIN_EVENT_CALLBACK(myDomainEventPMSuspendDiskCallback), strdup("pmsuspend-disk"), myFreeFunc); + callback15ret = virConnectDomainEventRegisterAny(dconn, + NULL, + VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED, + VIR_DOMAIN_EVENT_CALLBACK(myDomainEventDeviceRemovedCallback), + strdup("device removed"), myFreeFunc); + if ((callback1ret != -1) && (callback2ret != -1) && (callback3ret != -1) && @@ -587,7 +605,8 @@ int main(int argc, char **argv) (callback11ret != -1) && (callback12ret != -1) && (callback13ret != -1) && - (callback14ret != -1)) { + (callback14ret != -1) && + (callback15ret != -1)) { if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to start keepalive protocol: %s\n", @@ -616,6 +635,8 @@ int main(int argc, char **argv) virConnectDomainEventDeregisterAny(dconn, callback11ret); virConnectDomainEventDeregisterAny(dconn, callback12ret); virConnectDomainEventDeregisterAny(dconn, callback13ret); + virConnectDomainEventDeregisterAny(dconn, callback14ret); + virConnectDomainEventDeregisterAny(dconn, callback15ret); if (callback8ret != -1) virConnectDomainEventDeregisterAny(dconn, callback8ret); } diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py index b456dec..b7c10d1 100644 --- a/examples/domain-events/events-python/event-test.py +++ b/examples/domain-events/events-python/event-test.py @@ -495,6 +495,9 @@ def myDomainEventBalloonChangeCallback(conn, dom, actual, opaque): def myDomainEventPMSuspendDiskCallback(conn, dom, reason, opaque): print "myDomainEventPMSuspendDiskCallback: Domain %s(%s) system pmsuspend_disk" % ( dom.name(), dom.ID()) +def myDomainEventDeviceRemovedCallback(conn, dom, dev, opaque): + print "myDomainEventDeviceRemovedCallback: Domain %s(%s) device removed: %s" % ( + dom.name(), dom.ID(), dev) run = True @@ -570,6 +573,7 @@ def main(): vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_PMSUSPEND, myDomainEventPMSuspendCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE, myDomainEventBalloonChangeCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_PMSUSPEND_DISK, myDomainEventPMSuspendDiskCallback, None) + vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED, myDomainEventDeviceRemovedCallback, None) vc.setKeepAlive(5, 3) -- 1.8.3.2

--- src/libvirt.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index 4dc91d7..860188b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -10825,6 +10825,18 @@ 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. + * * Returns 0 in case of success, -1 in case of failure. */ int -- 1.8.3.2

--- src/qemu/qemu_command.c | 35 ++++++++++++++++++++--- src/qemu/qemu_command.h | 8 +++--- src/qemu/qemu_hotplug.c | 75 ++++++++----------------------------------------- 3 files changed, 47 insertions(+), 71 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d6ef9cd..b46462b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1144,8 +1144,9 @@ qemuDomainCCWAddressValidate(virDomainDefPtr def ATTRIBUTE_UNUSED, return qemuDomainCCWAddressAssign(info, data, false); } -int qemuDomainCCWAddressReleaseAddr(qemuDomainCCWAddressSetPtr addrs, - virDomainDeviceInfoPtr dev) +static int +qemuDomainCCWAddressReleaseAddr(qemuDomainCCWAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) { char *addr; int ret; @@ -1784,8 +1785,9 @@ int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, return 0; } -int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr) +static int +qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) { if (!qemuPCIAddressValidate(addrs, addr)) return -1; @@ -1878,6 +1880,31 @@ int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs, } +void +qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, + virDomainDeviceInfoPtr info, + const char *devstr) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (!devstr) + devstr = info->alias; + + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + STREQLEN(vm->def->os.machine, "s390-ccw", 8) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW) && + qemuDomainCCWAddressReleaseAddr(priv->ccwaddrs, info) < 0) + VIR_WARN("Unable to release CCW address on %s", + NULLSTR(devstr)); + else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && + qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, + &info->addr.pci) < 0) + VIR_WARN("Unable to release PCI address on %s", + NULLSTR(devstr)); +} + + #define IS_USB2_CONTROLLER(ctrl) \ (((ctrl)->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && \ ((ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1 || \ diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 88d7099..e15fe64 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -221,6 +221,10 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); +void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, + virDomainDeviceInfoPtr info, + const char *devstr); + int qemuDomainAssignPCIAddresses(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainObjPtr obj); @@ -237,16 +241,12 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev); int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs, virDevicePCIAddressPtr addr); -int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr); void qemuDomainPCIAddressSetFree(qemuDomainPCIAddressSetPtr addrs); int qemuAssignDevicePCISlots(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, qemuDomainPCIAddressSetPtr addrs); -int qemuDomainCCWAddressReleaseAddr(qemuDomainCCWAddressSetPtr addrs, - virDomainDeviceInfoPtr dev); int qemuDomainCCWAddressAssign(virDomainDeviceInfoPtr dev, qemuDomainCCWAddressSetPtr addrs, bool autoassign); void qemuDomainCCWAddressSetFree(qemuDomainCCWAddressSetPtr addrs); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ac9350b..2a57fef 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -327,16 +327,8 @@ cleanup: return ret; error: - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && releaseaddr) { - if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &disk->info.addr.pci) < 0) - VIR_WARN("Unable to release PCI address on %s", disk->src); - else if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - qemuDomainCCWAddressReleaseAddr(priv->ccwaddrs, - &disk->info) < 0) - VIR_WARN("Unable to release CCW address on %s", disk->src); - } + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &disk->info, disk->src); if (virSecurityManagerRestoreImageLabel(driver->securityManager, vm->def, disk) < 0) @@ -405,13 +397,8 @@ int qemuDomainAttachPciControllerDevice(virQEMUDriverPtr driver, } cleanup: - if ((ret != 0) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && - releaseaddr && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &controller->info.addr.pci) < 0) - VIR_WARN("Unable to release PCI address on controller"); + if (ret != 0 && releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &controller->info, NULL); VIR_FREE(devstr); return ret; @@ -930,19 +917,8 @@ cleanup: if (!ret) { vm->def->nets[vm->def->nnets++] = net; } else { - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - releaseaddr && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &net->info.addr.pci) < 0) - VIR_WARN("Unable to release PCI address on NIC"); - else if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW) && - net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - releaseaddr && - qemuDomainCCWAddressReleaseAddr(priv->ccwaddrs, - &net->info) < 0) - VIR_WARN("Unable to release CCW address on NIC"); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, &net->info, NULL); if (iface_connected) { virDomainConfNWFilterTeardown(net); @@ -1113,12 +1089,8 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, return 0; error: - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && - releaseaddr && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &hostdev->info->addr.pci) < 0) - VIR_WARN("Unable to release PCI address on host device"); + if (releaseaddr) + qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); qemuDomainReAttachHostdevDevices(driver, vm->def->name, &hostdev, 1); @@ -2213,16 +2185,7 @@ int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, virDomainAuditDisk(vm, detach->src, NULL, "detach", true); - if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && - STREQLEN(vm->def->os.machine, "s390-ccw", 8) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW) && - qemuDomainCCWAddressReleaseAddr(priv->ccwaddrs, &detach->info) < 0) { - VIR_WARN("Unable to release CCW address on %s", dev->data.disk->src); - } else if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &detach->info.addr.pci) < 0) - VIR_WARN("Unable to release PCI address on %s", dev->data.disk->src); + qemuDomainReleaseDeviceAddress(vm, &detach->info, dev->data.disk->src); virDomainDiskRemove(vm->def, idx); @@ -2437,13 +2400,9 @@ int qemuDomainDetachPciControllerDevice(virQEMUDriverPtr driver, qemuDomainObjExitMonitor(driver, vm); virDomainControllerRemove(vm->def, idx); + qemuDomainReleaseDeviceAddress(vm, &detach->info, NULL); virDomainControllerDefFree(detach); - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &detach->info.addr.pci) < 0) - VIR_WARN("Unable to release PCI address on controller"); - ret = 0; cleanup: @@ -2515,10 +2474,7 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver, virObjectUnlock(driver->activePciHostdevs); virObjectUnlock(driver->inactivePciHostdevs); - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &detach->info->addr.pci) < 0) - VIR_WARN("Unable to release PCI address on host device"); + qemuDomainReleaseDeviceAddress(vm, detach->info, NULL); cleanup: virObjectUnref(cfg); @@ -2841,14 +2797,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainAuditNet(vm, detach, NULL, "detach", true); - if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { - if (qemuDomainCCWAddressReleaseAddr(priv->ccwaddrs, &detach->info) < 0) - VIR_WARN("Unable to release CCW address on NIC"); - } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && - qemuDomainPCIAddressReleaseSlot(priv->pciaddrs, - &detach->info.addr.pci) < 0) - VIR_WARN("Unable to release PCI address on NIC"); + qemuDomainReleaseDeviceAddress(vm, &detach->info, NULL); virDomainConfNWFilterTeardown(detach); -- 1.8.3.2

--- src/qemu/qemu_hotplug.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6db789d..0093245 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2139,6 +2139,28 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } +static void +qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + virDomainControllerDefPtr controller) +{ + size_t i; + + VIR_DEBUG("Removing controller %s from domain %p %s", + controller->info.alias, vm, vm->def->name); + + for (i = 0; i < vm->def->ncontrollers; i++) { + if (vm->def->controllers[i] == controller) { + virDomainControllerRemove(vm->def, i); + break; + } + } + + qemuDomainReleaseDeviceAddress(vm, &controller->info, NULL); + virDomainControllerDefFree(controller); +} + + int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr detach) @@ -2364,9 +2386,7 @@ int qemuDomainDetachPciControllerDevice(virQEMUDriverPtr driver, } qemuDomainObjExitMonitor(driver, vm); - virDomainControllerRemove(vm->def, idx); - qemuDomainReleaseDeviceAddress(vm, &detach->info, NULL); - virDomainControllerDefFree(detach); + qemuDomainRemoveControllerDevice(driver, vm, detach); ret = 0; -- 1.8.3.2

--- src/qemu/qemu_hotplug.c | 240 ++++++++++++++++++++++++++++-------------------- 1 file changed, 142 insertions(+), 98 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 26a6d42..169cce2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2217,6 +2217,136 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, } +static void +qemuDomainRemovePCIHostDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + virPCIDevicePtr pci; + virPCIDevicePtr activePci; + + /* + * For SRIOV net host devices, unset mac and port profile before + * reset and reattach device + */ + if (hostdev->parent.data.net) + qemuDomainHostdevNetConfigRestore(hostdev, cfg->stateDir); + + virObjectLock(driver->activePciHostdevs); + virObjectLock(driver->inactivePciHostdevs); + pci = virPCIDeviceNew(subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, + subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + if (pci) { + activePci = virPCIDeviceListSteal(driver->activePciHostdevs, pci); + if (activePci && + virPCIDeviceReset(activePci, driver->activePciHostdevs, + driver->inactivePciHostdevs) == 0) { + qemuReattachPciDevice(activePci, driver); + } else { + /* reset of the device failed, treat it as if it was returned */ + virPCIDeviceFree(activePci); + } + virPCIDeviceFree(pci); + } + virObjectUnlock(driver->activePciHostdevs); + virObjectUnlock(driver->inactivePciHostdevs); + + qemuDomainReleaseDeviceAddress(vm, hostdev->info, NULL); + virObjectUnref(cfg); +} + +static void +qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr hostdev) +{ + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + virUSBDevicePtr usb; + + usb = virUSBDeviceNew(subsys->u.usb.bus, subsys->u.usb.device, NULL); + if (usb) { + virObjectLock(driver->activeUsbHostdevs); + virUSBDeviceListDel(driver->activeUsbHostdevs, usb); + virObjectUnlock(driver->activeUsbHostdevs); + virUSBDeviceFree(usb); + } else { + VIR_WARN("Unable to find device %03d.%03d in list of used USB devices", + subsys->u.usb.bus, subsys->u.usb.device); + } +} + +static void +qemuDomainRemoveSCSIHostDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) +{ + qemuDomainReAttachHostScsiDevices(driver, vm->def->name, &hostdev, 1); +} + +static void +qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) +{ + virDomainNetDefPtr net = NULL; + size_t i; + + VIR_DEBUG("Removing host device %s from domain %p %s", + hostdev->info->alias, vm, vm->def->name); + + if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET) { + net = hostdev->parent.data.net; + + for (i = 0; i < vm->def->nnets; i++) { + if (vm->def->nets[i] == net) { + virDomainNetRemove(vm->def, i); + break; + } + } + } + + for (i = 0; i < vm->def->nhostdevs; i++) { + if (vm->def->hostdevs[i] == hostdev) { + virDomainHostdevRemove(vm->def, i); + break; + } + } + + virDomainAuditHostdev(vm, hostdev, "detach", true); + + switch ((enum virDomainHostdevSubsysType) hostdev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + qemuDomainRemovePCIHostDevice(driver, vm, hostdev); + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + qemuDomainRemoveUSBHostDevice(driver, vm, hostdev); + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + qemuDomainRemoveSCSIHostDevice(driver, vm, hostdev); + break; + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: + break; + } + + if (qemuTeardownHostdevCgroup(vm, hostdev) < 0) + VIR_WARN("Failed to remove host device cgroup ACL"); + + if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, + vm->def, hostdev, NULL) < 0) { + VIR_WARN("Failed to restore host device labelling"); + } + + virDomainHostdevDefFree(hostdev); + + if (net) { + networkReleaseActualDevice(net); + virDomainNetDefFree(net); + } +} + + int qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr detach) @@ -2457,68 +2587,31 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevSubsysPtr subsys = &detach->source.subsys; - int ret = -1, rv; - virPCIDevicePtr pci; - virPCIDevicePtr activePci; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret; if (qemuIsMultiFunctionDevice(vm->def, detach->info)) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug multifunction PCI device: %.4x:%.2x:%.2x.%.1x"), subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, subsys->u.pci.addr.slot, subsys->u.pci.addr.function); - goto cleanup; + return -1; } if (!virDomainDeviceAddressIsValid(detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("device cannot be detached without a PCI address")); - goto cleanup; + return -1; } qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { - rv = qemuMonitorDelDevice(priv->mon, detach->info->alias); + ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); } else { - rv = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci); + ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci); } qemuDomainObjExitMonitor(driver, vm); - virDomainAuditHostdev(vm, detach, "detach", rv == 0); - if (rv < 0) - goto cleanup; - - /* - * For SRIOV net host devices, unset mac and port profile before - * reset and reattach device - */ - if (detach->parent.data.net) - qemuDomainHostdevNetConfigRestore(detach, cfg->stateDir); - - virObjectLock(driver->activePciHostdevs); - virObjectLock(driver->inactivePciHostdevs); - pci = virPCIDeviceNew(subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, - subsys->u.pci.addr.slot, subsys->u.pci.addr.function); - if (pci) { - activePci = virPCIDeviceListSteal(driver->activePciHostdevs, pci); - if (activePci && - virPCIDeviceReset(activePci, driver->activePciHostdevs, - driver->inactivePciHostdevs) == 0) { - qemuReattachPciDevice(activePci, driver); - ret = 0; - } else { - /* reset of the device failed, treat it as if it was returned */ - virPCIDeviceFree(activePci); - } - virPCIDeviceFree(pci); - } - virObjectUnlock(driver->activePciHostdevs); - virObjectUnlock(driver->inactivePciHostdevs); - - qemuDomainReleaseDeviceAddress(vm, detach->info, NULL); -cleanup: - virObjectUnref(cfg); return ret; } @@ -2528,8 +2621,6 @@ qemuDomainDetachHostUsbDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr detach) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainHostdevSubsysPtr subsys = &detach->source.subsys; - virUSBDevicePtr usb; int ret; if (!detach->info->alias) { @@ -2547,20 +2638,7 @@ qemuDomainDetachHostUsbDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorDelDevice(priv->mon, detach->info->alias); qemuDomainObjExitMonitor(driver, vm); - virDomainAuditHostdev(vm, detach, "detach", ret == 0); - if (ret < 0) - return -1; - usb = virUSBDeviceNew(subsys->u.usb.bus, subsys->u.usb.device, NULL); - if (usb) { - virObjectLock(driver->activeUsbHostdevs); - virUSBDeviceListDel(driver->activeUsbHostdevs, usb); - virObjectUnlock(driver->activeUsbHostdevs); - virUSBDeviceFree(usb); - } else { - VIR_WARN("Unable to find device %03d.%03d in list of used USB devices", - subsys->u.usb.bus, subsys->u.usb.device); - } return ret; } @@ -2608,11 +2686,6 @@ qemuDomainDetachHostScsiDevice(virQEMUDriverPtr driver, } qemuDomainObjExitMonitor(driver, vm); - virDomainAuditHostdev(vm, detach, "detach", ret == 0); - - if (ret == 0) - qemuDomainReAttachHostScsiDevices(driver, vm->def->name, &detach, 1); - cleanup: VIR_FREE(drvstr); VIR_FREE(devstr); @@ -2622,27 +2695,10 @@ cleanup: static int qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainHostdevDefPtr detach, - int idx) + virDomainHostdevDefPtr detach) { int ret = -1; - if (idx < 0) { - /* caller didn't know index of hostdev in hostdevs list, so we - * need to find it. - */ - for (idx = 0; idx < vm->def->nhostdevs; idx++) { - if (vm->def->hostdevs[idx] == detach) - break; - } - if (idx >= vm->def->nhostdevs) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("device not found in hostdevs list (%zu entries)"), - vm->def->nhostdevs); - return ret; - } - } - switch (detach->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: ret = qemuDomainDetachHostPciDevice(driver, vm, detach); @@ -2660,17 +2716,11 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, return -1; } - if (!ret) { - if (qemuTeardownHostdevCgroup(vm, detach) < 0) - VIR_WARN("Failed to remove host device cgroup ACL"); + if (ret < 0) + virDomainAuditHostdev(vm, detach, "detach", false); + else + qemuDomainRemoveHostDevice(driver, vm, detach); - if (virSecurityManagerRestoreHostdevLabel(driver->securityManager, - vm->def, detach, NULL) < 0) { - VIR_WARN("Failed to restore host device labelling"); - } - virDomainHostdevRemove(vm->def, idx); - virDomainHostdevDefFree(detach); - } return ret; } @@ -2732,7 +2782,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, if (detach->parent.type == VIR_DOMAIN_DEVICE_NET) return qemuDomainDetachNetDevice(driver, vm, &detach->parent); else - return qemuDomainDetachThisHostDevice(driver, vm, detach, idx); + return qemuDomainDetachThisHostDevice(driver, vm, detach); } int @@ -2765,13 +2815,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* coverity[negative_returns] */ ret = qemuDomainDetachThisHostDevice(driver, vm, - virDomainNetGetActualHostdev(detach), - -1); - if (!ret) { - networkReleaseActualDevice(detach); - virDomainNetRemove(vm->def, detachidx); - virDomainNetDefFree(detach); - } + virDomainNetGetActualHostdev(detach)); goto cleanup; } if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && -- 1.8.3.2

On Mon, Jul 15, 2013 at 07:11:12PM +0200, Jiri Denemark wrote: This field just feels very wrong to me. It is storing state related to a single method call, outside the execution lifetime of the method call. Can't we get rid of this bit here... And change this to if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT)) qemuDomainRemoveDiskDevice(driver, vm, detach); And kill the first part of this if() block, leaving only the else(), so removal of the device is fully delegated to this callback when the event is known to exist. Also, if we have the event, I think we ought to make the virDomainDetachDevice() API block waiting for arrival of the event, for some reasonable finite period of time. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/qemu/qemu_hotplug.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8c2f756..ea62e71 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2105,6 +2105,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainDiskDefPtr disk) { virDomainDeviceDef dev; + virDomainEventPtr event; size_t i; VIR_DEBUG("Removing disk %s from domain %p %s", @@ -2112,6 +2113,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); @@ -2140,15 +2145,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); @@ -2168,6 +2178,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", @@ -2175,6 +2186,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); @@ -2291,11 +2306,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; -- 1.8.3.2