[libvirt] [PATCH 0/6] arbitrary qemu event detection

Originally proposed as an RFC: https://www.redhat.com/archives/libvir-list/2013-December/msg01100.html Requires these patches to be reviewed first: https://www.redhat.com/archives/libvir-list/2014-January/msg01406.html https://www.redhat.com/archives/libvir-list/2014-January/msg01457.html Eric Blake (6): qemu: new API for tracking arbitrary monitor events qemu: virsh wrapper for qemu events qemu: create object for qemu monitor events qemu: wire up RPC for qemu monitor events qemu: enable monitor event reporting qemu: enable monitor event filtering by name daemon/libvirtd.h | 2 + daemon/remote.c | 209 +++++++++++++++++++++++++++++++++++++++++ include/libvirt/libvirt-qemu.h | 36 ++++++- src/conf/domain_event.c | 195 ++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 +++++ src/conf/object_event.c | 40 +++++--- src/driver.h | 17 +++- src/libvirt-qemu.c | 121 ++++++++++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_qemu.syms | 6 ++ src/qemu/qemu_driver.c | 52 ++++++++++ src/qemu/qemu_monitor.c | 16 +++- src/qemu/qemu_monitor.h | 13 ++- src/qemu/qemu_monitor_json.c | 23 ++++- src/qemu/qemu_process.c | 29 ++++++ src/qemu_protocol-structs | 22 +++++ src/remote/qemu_protocol.x | 50 +++++++++- src/remote/remote_driver.c | 149 ++++++++++++++++++++++++++++- src/rpc/gendispatch.pl | 13 +-- tools/virsh-domain.c | 193 +++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 27 +++++- 21 files changed, 1202 insertions(+), 35 deletions(-) -- 1.8.5.3

Several times in the past, qemu has implemented a new event, but libvirt has not yet caught up to reporting that event to the user applications. While it is possible to track libvirt logs to see that an unknown event was received and ignored, it would be nicer to copy what 'virsh qemu-monitor-command' does, and expose this information to the end developer as one of our unsupported qemu-specific commands. If you find yourself needing to use this API for more than just development purposes, please ask on the libvirt list for a supported counterpart event to be added in libvirt.so. While the supported virConnectDomainEventRegisterAny() API takes an id which determines the signature of the callback, this version takes a string filter and always uses the same signature. Furthermore, I chose to expose this as a new API instead of trying to add a new eventID at the top level, in part because the generic option lacks event name filtering, and in part because the normal domain event namespace should not be polluted by a qemu-only event. I also added a flags argument; unused for now, but we might decide to use it to allow a user to request event names by glob or regex instead of literal match. This API intentionally requires full write access (while normal event registration is allowed on read-only clients); this is in part due to the fact that it should only be used by debugging situations, and in part because the design of per-event filtering in later patches ended up allowing for duplicate registrations that could potentially be abused to exhaust server memory - requiring write privileges means that such abuse will not serve as a denial of service attack against users with higher privileges. * include/libvirt/libvirt-qemu.h (virConnectDomainQemuMonitorEventCallback) (virConnectDomainQemuMonitorEventRegister) (virConnectDomainQemuMonitorEventDeregister): New prototypes. * src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister) (virConnectDomainQemuMonitorEventDeregister): New functions. * src/libvirt_qemu.syms (LIBVIRT_QEMU_1.2.1): Export them. * src/driver.h (virDrvConnectDomainQemuMonitorEventRegister) (virDrvConnectDomainQemuMonitorEventDeregister): New callbacks. Signed-off-by: Eric Blake <eblake@redhat.com> --- include/libvirt/libvirt-qemu.h | 36 +++++++++++- src/driver.h | 17 +++++- src/libvirt-qemu.c | 121 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_qemu.syms | 6 ++ 4 files changed, 178 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index 3e79a8c..ed6d3d2 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * qemu specific methods * - * Copyright (C) 2010, 2012 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -54,6 +54,40 @@ typedef enum { char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd, int timeout, unsigned int flags); +/** + * virConnectDomainQemuMonitorEventCallback: + * @conn: the connection pointer + * @dom: the domain pointer + * @event: the name of the event + * @seconds: the qemu timestamp of the event: seconds since Epoch, or -1 if + * not available + * @micros: the qemu timestamp of the event: microseconds within the second + * @details: the JSON details of the event, if any were given + * @opaque: application specified data + * + * The callback signature to use when registering for a qemu monitor + * event with virConnectDomainQemuMonitorEventRegister(). + */ +typedef void (*virConnectDomainQemuMonitorEventCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *event, + long long seconds, + unsigned int micros, + const char *details, + void *opaque); + +int virConnectDomainQemuMonitorEventRegister(virConnectPtr conn, + virDomainPtr dom, + const char *event, + virConnectDomainQemuMonitorEventCallback cb, + void *opaque, + virFreeCallback freecb, + unsigned int flags); + +int virConnectDomainQemuMonitorEventDeregister(virConnectPtr conn, + int callbackID); + + # ifdef __cplusplus } # endif diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..55b2a33 100644 --- a/src/driver.h +++ b/src/driver.h @@ -2,7 +2,7 @@ * driver.h: description of the set of interfaces provided by a * entry point to the virtualization engine * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -842,6 +842,19 @@ typedef virDomainPtr unsigned int flags); typedef int +(*virDrvConnectDomainQemuMonitorEventRegister)(virConnectPtr conn, + virDomainPtr dom, + const char *event, + virConnectDomainQemuMonitorEventCallback cb, + void *opaque, + virFreeCallback freecb, + unsigned int flags); + +typedef int +(*virDrvConnectDomainQemuMonitorEventDeregister)(virConnectPtr conn, + int callbackID); + +typedef int (*virDrvDomainOpenConsole)(virDomainPtr dom, const char *dev_name, virStreamPtr st, @@ -1301,6 +1314,8 @@ struct _virDriver { virDrvDomainQemuMonitorCommand domainQemuMonitorCommand; virDrvDomainQemuAttach domainQemuAttach; virDrvDomainQemuAgentCommand domainQemuAgentCommand; + virDrvConnectDomainQemuMonitorEventRegister connectDomainQemuMonitorEventRegister; + virDrvConnectDomainQemuMonitorEventDeregister connectDomainQemuMonitorEventDeregister; virDrvDomainOpenConsole domainOpenConsole; virDrvDomainOpenChannel domainOpenChannel; virDrvDomainOpenGraphics domainOpenGraphics; diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c index 617ad05..e5c6311 100644 --- a/src/libvirt-qemu.c +++ b/src/libvirt-qemu.c @@ -215,3 +215,124 @@ error: virDispatchError(conn); return NULL; } + + +/** + * virConnectDomainQemuMonitorEventRegister: + * @conn: pointer to the connection + * @dom: pointer to the domain, or NULL + * @event: name of the event, or NULL + * @cb: callback to the function handling monitor events + * @opaque: opaque data to pass on to the callback + * @freecb: optional function to deallocate opaque when not used anymore + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * This API is QEMU specific, so it will only work with hypervisor + * connections to the QEMU driver. + * + * Adds a callback to receive notifications of arbitrary qemu monitor events + * occurring on a domain. Many qemu monitor events also result in a libvirt + * event which can be delivered via virConnectDomainEventRegisterAny(); this + * command is primarily for testing new qemu events that have not yet been + * given a libvirt counterpart event. + * + * If @dom is NULL, then events will be monitored for any domain. If @dom + * is non-NULL, then only the specific domain will be monitored. + * + * If @event is NULL, then all monitor events will be reported. If @event is + * non-NULL, then only the specific monitor event will be reported. @flags + * is currently unused, but in the future may support a flag for passing + * @event as a glob instead of a literal name to match a category of events. + * + * The virDomainPtr object handle passed into the callback upon delivery + * of an event is only valid for the duration of execution of the callback. + * If the callback wishes to keep the domain object after the callback returns, + * it shall take a reference to it, by calling virDomainRef(). + * The reference can be released once the object is no longer required + * by calling virDomainFree(). + * + * The return value from this method is a positive integer identifier + * for the callback. To unregister a callback, this callback ID should + * be passed to the virConnectDomainQemuMonitorEventDeregister() method. + * + * Returns a callback identifier on success, -1 on failure + */ +int +virConnectDomainQemuMonitorEventRegister(virConnectPtr conn, + virDomainPtr dom, + const char *event, + virConnectDomainQemuMonitorEventCallback cb, + void *opaque, + virFreeCallback freecb, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, + "conn=%p, event=%s, cb=%p, opaque=%p, freecb=%p, flags=%x", + conn, NULLSTR(event), cb, opaque, freecb, flags); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + if (dom) { + virCheckDomainGoto(dom, error); + if (dom->conn != conn) { + virReportInvalidArg(dom, + _("domain '%s' in %s must match connection"), + dom->name, __FUNCTION__); + goto error; + } + } + virCheckNonNullArgGoto(cb, error); + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver && conn->driver->connectDomainQemuMonitorEventRegister) { + int ret; + ret = conn->driver->connectDomainQemuMonitorEventRegister(conn, dom, event, cb, opaque, freecb, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); +error: + virDispatchError(conn); + return -1; +} + + +/** + * virConnectDomainQemuMonitorEventDeregister: + * @conn: pointer to the connection + * @callbackID: the callback identifier + * + * Removes an event callback. The callbackID parameter should be the + * value obtained from a previous virConnectDomainQemuMonitorEventRegister() + * method. + * + * Returns 0 on success, -1 on failure + */ +int +virConnectDomainQemuMonitorEventDeregister(virConnectPtr conn, + int callbackID) +{ + VIR_DEBUG("conn=%p, callbackID=%d", conn, callbackID); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckNonNegativeArgGoto(callbackID, error); + virCheckReadOnlyGoto(conn->flags, error); + + if (conn->driver && conn->driver->connectDomainQemuMonitorEventDeregister) { + int ret; + ret = conn->driver->connectDomainQemuMonitorEventDeregister(conn, callbackID); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); +error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_qemu.syms b/src/libvirt_qemu.syms index f968d91..21a740b 100644 --- a/src/libvirt_qemu.syms +++ b/src/libvirt_qemu.syms @@ -24,3 +24,9 @@ LIBVIRT_QEMU_0.10.0 { global: virDomainQemuAgentCommand; } LIBVIRT_QEMU_0.9.4; + +LIBVIRT_QEMU_1.2.2 { + global: + virConnectDomainQemuMonitorEventDeregister; + virConnectDomainQemuMonitorEventRegister; +} LIBVIRT_QEMU_0.10.0; -- 1.8.5.3

