[PATCH 0/2] qemu: Introduce VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE
Add a generic domain event that fires when libvirt detects a state change on any virtio-serial channel of a domain (connected / disconnected). The existing VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE event is restricted to the QEMU guest agent channel ("org.qemu.guest_agent.0"), making it impossible for management applications to observe lifecycle transitions of other channels (custom guest agents, SPICE, etc.) without polling the domain XML status file. The new event is emitted for every virtio-serial channel, including the guest agent channel, and carries the affected channels name. The hypervisor must support virtio-serial port state notifications (e.g. QEMU's VSERPORT_CHANGE event) for the event to be delivered. Some parts were lifted from the v2 series originally posted to libvirt-devel in 2016 by Matt Broadstone <mbroadst@gmail.com>. Lucas Kornicki (2): conf,remote: add channel lifecycle domain event qemu: emit channel lifecycle event examples/c/misc/event-test.c | 57 +++++++++++++++++ include/libvirt/libvirt-domain.h | 65 +++++++++++++++++++ src/conf/domain_event.c | 97 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 8 +++ src/qemu/qemu_process.c | 28 +++++++-- src/remote/remote_daemon_dispatch.c | 34 ++++++++++ src/remote/remote_driver.c | 34 ++++++++++ src/remote/remote_protocol.x | 16 ++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain-event.c | 35 +++++++++++ 12 files changed, 389 insertions(+), 7 deletions(-) -- 2.43.0
Add support for a new domain event which can be used to track the state of any virtio channel. Previously one could only monitor the "org.qemu.guest_agent.0" channel which had a dedicated agent lifecycle event. The channel lifecycle event will be emitted alongside the agent specific one. Signed-off-by: Lucas Kornicki <lucas.kornicki@nutanix.com> --- examples/c/misc/event-test.c | 57 +++++++++++++++++ include/libvirt/libvirt-domain.h | 65 +++++++++++++++++++ src/conf/domain_event.c | 97 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/remote/remote_daemon_dispatch.c | 34 ++++++++++ src/remote/remote_driver.c | 34 ++++++++++ src/remote/remote_protocol.x | 16 ++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain-event.c | 35 +++++++++++ 10 files changed, 359 insertions(+), 1 deletion(-) diff --git a/examples/c/misc/event-test.c b/examples/c/misc/event-test.c index f9e65c55f0..601f5eafcf 100644 --- a/examples/c/misc/event-test.c +++ b/examples/c/misc/event-test.c @@ -353,6 +353,45 @@ guestAgentLifecycleEventReasonToString(int event) return "unknown"; } + +static const char * +guestChannelLifecycleEventStateToString(int event) +{ + switch ((virConnectDomainEventChannelLifecycleState) event) { + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED: + return "Disconnected"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED: + return "Connected"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST: + break; + } + + return "unknown"; +} + + +static const char * +guestChannelLifecycleEventReasonToString(int event) +{ + switch ((virConnectDomainEventChannelLifecycleReason) event) { + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_UNKNOWN: + return "Unknown"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED: + return "Domain started"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL: + return "Channel event"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST: + break; + } + + return "unknown"; +} + static const char * storagePoolEventToString(int event) { @@ -869,6 +908,23 @@ myDomainEventAgentLifecycleCallback(virConnectPtr conn G_GNUC_UNUSED, } +static int +myDomainEventChannelLifecycleCallback(virConnectPtr conn G_GNUC_UNUSED, + virDomainPtr dom, + const char *channelName, + int state, + int reason, + void *opaque G_GNUC_UNUSED) +{ + printf("%s EVENT: Domain %s(%d) guest channel(%s) state changed: %s reason: %s\n", + __func__, virDomainGetName(dom), virDomainGetID(dom), channelName, + guestChannelLifecycleEventStateToString(state), + guestChannelLifecycleEventReasonToString(reason)); + + return 0; +} + + static int myDomainEventDeviceAddedCallback(virConnectPtr conn G_GNUC_UNUSED, virDomainPtr dom, @@ -1195,6 +1251,7 @@ struct domainEventData domainEvents[] = { DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_MEMORY_DEVICE_SIZE_CHANGE, myDomainEventMemoryDeviceSizeChangeCallback), DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_NIC_MAC_CHANGE, myDomainEventNICMACChangeCallback), DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_VCPU_REMOVED, myDomainEventVcpuRemovedCallback), + DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE, myDomainEventChannelLifecycleCallback), }; struct storagePoolEventData { diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1066a0b3f1..abc3be0252 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -7673,6 +7673,70 @@ typedef void (*virConnectDomainEventNICMACChangeCallback)(virConnectPtr conn, const char *newMAC, void *opaque); + +/** + * virConnectDomainEventChannelLifecycleState: + * + * Since: 12.4.0 + */ +typedef enum { + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED = 1, /* channel connected (Since: 12.4.0) */ + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED = 2, /* channel disconnected (Since: 12.4.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST /* (Since: 12.4.0) */ +# endif +} virConnectDomainEventChannelLifecycleState; + +/** + * virConnectDomainEventChannelLifecycleReason: + * + * The reason values are intentionally numerically aligned with + * virConnectDomainEventAgentLifecycleReason so that the qemu driver + * can pass the same int through both events. + * + * Since: 12.4.0 + */ +typedef enum { + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_UNKNOWN = 0, /* unknown state change reason (Since: 12.4.0) */ + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED = 1, /* state changed due to domain start (Since: 12.4.0) */ + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL = 2, /* channel state changed (Since: 12.4.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST /* (Since: 12.4.0) */ +# endif +} virConnectDomainEventChannelLifecycleReason; + +/** + * virConnectDomainEventChannelLifecycleCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @channelName: the name of the channel on which the event occurred + * @state: new state of the guest channel, one of virConnectDomainEventChannelLifecycleState + * @reason: reason for state change, one of virConnectDomainEventChannelLifecycleReason + * @opaque: application specified data + * + * This callback occurs when libvirt detects a change in the state of a guest + * virtio-serial channel. Unlike VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE which is + * tied to the QEMU guest agent channel ("org.qemu.guest_agent.0"), this event + * is emitted for every virtio-serial channel attached to the domain, + * including the guest agent channel. + * + * The hypervisor must support virtio-serial port state notifications for the + * event to be delivered. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE with virConnectDomainEventRegisterAny() + * + * Since: 12.4.0 + */ +typedef void (*virConnectDomainEventChannelLifecycleCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *channelName, + int state, + int reason, + void *opaque); + /** * VIR_DOMAIN_EVENT_CALLBACK: * @@ -7723,6 +7787,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_MEMORY_DEVICE_SIZE_CHANGE = 26, /* virConnectDomainEventMemoryDeviceSizeChangeCallback (Since: 7.9.0) */ VIR_DOMAIN_EVENT_ID_NIC_MAC_CHANGE = 27, /* virConnectDomainEventNICMACChangeCallback (Since: 11.2.0) */ VIR_DOMAIN_EVENT_ID_VCPU_REMOVED = 28, /* virConnectDomainEventVcpuRemovedCallback (Since: 12.4.0) */ + VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE = 29, /* virConnectDomainEventChannelLifecycleCallback (Since: 12.4.0) */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index f09c6a9816..e44dae7922 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -59,6 +59,7 @@ static virClass *virDomainEventBlockThresholdClass; static virClass *virDomainEventMemoryFailureClass; static virClass *virDomainEventMemoryDeviceSizeChangeClass; static virClass *virDomainEventNICMACChangeClass; +static virClass *virDomainEventChannelLifecycleClass; static void virDomainEventDispose(void *obj); static void virDomainEventLifecycleDispose(void *obj); @@ -85,6 +86,7 @@ static void virDomainEventBlockThresholdDispose(void *obj); static void virDomainEventMemoryFailureDispose(void *obj); static void virDomainEventMemoryDeviceSizeChangeDispose(void *obj); static void virDomainEventNICMACChangeDispose(void *obj); +static void virDomainEventChannelLifecycleDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -305,6 +307,23 @@ struct _virDomainEventNICMACChange { }; typedef struct _virDomainEventNICMACChange virDomainEventNICMACChange; +struct _virDomainEventChannelLifecycle { + virDomainEvent parent; + + char *channelName; + int state; + int reason; +}; +typedef struct _virDomainEventChannelLifecycle virDomainEventChannelLifecycle; + +/* Make sure the AGENT and CHANNEL lifecycle enums stay in sync with each other. */ +G_STATIC_ASSERT((int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_DOMAIN_STARTED == + (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED); +G_STATIC_ASSERT((int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL == + (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL); +G_STATIC_ASSERT((int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_LAST == + (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST); + static int virDomainEventsOnceInit(void) { @@ -358,6 +377,8 @@ virDomainEventsOnceInit(void) return -1; if (!VIR_CLASS_NEW(virDomainEventNICMACChange, virDomainEventClass)) return -1; + if (!VIR_CLASS_NEW(virDomainEventChannelLifecycle, virDomainEventClass)) + return -1; return 0; } @@ -600,6 +621,14 @@ virDomainEventNICMACChangeDispose(void *obj) g_free(event->newMAC); } +static void +virDomainEventChannelLifecycleDispose(void *obj) +{ + virDomainEventChannelLifecycle *event = obj; + + g_free(event->channelName); +} + static void * virDomainEventNew(virClass *klass, int eventID, @@ -1867,6 +1896,61 @@ virDomainEventNICMACChangeNewFromDom(virDomainPtr dom, } + +static virObjectEvent * +virDomainEventChannelLifecycleNew(int id, + const char *name, + const unsigned char *uuid, + const char *channelName, + int state, + int reason) +{ + virDomainEventChannelLifecycle *ev; + + if (virDomainEventsInitialize() < 0) + return NULL; + + if (!(ev = virDomainEventNew(virDomainEventChannelLifecycleClass, + VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE, + id, name, uuid))) + return NULL; + + ev->channelName = g_strdup(channelName); + ev->state = state; + ev->reason = reason; + + return (virObjectEvent *)ev; +} + + +virObjectEvent * +virDomainEventChannelLifecycleNewFromObj(virDomainObj *obj, + const char *channelName, + int state, + int reason) +{ + return virDomainEventChannelLifecycleNew(obj->def->id, + obj->def->name, + obj->def->uuid, + channelName, + state, + reason); +} + +virObjectEvent * +virDomainEventChannelLifecycleNewFromDom(virDomainPtr dom, + const char *channelName, + int state, + int reason) +{ + return virDomainEventChannelLifecycleNew(dom->id, + dom->name, + dom->uuid, + channelName, + state, + reason); +} + static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, virObjectEvent *event, @@ -2200,6 +2284,19 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, goto cleanup; } + case VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE: + { + virDomainEventChannelLifecycle *channelLifecycleEvent; + + channelLifecycleEvent = (virDomainEventChannelLifecycle *)event; + ((virConnectDomainEventChannelLifecycleCallback)cb)(conn, dom, + channelLifecycleEvent->channelName, + channelLifecycleEvent->state, + channelLifecycleEvent->reason, + 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 930b66b13a..4678bec385 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -295,6 +295,18 @@ virDomainEventNICMACChangeNewFromDom(virDomainPtr dom, const char *oldMAC, const char *newMAC); +virObjectEvent * +virDomainEventChannelLifecycleNewFromObj(virDomainObj *obj, + const char *channelName, + int state, + int reason); + +virObjectEvent * +virDomainEventChannelLifecycleNewFromDom(virDomainPtr dom, + const char *channelName, + int state, + int reason); + int virDomainEventStateRegister(virConnectPtr conn, virObjectEventState *state, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7232b7c663..a76da45fb9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -761,6 +761,8 @@ virDomainEventBlockJobNewFromDom; virDomainEventBlockJobNewFromObj; virDomainEventBlockThresholdNewFromDom; virDomainEventBlockThresholdNewFromObj; +virDomainEventChannelLifecycleNewFromDom; +virDomainEventChannelLifecycleNewFromObj; virDomainEventControlErrorNewFromDom; virDomainEventControlErrorNewFromObj; virDomainEventDeviceAddedNewFromDom; diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 55a1bd2af8..329853b6da 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -1379,6 +1379,39 @@ remoteRelayDomainEventNICMACChange(virConnectPtr conn, } +static int +remoteRelayDomainEventChannelLifecycle(virConnectPtr conn, + virDomainPtr dom, + const char *channelName, + int state, + int reason, + void *opaque) +{ + daemonClientEventCallback *callback = opaque; + remote_domain_event_callback_channel_lifecycle_msg data = { 0 }; + + if (callback->callbackID < 0 || + !remoteRelayDomainEventCheckACL(callback->client, conn, dom)) + return -1; + + VIR_DEBUG("Relaying domain channel lifecycle event %s %d, callback %d, " + "name %s, state %d, reason %d", + dom->name, dom->id, callback->callbackID, channelName, state, reason); + + data.callbackID = callback->callbackID; + make_nonnull_domain(&data.dom, dom); + data.channelName = g_strdup(channelName); + data.state = state; + data.reason = reason; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_CHANNEL_LIFECYCLE, + (xdrproc_t)xdr_remote_domain_event_callback_channel_lifecycle_msg, + &data); + return 0; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -1409,6 +1442,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventMemoryDeviceSizeChange), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventNICMACChange), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventVcpuRemoved), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventChannelLifecycle), }; G_STATIC_ASSERT(G_N_ELEMENTS(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e83b0abcbe..873e3d173c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -441,6 +441,11 @@ remoteDomainBuildEventNICMACChange(virNetClientProgram *prog, virNetClient *client, void *evdata, void *opaque); +static void +remoteDomainBuildEventCallbackChannelLifecycle(virNetClientProgram *prog, + virNetClient *client, + void *evdata, void *opaque); + static virNetClientProgramEvent remoteEvents[] = { { REMOTE_PROC_DOMAIN_EVENT_LIFECYCLE, remoteDomainBuildEventLifecycle, @@ -667,6 +672,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteDomainBuildEventVcpuRemoved, sizeof(remote_domain_event_vcpu_removed_msg), (xdrproc_t)xdr_remote_domain_event_vcpu_removed_msg }, + { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_CHANNEL_LIFECYCLE, + remoteDomainBuildEventCallbackChannelLifecycle, + sizeof(remote_domain_event_callback_channel_lifecycle_msg), + (xdrproc_t)xdr_remote_domain_event_callback_channel_lifecycle_msg }, }; static void @@ -5192,6 +5201,31 @@ remoteDomainBuildEventNICMACChange(virNetClientProgram *prog G_GNUC_UNUSED, } +static void +remoteDomainBuildEventCallbackChannelLifecycle(virNetClientProgram *prog G_GNUC_UNUSED, + virNetClient *client G_GNUC_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + remote_domain_event_callback_channel_lifecycle_msg *msg = evdata; + struct private_data *priv = conn->privateData; + virDomainPtr dom; + virObjectEvent *event = NULL; + + if (!(dom = get_nonnull_domain(conn, msg->dom))) + return; + + event = virDomainEventChannelLifecycleNewFromDom(dom, + msg->channelName, + msg->state, + msg->reason); + + virObjectUnref(dom); + + virObjectEventStateQueueRemote(priv->eventState, event, msg->callbackID); +} + + static int remoteStreamSend(virStreamPtr st, const char *data, diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 23699e99a6..4adba82f6d 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -4015,6 +4015,14 @@ struct remote_domain_event_nic_mac_change_msg { remote_nonnull_string newMAC; }; +struct remote_domain_event_callback_channel_lifecycle_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string channelName; + int state; + int reason; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -7132,5 +7140,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_VCPU_REMOVED = 454 + REMOTE_PROC_DOMAIN_EVENT_VCPU_REMOVED = 454, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_CHANNEL_LIFECYCLE = 455 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4c24245d2b..dd297bffff 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3342,6 +3342,13 @@ struct remote_domain_event_nic_mac_change_msg { remote_nonnull_string oldMAC; remote_nonnull_string newMAC; }; +struct remote_domain_event_callback_channel_lifecycle_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string channelName; + int state; + int reason; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3797,4 +3804,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_DEL_THROTTLE_GROUP = 452, REMOTE_PROC_DOMAIN_EVENT_NIC_MAC_CHANGE = 453, REMOTE_PROC_DOMAIN_EVENT_VCPU_REMOVED = 454, + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_CHANNEL_LIFECYCLE = 455, }; diff --git a/tools/virsh-domain-event.c b/tools/virsh-domain-event.c index 4a9a831b45..a541b155f4 100644 --- a/tools/virsh-domain-event.c +++ b/tools/virsh-domain-event.c @@ -655,6 +655,39 @@ virshEventAgentLifecyclePrint(virConnectPtr conn G_GNUC_UNUSED, virshEventPrint(opaque, &buf); } +VIR_ENUM_DECL(virshEventChannelLifecycleState); +VIR_ENUM_IMPL(virshEventChannelLifecycleState, + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST, + N_("unknown"), + N_("connected"), + N_("disconnected")); + +VIR_ENUM_DECL(virshEventChannelLifecycleReason); +VIR_ENUM_IMPL(virshEventChannelLifecycleReason, + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST, + N_("unknown"), + N_("domain started"), + N_("channel event")); + +static void +virshEventChannelLifecyclePrint(virConnectPtr conn G_GNUC_UNUSED, + virDomainPtr dom, + const char *channelName, + int state, + int reason, + void *opaque) +{ + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&buf, + _("event 'channel-lifecycle' for domain '%1$s': channel name: '%2$s' state: '%3$s' reason: '%4$s'\n"), + virDomainGetName(dom), + channelName, + UNKNOWNSTR(virshEventChannelLifecycleStateTypeToString(state)), + UNKNOWNSTR(virshEventChannelLifecycleReasonTypeToString(reason))); + virshEventPrint(opaque, &buf); +} + static void virshEventMigrationIterationPrint(virConnectPtr conn G_GNUC_UNUSED, virDomainPtr dom, @@ -889,6 +922,8 @@ virshDomainEventCallback virshDomainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(virshEventNICMACChangePrint), }, { "vcpu-removed", VIR_DOMAIN_EVENT_CALLBACK(virshEventVcpuRemovedPrint), }, + { "channel-lifecycle", + VIR_DOMAIN_EVENT_CALLBACK(virshEventChannelLifecyclePrint), }, }; G_STATIC_ASSERT(VIR_DOMAIN_EVENT_ID_LAST == G_N_ELEMENTS(virshDomainEventCallbacks)); -- 2.43.0
On 5/19/26 15:11, Lucas Kornicki wrote:
Add support for a new domain event which can be used to track the state of any virtio channel.
Previously one could only monitor the "org.qemu.guest_agent.0" channel which had a dedicated agent lifecycle event. The channel lifecycle event will be emitted alongside the agent specific one.
Signed-off-by: Lucas Kornicki <lucas.kornicki@nutanix.com> --- examples/c/misc/event-test.c | 57 +++++++++++++++++ include/libvirt/libvirt-domain.h | 65 +++++++++++++++++++ src/conf/domain_event.c | 97 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/remote/remote_daemon_dispatch.c | 34 ++++++++++ src/remote/remote_driver.c | 34 ++++++++++ src/remote/remote_protocol.x | 16 ++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain-event.c | 35 +++++++++++ 10 files changed, 359 insertions(+), 1 deletion(-)
diff --git a/examples/c/misc/event-test.c b/examples/c/misc/event-test.c index f9e65c55f0..601f5eafcf 100644 --- a/examples/c/misc/event-test.c +++ b/examples/c/misc/event-test.c @@ -353,6 +353,45 @@ guestAgentLifecycleEventReasonToString(int event) return "unknown"; }
+ +static const char * +guestChannelLifecycleEventStateToString(int event) +{ + switch ((virConnectDomainEventChannelLifecycleState) event) { + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED: + return "Disconnected"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED: + return "Connected"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST: + break; + } + + return "unknown"; +} + + +static const char * +guestChannelLifecycleEventReasonToString(int event) +{ + switch ((virConnectDomainEventChannelLifecycleReason) event) { + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_UNKNOWN: + return "Unknown"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED: + return "Domain started"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL: + return "Channel event"; + + case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST: + break; + } + + return "unknown"; +} + static const char * storagePoolEventToString(int event) { @@ -869,6 +908,23 @@ myDomainEventAgentLifecycleCallback(virConnectPtr conn G_GNUC_UNUSED, }
+static int +myDomainEventChannelLifecycleCallback(virConnectPtr conn G_GNUC_UNUSED, + virDomainPtr dom, + const char *channelName, + int state, + int reason, + void *opaque G_GNUC_UNUSED) +{ + printf("%s EVENT: Domain %s(%d) guest channel(%s) state changed: %s reason: %s\n", + __func__, virDomainGetName(dom), virDomainGetID(dom), channelName, + guestChannelLifecycleEventStateToString(state), + guestChannelLifecycleEventReasonToString(reason)); + + return 0; +} + + static int myDomainEventDeviceAddedCallback(virConnectPtr conn G_GNUC_UNUSED, virDomainPtr dom, @@ -1195,6 +1251,7 @@ struct domainEventData domainEvents[] = { DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_MEMORY_DEVICE_SIZE_CHANGE, myDomainEventMemoryDeviceSizeChangeCallback), DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_NIC_MAC_CHANGE, myDomainEventNICMACChangeCallback), DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_VCPU_REMOVED, myDomainEventVcpuRemovedCallback), + DOMAIN_EVENT(VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE, myDomainEventChannelLifecycleCallback), };
struct storagePoolEventData { diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1066a0b3f1..abc3be0252 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -7673,6 +7673,70 @@ typedef void (*virConnectDomainEventNICMACChangeCallback)(virConnectPtr conn, const char *newMAC, void *opaque);
+ +/** + * virConnectDomainEventChannelLifecycleState: + * + * Since: 12.4.0 + */ +typedef enum { + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED = 1, /* channel connected (Since: 12.4.0) */ + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED = 2, /* channel disconnected (Since: 12.4.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST /* (Since: 12.4.0) */ +# endif +} virConnectDomainEventChannelLifecycleState; + +/** + * virConnectDomainEventChannelLifecycleReason: + * + * The reason values are intentionally numerically aligned with + * virConnectDomainEventAgentLifecycleReason so that the qemu driver + * can pass the same int through both events.
True, but this is internal detail and I would not worry users with it. They should use these enum values instead of those from virConnectDomainEventAgentLifecycleReason enum.
+ * + * Since: 12.4.0 + */ +typedef enum { + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_UNKNOWN = 0, /* unknown state change reason (Since: 12.4.0) */ + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED = 1, /* state changed due to domain start (Since: 12.4.0) */ + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL = 2, /* channel state changed (Since: 12.4.0) */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST /* (Since: 12.4.0) */ +# endif +} virConnectDomainEventChannelLifecycleReason; + +/** + * virConnectDomainEventChannelLifecycleCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @channelName: the name of the channel on which the event occurred + * @state: new state of the guest channel, one of virConnectDomainEventChannelLifecycleState + * @reason: reason for state change, one of virConnectDomainEventChannelLifecycleReason + * @opaque: application specified data + * + * This callback occurs when libvirt detects a change in the state of a guest + * virtio-serial channel. Unlike VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE which is + * tied to the QEMU guest agent channel ("org.qemu.guest_agent.0"), this event + * is emitted for every virtio-serial channel attached to the domain, + * including the guest agent channel. + * + * The hypervisor must support virtio-serial port state notifications for the + * event to be delivered. + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE with virConnectDomainEventRegisterAny() + * + * Since: 12.4.0 + */ +typedef void (*virConnectDomainEventChannelLifecycleCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *channelName, + int state, + int reason, + void *opaque); + /** * VIR_DOMAIN_EVENT_CALLBACK: * @@ -7723,6 +7787,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_MEMORY_DEVICE_SIZE_CHANGE = 26, /* virConnectDomainEventMemoryDeviceSizeChangeCallback (Since: 7.9.0) */ VIR_DOMAIN_EVENT_ID_NIC_MAC_CHANGE = 27, /* virConnectDomainEventNICMACChangeCallback (Since: 11.2.0) */ VIR_DOMAIN_EVENT_ID_VCPU_REMOVED = 28, /* virConnectDomainEventVcpuRemovedCallback (Since: 12.4.0) */ + VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE = 29, /* virConnectDomainEventChannelLifecycleCallback (Since: 12.4.0) */
# ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_ID_LAST diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index f09c6a9816..e44dae7922 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -59,6 +59,7 @@ static virClass *virDomainEventBlockThresholdClass; static virClass *virDomainEventMemoryFailureClass; static virClass *virDomainEventMemoryDeviceSizeChangeClass; static virClass *virDomainEventNICMACChangeClass; +static virClass *virDomainEventChannelLifecycleClass;
static void virDomainEventDispose(void *obj); static void virDomainEventLifecycleDispose(void *obj); @@ -85,6 +86,7 @@ static void virDomainEventBlockThresholdDispose(void *obj); static void virDomainEventMemoryFailureDispose(void *obj); static void virDomainEventMemoryDeviceSizeChangeDispose(void *obj); static void virDomainEventNICMACChangeDispose(void *obj); +static void virDomainEventChannelLifecycleDispose(void *obj);
static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -305,6 +307,23 @@ struct _virDomainEventNICMACChange { }; typedef struct _virDomainEventNICMACChange virDomainEventNICMACChange;
+struct _virDomainEventChannelLifecycle { + virDomainEvent parent; + + char *channelName; + int state; + int reason; +}; +typedef struct _virDomainEventChannelLifecycle virDomainEventChannelLifecycle; + +/* Make sure the AGENT and CHANNEL lifecycle enums stay in sync with each other. */ +G_STATIC_ASSERT((int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_DOMAIN_STARTED == + (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED); +G_STATIC_ASSERT((int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL == + (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL); +G_STATIC_ASSERT((int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_LAST == + (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST); +
What we are lacking is: G_STATIC_ASSERT((int)VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED == (int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_STATE_CONNECTED); G_STATIC_ASSERT((int)VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED == (int)VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_STATE_DISCONNECTED); (I will post a patch for this shortly, as it is pre-existing) And this patch should then have: G_STATIC_ASSERT((int)VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED == (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED); G_STATIC_ASSERT((int)VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED == (int)VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED); (both should be in src/conf/domain_conf.h) The reason stems from processSerialChangedEvent() which declares a variable like this: virDomainChrDeviceState newstate; and then uses it to create events: event = virDomainEventAgentLifecycleNewFromObj(vm, newstate, VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL); Michal
Emit the channel lifecycle event on VSERPORT_CHANGE and when refreshing virtio state. On "org.qemu.guest_agent.0" channel state change both agent and channel lifecycle events are emitted in that order. Signed-off-by: Lucas Kornicki <lucas.kornicki@nutanix.com> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_process.c | 28 ++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eda1f42054..d260c1cc74 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3906,6 +3906,14 @@ processSerialChangedEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); } + /* we deliberately allow for goto endjob to skip generic event emission + * to ensure identical semantics for "org.qemu.guest_agent.0" */ + event = virDomainEventChannelLifecycleNewFromObj(vm, + dev.data.chr->target.name, + newstate, + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL); + virObjectEventStateQueue(driver->domainEventState, event); + endjob: virDomainObjEndJob(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 83f5ebb19c..38c1fa05ce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2253,7 +2253,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, size_t i; int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL; qemuMonitorChardevInfo *entry; - virObjectEvent *event = NULL; + virObjectEvent *events[2]; g_autofree char *id = NULL; if (booted) @@ -2271,11 +2271,27 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, !entry->state) continue; - if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && - STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0") && - (event = virDomainEventAgentLifecycleNewFromObj(vm, entry->state, - agentReason))) - virObjectEventStateQueue(driver->domainEventState, event); + if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT) { + events[0] = virDomainEventChannelLifecycleNewFromObj(vm, + chr->target.name, + entry->state, + agentReason); + if (STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0")) { + events[1] = virDomainEventAgentLifecycleNewFromObj(vm, + entry->state, + agentReason); + } else { + events[1] = NULL; + } + + /* emit agent then channel when emitting both events */ + if (events[0] && events[1]) { + virObjectEventStateQueue(driver->domainEventState, events[1]); + virObjectEventStateQueue(driver->domainEventState, events[0]); + } else if (events[0]) { + virObjectEventStateQueue(driver->domainEventState, events[0]); + } + } chr->state = entry->state; } -- 2.43.0
On 5/19/26 15:11, Lucas Kornicki wrote:
Emit the channel lifecycle event on VSERPORT_CHANGE and when refreshing virtio state.
On "org.qemu.guest_agent.0" channel state change both agent and channel lifecycle events are emitted in that order.
Signed-off-by: Lucas Kornicki <lucas.kornicki@nutanix.com> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_process.c | 28 ++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eda1f42054..d260c1cc74 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3906,6 +3906,14 @@ processSerialChangedEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); }
+ /* we deliberately allow for goto endjob to skip generic event emission + * to ensure identical semantics for "org.qemu.guest_agent.0" */ + event = virDomainEventChannelLifecycleNewFromObj(vm,
I'd rather avoid reusing the same variable. What can be utilized is the fact that agent event is created and queued in a different code block, i.e. in there new variable can be declared (say agentEvent) leaving this 'event' var free for this use. Or use an array, like you do in the hunk below.
+ dev.data.chr->target.name, + newstate, + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL); + virObjectEventStateQueue(driver->domainEventState, event); + endjob: virDomainObjEndJob(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 83f5ebb19c..38c1fa05ce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2253,7 +2253,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, size_t i; int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL; qemuMonitorChardevInfo *entry; - virObjectEvent *event = NULL; + virObjectEvent *events[2];
I'd rather have these initialized. Dangling pointers are a problem waiting to happen.
g_autofree char *id = NULL;
if (booted) @@ -2271,11 +2271,27 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, !entry->state) continue;
- if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && - STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0") && - (event = virDomainEventAgentLifecycleNewFromObj(vm, entry->state, - agentReason))) - virObjectEventStateQueue(driver->domainEventState, event); + if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT) { + events[0] = virDomainEventChannelLifecycleNewFromObj(vm, + chr->target.name, + entry->state, + agentReason); + if (STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0")) { + events[1] = virDomainEventAgentLifecycleNewFromObj(vm, + entry->state, + agentReason); + } else { + events[1] = NULL; + } + + /* emit agent then channel when emitting both events */ + if (events[0] && events[1]) { + virObjectEventStateQueue(driver->domainEventState, events[1]); + virObjectEventStateQueue(driver->domainEventState, events[0]); + } else if (events[0]) { + virObjectEventStateQueue(driver->domainEventState, events[0]); + }
No need to check for NULL, virObjectEventStateQueue() is a NOP when event is NULL.
+ }
chr->state = entry->state; }
Michal
On 5/19/26 15:11, Lucas Kornicki wrote:
Emit the channel lifecycle event on VSERPORT_CHANGE and when refreshing virtio state.
On "org.qemu.guest_agent.0" channel state change both agent and channel lifecycle events are emitted in that order.
Signed-off-by: Lucas Kornicki<lucas.kornicki@nutanix.com> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_process.c | 28 ++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eda1f42054..d260c1cc74 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3906,6 +3906,14 @@ processSerialChangedEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); }
+ /* we deliberately allow for goto endjob to skip generic event emission + * to ensure identical semantics for "org.qemu.guest_agent.0" */ + event = virDomainEventChannelLifecycleNewFromObj(vm, I'd rather avoid reusing the same variable. What can be utilized is the fact that agent event is created and queued in a different code block, i.e. in there new variable can be declared (say agentEvent) leaving this 'event' var free for this use. Or use an array, like you do in the hunk below. Personally I'm fine with either, but it was Peter's suggestion to reuse
On 5/20/26 12:41, Michal Prívozník wrote: the event variable. This should have been marked as v2. Sorry for the commotion
+ dev.data.chr->target.name, + newstate, + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL); + virObjectEventStateQueue(driver->domainEventState, event); + endjob: virDomainObjEndJob(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 83f5ebb19c..38c1fa05ce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2253,7 +2253,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, size_t i; int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL; qemuMonitorChardevInfo *entry; - virObjectEvent *event = NULL; + virObjectEvent *events[2]; I'd rather have these initialized. Dangling pointers are a problem waiting to happen.
I've decided to drop the initialization of `events` given how they're used and given that `entry` above is not initialized either.
g_autofree char *id = NULL;
if (booted) @@ -2271,11 +2271,27 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, !entry->state) continue;
- if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && - STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0") && - (event = virDomainEventAgentLifecycleNewFromObj(vm, entry->state, - agentReason))) - virObjectEventStateQueue(driver->domainEventState, event); + if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT) { + events[0] = virDomainEventChannelLifecycleNewFromObj(vm, + chr->target.name, + entry->state, + agentReason); + if (STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0")) { + events[1] = virDomainEventAgentLifecycleNewFromObj(vm, + entry->state, + agentReason); + } else { + events[1] = NULL; + } + + /* emit agent then channel when emitting both events */ + if (events[0] && events[1]) { + virObjectEventStateQueue(driver->domainEventState, events[1]); + virObjectEventStateQueue(driver->domainEventState, events[0]); + } else if (events[0]) { + virObjectEventStateQueue(driver->domainEventState, events[0]); + }
No need to check for NULL, virObjectEventStateQueue() is a NOP when event is NULL.
So you'd want to drop the `if (events[0])` and leave just the `else`? Or drop the `else` altogether? I see no harm in keeping it this way, but I'll defer to your judgement.
+ }
chr->state = entry->state; } Michal
On 5/20/26 16:14, Lucas Kornicki wrote:
On 5/19/26 15:11, Lucas Kornicki wrote:
Emit the channel lifecycle event on VSERPORT_CHANGE and when refreshing virtio state.
On "org.qemu.guest_agent.0" channel state change both agent and channel lifecycle events are emitted in that order.
Signed-off-by: Lucas Kornicki <lucas.kornicki@nutanix.com> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_process.c | 28 ++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eda1f42054..d260c1cc74 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3906,6 +3906,14 @@ processSerialChangedEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); }
+ /* we deliberately allow for goto endjob to skip generic event emission + * to ensure identical semantics for "org.qemu.guest_agent.0" */ + event = virDomainEventChannelLifecycleNewFromObj(vm, I'd rather avoid reusing the same variable. What can be utilized is the fact that agent event is created and queued in a different code block, i.e. in there new variable can be declared (say agentEvent) leaving this 'event' var free for this use. Or use an array, like you do in the hunk below. Personally I'm fine with either, but it was Peter's suggestion to reuse
On 5/20/26 12:41, Michal Prívozník wrote: the event variable. This should have been marked as v2. Sorry for the commotion
Ah, it was to avoid long lines. I hear what he says, but at the same time I think only a handful of variables should be reused (maybe some int indexes if there are two loops). But I can live with this.
+ dev.data.chr->target.name, + newstate, + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL); + virObjectEventStateQueue(driver->domainEventState, event); + endjob: virDomainObjEndJob(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 83f5ebb19c..38c1fa05ce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2253,7 +2253,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, size_t i; int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL; qemuMonitorChardevInfo *entry; - virObjectEvent *event = NULL; + virObjectEvent *events[2]; I'd rather have these initialized. Dangling pointers are a problem waiting to happen.
I've decided to drop the initialization of `events` given how they're used and given that `entry` above is not initialized either.
True and this is a good example of how our coding standard evolves. I mean, we had one way of writing things, but then evolved to slightly different one (in this specific case influenced by introduction of automatic memory freeing - g_autofree) but never went through the code and fixed every single occurrence of the old one.
g_autofree char *id = NULL;
if (booted) @@ -2271,11 +2271,27 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, !entry->state) continue;
- if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && - STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0") && - (event = virDomainEventAgentLifecycleNewFromObj(vm, entry->state, - agentReason))) - virObjectEventStateQueue(driver->domainEventState, event); + if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT) { + events[0] = virDomainEventChannelLifecycleNewFromObj(vm, + chr->target.name, + entry->state, + agentReason); + if (STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0")) { + events[1] = virDomainEventAgentLifecycleNewFromObj(vm, + entry->state, + agentReason); + } else { + events[1] = NULL; + } + + /* emit agent then channel when emitting both events */ + if (events[0] && events[1]) { + virObjectEventStateQueue(driver->domainEventState, events[1]); + virObjectEventStateQueue(driver->domainEventState, events[0]); + } else if (events[0]) { + virObjectEventStateQueue(driver->domainEventState, events[0]); + }
No need to check for NULL, virObjectEventStateQueue() is a NOP when event is NULL.
So you'd want to drop the `if (events[0])` and leave just the `else`? Or drop the `else` altogether? I see no harm in keeping it this way, but I'll defer to your judgement.
I'd just keep queuing both events and drop else branch. Even in the first hunk (or pre-existing code) there's no NULL check. It's simply: event = virDomainEventXXXNewFromObj(...); virObjectEventStateQueue(..., event); And all of virDomainEventXXXNewFromObj() can fail and return NULL. Michal
On 5/21/26 10:39, Michal Prívozník wrote:
On 5/20/26 16:14, Lucas Kornicki wrote:
On 5/19/26 15:11, Lucas Kornicki wrote:
Emit the channel lifecycle event on VSERPORT_CHANGE and when refreshing virtio state.
On "org.qemu.guest_agent.0" channel state change both agent and channel lifecycle events are emitted in that order.
Signed-off-by: Lucas Kornicki<lucas.kornicki@nutanix.com> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_process.c | 28 ++++++++++++++++++++++------ 2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eda1f42054..d260c1cc74 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3906,6 +3906,14 @@ processSerialChangedEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); }
+ /* we deliberately allow for goto endjob to skip generic event emission + * to ensure identical semantics for "org.qemu.guest_agent.0" */ + event = virDomainEventChannelLifecycleNewFromObj(vm, I'd rather avoid reusing the same variable. What can be utilized is the fact that agent event is created and queued in a different code block, i.e. in there new variable can be declared (say agentEvent) leaving this 'event' var free for this use. Or use an array, like you do in the hunk below. Personally I'm fine with either, but it was Peter's suggestion to reuse
On 5/20/26 12:41, Michal Prívozník wrote: the event variable. This should have been marked as v2. Sorry for the commotion Ah, it was to avoid long lines. I hear what he says, but at the same time I think only a handful of variables should be reused (maybe some int indexes if there are two loops). But I can live with this.
+ dev.data.chr->target.name, + newstate, + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL); + virObjectEventStateQueue(driver->domainEventState, event); + endjob: virDomainObjEndJob(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 83f5ebb19c..38c1fa05ce 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2253,7 +2253,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, size_t i; int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL; qemuMonitorChardevInfo *entry; - virObjectEvent *event = NULL; + virObjectEvent *events[2]; I'd rather have these initialized. Dangling pointers are a problem waiting to happen. I've decided to drop the initialization of `events` given how they're used and given that `entry` above is not initialized either. True and this is a good example of how our coding standard evolves. I mean, we had one way of writing things, but then evolved to slightly different one (in this specific case influenced by introduction of automatic memory freeing - g_autofree) but never went through the code and fixed every single occurrence of the old one.
g_autofree char *id = NULL;
if (booted) @@ -2271,11 +2271,27 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriver *driver, !entry->state) continue;
- if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT && - STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0") && - (event = virDomainEventAgentLifecycleNewFromObj(vm, entry->state, - agentReason))) - virObjectEventStateQueue(driver->domainEventState, event); + if (entry->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT) { + events[0] = virDomainEventChannelLifecycleNewFromObj(vm, + chr->target.name, + entry->state, + agentReason); + if (STREQ_NULLABLE(chr->target.name, "org.qemu.guest_agent.0")) { + events[1] = virDomainEventAgentLifecycleNewFromObj(vm, + entry->state, + agentReason); + } else { + events[1] = NULL; + } + + /* emit agent then channel when emitting both events */ + if (events[0] && events[1]) { + virObjectEventStateQueue(driver->domainEventState, events[1]); + virObjectEventStateQueue(driver->domainEventState, events[0]); + } else if (events[0]) { + virObjectEventStateQueue(driver->domainEventState, events[0]); + }
No need to check for NULL, virObjectEventStateQueue() is a NOP when event is NULL.
So you'd want to drop the `if (events[0])` and leave just the `else`? Or drop the `else` altogether? I see no harm in keeping it this way, but I'll defer to your judgement. I'd just keep queuing both events and drop else branch. Even in the first hunk (or pre-existing code) there's no NULL check. It's simply:
event = virDomainEventXXXNewFromObj(...); virObjectEventStateQueue(..., event);
And all of virDomainEventXXXNewFromObj() can fail and return NULL.
Michal
Ah I think I remember why I've done it this way. If we just queue events[1] and events[0] there is a possibility that we only end up sending the agent event if construction of channel event fails. I don't think this is much of a practical concern as if NewFromObj fails, it will likely fail for both events. At least with explicit control flow it will be evident what the intention was. If we don't care about this sort of consistency we can rely on NOP queuing behavior. Thoughts? Do you want me to send in a v3, or will you apply the changes when merging?
On 5/21/26 14:22, Lucas Kornicki wrote:
On 5/21/26 10:39, Michal Prívozník wrote:
So you'd want to drop the `if (events[0])` and leave just the `else`? Or drop the `else` altogether? I see no harm in keeping it this way, but I'll defer to your judgement. I'd just keep queuing both events and drop else branch. Even in the first hunk (or pre-existing code) there's no NULL check. It's simply:
event = virDomainEventXXXNewFromObj(...); virObjectEventStateQueue(..., event);
And all of virDomainEventXXXNewFromObj() can fail and return NULL.
Michal
Ah I think I remember why I've done it this way. If we just queue events[1] and events[0] there is a possibility that we only end up sending the agent event if construction of channel event fails. I don't think this is much of a practical concern as if NewFromObj fails, it will likely fail for both events. At least with explicit control flow it will be evident what the intention was.
If we don't care about this sort of consistency we can rely on NOP queuing behavior. Thoughts?
Yeah, we already rely on NOP behavior so let's keep the pattern.
Do you want me to send in a v3, or will you apply the changes when merging?
Nah, I'll change it before merging. Michal
On 5/19/26 15:11, Lucas Kornicki wrote:
Add a generic domain event that fires when libvirt detects a state change on any virtio-serial channel of a domain (connected / disconnected). The existing VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE event is restricted to the QEMU guest agent channel ("org.qemu.guest_agent.0"), making it impossible for management applications to observe lifecycle transitions of other channels (custom guest agents, SPICE, etc.) without polling the domain XML status file.
The new event is emitted for every virtio-serial channel, including the guest agent channel, and carries the affected channels name.
The hypervisor must support virtio-serial port state notifications (e.g. QEMU's VSERPORT_CHANGE event) for the event to be delivered.
Some parts were lifted from the v2 series originally posted to libvirt-devel in 2016 by Matt Broadstone <mbroadst@gmail.com>.
Lucas Kornicki (2): conf,remote: add channel lifecycle domain event qemu: emit channel lifecycle event
examples/c/misc/event-test.c | 57 +++++++++++++++++ include/libvirt/libvirt-domain.h | 65 +++++++++++++++++++ src/conf/domain_event.c | 97 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 8 +++ src/qemu/qemu_process.c | 28 +++++++-- src/remote/remote_daemon_dispatch.c | 34 ++++++++++ src/remote/remote_driver.c | 34 ++++++++++ src/remote/remote_protocol.x | 16 ++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain-event.c | 35 +++++++++++ 12 files changed, 389 insertions(+), 7 deletions(-)
I like this. There's only a couple of small nits I've raised. If you want I can fix that before merging or you can send v2. Michal
On 5/20/26 12:41, Michal Prívozník wrote:
On 5/19/26 15:11, Lucas Kornicki wrote:
Add a generic domain event that fires when libvirt detects a state change on any virtio-serial channel of a domain (connected / disconnected). The existing VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE event is restricted to the QEMU guest agent channel ("org.qemu.guest_agent.0"), making it impossible for management applications to observe lifecycle transitions of other channels (custom guest agents, SPICE, etc.) without polling the domain XML status file.
The new event is emitted for every virtio-serial channel, including the guest agent channel, and carries the affected channels name.
The hypervisor must support virtio-serial port state notifications (e.g. QEMU's VSERPORT_CHANGE event) for the event to be delivered.
Some parts were lifted from the v2 series originally posted to libvirt-devel in 2016 by Matt Broadstone<mbroadst@gmail.com>.
Lucas Kornicki (2): conf,remote: add channel lifecycle domain event qemu: emit channel lifecycle event
examples/c/misc/event-test.c | 57 +++++++++++++++++ include/libvirt/libvirt-domain.h | 65 +++++++++++++++++++ src/conf/domain_event.c | 97 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 8 +++ src/qemu/qemu_process.c | 28 +++++++-- src/remote/remote_daemon_dispatch.c | 34 ++++++++++ src/remote/remote_driver.c | 34 ++++++++++ src/remote/remote_protocol.x | 16 ++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain-event.c | 35 +++++++++++ 12 files changed, 389 insertions(+), 7 deletions(-)
I like this. There's only a couple of small nits I've raised. If you want I can fix that before merging or you can send v2.
Michal
I've sent my comments. Please have a look at v1 first, and if everything is in order you can merge with appropriate fixes.
On 5/19/26 15:11, Lucas Kornicki wrote:
Add a generic domain event that fires when libvirt detects a state change on any virtio-serial channel of a domain (connected / disconnected). The existing VIR_DOMAIN_EVENT_ID_AGENT_LIFECYCLE event is restricted to the QEMU guest agent channel ("org.qemu.guest_agent.0"), making it impossible for management applications to observe lifecycle transitions of other channels (custom guest agents, SPICE, etc.) without polling the domain XML status file.
The new event is emitted for every virtio-serial channel, including the guest agent channel, and carries the affected channels name.
The hypervisor must support virtio-serial port state notifications (e.g. QEMU's VSERPORT_CHANGE event) for the event to be delivered.
Some parts were lifted from the v2 series originally posted to libvirt-devel in 2016 by Matt Broadstone <mbroadst@gmail.com>.
Lucas Kornicki (2): conf,remote: add channel lifecycle domain event qemu: emit channel lifecycle event
examples/c/misc/event-test.c | 57 +++++++++++++++++ include/libvirt/libvirt-domain.h | 65 +++++++++++++++++++ src/conf/domain_event.c | 97 +++++++++++++++++++++++++++++ src/conf/domain_event.h | 12 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_driver.c | 8 +++ src/qemu/qemu_process.c | 28 +++++++-- src/remote/remote_daemon_dispatch.c | 34 ++++++++++ src/remote/remote_driver.c | 34 ++++++++++ src/remote/remote_protocol.x | 16 ++++- src/remote_protocol-structs | 8 +++ tools/virsh-domain-event.c | 35 +++++++++++ 12 files changed, 389 insertions(+), 7 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Congratulations on your first libvirt contribution! Michal
participants (2)
-
Lucas Kornicki -
Michal Prívozník