On Mon, Nov 14, 2016 at 08:19:15AM -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 | 44 ++++++++++++++++++
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, 347 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index e414f92..25b32f2 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1086,6 +1086,47 @@ remoteRelayDomainEventAgentLifecycle(virConnectPtr conn,
static int
+remoteRelayDomainEventChannelLifecycle(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *channelName,
+ int state,
+ int reason,
+ void *opaque)
+{
+ daemonClientEventCallbackPtr callback = opaque;
+ remote_domain_event_callback_channel_lifecycle_msg data;
+
+ 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",
+ dom->name, dom->id, callback->callbackID, channelName, state, reason);
+
+ /* build return data */
+ memset(&data, 0, sizeof(data));
+ data.callbackID = callback->callbackID;
+ make_nonnull_domain(&data.dom, dom);
+
+ if (VIR_STRDUP(data.channelName, channelName) < 0)
+ goto error;
No need for the error, just return -1, it will not allocate anything on error.
+ 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; + error: + VIR_FREE(data.channelName);
So this is not needed either.
diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c
index 730cb8b..da023eb 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -337,6 +337,46 @@ guestAgentLifecycleEventReasonToString(int event)
return "unknown";
}
+
+static const char *
+guestChannelLifecycleEventStateToString(int event)
+{
+ switch ((virConnectDomainEventChannelLifecycleState) event) {
+ case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_DISCONNECTED:
+ return "Disconnected";
+
+ case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_CONNECTED:
+ return "Connected";
+
+ case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_STATE_LAST:
+ break;
+ }
+
+ return "unknown";
+}
+
+
+static const char *
+guestChannelLifecycleEventReasonToString(int event)
+{
+ switch ((virConnectDomainEventChannelLifecycleReason) event) {
+ case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_UNKNOWN:
+ return "Unknown";
+
+ case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_DOMAIN_STARTED:
+ return "Domain started";
+
+ case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL:
+ return "Channel event";
+
+ case VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_LAST:
+ break;
+ }
+
+ return "unknown";
+}
+
+
Maybe these could be share with the Agent one (with added comment and maybe sanity check (AGENT_..._LAST == CHANNEL_..._LAST)? Not required.
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 5f50660..9a5c664 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3965,6 +3965,49 @@ 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 =
+ VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_DOMAIN_STARTED, /* state changed due to domain start */
+ VIR_CONNECT_DOMAIN_EVENT_CHANNEL_LIFECYCLE_REASON_CHANNEL =
+ VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL, /* channel state changed */
Good that it's trying to keep the same values (even though it's just for the simplicity of the code few hunks below), but why not do that in previous enum as well? We could then document that agent lifecycle should not be used (because why would you) and we'd just make it back-compat (basically AGENT_ enum values would reference CHANNEL_ enum values). But that's thinking way ahead =)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 63ae9e1..f971a0d 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1460,6 +1487,56 @@ virDomainEventAgentLifecycleNewFromDom(virDomainPtr dom,
}
static virObjectEventPtr
+virDomainEventChannelLifecycleNew(int id,
+ const char *name,
+ const unsigned char *uuid,
+ const char *channelName,
+ int state,
+ int reason)
+{
+ virDomainEventChannelLifecyclePtr ev;
+
+ if (virDomainEventsInitialize() < 0)
+ return NULL;
+
+ if (!(ev = virDomainEventNew(virDomainEventChannelLifecycleClass,
+ VIR_DOMAIN_EVENT_ID_CHANNEL_LIFECYCLE,
+ id, name, uuid)))
+ return NULL;
+
+ if (VIR_STRDUP(ev->channelName, channelName) < 0) {
+ virObjectUnref(ev);
+ return NULL;
+ }
+
+ ev->state = state;
+ ev->reason = reason;
+
+ return (virObjectEventPtr)ev;
+}
+
+virObjectEventPtr
+virDomainEventChannelLifecycleNewFromObj(virDomainObjPtr obj,
+ const char *channelName,
+ int state,
+ int reason)
+{
+ return virDomainEventChannelLifecycleNew(obj->def->id, obj->def->name,
+ obj->def->uuid, channelName, state, reason);
Long line ^^
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
@@ -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);
+ qemuDomainEventQueue(driver, channelEvent);
+
Create and queue it before the previous agent event since that one might fail and skip this.
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
@@ -1931,6 +1931,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr driver,
int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL;
qemuMonitorChardevInfoPtr entry;
virObjectEventPtr event = NULL;
+ virObjectEventPtr channelEvent = NULL;
char id[32];
if (booted)
So you're reusing the constants here (the reason for marking them equal), but can we prevent it from biting us in the future? I don't wxpect many more events to be added, I'm just trying to be safe =)
@@ -1958,6 +1959,11 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr driver,
agentReason)))
qemuDomainEventQueue(driver, event);
+
+ channelEvent =
+ virDomainEventChannelLifecycleNewFromObj(vm, chr->target.name, entry->state, agentReason);
Long line, split it where the arguments are, not after the equals sign.
+ qemuDomainEventQueue(driver, channelEvent); +
Also this should be emitted only if state != STATE_DEFAULT, you can utilize the if above.
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index e8382dc..e5227c4 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -3242,6 +3242,15 @@ struct remote_domain_event_callback_agent_lifecycle_msg {
int reason;
};
+struct remote_domain_event_callback_channel_lifecycle_msg {
+ int callbackID;
+ remote_nonnull_domain dom;
+
+ char *channelName;
remote_string, not char *
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 184f64d..e085358 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12656,6 +12670,25 @@ virshEventAgentLifecyclePrint(virConnectPtr conn ATTRIBUTE_UNUSED,
}
static void
+virshEventChannelLifecyclePrint(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virDomainPtr dom,
+ const char *channelName,
+ int state,
+ int reason,
+ void *opaque)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ virBufferAsprintf(&buf, _("event 'channel-lifecycle' for domain %s: name: '%s', "
s/name/channel name/ ?
+ "state: '%s' reason: '%s'\n"), + virDomainGetName(dom), + channelName,
I totally missed the channelName here, please put the format string on new line as well, thanks. Otherwise looks good and I think it should go in with those fixes. Sorry for the wait on the review. Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