On Fri, Jan 31, 2014 at 07:12:06PM -0700, Eric Blake wrote:
Several times in the past, qemu has implemented a new event, but libvirt has not yet caught up to reporting that event to the user applications. While it is possible to track libvirt logs to see that an unknown event was received and ignored, it would be nicer to copy what 'virsh qemu-monitor-command' does, and expose this information to the end developer as one of our unsupported qemu-specific commands.
If you find yourself needing to use this API for more than just development purposes, please ask on the libvirt list for a supported counterpart event to be added in libvirt.so.
While the supported virConnectDomainEventRegisterAny() API takes an id which determines the signature of the callback, this version takes a string filter and always uses the same signature. Furthermore, I chose to expose this as a new API instead of trying to add a new eventID at the top level, in part because the generic option lacks event name filtering, and in part because the normal domain event namespace should not be polluted by a qemu-only event. I also added a flags argument; unused for now, but we might decide to use it to allow a user to request event names by glob or regex instead of literal match.
This API intentionally requires full write access (while normal event registration is allowed on read-only clients); this is in part due to the fact that it should only be used by debugging situations, and in part because the design of per-event filtering in later patches ended up allowing for duplicate registrations that could potentially be abused to exhaust server memory - requiring write privileges means that such abuse will not serve as a denial of service attack against users with higher privileges.
* include/libvirt/libvirt-qemu.h (virConnectDomainQemuMonitorEventCallback) (virConnectDomainQemuMonitorEventRegister) (virConnectDomainQemuMonitorEventDeregister): New prototypes. * src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister) (virConnectDomainQemuMonitorEventDeregister): New functions. * src/libvirt_qemu.syms (LIBVIRT_QEMU_1.2.1): Export them. * src/driver.h (virDrvConnectDomainQemuMonitorEventRegister) (virDrvConnectDomainQemuMonitorEventDeregister): New callbacks.
Signed-off-by: Eric Blake <eblake@redhat.com> --- include/libvirt/libvirt-qemu.h | 36 +++++++++++- src/driver.h | 17 +++++- src/libvirt-qemu.c | 121 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_qemu.syms | 6 ++ 4 files changed, 178 insertions(+), 2 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Any new API deserves a good virsh wrapper :) qemu-monitor-event [<domain>] [<event>] [--pretty] [--loop] [--timeout <number>] It helps that we already have an event loop running in a dedicated thread, so a blocking read() on a pipe-to-self is sufficient to handle all three means of ending the command: SIGINT, timeout, and oneshot behavior of an event received. For an example session (once subsequent qemu patches are applied): $ virsh -c qemu:///system qemu-monitor-event --event SHUTDOWN & $ virsh -c qemu:///system start f18-live Domain f18-live started $ virsh -c qemu:///system destroy f18-live Domain f18-live destroyed event SHUTDOWN at 1391212552.026544 for domain f18-live: (null) events received: 1 [1]+ Done virsh -c qemu:///system qemu-monitor-event --event SHUTDOWN $ * tools/virsh-domain.c (cmdQemuMonitorEvent): New command. * tools/virsh.pod (qemu-monitor-event): Document it. Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-domain.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 27 +++++-- 2 files changed, 216 insertions(+), 4 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ce7aea9..a6dc80a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7933,6 +7933,193 @@ cleanup: } /* + * "qemu-monitor-event" command + */ +static int vshEventFd = -1; + +static void +vshEventInt(int sig ATTRIBUTE_UNUSED, + siginfo_t *siginfo ATTRIBUTE_UNUSED, + void *context ATTRIBUTE_UNUSED) +{ + if (vshEventFd >= 0) + ignore_value(safewrite(vshEventFd, "i", 1)); +} + +static void +vshEventTimeout(int timer ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + if (vshEventFd >= 0) + ignore_value(safewrite(vshEventFd, "t", 1)); +} + +struct vshEventData { + vshControl *ctl; + bool loop; + bool pretty; + int count; +}; +typedef struct vshEventData vshEventData; + +static void +vshEventPrint(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, + const char *event, long long seconds, unsigned int micros, + const char *details, void *opaque) +{ + vshEventData *data = opaque; + virJSONValuePtr pretty = NULL; + char *str = NULL; + + if (data->pretty && details) { + pretty = virJSONValueFromString(details); + if (pretty && (str = virJSONValueToString(pretty, true))) + details = str; + } + vshPrint(data->ctl, "event %s at %lld.%06u for domain %s: %s\n", + event, seconds, micros, virDomainGetName(dom), NULLSTR(details)); + data->count++; + if (!data->loop && vshEventFd >= 0) + ignore_value(safewrite(vshEventFd, "done", 1)); + + VIR_FREE(str); +} + +static const vshCmdInfo info_qemu_monitor_event[] = { + {.name = "help", + .data = N_("QEMU Monitor Events") + }, + {.name = "desc", + .data = N_("Listen for QEMU Monitor Events") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_qemu_monitor_event[] = { + {.name = "domain", + .type = VSH_OT_DATA, + .help = N_("filter by domain name, id or uuid") + }, + {.name = "event", + .type = VSH_OT_DATA, + .help = N_("filter by event name") + }, + {.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("pretty-print any JSON output") + }, + {.name = "loop", + .type = VSH_OT_BOOL, + .help = N_("loop until timeout or interrupt, rather than one-shot") + }, + {.name = "timeout", + .type = VSH_OT_INT, + .help = N_("timeout seconds") + }, + {.name = NULL} +}; + +static bool +cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + unsigned int flags = 0; + int timerId = -1; + int eventId = -1; + int fd[2] = { -1, -1 }; + int timeout = 0; + const char *event = NULL; + struct sigaction sig_action; + struct sigaction old_sig_action; + int rv; + char buf = '\0'; + vshEventData data; + + data.ctl = ctl; + data.loop = vshCommandOptBool(cmd, "loop"); + data.pretty = vshCommandOptBool(cmd, "pretty"); + data.count = 0; + if ((rv = vshCommandOptInt(cmd, "timeout", &timeout)) < 0 || + (rv > 0 && timeout < 1)) { + vshError(ctl, "%s", _("invalid timeout")); + return false; + } else if (rv > 0) { + /* Ensure that we can multiply by 1000 without overflowing. */ + if (timeout > INT_MAX / 1000) { + vshError(ctl, "%s", _("timeout is too big")); + return false; + } + timeout *= 1000; + } + if (vshCommandOptString(cmd, "event", &event) < 0) + return false; + + if (vshCommandOptBool(cmd, "domain")) + dom = vshCommandOptDomain(ctl, cmd, NULL); + + /* virsh.c already set up an event loop; this command blocks until + * a callback informs us that we are done. */ + if (pipe2(fd, O_CLOEXEC) < 0) + goto cleanup; + vshEventFd = fd[1]; + + if ((eventId = virConnectDomainQemuMonitorEventRegister(ctl->conn, dom, + event, + vshEventPrint, + &data, NULL, + flags)) < 0) + goto cleanup; + + sig_action.sa_sigaction = vshEventInt; + sig_action.sa_flags = SA_SIGINFO; + sigemptyset(&sig_action.sa_mask); + sigaction(SIGINT, &sig_action, &old_sig_action); + + if (timeout) + timerId = virEventAddTimeout(timeout, vshEventTimeout, NULL, NULL); + + while ((rv = read(fd[0], &buf, 1)) < 0 && errno == EINTR); + if (rv != 1) { + char ebuf[1024]; + + if (!rv) + errno = EPIPE; + vshError(ctl, _("failed to determine loop exit status: %s"), + virStrerror(errno, ebuf, sizeof(ebuf))); + goto cleanup; + } + + switch (buf) { + case 'i': + vshPrint(ctl, _("event loop interrupted\n")); + break; + case 't': + vshPrint(ctl, _("event loop timed out\n")); + break; + } + vshPrint(ctl, _("events received: %d\n"), data.count); + if (data.count) + ret = true; + +cleanup: + if (eventId >= 0) { + sigaction(SIGINT, &old_sig_action, NULL); + if (virConnectDomainQemuMonitorEventDeregister(ctl->conn, eventId) < 0) + ret = false; + } + vshEventFd = -1; + VIR_FORCE_CLOSE(fd[0]); + VIR_FORCE_CLOSE(fd[1]); + if (timerId >= 0) + virEventRemoveTimeout(timerId); + if (dom) + virDomainFree(dom); + + return ret; +} + +/* * "qemu-attach" command */ static const vshCmdInfo info_qemu_attach[] = { @@ -10903,6 +11090,12 @@ const vshCmdDef domManagementCmds[] = { .info = info_qemu_monitor_command, .flags = 0 }, + {.name = "qemu-monitor-event", + .handler = cmdQemuMonitorEvent, + .opts = opts_qemu_monitor_event, + .info = info_qemu_monitor_event, + .flags = 0 + }, {.name = "qemu-agent-command", .handler = cmdQemuAgentCommand, .opts = opts_qemu_agent_command, diff --git a/tools/virsh.pod b/tools/virsh.pod index d052b3a..22b1373 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3272,12 +3272,15 @@ variables, and defaults to C<vi>. =back -=head1 QEMU-SPECIFIC COMMANDS +=head1 HYPERVISOR-SPECIFIC COMMANDS NOTE: Use of the following commands is B<strongly> discouraged. They can cause libvirt to become confused and do the wrong thing on subsequent -operations. Once you have used this command, please do not report -problems to the libvirt developers; the reports will be ignored. +operations. Once you have used these commands, please do not report +problems to the libvirt developers; the reports will be ignored. If +you find that these commands are the only way to accomplish something, +then it is better to request that the feature be added as a first-class +citizen in the regular libvirt library. =over 4 @@ -3316,7 +3319,8 @@ and the monitor uses QMP, then the output will be pretty-printed. If more than one argument is provided for I<command>, they are concatenated with a space in between before passing the single command to the monitor. -=item B<qemu-agent-command> I<domain> [I<--timeout> I<seconds> | I<--async> | I<--block>] I<command>... +=item B<qemu-agent-command> I<domain> [I<--timeout> I<seconds> | I<--async> | +I<--block>] I<command>... Send an arbitrary guest agent command I<command> to domain I<domain> through qemu agent. @@ -3326,6 +3330,21 @@ When I<--aysnc> is given, the command waits for timeout whether success or failed. And when I<--block> is given, the command waits forever with blocking timeout. +=item B<qemu-monitor-event> [I<domain>] [I<--event> I<event-name>] [I<--loop>] +[I<--timeout> I<seconds>] [I<--pretty>] + +Wait for arbitrary QEMU monitor events to occur, and print out the +details of events as they happen. The events can optionally be filtered +by I<domain> or I<event-name>. The 'query-events' QMP command can be +used via I<qemu-monitor-command> to learn what events are supported. + +By default, this command is one-shot, and returns success once an event +occurs; you can send SIGINT (usually via C<Ctrl-C>) to quit immediately. +If I<--timeout> is specified, the command gives up waiting for events +after I<seconds> have elapsed. With I<--loop>, the command prints all +events until a timeout or interrupt key. If I<--pretty> is specified, +any JSON event details are pretty-printed for better legibility. + =item B<lxc-enter-namespace> I<domain> -- /path/to/binary [arg1, [arg2, ...]] Enter the namespace of I<domain> and execute the command C</path/to/binary> -- 1.8.5.3

