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 <[email protected]>
> ---
>  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

Reply via email to