On Wed, May 13, 2026 at 12:20:31 +0200, 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.
> 
> Forward-port of the v2 series originally posted to libvirt-devel in
> 2016 by Matt Broadstone <[email protected]>.

I'd expect that authorship is preserved if you forward port the patch.
Did you change it substantially? Did the original author include a
'signed-off-by' tag?

> 
> Signed-off-by: Lucas Kornicki <[email protected]>
> ---
>  examples/c/misc/event-test.c        | 57 +++++++++++++++++
>  include/libvirt/libvirt-domain.h    | 65 ++++++++++++++++++++
>  src/conf/domain_event.c             | 95 +++++++++++++++++++++++++++++
>  src/conf/domain_event.h             | 12 ++++
>  src/libvirt_private.syms            |  2 +
>  src/qemu/qemu_driver.c              |  8 +++
>  src/qemu/qemu_process.c             | 17 ++++--

Please seaprate the implementation in the qemu driver from teh other
changes which add the public side of the event.

>  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, 377 insertions(+), 6 deletions(-)

[...]


> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 4a8e3114b3..fc0fc30e83 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -7568,6 +7568,70 @@ typedef void 
> (*virConnectDomainEventNICMACChangeCallback)(virConnectPtr conn,
>                                                            const char *newMAC,
>                                                            void *opaque);
>  
> +
> +/**
> + * virConnectDomainEventChannelLifecycleState:
> + *
> + * Since: 12.3.0

This (and all other below) needs to be at least 12.4.0 since you've
sent the patch half-way through the development cycle which will become
libvirt-12.4.0.

Note that the freeze for the release will be around 25.5, so if you
don't send v2 before that (including some leeway for review) change them
directly to 12.5.0.


> + */
> +typedef enum {
> +    VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED = 1, /* 
> channel connected (Since: 12.3.0) */
> +    VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED = 2, /* 
> channel disconnected (Since: 12.3.0) */
> +
> +# ifdef VIR_ENUM_SENTINELS
> +    VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST /* (Since: 12.3.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.3.0
> + */
> +typedef enum {
> +    VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_UNKNOWN = 0, /* 
> unknown state change reason (Since: 12.3.0) */
> +    VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED = 1, /* 
> state changed due to domain start (Since: 12.3.0) */
> +    VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL = 2, /* 
> channel state changed (Since: 12.3.0) */
> +
> +# ifdef VIR_ENUM_SENTINELS
> +    VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST /* (Since: 
> 12.3.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.3.0
> + */
> +typedef void (*virConnectDomainEventChannelLifecycleCallback)(virConnectPtr 
> conn,
> +                                                              virDomainPtr 
> dom,
> +                                                              const char 
> *channelName,
> +                                                              int state,
> +                                                              int reason,
> +                                                              void *opaque);
> +
>  /**
>   * VIR_DOMAIN_EVENT_CALLBACK:
>   *
> @@ -7617,6 +7681,7 @@ typedef enum {
>      VIR_DOMAIN_EVENT_ID_MEMORY_FAILURE = 25,  /* 
> virConnectDomainEventMemoryFailureCallback (Since: 6.9.0) */
>      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_CHANNEL_LIFECYCLE = 28, /* 
> virConnectDomainEventChannelLifecycleCallback (Since: 12.3.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 88087bad4f..38ab902aa2 100644
> --- a/src/conf/domain_event.c
> +++ b/src/conf/domain_event.c
> @@ -58,6 +58,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);
> @@ -83,6 +84,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,
> @@ -296,6 +298,21 @@ 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);
> +
>  static int
>  virDomainEventsOnceInit(void)
>  {

[...]

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 529e9fe3be..dcb664834a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3834,6 +3834,14 @@ processSerialChangedEvent(virQEMUDriver *driver,
>  
>      qemuDomainSaveStatus(vm);
>  
> +    /* queue the generic channel-lifecycle event before the agent-specific 
> one
> +     * because the latter might goto endjob if qemuConnectAgent() fails */

I'd argue that for the guest agent channel these ought to behave
identically.

I no longer remember the reason (or whetherr there was any) for not
emitting the event if agent connection fails but the events ought to
have same semantics.

> +    virObjectEventStateQueue(driver->domainEventState,
> +                             virDomainEventChannelLifecycleNewFromObj(vm,
> +                                                                      
> dev.data.chr->target.name,
> +                                                                      
> newstate,
> +                                                                      
> VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL));
> +

See below about 'event' variable reuse to avoidl long lines.


>      if (STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0")) 
> {
>          if (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) {
>              if (qemuConnectAgent(driver, vm) < 0)
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a6d33f6746..ba1d17d91a 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2270,11 +2270,18 @@ 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) {
> +                if (STREQ_NULLABLE(chr->target.name, 
> "org.qemu.guest_agent.0") &&
> +                    (event = virDomainEventAgentLifecycleNewFromObj(vm, 
> entry->state,
> +                                                                    
> agentReason)))
> +                    virObjectEventStateQueue(driver->domainEventState, 
> event);
> +
> +                virObjectEventStateQueue(driver->domainEventState,
> +                                         
> virDomainEventChannelLifecycleNewFromObj(vm,
> +                                                                             
>      chr->target.name,
> +                                                                             
>      entry->state,
> +                                                                             
>      agentReason));

I suggest you reuse 'event' to construct the event to avoid overly long
line.

Also note that the code for emitting
virDomainEventAgentLifecycleNewFromObj which you've refactored here is
NULL checking the result but your addition isn't despite using
effectively identical APIs.

Since virObjectEventStateQueueRemote seems to NULL check 'event' it
seems to be okay if you don't do it here, but make it consistent.


> +            }
>  
>              chr->state = entry->state;
>          }
> diff --git a/src/remote/remote_daemon_dispatch.c 
> b/src/remote/remote_daemon_dispatch.c
> index 7e74ff063f..ac0440eb0b 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -1354,6 +1354,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",

Inconsistent separation of fields "name" has a colon, rest has nothing.

We also usually now enclose substituitions in debug strings in single
quotes (e.g. '%s') so that it's obvious if the string doesn't contain
e.g. whitespace.


> +              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),
> @@ -1383,6 +1416,7 @@ static virConnectDomainEventGenericCallback 
> domainEventCallbacks[] = {
>      VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventMemoryFailure),
>      VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventMemoryDeviceSizeChange),
>      VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventNICMACChange),
> +    VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventChannelLifecycle),
>  };
>  
>  G_STATIC_ASSERT(G_N_ELEMENTS(domainEventCallbacks) == 
> VIR_DOMAIN_EVENT_ID_LAST);

Reply via email to