On Fri, Jan 31, 2014 at 07:12:07PM -0700, Eric Blake wrote:
Any new API deserves a good virsh wrapper :)
qemu-monitor-event [<domain>] [<event>] [--pretty] [--loop] [--timeout <number>]
It helps that we already have an event loop running in a dedicated thread, so a blocking read() on a pipe-to-self is sufficient to handle all three means of ending the command: SIGINT, timeout, and oneshot behavior of an event received.
For an example session (once subsequent qemu patches are applied):
$ virsh -c qemu:///system qemu-monitor-event --event SHUTDOWN & $ virsh -c qemu:///system start f18-live Domain f18-live started
$ virsh -c qemu:///system destroy f18-live Domain f18-live destroyed
event SHUTDOWN at 1391212552.026544 for domain f18-live: (null) events received: 1
[1]+ Done virsh -c qemu:///system qemu-monitor-event --event SHUTDOWN $
* tools/virsh-domain.c (cmdQemuMonitorEvent): New command. * tools/virsh.pod (qemu-monitor-event): Document it.
I can't help thinking that we should have a general 'virsh domain-event' command before we go adding a qemu specific one here. Without a generic command we'd be forcing people to use the QEMU specific command even for events where we have general support like your shutdown example. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/11/2014 08:07 AM, Daniel P. Berrange wrote:
On Fri, Jan 31, 2014 at 07:12:07PM -0700, Eric Blake wrote:
Any new API deserves a good virsh wrapper :)
qemu-monitor-event [<domain>] [<event>] [--pretty] [--loop] [--timeout <number>]
* tools/virsh-domain.c (cmdQemuMonitorEvent): New command. * tools/virsh.pod (qemu-monitor-event): Document it.
I can't help thinking that we should have a general 'virsh domain-event' command before we go adding a qemu specific one here. Without a generic command we'd be forcing people to use the QEMU specific command even for events where we have general support like your shutdown example.
Sure; I'll write the generic domain-event as patch 1.5/6 and submit it shortly :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Create qemu monitor events as a distinct class to normal domain events, because they will be filtered differently. For ease of review, the logic for filtering by event name is saved for a later patch. * src/conf/domain_event.c (virDomainQemuMonitorEventClass): New class. (virDomainEventsOnceInit): Register it. (virDomainQemuMonitorEventDispose, virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventDispatchFunc) (virDomainQemuMonitorEventStateRegisterID): New functions. * src/conf/domain_event.h (virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventStateRegisterID): New prototypes. * src/libvirt_private.syms (conf/domain_conf.h): Export them. --- src/conf/domain_event.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 ++++++++ src/libvirt_private.syms | 2 + 3 files changed, 164 insertions(+) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 9c18922..66fe769 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -48,6 +48,7 @@ static virClassPtr virDomainEventTrayChangeClass; static virClassPtr virDomainEventBalloonChangeClass; static virClassPtr virDomainEventDeviceRemovedClass; static virClassPtr virDomainEventPMClass; +static virClassPtr virDomainQemuMonitorEventClass; static void virDomainEventDispose(void *obj); @@ -62,6 +63,7 @@ static void virDomainEventTrayChangeDispose(void *obj); static void virDomainEventBalloonChangeDispose(void *obj); static void virDomainEventDeviceRemovedDispose(void *obj); static void virDomainEventPMDispose(void *obj); +static void virDomainQemuMonitorEventDispose(void *obj); static void virDomainEventDispatchDefaultFunc(virConnectPtr conn, @@ -69,6 +71,12 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, virConnectObjectEventGenericCallback cb, void *cbopaque); +static void +virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, + virObjectEventPtr event, + virConnectObjectEventGenericCallback cb, + void *cbopaque); + struct _virDomainEvent { virObjectEvent parent; @@ -181,6 +189,17 @@ struct _virDomainEventPM { typedef struct _virDomainEventPM virDomainEventPM; typedef virDomainEventPM *virDomainEventPMPtr; +struct _virDomainQemuMonitorEvent { + virObjectEvent parent; + + char *event; + long long seconds; + unsigned int micros; + char *details; +}; +typedef struct _virDomainQemuMonitorEvent virDomainQemuMonitorEvent; +typedef virDomainQemuMonitorEvent *virDomainQemuMonitorEventPtr; + static int virDomainEventsOnceInit(void) @@ -257,6 +276,12 @@ virDomainEventsOnceInit(void) sizeof(virDomainEventPM), virDomainEventPMDispose))) return -1; + if (!(virDomainQemuMonitorEventClass = + virClassNew(virClassForObjectEvent(), + "virDomainQemuMonitorEvent", + sizeof(virDomainQemuMonitorEvent), + virDomainQemuMonitorEventDispose))) + return -1; return 0; } @@ -382,6 +407,16 @@ virDomainEventPMDispose(void *obj) VIR_DEBUG("obj=%p", event); } +static void +virDomainQemuMonitorEventDispose(void *obj) +{ + virDomainQemuMonitorEventPtr event = obj; + VIR_DEBUG("obj=%p", event); + + VIR_FREE(event->event); + VIR_FREE(event->details); +} + static void * virDomainEventNew(virClassPtr klass, @@ -1313,6 +1348,65 @@ cleanup: } +virObjectEventPtr +virDomainQemuMonitorEventNew(int id, + const char *name, + const unsigned char *uuid, + const char *event, + long long seconds, + unsigned int micros, + const char *details) +{ + virDomainQemuMonitorEventPtr ev; + + if (virDomainEventsInitialize() < 0) + return NULL; + + if (!(ev = virObjectEventNew(virDomainQemuMonitorEventClass, + virDomainQemuMonitorEventDispatchFunc, + 0, id, name, uuid))) + return NULL; + + /* event is mandatory, details are optional */ + if (VIR_STRDUP(ev->event, event) <= 0) + goto error; + ev->seconds = seconds; + ev->micros = micros; + if (VIR_STRDUP(ev->details, details) < 0) + goto error; + + return (virObjectEventPtr)ev; + +error: + virObjectUnref(ev); + return NULL; +} + + +static void +virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, + virObjectEventPtr event, + virConnectObjectEventGenericCallback cb, + void *cbopaque) +{ + virDomainPtr dom = virGetDomain(conn, event->meta.name, event->meta.uuid); + virDomainQemuMonitorEventPtr qemuMonitorEvent; + + if (!dom) + return; + dom->id = event->meta.id; + + qemuMonitorEvent = (virDomainQemuMonitorEventPtr)event; + ((virConnectDomainQemuMonitorEventCallback)cb)(conn, dom, + qemuMonitorEvent->event, + qemuMonitorEvent->seconds, + qemuMonitorEvent->micros, + qemuMonitorEvent->details, + cbopaque); + virDomainFree(dom); +} + + /** * virDomainEventStateRegister: * @conn: connection to associate with callback @@ -1480,3 +1574,49 @@ virDomainEventStateDeregister(virConnectPtr conn, return -1; return virObjectEventStateDeregisterID(conn, state, callbackID); } + + +/** + * virDomainQemuMonitorEventStateRegisterID: + * @conn: connection to associate with callback + * @state: object event state + * @dom: optional domain where event must occur + * @event: optional name of event to register for + * @cb: function to invoke when event occurs + * @opaque: data blob to pass to callback + * @freecb: callback to free @opaque + * @callbackID: filled with callback ID + * + * Register the function @cb with connection @conn, from @state, for + * events of type @eventID. + * + * Returns: the number of callbacks now registered, or -1 on error + */ +int +virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, + virObjectEventStatePtr state, + virDomainPtr dom, + const char *event, + virConnectDomainQemuMonitorEventCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) +{ + if (virDomainEventsInitialize() < 0) + return -1; + + /* FIXME support event filtering */ + if (event) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("event filtering on '%s' not implemented yet"), + event); + return -1; + } + + return virObjectEventStateRegisterID(conn, state, dom ? dom->uuid : NULL, + NULL, NULL, + virDomainQemuMonitorEventClass, 0, + VIR_OBJECT_EVENT_CALLBACK(cb), + opaque, freecb, + false, callbackID, false); +} diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index 300c41b..eb5183e 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -219,4 +219,26 @@ virDomainEventStateDeregister(virConnectPtr conn, virConnectDomainEventCallback callback) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int +virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, + virObjectEventStatePtr state, + virDomainPtr dom, + const char *event, + virConnectDomainQemuMonitorEventCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) + ATTRIBUTE_NONNULL(8); + +virObjectEventPtr +virDomainQemuMonitorEventNew(int id, + const char *name, + const unsigned char *uuid, + const char *event, + long long seconds, + unsigned int micros, + const char *details) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + #endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dead33b..1d981b2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -454,6 +454,8 @@ virDomainEventTrayChangeNewFromDom; virDomainEventTrayChangeNewFromObj; virDomainEventWatchdogNewFromDom; virDomainEventWatchdogNewFromObj; +virDomainQemuMonitorEventNew; +virDomainQemuMonitorEventStateRegisterID; # conf/domain_nwfilter.h -- 1.8.5.3

