On Mon, Nov 07, 2016 at 15:48:35 -0500, Matt Broadstone wrote:
> Presently domain events are emitted for the "agent lifecycle" which
> is a specialization on virtio-serial events specific to the channel
> named "org.qemu.guest_agent.0". This patch adds a generic event for
> "channel lifecycles", which emit state change events for all
> attached channels.
> ---
> daemon/remote.c | 42 +++++++++++++++++
> examples/object-events/event-test.c | 57 ++++++++++++++++++++++++
> include/libvirt/libvirt-domain.h | 41 +++++++++++++++++
> src/conf/domain_event.c | 89 +++++++++++++++++++++++++++++++++++++
> src/conf/domain_event.h | 12 +++++
> src/libvirt_private.syms | 2 +
> src/qemu/qemu_driver.c | 5 +++
> src/qemu/qemu_process.c | 6 +++
> src/remote/remote_driver.c | 32 +++++++++++++
> src/remote/remote_protocol.x | 17 ++++++-
> src/remote_protocol-structs | 7 +++
> tools/virsh-domain.c | 35 +++++++++++++++
> 12 files changed, 344 insertions(+), 1 deletion(-)
>
[...]
> 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.
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.
> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL = 2, /* channel state changed */
> +
> +# ifdef VIR_ENUM_SENTINELS
> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST
> +# 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
> + * serial channel.
You should note that the hypervisor needs to support channel state
notifications, otherwise it won't work. (Qemu supporting this has been
around for ages now, but we still support versions older than that. Also
other hypervisors don't support this currently)
> + *
> + * The callback signature to use when registering for an event of type
> + * VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE with virConnectDomainEventRegisterAny()
> + */
> +typedef void (*virConnectDomainEventChannelLifecycleCallback)(virConnectPtr conn,
> + virDomainPtr dom,
> + const char *channelName,
> + int state,
> + int reason,
> + void *opaque);
>
> /**
> * VIR_DOMAIN_EVENT_CALLBACK:
> @@ -4006,6 +4046,7 @@ typedef enum {
> VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION = 20, /* virConnectDomainEventMigrationIterationCallback */
> VIR_DOMAIN_EVENT_ID_JOB_COMPLETED = 21, /* virConnectDomainEventJobCompletedCallback */
> VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED = 22, /* virConnectDomainEventDeviceRemovalFailedCallback */
> + VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE = 23, /* virConnectDomainEventChannelLifecycleCallback */
>
> # ifdef VIR_ENUM_SENTINELS
> VIR_DOMAIN_EVENT_ID_LAST
[...]
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 38c8414..b464412 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4407,6 +4407,7 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> virDomainChrDeviceState newstate;
> virObjectEventPtr event = NULL;
> + virObjectEventPtr channelEvent = NULL;
> virDomainDeviceDef dev;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> int rc;
> @@ -4482,6 +4483,10 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
> qemuDomainEventQueue(driver, event);
> }
>
> + channelEvent = virDomainEventChannelLifecycleNewFromObj(vm, dev.data.chr->target.name, newstate,
> + VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL);
I don't think we should emit this event for the guest agent channel. It
has a separate one and the channel is reserved for libvirt anyways, so
client applications should use the guest agent event for this.
> + qemuDomainEventQueue(driver, channelEvent);
> +
> endjob:
> qemuDomainObjEndJob(driver, vm);
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 1b67aee..31b5028 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
[...]
> @@ -1958,6 +1959,11 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr driver,
> agentReason)))
> qemuDomainEventQueue(driver, event);
>
> +
> + channelEvent =
> + virDomainEventChannelLifecycleNewFromObj(vm, chr->target.name, entry->state, agentReason);
> + qemuDomainEventQueue(driver, channelEvent);
Same comment as above.
> +
> chr->state = entry->state;
> }
> }
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index a3cd7cd..c778c42 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
[...]
> @@ -538,6 +543,10 @@ static virNetClientProgramEvent remoteEvents[] = {
> remoteDomainBuildEventCallbackAgentLifecycle,
> sizeof(remote_domain_event_callback_agent_lifecycle_msg),
> (xdrproc_t)xdr_remote_domain_event_callback_agent_lifecycle_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 },
> { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED,
> remoteDomainBuildEventCallbackDeviceAdded,
> sizeof(remote_domain_event_callback_device_added_msg),
These are ordered in the way the events were added, and you are not
putting yours at the end.
Other than the few details looks good.
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list