[libvirt] [PATCH 0/2] qemu: add VIR_DOMAIN_EVENT_ID_DEVICE_ADDED event
https://bugzilla.redhat.com/show_bug.cgi?id=1206114 Ján Tomko (2): Add VIR_DOMAIN_EVENT_ID_DEVICE_ADDED event Emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED in the QEMU driver daemon/remote.c | 37 +++++++++++++++++++ include/libvirt/libvirt-domain.h | 18 ++++++++++ src/conf/domain_event.c | 77 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 6 ++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 17 ++++++++- src/qemu/qemu_hotplug.c | 5 +++ src/remote/remote_driver.c | 29 +++++++++++++++ src/remote/remote_protocol.x | 14 +++++++- src/remote_protocol-structs | 6 ++++ tools/virsh-domain.c | 20 +++++++++++ 11 files changed, 229 insertions(+), 2 deletions(-) -- 2.0.5
The counterpart to VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED. https://bugzilla.redhat.com/show_bug.cgi?id=1206114 --- daemon/remote.c | 37 +++++++++++++++++++ include/libvirt/libvirt-domain.h | 18 ++++++++++ src/conf/domain_event.c | 77 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 6 ++++ src/libvirt_private.syms | 2 ++ src/remote/remote_driver.c | 29 +++++++++++++++ src/remote/remote_protocol.x | 14 +++++++- src/remote_protocol-structs | 6 ++++ tools/virsh-domain.c | 20 +++++++++++ 9 files changed, 208 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2e1f973..3a3f168 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1045,6 +1045,42 @@ remoteRelayDomainEventAgentLifecycle(virConnectPtr conn, } +static int +remoteRelayDomainEventDeviceAdded(virConnectPtr conn, + virDomainPtr dom, + const char *devAlias, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_callback_device_added_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain device added 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_ADDED, + (xdrproc_t)xdr_remote_domain_event_callback_device_added_msg, + &data); + + return 0; +} + + + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -1065,6 +1101,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob2), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventAgentLifecycle), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7be4219..8a4fe53 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3202,6 +3202,23 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, void *opaque); /** + * virConnectDomainEventDeviceAddedCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @devAlias: device alias + * @opaque: application specified data + * + * This callback occurs when a device is added to the domain. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_DEVICE_ADDED with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventDeviceAddedCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *devAlias, + void *opaque); + +/** * VIR_DOMAIN_TUNABLE_CPU_VCPUPIN: * * Macro represents formatted pinning for one vcpu specified by id which is @@ -3483,6 +3500,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2 = 16, /* virConnectDomainEventBlockJobCallback */ VIR_DOMAIN_EVENT_ID_TUNABLE = 17, /* virConnectDomainEventTunableCallback */ VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE = 18,/* virConnectDomainEventAgentLifecycleCallback */ + VIR_DOMAIN_EVENT_ID_DEVICE_ADDED = 19, /* virConnectDomainEventDeviceAddedCallback */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 2786c1e..20d66e1 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -55,6 +55,7 @@ static virClassPtr virDomainEventPMClass; static virClassPtr virDomainQemuMonitorEventClass; static virClassPtr virDomainEventTunableClass; static virClassPtr virDomainEventAgentLifecycleClass; +static virClassPtr virDomainEventDeviceAddedClass; static void virDomainEventDispose(void *obj); @@ -72,6 +73,7 @@ static void virDomainEventPMDispose(void *obj); static void virDomainQemuMonitorEventDispose(void *obj); static void virDomainEventTunableDispose(void *obj); static void virDomainEventAgentLifecycleDispose(void *obj); +static void virDomainEventDeviceAddedDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -189,6 +191,14 @@ struct _virDomainEventDeviceRemoved { typedef struct _virDomainEventDeviceRemoved virDomainEventDeviceRemoved; typedef virDomainEventDeviceRemoved *virDomainEventDeviceRemovedPtr; +struct _virDomainEventDeviceAdded { + virDomainEvent parent; + + char *devAlias; +}; +typedef struct _virDomainEventDeviceAdded virDomainEventDeviceAdded; +typedef virDomainEventDeviceAdded *virDomainEventDeviceAddedPtr; + struct _virDomainEventPM { virDomainEvent parent; @@ -296,6 +306,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainEventDeviceRemoved), virDomainEventDeviceRemovedDispose))) return -1; + if (!(virDomainEventDeviceAddedClass = + virClassNew(virDomainEventClass, + "virDomainEventDeviceAdded", + sizeof(virDomainEventDeviceAdded), + virDomainEventDeviceAddedDispose))) + return -1; if (!(virDomainEventPMClass = virClassNew(virDomainEventClass, "virDomainEventPM", @@ -439,6 +455,15 @@ virDomainEventDeviceRemovedDispose(void *obj) } static void +virDomainEventDeviceAddedDispose(void *obj) +{ + virDomainEventDeviceAddedPtr event = obj; + VIR_DEBUG("obj=%p", event); + + VIR_FREE(event->devAlias); +} + +static void virDomainEventPMDispose(void *obj) { virDomainEventPMPtr event = obj; @@ -1226,6 +1251,47 @@ virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, devAlias); } +static virObjectEventPtr +virDomainEventDeviceAddedNew(int id, + const char *name, + unsigned char *uuid, + const char *devAlias) +{ + virDomainEventDeviceAddedPtr ev; + + if (virDomainEventsInitialize() < 0) + return NULL; + + if (!(ev = virDomainEventNew(virDomainEventDeviceAddedClass, + VIR_DOMAIN_EVENT_ID_DEVICE_ADDED, + id, name, uuid))) + return NULL; + + if (VIR_STRDUP(ev->devAlias, devAlias) < 0) + goto error; + + return (virObjectEventPtr)ev; + + error: + virObjectUnref(ev); + return NULL; +} + +virObjectEventPtr +virDomainEventDeviceAddedNewFromObj(virDomainObjPtr obj, + const char *devAlias) +{ + return virDomainEventDeviceAddedNew(obj->def->id, obj->def->name, + obj->def->uuid, devAlias); +} + +virObjectEventPtr +virDomainEventDeviceAddedNewFromDom(virDomainPtr dom, + const char *devAlias) +{ + return virDomainEventDeviceAddedNew(dom->id, dom->name, dom->uuid, + devAlias); +} static virObjectEventPtr virDomainEventAgentLifecycleNew(int id, @@ -1537,6 +1603,17 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, goto cleanup; } + case VIR_DOMAIN_EVENT_ID_DEVICE_ADDED: + { + virDomainEventDeviceAddedPtr deviceAddedEvent; + + deviceAddedEvent = (virDomainEventDeviceAddedPtr)event; + ((virConnectDomainEventDeviceAddedCallback)cb)(conn, dom, + deviceAddedEvent->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 534ff9e..afbed89 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -185,6 +185,12 @@ virObjectEventPtr virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, const char *devAlias); virObjectEventPtr +virDomainEventDeviceAddedNewFromObj(virDomainObjPtr obj, + const char *devAlias); +virObjectEventPtr +virDomainEventDeviceAddedNewFromDom(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 9f82926..cbd8089 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -486,6 +486,8 @@ virDomainEventBlockJobNewFromDom; virDomainEventBlockJobNewFromObj; virDomainEventControlErrorNewFromDom; virDomainEventControlErrorNewFromObj; +virDomainEventDeviceAddedNewFromDom; +virDomainEventDeviceAddedNewFromObj; virDomainEventDeviceRemovedNewFromDom; virDomainEventDeviceRemovedNewFromObj; virDomainEventDiskChangeNewFromDom; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b275d86..9c3b53f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -321,6 +321,10 @@ static void remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog, virNetClientPtr client, void *evdata, void *opaque); +static void +remoteDomainBuildEventCallbackDeviceAdded(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); static void remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog, @@ -496,6 +500,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventCallbackAgentLifecycle, sizeof(remote_domain_event_callback_agent_lifecycle_msg), (xdrproc_t)xdr_remote_domain_event_callback_agent_lifecycle_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED, + remoteDomainBuildEventCallbackDeviceAdded, + sizeof(remote_domain_event_callback_device_added_msg), + (xdrproc_t)xdr_remote_domain_event_callback_device_added_msg }, }; @@ -5431,6 +5439,27 @@ remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog ATTRIBUT remoteDomainBuildEventDeviceRemovedHelper(conn, &msg->msg, msg->callbackID); } +static void +remoteDomainBuildEventCallbackDeviceAdded(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; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) + return; + + event = virDomainEventDeviceAddedNewFromDom(dom, msg->devAlias); + + virObjectUnref(dom); + + remoteEventQueue(priv, event, msg->callbackID); +} static void remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d90e6b5..b02e58c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3045,6 +3045,12 @@ struct remote_domain_event_callback_tunable_msg { remote_typed_param params<REMOTE_DOMAIN_EVENT_TUNABLE_MAX>; }; +struct remote_domain_event_callback_device_added_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string devAlias; +}; + struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -5643,5 +5649,11 @@ enum remote_procedure { * @generate: none * @acl: domain:read */ - REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353 + REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED = 354 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e614f77..2b6b47a 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2487,6 +2487,11 @@ struct remote_domain_event_callback_tunable_msg { remote_typed_param * params_val; } params; }; +struct remote_domain_event_callback_device_added_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string devAlias; +}; struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -3017,4 +3022,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO = 351, REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 352, REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED = 354, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 020a308..aeacf59 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11959,6 +11959,24 @@ vshEventDeviceRemovedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, } static void +vshEventDeviceAddedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *alias, + void *opaque) +{ + vshDomEventData *data = opaque; + + if (!data->loop && *data->count) + return; + vshPrint(data->ctl, + _("event 'device-added' for domain %s: %s\n"), + virDomainGetName(dom), alias); + (*data->count)++; + if (!data->loop) + vshEventDone(data->ctl); +} + +static void vshEventTunablePrint(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, virTypedParameterPtr params, @@ -12063,6 +12081,8 @@ static vshEventCallback vshEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(vshEventTunablePrint), }, { "agent-lifecycle", VIR_DOMAIN_EVENT_CALLBACK(vshEventAgentLifecyclePrint), }, + { "device-added", + VIR_DOMAIN_EVENT_CALLBACK(vshEventDeviceAddedPrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks)); -- 2.0.5
On 04/04/2015 01:16 PM, Ján Tomko wrote:
The counterpart to VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1206114 --- daemon/remote.c | 37 +++++++++++++++++++ include/libvirt/libvirt-domain.h | 18 ++++++++++ src/conf/domain_event.c | 77 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 6 ++++ src/libvirt_private.syms | 2 ++ src/remote/remote_driver.c | 29 +++++++++++++++ src/remote/remote_protocol.x | 14 +++++++- src/remote_protocol-structs | 6 ++++ tools/virsh-domain.c | 20 +++++++++++ 9 files changed, 208 insertions(+), 1 deletion(-)
I searched on VIR_DOMAIN_EVENT_ID_DEVICE - what about examples/object-events/event-test.c ? Also should 'src/libvirt-domain.c' have a description for the _ADDED flag in 'virDomainAttachDeviceFlags' like there is for _REMOVED in 'virDomainDetachDeviceFlags'? (although even that text is a bit shy of an 'a' - as in "or add a handler for" rather than "or add handler for" <sigh> ACK in general for what's here and with the new test and change to AttachDevice description... John
diff --git a/daemon/remote.c b/daemon/remote.c index 2e1f973..3a3f168 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1045,6 +1045,42 @@ remoteRelayDomainEventAgentLifecycle(virConnectPtr conn, }
+static int +remoteRelayDomainEventDeviceAdded(virConnectPtr conn, + virDomainPtr dom, + const char *devAlias, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_domain_event_callback_device_added_msg data; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain device added 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_ADDED, + (xdrproc_t)xdr_remote_domain_event_callback_device_added_msg, + &data); + + return 0; +} + + + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -1065,6 +1101,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockJob2), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventAgentLifecycle), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded), };
verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 7be4219..8a4fe53 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3202,6 +3202,23 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, void *opaque);
/** + * virConnectDomainEventDeviceAddedCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @devAlias: device alias + * @opaque: application specified data + * + * This callback occurs when a device is added to the domain. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_DEVICE_ADDED with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventDeviceAddedCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *devAlias, + void *opaque); + +/** * VIR_DOMAIN_TUNABLE_CPU_VCPUPIN: * * Macro represents formatted pinning for one vcpu specified by id which is @@ -3483,6 +3500,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2 = 16, /* virConnectDomainEventBlockJobCallback */ VIR_DOMAIN_EVENT_ID_TUNABLE = 17, /* virConnectDomainEventTunableCallback */ VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE = 18,/* virConnectDomainEventAgentLifecycleCallback */ + VIR_DOMAIN_EVENT_ID_DEVICE_ADDED = 19, /* virConnectDomainEventDeviceAddedCallback */
# ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 2786c1e..20d66e1 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -55,6 +55,7 @@ static virClassPtr virDomainEventPMClass; static virClassPtr virDomainQemuMonitorEventClass; static virClassPtr virDomainEventTunableClass; static virClassPtr virDomainEventAgentLifecycleClass; +static virClassPtr virDomainEventDeviceAddedClass;
static void virDomainEventDispose(void *obj); @@ -72,6 +73,7 @@ static void virDomainEventPMDispose(void *obj); static void virDomainQemuMonitorEventDispose(void *obj); static void virDomainEventTunableDispose(void *obj); static void virDomainEventAgentLifecycleDispose(void *obj); +static void virDomainEventDeviceAddedDispose(void *obj);
static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -189,6 +191,14 @@ struct _virDomainEventDeviceRemoved { typedef struct _virDomainEventDeviceRemoved virDomainEventDeviceRemoved; typedef virDomainEventDeviceRemoved *virDomainEventDeviceRemovedPtr;
+struct _virDomainEventDeviceAdded { + virDomainEvent parent; + + char *devAlias; +}; +typedef struct _virDomainEventDeviceAdded virDomainEventDeviceAdded; +typedef virDomainEventDeviceAdded *virDomainEventDeviceAddedPtr; + struct _virDomainEventPM { virDomainEvent parent;
@@ -296,6 +306,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainEventDeviceRemoved), virDomainEventDeviceRemovedDispose))) return -1; + if (!(virDomainEventDeviceAddedClass = + virClassNew(virDomainEventClass, + "virDomainEventDeviceAdded", + sizeof(virDomainEventDeviceAdded), + virDomainEventDeviceAddedDispose))) + return -1; if (!(virDomainEventPMClass = virClassNew(virDomainEventClass, "virDomainEventPM", @@ -439,6 +455,15 @@ virDomainEventDeviceRemovedDispose(void *obj) }
static void +virDomainEventDeviceAddedDispose(void *obj) +{ + virDomainEventDeviceAddedPtr event = obj; + VIR_DEBUG("obj=%p", event); + + VIR_FREE(event->devAlias); +} + +static void virDomainEventPMDispose(void *obj) { virDomainEventPMPtr event = obj; @@ -1226,6 +1251,47 @@ virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, devAlias); }
+static virObjectEventPtr +virDomainEventDeviceAddedNew(int id, + const char *name, + unsigned char *uuid, + const char *devAlias) +{ + virDomainEventDeviceAddedPtr ev; + + if (virDomainEventsInitialize() < 0) + return NULL; + + if (!(ev = virDomainEventNew(virDomainEventDeviceAddedClass, + VIR_DOMAIN_EVENT_ID_DEVICE_ADDED, + id, name, uuid))) + return NULL; + + if (VIR_STRDUP(ev->devAlias, devAlias) < 0) + goto error; + + return (virObjectEventPtr)ev; + + error: + virObjectUnref(ev); + return NULL; +} + +virObjectEventPtr +virDomainEventDeviceAddedNewFromObj(virDomainObjPtr obj, + const char *devAlias) +{ + return virDomainEventDeviceAddedNew(obj->def->id, obj->def->name, + obj->def->uuid, devAlias); +} + +virObjectEventPtr +virDomainEventDeviceAddedNewFromDom(virDomainPtr dom, + const char *devAlias) +{ + return virDomainEventDeviceAddedNew(dom->id, dom->name, dom->uuid, + devAlias); +}
static virObjectEventPtr virDomainEventAgentLifecycleNew(int id, @@ -1537,6 +1603,17 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, goto cleanup; }
+ case VIR_DOMAIN_EVENT_ID_DEVICE_ADDED: + { + virDomainEventDeviceAddedPtr deviceAddedEvent; + + deviceAddedEvent = (virDomainEventDeviceAddedPtr)event; + ((virConnectDomainEventDeviceAddedCallback)cb)(conn, dom, + deviceAddedEvent->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 534ff9e..afbed89 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -185,6 +185,12 @@ virObjectEventPtr virDomainEventDeviceRemovedNewFromDom(virDomainPtr dom, const char *devAlias); virObjectEventPtr +virDomainEventDeviceAddedNewFromObj(virDomainObjPtr obj, + const char *devAlias); +virObjectEventPtr +virDomainEventDeviceAddedNewFromDom(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 9f82926..cbd8089 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -486,6 +486,8 @@ virDomainEventBlockJobNewFromDom; virDomainEventBlockJobNewFromObj; virDomainEventControlErrorNewFromDom; virDomainEventControlErrorNewFromObj; +virDomainEventDeviceAddedNewFromDom; +virDomainEventDeviceAddedNewFromObj; virDomainEventDeviceRemovedNewFromDom; virDomainEventDeviceRemovedNewFromObj; virDomainEventDiskChangeNewFromDom; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b275d86..9c3b53f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -321,6 +321,10 @@ static void remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog, virNetClientPtr client, void *evdata, void *opaque); +static void +remoteDomainBuildEventCallbackDeviceAdded(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque);
static void remoteDomainBuildEventBlockJob2(virNetClientProgramPtr prog, @@ -496,6 +500,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventCallbackAgentLifecycle, sizeof(remote_domain_event_callback_agent_lifecycle_msg), (xdrproc_t)xdr_remote_domain_event_callback_agent_lifecycle_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED, + remoteDomainBuildEventCallbackDeviceAdded, + sizeof(remote_domain_event_callback_device_added_msg), + (xdrproc_t)xdr_remote_domain_event_callback_device_added_msg }, };
@@ -5431,6 +5439,27 @@ remoteDomainBuildEventCallbackDeviceRemoved(virNetClientProgramPtr prog ATTRIBUT remoteDomainBuildEventDeviceRemovedHelper(conn, &msg->msg, msg->callbackID); }
+static void +remoteDomainBuildEventCallbackDeviceAdded(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; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) + return; + + event = virDomainEventDeviceAddedNewFromDom(dom, msg->devAlias); + + virObjectUnref(dom); + + remoteEventQueue(priv, event, msg->callbackID); +}
static void remoteDomainBuildEventCallbackTunable(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index d90e6b5..b02e58c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3045,6 +3045,12 @@ struct remote_domain_event_callback_tunable_msg { remote_typed_param params<REMOTE_DOMAIN_EVENT_TUNABLE_MAX>; };
+struct remote_domain_event_callback_device_added_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string devAlias; +}; + struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -5643,5 +5649,11 @@ enum remote_procedure { * @generate: none * @acl: domain:read */ - REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353 + REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED = 354 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e614f77..2b6b47a 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2487,6 +2487,11 @@ struct remote_domain_event_callback_tunable_msg { remote_typed_param * params_val; } params; }; +struct remote_domain_event_callback_device_added_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string devAlias; +}; struct remote_connect_get_cpu_model_names_args { remote_nonnull_string arch; int need_results; @@ -3017,4 +3022,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_IOTHREAD_INFO = 351, REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 352, REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED = 354, }; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 020a308..aeacf59 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11959,6 +11959,24 @@ vshEventDeviceRemovedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, }
static void +vshEventDeviceAddedPrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *alias, + void *opaque) +{ + vshDomEventData *data = opaque; + + if (!data->loop && *data->count) + return; + vshPrint(data->ctl, + _("event 'device-added' for domain %s: %s\n"), + virDomainGetName(dom), alias); + (*data->count)++; + if (!data->loop) + vshEventDone(data->ctl); +} + +static void vshEventTunablePrint(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, virTypedParameterPtr params, @@ -12063,6 +12081,8 @@ static vshEventCallback vshEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(vshEventTunablePrint), }, { "agent-lifecycle", VIR_DOMAIN_EVENT_CALLBACK(vshEventAgentLifecyclePrint), }, + { "device-added", + VIR_DOMAIN_EVENT_CALLBACK(vshEventDeviceAddedPrint), }, }; verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));
On Mon, Apr 13, 2015 at 01:59:39PM -0400, John Ferlan wrote:
On 04/04/2015 01:16 PM, Ján Tomko wrote:
The counterpart to VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1206114 --- daemon/remote.c | 37 +++++++++++++++++++ include/libvirt/libvirt-domain.h | 18 ++++++++++ src/conf/domain_event.c | 77 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 6 ++++ src/libvirt_private.syms | 2 ++ src/remote/remote_driver.c | 29 +++++++++++++++ src/remote/remote_protocol.x | 14 +++++++- src/remote_protocol-structs | 6 ++++ tools/virsh-domain.c | 20 +++++++++++ 9 files changed, 208 insertions(+), 1 deletion(-)
I searched on VIR_DOMAIN_EVENT_ID_DEVICE - what about examples/object-events/event-test.c ?
This patch already includes an example in tools/virsh :) I have sent a separate patch adding it to event-test as well. (I would have separated this patch too, as suggested by the http://libvirt.org/api_extension.html page, but it failed to compile separately due to either ACL check or unhandled enum values)
Also should 'src/libvirt-domain.c' have a description for the _ADDED flag in 'virDomainAttachDeviceFlags' like there is for _REMOVED in 'virDomainDetachDeviceFlags'?
No. With DetachDevice, the device may still not be detached even if the API returns and the caller needs to wait for the event to be sure. Attaching a device is synchronous, so waiting for the event only makes sense if the domain was changed from another connection. Generally, we don't seem to document events in the APIs that cause them.
(although even that text is a bit shy of an 'a' - as in "or add a handler for" rather than "or add handler for" <sigh>
Fixed: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=682ba8e9
ACK in general for what's here and with the new test and change to AttachDevice description...
I don't think either of those should be included in the patch. Jan
John
On 04/14/2015 09:35 AM, Ján Tomko wrote:
On Mon, Apr 13, 2015 at 01:59:39PM -0400, John Ferlan wrote:
On 04/04/2015 01:16 PM, Ján Tomko wrote:
The counterpart to VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1206114 --- daemon/remote.c | 37 +++++++++++++++++++ include/libvirt/libvirt-domain.h | 18 ++++++++++ src/conf/domain_event.c | 77 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 6 ++++ src/libvirt_private.syms | 2 ++ src/remote/remote_driver.c | 29 +++++++++++++++ src/remote/remote_protocol.x | 14 +++++++- src/remote_protocol-structs | 6 ++++ tools/virsh-domain.c | 20 +++++++++++ 9 files changed, 208 insertions(+), 1 deletion(-)
I searched on VIR_DOMAIN_EVENT_ID_DEVICE - what about examples/object-events/event-test.c ?
This patch already includes an example in tools/virsh :)
I have sent a separate patch adding it to event-test as well.
(I would have separated this patch too, as suggested by the http://libvirt.org/api_extension.html page, but it failed to compile separately due to either ACL check or unhandled enum values)
Also should 'src/libvirt-domain.c' have a description for the _ADDED flag in 'virDomainAttachDeviceFlags' like there is for _REMOVED in 'virDomainDetachDeviceFlags'?
No. With DetachDevice, the device may still not be detached even if the API returns and the caller needs to wait for the event to be sure. Attaching a device is synchronous, so waiting for the event only makes sense if the domain was changed from another connection.
Generally, we don't seem to document events in the APIs that cause them.
(although even that text is a bit shy of an 'a' - as in "or add a handler for" rather than "or add handler for" <sigh>
Fixed: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=682ba8e9
ACK in general for what's here and with the new test and change to AttachDevice description...
I don't think either of those should be included in the patch.
Jan
OK - explanation is fine with me. ACK - John
Only for devices that have an alias. --- src/qemu/qemu_driver.c | 17 ++++++++++++++++- src/qemu/qemu_hotplug.c | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..c13f22b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7586,17 +7586,20 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; + const char *alias = NULL; switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1); ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev); + alias = dev->data.disk->info.alias; if (!ret) dev->data.disk = NULL; break; case VIR_DOMAIN_DEVICE_CONTROLLER: ret = qemuDomainAttachDeviceControllerLive(driver, vm, dev); + alias = dev->data.controller->info.alias; if (!ret) dev->data.controller = NULL; break; @@ -7612,6 +7615,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, -1); ret = qemuDomainAttachNetDevice(dom->conn, driver, vm, dev->data.net); + alias = dev->data.net->info.alias; if (!ret) dev->data.net = NULL; break; @@ -7620,6 +7624,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1); ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, dev->data.hostdev); + alias = dev->data.hostdev->info->alias; if (!ret) dev->data.hostdev = NULL; break; @@ -7627,6 +7632,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_REDIRDEV: ret = qemuDomainAttachRedirdevDevice(driver, vm, dev->data.redirdev); + alias = dev->data.redirdev->info.alias; if (!ret) dev->data.redirdev = NULL; break; @@ -7634,6 +7640,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_CHR: ret = qemuDomainAttachChrDevice(driver, vm, dev->data.chr); + alias = dev->data.chr->info.alias; if (!ret) dev->data.chr = NULL; break; @@ -7641,6 +7648,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_RNG: ret = qemuDomainAttachRNGDevice(driver, vm, dev->data.rng); + alias = dev->data.rng->info.alias; if (!ret) dev->data.rng = NULL; break; @@ -7673,8 +7681,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; } - if (ret == 0) + if (ret == 0) { ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + if (alias) { + virObjectEventPtr event; + event = virDomainEventDeviceAddedNewFromObj(vm, alias); + if (event) + qemuDomainEventQueue(driver, event); + } + } return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2f0549e..f07c54d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1726,6 +1726,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, char *objalias = NULL; const char *backendType; virJSONValuePtr props = NULL; + virObjectEventPtr event; int id; int ret = -1; @@ -1769,6 +1770,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, goto cleanup; } + event = virDomainEventDeviceAddedNewFromObj(vm, objalias); + if (event) + qemuDomainEventQueue(driver, event); + /* mem is consumed by vm->def */ mem = NULL; -- 2.0.5
On 04/04/2015 01:16 PM, Ján Tomko wrote:
Only for devices that have an alias. --- src/qemu/qemu_driver.c | 17 ++++++++++++++++- src/qemu/qemu_hotplug.c | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..c13f22b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7586,17 +7586,20 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; + const char *alias = NULL;
switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1); ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev); + alias = dev->data.disk->info.alias; if (!ret) dev->data.disk = NULL; break;
case VIR_DOMAIN_DEVICE_CONTROLLER: ret = qemuDomainAttachDeviceControllerLive(driver, vm, dev); + alias = dev->data.controller->info.alias;
The one concern I'd have for all of these is - if (ret != 0) - is there any path that free's anything along the way that you're using a pointer in the alias fetching? Additionally of course, since the only way to print the alias is if (ret == 0) later, one could point out that setting it when ret != 0 is pointless; however, at least if ret == 0, you should be able to assume no one as deleted the alias! Perhaps it's best to only get the alias if (!ret) Your call if you want to add a "note" for case VIR_DOMAIN_DEVICE_MEMORY that the event is elicited inside the call since the call consumes dev->data.memory and hence the alias. I think with the alias setting inside !ret I'd feel comfortable giving an ACK - although I suspect in the other case it's not deleted until after this call exits John
if (!ret) dev->data.controller = NULL; break; @@ -7612,6 +7615,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, -1); ret = qemuDomainAttachNetDevice(dom->conn, driver, vm, dev->data.net); + alias = dev->data.net->info.alias; if (!ret) dev->data.net = NULL; break; @@ -7620,6 +7624,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1); ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, dev->data.hostdev); + alias = dev->data.hostdev->info->alias; if (!ret) dev->data.hostdev = NULL; break; @@ -7627,6 +7632,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_REDIRDEV: ret = qemuDomainAttachRedirdevDevice(driver, vm, dev->data.redirdev); + alias = dev->data.redirdev->info.alias; if (!ret) dev->data.redirdev = NULL; break; @@ -7634,6 +7640,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_CHR: ret = qemuDomainAttachChrDevice(driver, vm, dev->data.chr); + alias = dev->data.chr->info.alias; if (!ret) dev->data.chr = NULL; break; @@ -7641,6 +7648,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_RNG: ret = qemuDomainAttachRNGDevice(driver, vm, dev->data.rng); + alias = dev->data.rng->info.alias; if (!ret) dev->data.rng = NULL; break; @@ -7673,8 +7681,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; }
- if (ret == 0) + if (ret == 0) { ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); + if (alias) { + virObjectEventPtr event; + event = virDomainEventDeviceAddedNewFromObj(vm, alias); + if (event) + qemuDomainEventQueue(driver, event); + } + }
return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2f0549e..f07c54d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1726,6 +1726,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, char *objalias = NULL; const char *backendType; virJSONValuePtr props = NULL; + virObjectEventPtr event; int id; int ret = -1;
@@ -1769,6 +1770,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, goto cleanup; }
+ event = virDomainEventDeviceAddedNewFromObj(vm, objalias); + if (event) + qemuDomainEventQueue(driver, event); + /* mem is consumed by vm->def */ mem = NULL;
On Mon, Apr 13, 2015 at 02:03:57PM -0400, John Ferlan wrote:
On 04/04/2015 01:16 PM, Ján Tomko wrote:
Only for devices that have an alias. --- src/qemu/qemu_driver.c | 17 ++++++++++++++++- src/qemu/qemu_hotplug.c | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..c13f22b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7586,17 +7586,20 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; + const char *alias = NULL;
switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1); ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev); + alias = dev->data.disk->info.alias; if (!ret) dev->data.disk = NULL; break;
case VIR_DOMAIN_DEVICE_CONTROLLER: ret = qemuDomainAttachDeviceControllerLive(driver, vm, dev); + alias = dev->data.controller->info.alias;
The one concern I'd have for all of these is - if (ret != 0) - is there any path that free's anything along the way that you're using a pointer in the alias fetching?
All the functions except AttachMemory should only add the device to the domain definition right after setting ret to 0, so if ret == -1, it should still be owned by the caller.
Additionally of course, since the only way to print the alias is if (ret == 0) later, one could point out that setting it when ret != 0 is pointless; however, at least if ret == 0, you should be able to assume no one as deleted the alias!
Actually, this is wrong. When ret == 0, the device is already a part of the domain definition. And the domain object is unlocked when we enter the monitor in qemuDomainUpdateDeviceList. So the Event should be queued before the call to UpdateDeviceList, or a local copy of the alias is needed. Either way, another version of this patch is needed.
Perhaps it's best to only get the alias if (!ret)
Your call if you want to add a "note" for case VIR_DOMAIN_DEVICE_MEMORY that the event is elicited inside the call since the call consumes dev->data.memory and hence the alias.
Good idea. Jan
I think with the alias setting inside !ret I'd feel comfortable giving an ACK - although I suspect in the other case it's not deleted until after this call exits
John
Only for devices that have an alias. --- v2: only set the alias when ret == 0 and emit the event before the alias can disappear mention that AttachMemory emits the event src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_hotplug.c | 5 +++++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9e3fe7b..fdcdbdc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7661,19 +7661,24 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; + const char *alias = NULL; switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1); ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev); - if (!ret) + if (!ret) { + alias = dev->data.disk->info.alias; dev->data.disk = NULL; + } break; case VIR_DOMAIN_DEVICE_CONTROLLER: ret = qemuDomainAttachDeviceControllerLive(driver, vm, dev); - if (!ret) + if (!ret) { + alias = dev->data.controller->info.alias; dev->data.controller = NULL; + } break; case VIR_DOMAIN_DEVICE_LEASE: @@ -7687,41 +7692,52 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, -1); ret = qemuDomainAttachNetDevice(dom->conn, driver, vm, dev->data.net); - if (!ret) + if (!ret) { + alias = dev->data.net->info.alias; dev->data.net = NULL; + } break; case VIR_DOMAIN_DEVICE_HOSTDEV: qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1); ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, dev->data.hostdev); - if (!ret) + if (!ret) { + alias = dev->data.hostdev->info->alias; dev->data.hostdev = NULL; + } break; case VIR_DOMAIN_DEVICE_REDIRDEV: ret = qemuDomainAttachRedirdevDevice(driver, vm, dev->data.redirdev); - if (!ret) + if (!ret) { + alias = dev->data.redirdev->info.alias; dev->data.redirdev = NULL; + } break; case VIR_DOMAIN_DEVICE_CHR: ret = qemuDomainAttachChrDevice(driver, vm, dev->data.chr); - if (!ret) + if (!ret) { + alias = dev->data.chr->info.alias; dev->data.chr = NULL; + } break; case VIR_DOMAIN_DEVICE_RNG: ret = qemuDomainAttachRNGDevice(driver, vm, dev->data.rng); - if (!ret) + if (!ret) { + alias = dev->data.rng->info.alias; dev->data.rng = NULL; + } break; case VIR_DOMAIN_DEVICE_MEMORY: - /* note that qemuDomainAttachMemory always consumes dev->data.memory */ + /* note that qemuDomainAttachMemory always consumes dev->data.memory + * and dispatches DeviceAdded event on success */ ret = qemuDomainAttachMemory(driver, vm, dev->data.memory); dev->data.memory = NULL; @@ -7748,6 +7764,16 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; } + if (alias) { + /* queue the event before the alias has a chance to get freed + * if the domain disappears while qemuDomainUpdateDeviceList + * is in monitor */ + virObjectEventPtr event; + event = virDomainEventDeviceAddedNewFromObj(vm, alias); + if (event) + qemuDomainEventQueue(driver, event); + } + if (ret == 0) ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2f0549e..f07c54d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1726,6 +1726,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, char *objalias = NULL; const char *backendType; virJSONValuePtr props = NULL; + virObjectEventPtr event; int id; int ret = -1; @@ -1769,6 +1770,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, goto cleanup; } + event = virDomainEventDeviceAddedNewFromObj(vm, objalias); + if (event) + qemuDomainEventQueue(driver, event); + /* mem is consumed by vm->def */ mem = NULL; -- 2.0.5
On 04/14/2015 01:27 PM, Ján Tomko wrote:
Only for devices that have an alias. --- v2: only set the alias when ret == 0 and emit the event before the alias can disappear mention that AttachMemory emits the event
src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++++-------- src/qemu/qemu_hotplug.c | 5 +++++ 2 files changed, 39 insertions(+), 8 deletions(-)
ACK - John
--- examples/object-events/event-test.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index db7acd9..4f17273 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -565,6 +565,17 @@ myDomainEventAgentLifecycleCallback(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int +myDomainEventDeviceAddedCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *devAlias, + void *opaque ATTRIBUTE_UNUSED) +{ + printf("%s EVENT: Domain %s(%d) device added: %s\n", + __func__, virDomainGetName(dom), virDomainGetID(dom), devAlias); + return 0; +} + static void myFreeFunc(void *opaque) { char *str = opaque; @@ -608,6 +619,7 @@ int main(int argc, char **argv) int callback16ret = -1; int callback17ret = -1; int callback18ret = -1; + int callback19ret = -1; struct sigaction action_stop; memset(&action_stop, 0, sizeof(action_stop)); @@ -736,6 +748,11 @@ int main(int argc, char **argv) VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE, VIR_DOMAIN_EVENT_CALLBACK(myDomainEventAgentLifecycleCallback), strdup("guest agent lifecycle"), myFreeFunc); + callback19ret = virConnectDomainEventRegisterAny(dconn, + NULL, + VIR_DOMAIN_EVENT_ID_DEVICE_ADDED, + VIR_DOMAIN_EVENT_CALLBACK(myDomainEventDeviceAddedCallback), + strdup("device added"), myFreeFunc); if ((callback1ret != -1) && (callback2ret != -1) && @@ -753,7 +770,8 @@ int main(int argc, char **argv) (callback15ret != -1) && (callback16ret != -1) && (callback17ret != -1) && - (callback18ret != -1)) { + (callback18ret != -1) && + (callback19ret != -1)) { if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to start keepalive protocol: %s\n", @@ -787,6 +805,7 @@ int main(int argc, char **argv) virConnectNetworkEventDeregisterAny(dconn, callback16ret); virConnectDomainEventDeregisterAny(dconn, callback17ret); virConnectDomainEventDeregisterAny(dconn, callback18ret); + virConnectDomainEventDeregisterAny(dconn, callback19ret); if (callback8ret != -1) virConnectDomainEventDeregisterAny(dconn, callback8ret); -- 2.0.5
participants (2)
-
John Ferlan -
Ján Tomko