On Fri, Jan 31, 2014 at 07:12:08PM -0700, Eric Blake wrote:
Create qemu monitor events as a distinct class to normal domain events, because they will be filtered differently. For ease of review, the logic for filtering by event name is saved for a later patch.
* src/conf/domain_event.c (virDomainQemuMonitorEventClass): New class. (virDomainEventsOnceInit): Register it. (virDomainQemuMonitorEventDispose, virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventDispatchFunc) (virDomainQemuMonitorEventStateRegisterID): New functions. * src/conf/domain_event.h (virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventStateRegisterID): New prototypes. * src/libvirt_private.syms (conf/domain_conf.h): Export them. --- src/conf/domain_event.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 ++++++++ src/libvirt_private.syms | 2 + 3 files changed, 164 insertions(+)
I think these APIs should be in src/qemu/qemu_event.{c,h} rather than in the shared codebase. I think it ought to be possible to split the code off, since we've already done separation of the network + domain events + base object evemt classes into distinct files. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Feb 11, 2014 at 03:08:46PM +0000, Daniel P. Berrange wrote:
On Fri, Jan 31, 2014 at 07:12:08PM -0700, Eric Blake wrote:
Create qemu monitor events as a distinct class to normal domain events, because they will be filtered differently. For ease of review, the logic for filtering by event name is saved for a later patch.
* src/conf/domain_event.c (virDomainQemuMonitorEventClass): New class. (virDomainEventsOnceInit): Register it. (virDomainQemuMonitorEventDispose, virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventDispatchFunc) (virDomainQemuMonitorEventStateRegisterID): New functions. * src/conf/domain_event.h (virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventStateRegisterID): New prototypes. * src/libvirt_private.syms (conf/domain_conf.h): Export them. --- src/conf/domain_event.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 ++++++++ src/libvirt_private.syms | 2 + 3 files changed, 164 insertions(+)
I think these APIs should be in src/qemu/qemu_event.{c,h} rather than in the shared codebase. I think it ought to be possible to split the code off, since we've already done separation of the network + domain events + base object evemt classes into distinct files.
Ah, later patches show why this is needed - the remote client driver needs access to this API Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/11/2014 08:15 AM, Daniel P. Berrange wrote:
--- src/conf/domain_event.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 ++++++++ src/libvirt_private.syms | 2 + 3 files changed, 164 insertions(+)
I think these APIs should be in src/qemu/qemu_event.{c,h} rather than in the shared codebase. I think it ought to be possible to split the code off, since we've already done separation of the network + domain events + base object evemt classes into distinct files.
Ah, later patches show why this is needed - the remote client driver needs access to this API
Yes, but the remote client could just as easily get them from a new file. I'll go ahead and make the file split (I was 50/50 on whether to create a new file, and chose the wrong way). Also, I still need a review on this series, as it is is a prereq to arbitrary events: https://www.redhat.com/archives/libvir-list/2014-January/msg01457.html -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Feb 11, 2014 at 08:26:50AM -0700, Eric Blake wrote:
On 02/11/2014 08:15 AM, Daniel P. Berrange wrote:
--- src/conf/domain_event.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 ++++++++ src/libvirt_private.syms | 2 + 3 files changed, 164 insertions(+)
I think these APIs should be in src/qemu/qemu_event.{c,h} rather than in the shared codebase. I think it ought to be possible to split the code off, since we've already done separation of the network + domain events + base object evemt classes into distinct files.
Ah, later patches show why this is needed - the remote client driver needs access to this API
Yes, but the remote client could just as easily get them from a new file.
I'll go ahead and make the file split (I was 50/50 on whether to create a new file, and chose the wrong way).
But if we put it in the src/qemu directory this would create complexity for our build process when remote driver is enabled but qemu driver is disabled. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/11/2014 08:28 AM, Daniel P. Berrange wrote:
On Tue, Feb 11, 2014 at 08:26:50AM -0700, Eric Blake wrote:
On 02/11/2014 08:15 AM, Daniel P. Berrange wrote:
--- src/conf/domain_event.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 ++++++++ src/libvirt_private.syms | 2 + 3 files changed, 164 insertions(+)
I think these APIs should be in src/qemu/qemu_event.{c,h} rather than in the shared codebase. I think it ought to be possible to split the code off, since we've already done separation of the network + domain events + base object evemt classes into distinct files.
Ah, later patches show why this is needed - the remote client driver needs access to this API
Yes, but the remote client could just as easily get them from a new file.
I'll go ahead and make the file split (I was 50/50 on whether to create a new file, and chose the wrong way).
But if we put it in the src/qemu directory this would create complexity for our build process when remote driver is enabled but qemu driver is disabled.
Oh, I misread. I was thinking a separate file in src/conf, whereas I see you were thinking of src/qemu. Yeah, that does get too difficult. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, Jan 31, 2014 at 07:12:08PM -0700, Eric Blake wrote:
Create qemu monitor events as a distinct class to normal domain events, because they will be filtered differently. For ease of review, the logic for filtering by event name is saved for a later patch.
* src/conf/domain_event.c (virDomainQemuMonitorEventClass): New class. (virDomainEventsOnceInit): Register it. (virDomainQemuMonitorEventDispose, virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventDispatchFunc) (virDomainQemuMonitorEventStateRegisterID): New functions. * src/conf/domain_event.h (virDomainQemuMonitorEventNew) (virDomainQemuMonitorEventStateRegisterID): New prototypes. * src/libvirt_private.syms (conf/domain_conf.h): Export them. --- src/conf/domain_event.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 22 ++++++++ src/libvirt_private.syms | 2 + 3 files changed, 164 insertions(+)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

