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

[Peter Krempa]

On Fri, Nov 11, 2016 at 07:51:25 -0500, Matt Broadstone wrote:
> On Fri, Nov 11, 2016 at 5:13 AM, Peter Krempa <pkrempa at redhat.com> 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.

It uses the AGENT_LIFECYCLE value not this declared here. Thus that
exact value is not used. If you want to rely on the fact that they stay
in sync, please note it in a comment. I've used 'git grep' and thus did
not see it.

> > 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.

Okay, fair enough. Note that, for channels, this basically will always
state that the VM has booted and the channel is disconnected as there is
no other option. With the regular guest agent with qemu that did not
support the channel state notification we always connected to the agent.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161111/f325d9f7/attachment-0001.sig>