Re: [libvirt] [PATCH] qemu: Emit domain events for all virtio-serial channels

On Fri, Nov 11, 2016 at 5:13 AM, Peter Krempa <pkrempa(a)redhat.com&gt; wrote: > On Mon, Nov 07, 2016 at 15:48:35 -0500, Matt Broadstone wrote:

>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h >> index 5f50660..d720132 100644 >> --- a/include/libvirt/libvirt-domain.h >> +++ b/include/libvirt/libvirt-domain.h >> @@ -3965,6 +3965,46 @@ typedef void (*virConnectDomainEventAgentLifecycleCallback)(virConnectPtr conn, >> int reason, >> void *opaque); >> >> +typedef enum { >> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED = 1, /* channel connected */ >> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED = 2, /* channel disconnected */ >> + >> +# ifdef VIR_ENUM_SENTINELS >> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST >> +# endif >> +} virConnectDomainEventChannelLifecycleState; >> + >> +typedef enum { >> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_UNKNOWN = 0, /* unknown state change reason */ >> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED = 1, /* state changed due to domain start */ > > This reason is not used at all and I don't think it even makes sense to > be reported for channel events. With guest agent it might make sense and > also other possible reasons as guest agent error. > This reason is, in fact, being used in qemu_process.c:1925 (qemuProcessRefreshChannelVirtioState). The state is actually aligned with the virConnectDomainEventChannelLifecycleReason enum in order to reuse the existing `agentReason` for emitting this event. I can explicitly add a `channelReason` if that makes the code clearer/more readable.

> With channels you mostly care about the connect and disconnect events. > It might make sense to report client connect or disconnect in the future > for certain backend types, but I don't think that qemu supports such > report currently. I agree that with chanels you generally care about `connected` and `disconnected`, but the real problem here is that there is no framing over the virtio-serial channel. As a result, it's important for developers of guest agents to know when a channel connection is being cycled or started for the first time. Remember that the reason for this patch in the first place is that I am creating an agent myself.