These are the first async events in the qemu protocol, so this patch looks rather big compared to most RPC additions. However, a large majority of this patch is just mechanical copy-and-paste from recently-added network events. It didn't help that this is also the first virConnect rather than virDomain prefix associated with a qemu-specific API. * src/remote/qemu_protocol.x (qemu_*_domain_monitor_event_*): New structs and RPC messages. * src/rpc/gendispatch.pl: Adjust naming conventions. * daemon/libvirtd.h (daemonClientPrivate): Track qemu events. * daemon/remote.c (remoteClientFreeFunc): Likewise. (remoteRelayDomainQemuMonitorEvent) (qemuDispatchConnectDomainMonitorEventRegister) (qemuDispatchConnectDomainMonitorEventDeregister): New functions. * src/remote/remote_driver.c (qemuEvents): Handle qemu events. (doRemoteOpen): Register for events. (remoteNetworkBuildEventLifecycle) (remoteConnectDomainQemuMonitorEventRegister) (remoteConnectDomainQemuMonitorEventDeregister): New functions. * src/qemu_protocol-structs: Regenerate. Signed-off-by: Eric Blake <eblake@redhat.com> --- daemon/libvirtd.h | 2 + daemon/remote.c | 209 +++++++++++++++++++++++++++++++++++++++++++++ src/qemu_protocol-structs | 22 +++++ src/remote/qemu_protocol.x | 50 ++++++++++- src/remote/remote_driver.c | 149 +++++++++++++++++++++++++++++++- src/rpc/gendispatch.pl | 13 +-- 6 files changed, 434 insertions(+), 11 deletions(-) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index c4f1f27..49df33e 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -55,6 +55,8 @@ struct daemonClientPrivate { size_t ndomainEventCallbacks; daemonClientEventCallbackPtr *networkEventCallbacks; size_t nnetworkEventCallbacks; + daemonClientEventCallbackPtr *qemuEventCallbacks; + size_t nqemuEventCallbacks; # if WITH_SASL virNetSASLSessionPtr sasl; diff --git a/daemon/remote.c b/daemon/remote.c index 932f65f..f8856f1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -53,6 +53,7 @@ #include "domain_conf.h" #include "network_conf.h" #include "viraccessapicheck.h" +#include "viraccessapicheckqemu.h" #define VIR_FROM_THIS VIR_FROM_RPC @@ -186,6 +187,33 @@ cleanup: } +static bool +remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, + virConnectPtr conn, virDomainPtr dom) +{ + virDomainDef def; + virIdentityPtr identity = NULL; + bool ret = false; + + /* For now, we just create a virDomainDef with enough contents to + * satisfy what viraccessdriverpolkit.c references. This is a bit + * fragile, but I don't know of anything better. */ + def.name = dom->name; + memcpy(def.uuid, dom->uuid, VIR_UUID_BUFLEN); + + if (!(identity = virNetServerClientGetIdentity(client))) + goto cleanup; + if (virIdentitySetCurrent(identity) < 0) + goto cleanup; + ret = virConnectDomainQemuMonitorEventRegisterCheckACL(conn, &def); + +cleanup: + ignore_value(virIdentitySetCurrent(NULL)); + virObjectUnref(identity); + return ret; +} + + static int remoteRelayDomainEventLifecycle(virConnectPtr conn, virDomainPtr dom, @@ -958,6 +986,52 @@ static virConnectNetworkEventGenericCallback networkEventCallbacks[] = { verify(ARRAY_CARDINALITY(networkEventCallbacks) == VIR_NETWORK_EVENT_ID_LAST); +static void +remoteRelayDomainQemuMonitorEvent(virConnectPtr conn, + virDomainPtr dom, + const char *event, + long long seconds, + unsigned int micros, + const char *details, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + qemu_domain_monitor_event_msg data; + char **details_p = NULL; + + if (callback->callbackID < 0 || + !remoteRelayDomainQemuMonitorEventCheckACL(callback->client, conn, + dom)) + return; + + VIR_DEBUG("Relaying qemu monitor event %s %s, callback %d", + event, details, callback->callbackID); + + /* build return data */ + memset(&data, 0, sizeof(data)); + data.callbackID = callback->callbackID; + if (VIR_STRDUP(data.event, event) < 0) + goto error; + data.seconds = seconds; + data.micros = micros; + if (details && + ((VIR_ALLOC(details_p) < 0) || + VIR_STRDUP(*details_p, details) < 0)) + goto error; + data.details = details_p; + make_nonnull_domain(&data.dom, dom); + + remoteDispatchObjectEventSend(callback->client, qemuProgram, + QEMU_PROC_DOMAIN_MONITOR_EVENT, + (xdrproc_t)xdr_qemu_domain_monitor_event_msg, + &data); + return; + +error: + VIR_FREE(data.event); + VIR_FREE(details_p); +} + /* * You must hold lock for at least the client * We don't free stuff here, merely disconnect the client's @@ -1005,6 +1079,21 @@ void remoteClientFreeFunc(void *data) } VIR_FREE(priv->networkEventCallbacks); + for (i = 0; i < priv->nqemuEventCallbacks; i++) { + int callbackID = priv->qemuEventCallbacks[i]->callbackID; + if (callbackID < 0) { + VIR_WARN("unexpected incomplete qemu monitor callback %zu", i); + continue; + } + VIR_DEBUG("Deregistering remote qemu monitor event relay %d", + callbackID); + priv->qemuEventCallbacks[i]->callbackID = -1; + if (virConnectDomainQemuMonitorEventDeregister(priv->conn, + callbackID) < 0) + VIR_WARN("unexpected qemu monitor event deregister failure"); + } + VIR_FREE(priv->qemuEventCallbacks); + virConnectClose(priv->conn); virIdentitySetCurrent(NULL); @@ -5862,6 +5951,126 @@ cleanup: } +static int +qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + qemu_connect_domain_monitor_event_register_args *args, + qemu_connect_domain_monitor_event_register_ret *ret) +{ + int callbackID; + int rv = -1; + daemonClientEventCallbackPtr callback = NULL; + daemonClientEventCallbackPtr ref; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + virDomainPtr dom = NULL; + const char *event = args->event ? *args->event : NULL; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + if (args->dom && + !(dom = get_nonnull_domain(priv->conn, *args->dom))) + goto cleanup; + + /* If we call register first, we could append a complete callback + * to our array, but on OOM append failure, we'd have to then hope + * deregister works to undo our register. So instead we append an + * incomplete callback to our array, then register, then fix up + * our callback; but since VIR_APPEND_ELEMENT clears 'callback' on + * success, we use 'ref' to save a copy of the pointer. */ + if (VIR_ALLOC(callback) < 0) + goto cleanup; + callback->client = client; + callback->callbackID = -1; + ref = callback; + if (VIR_APPEND_ELEMENT(priv->qemuEventCallbacks, + priv->nqemuEventCallbacks, + callback) < 0) + goto cleanup; + + if ((callbackID = virConnectDomainQemuMonitorEventRegister(priv->conn, + dom, + event, + remoteRelayDomainQemuMonitorEvent, + ref, + remoteEventCallbackFree, + args->flags)) < 0) { + VIR_SHRINK_N(priv->qemuEventCallbacks, + priv->nqemuEventCallbacks, 1); + callback = ref; + goto cleanup; + } + + ref->callbackID = callbackID; + ret->callbackID = callbackID; + + rv = 0; + +cleanup: + VIR_FREE(callback); + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +qemuDispatchConnectDomainMonitorEventDeregister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + qemu_connect_domain_monitor_event_deregister_args *args) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + for (i = 0; i < priv->nqemuEventCallbacks; i++) { + if (priv->qemuEventCallbacks[i]->callbackID == args->callbackID) + break; + } + if (i == priv->nqemuEventCallbacks) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("qemu monitor event callback %d not registered"), + args->callbackID); + goto cleanup; + } + + if (virConnectDomainQemuMonitorEventDeregister(priv->conn, + args->callbackID) < 0) + goto cleanup; + + VIR_DELETE_ELEMENT(priv->qemuEventCallbacks, i, + priv->nqemuEventCallbacks); + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + /*----- Helpers. -----*/ /* get_nonnull_domain and get_nonnull_network turn an on-wire diff --git a/src/qemu_protocol-structs b/src/qemu_protocol-structs index 0dcd2c6..8501543 100644 --- a/src/qemu_protocol-structs +++ b/src/qemu_protocol-structs @@ -28,8 +28,30 @@ struct qemu_domain_agent_command_args { struct qemu_domain_agent_command_ret { remote_string result; }; +struct qemu_connect_domain_monitor_event_register_args { + remote_domain dom; + remote_string event; + u_int flags; +}; +struct qemu_connect_domain_monitor_event_register_ret { + int callbackID; +}; +struct qemu_connect_domain_monitor_event_deregister_args { + int callbackID; +}; +struct qemu_domain_monitor_event_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string event; + int64_t seconds; + u_int micros; + remote_string details; +}; enum qemu_procedure { QEMU_PROC_DOMAIN_MONITOR_COMMAND = 1, QEMU_PROC_DOMAIN_ATTACH = 2, QEMU_PROC_DOMAIN_AGENT_COMMAND = 3, + QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_REGISTER = 4, + QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_DEREGISTER = 5, + QEMU_PROC_DOMAIN_MONITOR_EVENT = 6, }; diff --git a/src/remote/qemu_protocol.x b/src/remote/qemu_protocol.x index 1e7cf7c..f6b88a9 100644 --- a/src/remote/qemu_protocol.x +++ b/src/remote/qemu_protocol.x @@ -3,7 +3,7 @@ * remote_internal driver and libvirtd. This protocol is * internal and may change at any time. * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -58,6 +58,30 @@ struct qemu_domain_agent_command_ret { remote_string result; }; + +struct qemu_connect_domain_monitor_event_register_args { + remote_domain dom; + remote_string event; + unsigned int flags; +}; + +struct qemu_connect_domain_monitor_event_register_ret { + int callbackID; +}; + +struct qemu_connect_domain_monitor_event_deregister_args { + int callbackID; +}; + +struct qemu_domain_monitor_event_msg { + int callbackID; + remote_nonnull_domain dom; + remote_nonnull_string event; + hyper seconds; + unsigned int micros; + remote_string details; +}; + /* Define the program number, protocol version and procedure numbers here. */ const QEMU_PROGRAM = 0x20008087; const QEMU_PROTOCOL_VERSION = 1; @@ -108,5 +132,27 @@ enum qemu_procedure { * @priority: low * @acl: domain:write */ - QEMU_PROC_DOMAIN_AGENT_COMMAND = 3 + QEMU_PROC_DOMAIN_AGENT_COMMAND = 3, + + /** + * @generate: none + * @priority: high + * @acl: connect:search_domains + * @acl: connect:write + * @aclfilter: domain:getattr + */ + QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_REGISTER = 4, + + /** + * @generate: none + * @priority: high + * @acl: connect:write + */ + QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_DEREGISTER = 5, + + /** + * @generate: both + * @acl: none + */ + QEMU_PROC_DOMAIN_MONITOR_EVENT = 6 }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 955465a..d38b883 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -498,6 +498,19 @@ static virNetClientProgramEvent remoteEvents[] = { (xdrproc_t)xdr_remote_domain_event_callback_device_removed_msg }, }; + +static void +remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque); + +static virNetClientProgramEvent qemuEvents[] = { + { QEMU_PROC_DOMAIN_MONITOR_EVENT, + remoteDomainBuildQemuMonitorEvent, + sizeof(qemu_domain_monitor_event_msg), + (xdrproc_t)xdr_qemu_domain_monitor_event_msg }, +}; + enum virDrvOpenRemoteFlags { VIR_DRV_OPEN_REMOTE_RO = (1 << 0), VIR_DRV_OPEN_REMOTE_USER = (1 << 1), /* Use the per-user socket path */ @@ -975,9 +988,9 @@ doRemoteOpen(virConnectPtr conn, goto failed; if (!(priv->qemuProgram = virNetClientProgramNew(QEMU_PROGRAM, QEMU_PROTOCOL_VERSION, - NULL, - 0, - NULL))) + qemuEvents, + ARRAY_CARDINALITY(qemuEvents), + conn))) goto failed; if (virNetClientAddProgram(priv->client, priv->remoteProgram) < 0 || @@ -3174,6 +3187,109 @@ done: static int +remoteConnectDomainQemuMonitorEventRegister(virConnectPtr conn, + virDomainPtr dom, + const char *event, + virConnectDomainQemuMonitorEventCallback callback, + void *opaque, + virFreeCallback freecb, + unsigned int flags) +{ + int rv = -1; + struct private_data *priv = conn->privateData; + qemu_connect_domain_monitor_event_register_args args; + qemu_connect_domain_monitor_event_register_ret ret; + int callbackID; + int count; + remote_nonnull_domain domain; + + remoteDriverLock(priv); + + /* While most remote functions don't check flags, so that they can + * control a newer server that does understand the flag, this + * particular function may use flags to modify how 'event' is + * interpreted client side. */ + virCheckFlags(0, -1); + + if ((count = virDomainQemuMonitorEventStateRegisterID(conn, + priv->eventState, + dom, event, callback, + opaque, freecb, + &callbackID)) < 0) + goto done; + + /* If this is the first callback for this event, we need to enable + * events on the server */ + if (count == 1) { + if (dom) { + make_nonnull_domain(&domain, dom); + args.dom = &domain; + } else { + args.dom = NULL; + } + args.event = event ? (char **) &event : NULL; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, priv, REMOTE_CALL_QEMU, QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_REGISTER, + (xdrproc_t) xdr_qemu_connect_domain_monitor_event_register_args, (char *) &args, + (xdrproc_t) xdr_qemu_connect_domain_monitor_event_register_ret, (char *) &ret) == -1) { + virObjectEventStateDeregisterID(conn, priv->eventState, + callbackID); + goto done; + } + virObjectEventStateSetRemote(conn, priv->eventState, callbackID, + ret.callbackID); + } + + rv = callbackID; + +done: + remoteDriverUnlock(priv); + return rv; +} + + +static int +remoteConnectDomainQemuMonitorEventDeregister(virConnectPtr conn, + int callbackID) +{ + struct private_data *priv = conn->privateData; + int rv = -1; + qemu_connect_domain_monitor_event_deregister_args args; + int remoteID; + int count; + + remoteDriverLock(priv); + + if (virObjectEventStateEventID(conn, priv->eventState, + callbackID, &remoteID) < 0) + goto done; + + if ((count = virObjectEventStateDeregisterID(conn, priv->eventState, + callbackID)) < 0) + goto done; + + /* If that was the last callback for this event, we need to disable + * events on the server */ + if (count == 0) { + args.callbackID = remoteID; + + if (call(conn, priv, REMOTE_CALL_QEMU, QEMU_PROC_CONNECT_DOMAIN_MONITOR_EVENT_DEREGISTER, + (xdrproc_t) xdr_qemu_connect_domain_monitor_event_deregister_args, (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) + goto done; + } + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + + +static int remoteConnectListAllInterfaces(virConnectPtr conn, virInterfacePtr **ifaces, unsigned int flags) @@ -5408,6 +5524,31 @@ remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, } +static void +remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + struct private_data *priv = conn->privateData; + qemu_domain_monitor_event_msg *msg = evdata; + virDomainPtr dom; + virObjectEventPtr event = NULL; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) + return; + + event = virDomainQemuMonitorEventNew(dom->id, dom->name, dom->uuid, + msg->event, msg->seconds, + msg->micros, + msg->details ? *msg->details : NULL); + virDomainFree(dom); + + remoteEventQueue(priv, event, msg->callbackID); +} + + static virDrvOpenStatus ATTRIBUTE_NONNULL(1) remoteSecretOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) @@ -7618,6 +7759,8 @@ static virDriver remote_driver = { .domainQemuMonitorCommand = remoteDomainQemuMonitorCommand, /* 0.8.3 */ .domainQemuAttach = remoteDomainQemuAttach, /* 0.9.4 */ .domainQemuAgentCommand = remoteDomainQemuAgentCommand, /* 0.10.0 */ + .connectDomainQemuMonitorEventRegister = remoteConnectDomainQemuMonitorEventRegister, /* 1.2.2 */ + .connectDomainQemuMonitorEventDeregister = remoteConnectDomainQemuMonitorEventDeregister, /* 1.2.2 */ .domainOpenConsole = remoteDomainOpenConsole, /* 0.8.6 */ .domainOpenChannel = remoteDomainOpenChannel, /* 1.0.2 */ .domainOpenGraphics = remoteDomainOpenGraphics, /* 0.9.7 */ diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index ceb1ad8..b76bbac 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1,6 +1,6 @@ #!/usr/bin/perl -w # -# Copyright (C) 2010-2013 Red Hat, Inc. +# Copyright (C) 2010-2014 Red Hat, Inc. # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -927,8 +927,9 @@ elsif ($mode eq "server") { push(@args_list, "priv->conn"); } - if ($structprefix eq "qemu" && $call->{ProcName} =~ /^Domain/) { - $proc_name =~ s/^(Domain)/${1}Qemu/; + if ($structprefix eq "qemu" && + $call->{ProcName} =~ /^(Connect)?Domain/) { + $proc_name =~ s/^((Connect)?Domain)/${1}Qemu/; } if ($structprefix eq "lxc" && $call->{ProcName} =~ /^Domain/) { $proc_name =~ s/^(Domain)/${1}Lxc/; @@ -1704,7 +1705,7 @@ elsif ($mode eq "client") { if ($mode eq "aclsym") { my $apiname = "vir" . $call->{ProcName}; if ($structprefix eq "qemu") { - $apiname =~ s/virDomain/virDomainQemu/; + $apiname =~ s/(vir(Connect)?Domain)/${1}Qemu/; } elsif ($structprefix eq "lxc") { $apiname =~ s/virDomain/virDomainLxc/; } @@ -1744,7 +1745,7 @@ elsif ($mode eq "client") { my $apiname = "vir" . $call->{ProcName}; if ($structprefix eq "qemu") { - $apiname =~ s/virDomain/virDomainQemu/; + $apiname =~ s/(vir(Connect)?Domain)/${1}Qemu/; } elsif ($structprefix eq "lxc") { $apiname =~ s/virDomain/virDomainLxc/; } @@ -1856,7 +1857,7 @@ elsif ($mode eq "client") { my $apiname = "vir" . $call->{ProcName}; if ($structprefix eq "qemu") { - $apiname =~ s/virDomain/virDomainQemu/; + $apiname =~ s/(vir(Connect)?Domain)/${1}Qemu/; } elsif ($structprefix eq "lxc") { $apiname =~ s/virDomain/virDomainLxc/; } -- 1.8.5.3

On Fri, Jan 31, 2014 at 07:12:09PM -0700, Eric Blake wrote:
These are the first async events in the qemu protocol, so this patch looks rather big compared to most RPC additions. However, a large majority of this patch is just mechanical copy-and-paste from recently-added network events. It didn't help that this is also the first virConnect rather than virDomain prefix associated with a qemu-specific API.
* src/remote/qemu_protocol.x (qemu_*_domain_monitor_event_*): New structs and RPC messages. * src/rpc/gendispatch.pl: Adjust naming conventions. * daemon/libvirtd.h (daemonClientPrivate): Track qemu events. * daemon/remote.c (remoteClientFreeFunc): Likewise. (remoteRelayDomainQemuMonitorEvent) (qemuDispatchConnectDomainMonitorEventRegister) (qemuDispatchConnectDomainMonitorEventDeregister): New functions. * src/remote/remote_driver.c (qemuEvents): Handle qemu events. (doRemoteOpen): Register for events. (remoteNetworkBuildEventLifecycle) (remoteConnectDomainQemuMonitorEventRegister) (remoteConnectDomainQemuMonitorEventDeregister): New functions. * src/qemu_protocol-structs: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com> --- daemon/libvirtd.h | 2 + daemon/remote.c | 209 +++++++++++++++++++++++++++++++++++++++++++++ src/qemu_protocol-structs | 22 +++++ src/remote/qemu_protocol.x | 50 ++++++++++- src/remote/remote_driver.c | 149 +++++++++++++++++++++++++++++++- src/rpc/gendispatch.pl | 13 +-- 6 files changed, 434 insertions(+), 11 deletions(-)
It sure would be nice if we could keep all the QEMU specific bits in QEMU code and have the driver register against some daemon APIs somehow, but that's a cleanup for another day long in the future. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Wire up all the pieces to send arbitrary qemu events to a client using libvirt-qemu.so. If the extra bookkeeping of generating event objects even when no one is listening turns out to be noticeable, we can try to further optimize things by adding a counter for how many connections are using events, and only dump events when the counter is non-zero; but for now, I didn't think it was worth the code complexity. * src/qemu/qemu_driver.c (qemuConnectDomainQemuMonitorEventRegister) (qemuConnectDomainQemuMonitorEventDeregister): New functions. * src/qemu/qemu_monitor.h (qemuMonitorEmitEvent): New prototype. (qemuMonitorDomainEventCallback): New typedef. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONIOProcessEvent): Report events. * src/qemu/qemu_monitor.c (qemuMonitorEmitEvent): New function, to pass events through. * src/qemu/qemu_process.c (qemuProcessHandleEvent): Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 16 +++++++++++++- src/qemu/qemu_monitor.h | 13 ++++++++++- src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++-- src/qemu/qemu_process.c | 29 ++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dd7c91f..31553c3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16281,6 +16281,56 @@ cleanup: return result; } + +static int +qemuConnectDomainQemuMonitorEventRegister(virConnectPtr conn, + virDomainPtr dom, + const char *event, + virConnectDomainQemuMonitorEventCallback callback, + void *opaque, + virFreeCallback freecb, + unsigned int flags) +{ + virQEMUDriverPtr driver = conn->privateData; + int ret = -1; + + virCheckFlags(0, -1); + + if (virConnectDomainQemuMonitorEventRegisterEnsureACL(conn) < 0) + goto cleanup; + + if (virDomainQemuMonitorEventStateRegisterID(conn, + driver->domainEventState, + dom, event, callback, + opaque, freecb, &ret) < 0) + ret = -1; + +cleanup: + return ret; +} + + +static int +qemuConnectDomainQemuMonitorEventDeregister(virConnectPtr conn, + int callbackID) +{ + virQEMUDriverPtr driver = conn->privateData; + int ret = -1; + + if (virConnectDomainQemuMonitorEventDeregisterEnsureACL(conn) < 0) + goto cleanup; + + if (virObjectEventStateDeregisterID(conn, driver->domainEventState, + callbackID) < 0) + goto cleanup; + + ret = 0; + +cleanup: + return ret; +} + + static int qemuDomainFSTrim(virDomainPtr dom, const char *mountPoint, @@ -16624,6 +16674,8 @@ static virDriver qemuDriver = { .domainQemuMonitorCommand = qemuDomainQemuMonitorCommand, /* 0.8.3 */ .domainQemuAttach = qemuDomainQemuAttach, /* 0.9.4 */ .domainQemuAgentCommand = qemuDomainQemuAgentCommand, /* 0.10.0 */ + .connectDomainQemuMonitorEventRegister = qemuConnectDomainQemuMonitorEventRegister, /* 1.2.2 */ + .connectDomainQemuMonitorEventDeregister = qemuConnectDomainQemuMonitorEventDeregister, /* 1.2.2 */ .domainOpenConsole = qemuDomainOpenConsole, /* 0.8.6 */ .domainOpenGraphics = qemuDomainOpenGraphics, /* 0.9.7 */ .domainInjectNMI = qemuDomainInjectNMI, /* 0.9.2 */ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a968901..670ab0e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1,7 +1,7 @@ /* * qemu_monitor.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1157,6 +1157,20 @@ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, } +int +qemuMonitorEmitEvent(qemuMonitorPtr mon, const char *event, + long long seconds, unsigned int micros, + const char *details) +{ + int ret = -1; + VIR_DEBUG("mon=%p event=%s", mon, event); + + QEMU_MONITOR_CALLBACK(mon, ret, domainEvent, mon->vm, event, seconds, + micros, details); + return ret; +} + + int qemuMonitorEmitShutdown(qemuMonitorPtr mon) { int ret = -1; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index eabf000..51848ca 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1,7 +1,7 @@ /* * qemu_monitor.h: interaction with QEMU monitor console * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -90,6 +90,13 @@ typedef int (*qemuMonitorDiskSecretLookupCallback)(qemuMonitorPtr mon, char **secret, size_t *secretLen, void *opaque); +typedef int (*qemuMonitorDomainEventCallback)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *event, + long long seconds, + unsigned int micros, + const char *details, + void *opaque); typedef int (*qemuMonitorDomainShutdownCallback)(qemuMonitorPtr mon, virDomainObjPtr vm, void *opaque); @@ -171,6 +178,7 @@ struct _qemuMonitorCallbacks { qemuMonitorEofNotifyCallback eofNotify; qemuMonitorErrorNotifyCallback errorNotify; qemuMonitorDiskSecretLookupCallback diskSecretLookup; + qemuMonitorDomainEventCallback domainEvent; qemuMonitorDomainShutdownCallback domainShutdown; qemuMonitorDomainResetCallback domainReset; qemuMonitorDomainPowerdownCallback domainPowerdown; @@ -236,6 +244,9 @@ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, char **secret, size_t *secretLen); +int qemuMonitorEmitEvent(qemuMonitorPtr mon, const char *event, + long long seconds, unsigned int micros, + const char *details); int qemuMonitorEmitShutdown(qemuMonitorPtr mon); int qemuMonitorEmitReset(qemuMonitorPtr mon); int qemuMonitorEmitPowerdown(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ec3b958..d64fe49 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1,7 +1,7 @@ /* * qemu_monitor_json.c: interaction with QEMU monitor console * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2014 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -126,6 +126,12 @@ qemuMonitorJSONIOProcessEvent(qemuMonitorPtr mon, { const char *type; qemuEventHandler *handler; + virJSONValuePtr data; + char *details = NULL; + virJSONValuePtr timestamp; + long long seconds = -1; + unsigned int micros = 0; + VIR_DEBUG("mon=%p obj=%p", mon, obj); type = virJSONValueObjectGetString(obj, "event"); @@ -135,10 +141,23 @@ qemuMonitorJSONIOProcessEvent(qemuMonitorPtr mon, return -1; } + /* Not all events have data; and event reporting is best-effort only */ + if ((data = virJSONValueObjectGet(obj, "data"))) + details = virJSONValueToString(data, false); + if ((timestamp = virJSONValueObjectGet(obj, "timestamp"))) { + virJSONValuePtr elt; + + if ((elt = virJSONValueObjectGet(timestamp, "seconds"))) + ignore_value(virJSONValueGetNumberLong(elt, &seconds)); + if ((elt = virJSONValueObjectGet(timestamp, "microseconds"))) + ignore_value(virJSONValueGetNumberUint(elt, µs)); + } + qemuMonitorEmitEvent(mon, type, seconds, micros, details); + VIR_FREE(details); + handler = bsearch(type, eventHandlers, ARRAY_CARDINALITY(eventHandlers), sizeof(eventHandlers[0]), qemuMonitorEventCompare); if (handler) { - virJSONValuePtr data = virJSONValueObjectGet(obj, "data"); VIR_DEBUG("handle %s handler=%p data=%p", type, handler->handler, data); (handler->handler)(mon, data); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8bcd98e..8905590 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -646,6 +646,34 @@ qemuProcessShutdownOrReboot(virQEMUDriverPtr driver, } } + +static int +qemuProcessHandleEvent(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *eventName, + long long seconds, + unsigned int micros, + const char *details, + void *opaque) +{ + virQEMUDriverPtr driver = opaque; + virObjectEventPtr event = NULL; + + VIR_DEBUG("vm=%p", vm); + + virObjectLock(vm); + event = virDomainQemuMonitorEventNew(vm->def->id, vm->def->name, + vm->def->uuid, eventName, + seconds, micros, details); + + virObjectUnlock(vm); + if (event) + qemuDomainEventQueue(driver, event); + + return 0; +} + + static int qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm, @@ -1367,6 +1395,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .eofNotify = qemuProcessHandleMonitorEOF, .errorNotify = qemuProcessHandleMonitorError, .diskSecretLookup = qemuProcessFindVolumeQcowPassphrase, + .domainEvent = qemuProcessHandleEvent, .domainShutdown = qemuProcessHandleShutdown, .domainStop = qemuProcessHandleStop, .domainResume = qemuProcessHandleResume, -- 1.8.5.3

On Fri, Jan 31, 2014 at 07:12:10PM -0700, Eric Blake wrote:
Wire up all the pieces to send arbitrary qemu events to a client using libvirt-qemu.so. If the extra bookkeeping of generating event objects even when no one is listening turns out to be noticeable, we can try to further optimize things by adding a counter for how many connections are using events, and only dump events when the counter is non-zero; but for now, I didn't think it was worth the code complexity.
* src/qemu/qemu_driver.c (qemuConnectDomainQemuMonitorEventRegister) (qemuConnectDomainQemuMonitorEventDeregister): New functions. * src/qemu/qemu_monitor.h (qemuMonitorEmitEvent): New prototype. (qemuMonitorDomainEventCallback): New typedef. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONIOProcessEvent): Report events. * src/qemu/qemu_monitor.c (qemuMonitorEmitEvent): New function, to pass events through. * src/qemu/qemu_process.c (qemuProcessHandleEvent): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 16 +++++++++++++- src/qemu/qemu_monitor.h | 13 ++++++++++- src/qemu/qemu_monitor_json.c | 23 ++++++++++++++++++-- src/qemu/qemu_process.c | 29 ++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 4 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Filtering monitor events by name requires tracking the name for the duration of the filtering. In order to free the name, I found it easiest to just piggyback on the user's freecb function, which gets called when the event is deregistered. For events without a name filter, we have the design of multiple client registrations sharing a common server registration, because the server side uses the same callback function and we reject duplicate use of the same function. But with events in the mix, we want to be able to allow the same function pointer to be used with more than one event name. The solution is to tweak the duplicate detection code to only act when there is no additional filtering; if name filtering is in use, there is exactly one client registration per server registration. Yes, this means that there is no longer a bound on the number of server registrations possible, so a malicious client could repeatedly register for the same name event to exhaust server memory. On the other hand, we already restricted monitor events to require write access (compared to normal events only needing read access), and separated it into the intentionally unsuported libvirt-qemu.so, with documentation that using this function is for debug purposes only; so it is not a security risk worth worrying about a client trying to abuse multiple registrations. * src/conf/domain_event.c (virDomainQemuMonitorEventData): New struct. (virDomainQemuMonitorEventFilter) (virDomainQemuMonitorEventCleanup): New functions. (virDomainQemuMonitorEventDispatchFunc) (virDomainQemuMonitorEventStateRegisterID): Use new struct. * src/conf/object_event.c (virObjectEventCallbackListCount) (virObjectEventCallbackListAddID) (virObjectEventCallbackListRemoveID) (virObjectEventCallbackListMarkDeleteID): Drop duplicate detection when filtering is in effect. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/domain_event.c | 71 +++++++++++++++++++++++++++++++++++++++++++------ src/conf/object_event.c | 40 ++++++++++++++++++---------- 2 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 66fe769..1fb5243 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1383,6 +1383,18 @@ error: } +/* In order to filter by event name, we need to store a copy of the + * name to filter on. By wrapping the caller's freecb, we can + * piggyback our cleanup to happen at the same time the caller + * deregisters. */ +struct virDomainQemuMonitorEventData { + char *event; + void *opaque; + virFreeCallback freecb; +}; +typedef struct virDomainQemuMonitorEventData virDomainQemuMonitorEventData; + + static void virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, virObjectEventPtr event, @@ -1391,6 +1403,7 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, { virDomainPtr dom = virGetDomain(conn, event->meta.name, event->meta.uuid); virDomainQemuMonitorEventPtr qemuMonitorEvent; + virDomainQemuMonitorEventData *data = cbopaque; if (!dom) return; @@ -1402,7 +1415,7 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, qemuMonitorEvent->seconds, qemuMonitorEvent->micros, qemuMonitorEvent->details, - cbopaque); + data->opaque); virDomainFree(dom); } @@ -1577,6 +1590,41 @@ virDomainEventStateDeregister(virConnectPtr conn, /** + * virDomainQemuMonitorEventFilter: + * @conn: the connection pointer + * @event: the event about to be dispatched + * @opaque: the opaque data registered with the filter + * + * Callback for filtering based on event names. Returns true if the + * event should be dispatched. + */ +static bool +virDomainQemuMonitorEventFilter(virConnectPtr conn ATTRIBUTE_UNUSED, + virObjectEventPtr event, + void *opaque) +{ + virDomainQemuMonitorEventData *data = opaque; + virDomainQemuMonitorEventPtr monitorEvent; + + monitorEvent = (virDomainQemuMonitorEventPtr) event; + + return STREQ(monitorEvent->event, data->event); +} + + +static void +virDomainQemuMonitorEventCleanup(void *opaque) +{ + virDomainQemuMonitorEventData *data = opaque; + + VIR_FREE(data->event); + if (data->freecb) + (data->freecb)(data->opaque); + VIR_FREE(data); +} + + +/** * virDomainQemuMonitorEventStateRegisterID: * @conn: connection to associate with callback * @state: object event state @@ -1602,21 +1650,28 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, virFreeCallback freecb, int *callbackID) { + virDomainQemuMonitorEventData *data = NULL; + virObjectEventCallbackFilter filter = NULL; + if (virDomainEventsInitialize() < 0) return -1; - /* FIXME support event filtering */ - if (event) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("event filtering on '%s' not implemented yet"), - event); + if (VIR_ALLOC(data) < 0) + return -1; + if (VIR_STRDUP(data->event, event) < 0) { + VIR_FREE(data); return -1; } + data->opaque = opaque; + data->freecb = freecb; + if (event) + filter = virDomainQemuMonitorEventFilter; + freecb = virDomainQemuMonitorEventCleanup; return virObjectEventStateRegisterID(conn, state, dom ? dom->uuid : NULL, - NULL, NULL, + filter, data, virDomainQemuMonitorEventClass, 0, VIR_OBJECT_EVENT_CALLBACK(cb), - opaque, freecb, + data, freecb, false, callbackID, false); } diff --git a/src/conf/object_event.c b/src/conf/object_event.c index de45257..56df806 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -181,6 +181,8 @@ virObjectEventCallbackListCount(virConnectPtr conn, for (i = 0; i < cbList->count; i++) { virObjectEventCallbackPtr cb = cbList->callbacks[i]; + if (cb->filter) + continue; if (cb->klass == klass && cb->eventID == eventID && cb->conn == conn && @@ -216,10 +218,11 @@ virObjectEventCallbackListRemoveID(virConnectPtr conn, if (cb->callbackID == callbackID && cb->conn == conn) { int ret; - ret = virObjectEventCallbackListCount(conn, cbList, cb->klass, - cb->eventID, - cb->uuid_filter ? cb->uuid : NULL, - cb->remoteID >= 0) - 1; + ret = cb->filter ? 0 : + (virObjectEventCallbackListCount(conn, cbList, cb->klass, + cb->eventID, + cb->uuid_filter ? cb->uuid : NULL, + cb->remoteID >= 0) - 1); if (cb->freecb) (*cb->freecb)(cb->opaque); @@ -249,10 +252,11 @@ virObjectEventCallbackListMarkDeleteID(virConnectPtr conn, if (cb->callbackID == callbackID && cb->conn == conn) { cb->deleted = true; - return virObjectEventCallbackListCount(conn, cbList, cb->klass, - cb->eventID, - cb->uuid_filter ? cb->uuid : NULL, - cb->remoteID >= 0); + return cb->filter ? 0 : + virObjectEventCallbackListCount(conn, cbList, cb->klass, + cb->eventID, + cb->uuid_filter ? cb->uuid : NULL, + cb->remoteID >= 0); } } @@ -396,8 +400,10 @@ virObjectEventCallbackListAddID(virConnectPtr conn, return -1; } - /* check if we already have this callback on our list */ - if (virObjectEventCallbackLookup(conn, cbList, uuid, + /* If there is no additional filtering, then check if we already + * have this callback on our list. */ + if (!filter && + virObjectEventCallbackLookup(conn, cbList, uuid, klass, eventID, callback, legacy, serverFilter ? &remoteID : NULL) != -1) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -430,10 +436,16 @@ virObjectEventCallbackListAddID(virConnectPtr conn, if (VIR_APPEND_ELEMENT(cbList->callbacks, cbList->count, event) < 0) goto cleanup; - ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID, - uuid, serverFilter); - if (serverFilter && remoteID < 0) - ret++; + /* When additional filtering is being done, every client callback + * is matched to exactly one server callback. */ + if (filter) { + ret = 1; + } else { + ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID, + uuid, serverFilter); + if (serverFilter && remoteID < 0) + ret++; + } cleanup: if (event) -- 1.8.5.3

On Fri, Jan 31, 2014 at 07:12:11PM -0700, Eric Blake wrote:
Filtering monitor events by name requires tracking the name for the duration of the filtering. In order to free the name, I found it easiest to just piggyback on the user's freecb function, which gets called when the event is deregistered.
For events without a name filter, we have the design of multiple client registrations sharing a common server registration, because the server side uses the same callback function and we reject duplicate use of the same function. But with events in the mix, we want to be able to allow the same function pointer to be used with more than one event name. The solution is to tweak the duplicate detection code to only act when there is no additional filtering; if name filtering is in use, there is exactly one client registration per server registration. Yes, this means that there is no longer a bound on the number of server registrations possible, so a malicious client could repeatedly register for the same name event to exhaust server memory. On the other hand, we already restricted monitor events to require write access (compared to normal events only needing read access), and separated it into the intentionally unsuported libvirt-qemu.so, with documentation that using this function is for debug purposes only; so it is not a security risk worth worrying about a client trying to abuse multiple registrations.
* src/conf/domain_event.c (virDomainQemuMonitorEventData): New struct. (virDomainQemuMonitorEventFilter) (virDomainQemuMonitorEventCleanup): New functions. (virDomainQemuMonitorEventDispatchFunc) (virDomainQemuMonitorEventStateRegisterID): Use new struct. * src/conf/object_event.c (virObjectEventCallbackListCount) (virObjectEventCallbackListAddID) (virObjectEventCallbackListRemoveID) (virObjectEventCallbackListMarkDeleteID): Drop duplicate detection when filtering is in effect.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/conf/domain_event.c | 71 +++++++++++++++++++++++++++++++++++++++++++------ src/conf/object_event.c | 40 ++++++++++++++++++---------- 2 files changed, 89 insertions(+), 22 deletions(-)
I do wonder if we're adding too much functionality / complexity for an API that we basically don't want people to use in the common case. ACK, though personally I wouldn't bother supporting this feature, nor the following patch. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

When listening for a subset of monitor events, it can be tedious to register for each event name in series; nicer is to register for multiple events in one go. Implement a flag to use regex interpretation of the event filter. While at it, prove how much I hate the shift key, by adding a way to filter for 'shutdown' instead of 'SHUTDOWN'. :) * include/libvirt/libvirt-qemu.h (virConnectDomainQemuMonitorEventRegisterFlags): New enum. * src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister): Document flags. * tools/virsh-domain.c (cmdQemuMonitorEvent): Expose them. * tools/virsh.pod (qemu-monitor-event): Document this. * src/conf/domain_event.h (virDomainQemuMonitorEventStateRegisterID): Adjust signature. * src/conf/domain_event.c (virDomainQemuMonitorEventStateRegisterID): Add flags. (virDomainQemuMonitorEventFilter): Handle regex, and optimize client side. (virDomainQemuMonitorEventCleanup): Clean up regex. * src/qemu/qemu_driver.c (qemuConnectDomainQemuMonitorEventRegister): Adjust caller. * src/remote/remote_driver.c (remoteConnectDomainQemuMonitorEventRegister): New flags can always be safely handled server side. Signed-off-by: Eric Blake <eblake@redhat.com> --- include/libvirt/libvirt-qemu.h | 10 ++++++++++ src/conf/domain_event.c | 41 ++++++++++++++++++++++++++++++++++++++--- src/conf/domain_event.h | 3 ++- src/libvirt-qemu.c | 11 +++++++---- src/qemu/qemu_driver.c | 7 +++++-- src/remote/remote_driver.c | 8 +------- tools/virsh-domain.c | 13 +++++++++++++ tools/virsh.pod | 5 ++++- 8 files changed, 80 insertions(+), 18 deletions(-) diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h index ed6d3d2..652926e 100644 --- a/include/libvirt/libvirt-qemu.h +++ b/include/libvirt/libvirt-qemu.h @@ -76,6 +76,16 @@ typedef void (*virConnectDomainQemuMonitorEventCallback)(virConnectPtr conn, const char *details, void *opaque); + +typedef enum { + /* Event filter is a regex rather than a literal string */ + VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX = (1 << 0), + + /* Event filter is case insensitive */ + VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE = (1 << 1), +} virConnectDomainQemuMonitorEventRegisterFlags; + + int virConnectDomainQemuMonitorEventRegister(virConnectPtr conn, virDomainPtr dom, const char *event, diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 1fb5243..ab468c9 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -24,6 +24,8 @@ #include <config.h> +#include <regex.h> + #include "domain_event.h" #include "object_event.h" #include "object_event_private.h" @@ -1389,6 +1391,8 @@ error: * deregisters. */ struct virDomainQemuMonitorEventData { char *event; + regex_t regex; + unsigned int flags; void *opaque; virFreeCallback freecb; }; @@ -1608,6 +1612,12 @@ virDomainQemuMonitorEventFilter(virConnectPtr conn ATTRIBUTE_UNUSED, monitorEvent = (virDomainQemuMonitorEventPtr) event; + if (data->flags == -1) + return true; + if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) + return regexec(&data->regex, monitorEvent->event, 0, NULL, 0) == 0; + if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) + return STRCASEEQ(monitorEvent->event, data->event); return STREQ(monitorEvent->event, data->event); } @@ -1618,6 +1628,8 @@ virDomainQemuMonitorEventCleanup(void *opaque) virDomainQemuMonitorEventData *data = opaque; VIR_FREE(data->event); + if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) + regfree(&data->regex); if (data->freecb) (data->freecb)(data->opaque); VIR_FREE(data); @@ -1633,6 +1645,8 @@ virDomainQemuMonitorEventCleanup(void *opaque) * @cb: function to invoke when event occurs * @opaque: data blob to pass to callback * @freecb: callback to free @opaque + * @flags: -1 for client, valid virConnectDomainQemuMonitorEventRegisterFlags + * for server * @callbackID: filled with callback ID * * Register the function @cb with connection @conn, from @state, for @@ -1648,6 +1662,7 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, virConnectDomainQemuMonitorEventCallback cb, void *opaque, virFreeCallback freecb, + unsigned int flags, int *callbackID) { virDomainQemuMonitorEventData *data = NULL; @@ -1658,9 +1673,29 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, if (VIR_ALLOC(data) < 0) return -1; - if (VIR_STRDUP(data->event, event) < 0) { - VIR_FREE(data); - return -1; + data->flags = flags; + if (flags != -1) { + int rflags = REG_NOSUB; + + if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE) + rflags |= REG_ICASE; + if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) { + int err = regcomp(&data->regex, event, rflags); + + if (err) { + char error[100]; + regerror(err, &data->regex, error, sizeof(error)); + virReportError(VIR_ERR_INVALID_ARG, + _("failed to compile regex '%s': %s"), + event, error); + regfree(&data->regex); + VIR_FREE(data); + return -1; + } + } else if (VIR_STRDUP(data->event, event) < 0) { + VIR_FREE(data); + return -1; + } } data->opaque = opaque; data->freecb = freecb; diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index eb5183e..9c41090 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -227,9 +227,10 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, virConnectDomainQemuMonitorEventCallback cb, void *opaque, virFreeCallback freecb, + unsigned int flags, int *callbackID) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) - ATTRIBUTE_NONNULL(8); + ATTRIBUTE_NONNULL(9); virObjectEventPtr virDomainQemuMonitorEventNew(int id, diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c index e5c6311..0876d52 100644 --- a/src/libvirt-qemu.c +++ b/src/libvirt-qemu.c @@ -225,7 +225,7 @@ error: * @cb: callback to the function handling monitor events * @opaque: opaque data to pass on to the callback * @freecb: optional function to deallocate opaque when not used anymore - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virConnectDomainQemuMonitorEventRegisterFlags * * This API is QEMU specific, so it will only work with hypervisor * connections to the QEMU driver. @@ -240,9 +240,12 @@ error: * is non-NULL, then only the specific domain will be monitored. * * If @event is NULL, then all monitor events will be reported. If @event is - * non-NULL, then only the specific monitor event will be reported. @flags - * is currently unused, but in the future may support a flag for passing - * @event as a glob instead of a literal name to match a category of events. + * non-NULL, then only specific monitor events will be reported. @flags + * controls how the filtering is performed: 0 requests an exact match, while + * VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX states that @event + * is a basic regular expression. Additionally, including + * VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE lets @event match + * case-insensitively. * * The virDomainPtr object handle passed into the callback upon delivery * of an event is only valid for the duration of execution of the callback. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6b5076d..1639e53 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16322,7 +16322,9 @@ qemuConnectDomainQemuMonitorEventRegister(virConnectPtr conn, virQEMUDriverPtr driver = conn->privateData; int ret = -1; - virCheckFlags(0, -1); + virCheckFlags(VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX | + VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE, + -1); if (virConnectDomainQemuMonitorEventRegisterEnsureACL(conn) < 0) goto cleanup; @@ -16330,7 +16332,8 @@ qemuConnectDomainQemuMonitorEventRegister(virConnectPtr conn, if (virDomainQemuMonitorEventStateRegisterID(conn, driver->domainEventState, dom, event, callback, - opaque, freecb, &ret) < 0) + opaque, freecb, flags, + &ret) < 0) ret = -1; cleanup: diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d38b883..1d3d13a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3205,16 +3205,10 @@ remoteConnectDomainQemuMonitorEventRegister(virConnectPtr conn, remoteDriverLock(priv); - /* While most remote functions don't check flags, so that they can - * control a newer server that does understand the flag, this - * particular function may use flags to modify how 'event' is - * interpreted client side. */ - virCheckFlags(0, -1); - if ((count = virDomainQemuMonitorEventStateRegisterID(conn, priv->eventState, dom, event, callback, - opaque, freecb, + opaque, freecb, -1, &callbackID)) < 0) goto done; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index a6dc80a..fd04361 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8016,6 +8016,14 @@ static const vshCmdOptDef opts_qemu_monitor_event[] = { .type = VSH_OT_INT, .help = N_("timeout seconds") }, + {.name = "regex", + .type = VSH_OT_BOOL, + .help = N_("treat event as a regex rather than literal filter") + }, + {.name = "no-case", + .type = VSH_OT_BOOL, + .help = N_("treat event case-insensitively") + }, {.name = NULL} }; @@ -8036,6 +8044,11 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd) char buf = '\0'; vshEventData data; + if (vshCommandOptBool(cmd, "regex")) + flags |= VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX; + if (vshCommandOptBool(cmd, "no-case")) + flags |= VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE; + data.ctl = ctl; data.loop = vshCommandOptBool(cmd, "loop"); data.pretty = vshCommandOptBool(cmd, "pretty"); diff --git a/tools/virsh.pod b/tools/virsh.pod index a8af75b..cc3aac2 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3331,12 +3331,15 @@ failed. And when I<--block> is given, the command waits forever with blocking timeout. =item B<qemu-monitor-event> [I<domain>] [I<--event> I<event-name>] [I<--loop>] -[I<--timeout> I<seconds>] [I<--pretty>] +[I<--timeout> I<seconds>] [I<--pretty>] [I<--regex>] [I<--no-case>] Wait for arbitrary QEMU monitor events to occur, and print out the details of events as they happen. The events can optionally be filtered by I<domain> or I<event-name>. The 'query-events' QMP command can be used via I<qemu-monitor-command> to learn what events are supported. +If I<--regex> is used, I<event-name> is a basic regular expression +instead of a literal string. If I<--no-case> is used, I<event-name> +will match case-insensitively. By default, this command is one-shot, and returns success once an event occurs; you can send SIGINT (usually via C<Ctrl-C>) to quit immediately. -- 1.8.5.3

On Thu, Feb 06, 2014 at 04:19:53PM -0700, Eric Blake wrote:
When listening for a subset of monitor events, it can be tedious to register for each event name in series; nicer is to register for multiple events in one go. Implement a flag to use regex interpretation of the event filter.
While at it, prove how much I hate the shift key, by adding a way to filter for 'shutdown' instead of 'SHUTDOWN'. :)
* include/libvirt/libvirt-qemu.h (virConnectDomainQemuMonitorEventRegisterFlags): New enum. * src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister): Document flags. * tools/virsh-domain.c (cmdQemuMonitorEvent): Expose them. * tools/virsh.pod (qemu-monitor-event): Document this. * src/conf/domain_event.h (virDomainQemuMonitorEventStateRegisterID): Adjust signature. * src/conf/domain_event.c (virDomainQemuMonitorEventStateRegisterID): Add flags. (virDomainQemuMonitorEventFilter): Handle regex, and optimize client side. (virDomainQemuMonitorEventCleanup): Clean up regex. * src/qemu/qemu_driver.c (qemuConnectDomainQemuMonitorEventRegister): Adjust caller. * src/remote/remote_driver.c (remoteConnectDomainQemuMonitorEventRegister): New flags can always be safely handled server side.
Signed-off-by: Eric Blake <eblake@redhat.com> --- include/libvirt/libvirt-qemu.h | 10 ++++++++++ src/conf/domain_event.c | 41 ++++++++++++++++++++++++++++++++++++++--- src/conf/domain_event.h | 3 ++- src/libvirt-qemu.c | 11 +++++++---- src/qemu/qemu_driver.c | 7 +++++-- src/remote/remote_driver.c | 8 +------- tools/virsh-domain.c | 13 +++++++++++++ tools/virsh.pod | 5 ++++- 8 files changed, 80 insertions(+), 18 deletions(-)
Again I'm wondering whether this is really worth the extra maintainence pain. We're adding alot more crufty code to our common domain_event code to support a feature on an API we generally don't want people using Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange
-
Eric Blake