On Fri, Apr 01, 2011 at 05:51:06PM +0800, Lai Jiangshan wrote:
> Enable libvirt send some events to the guest.
> This command currently only supports NMI and key events.
>
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> ---
> daemon/remote.c | 52 +++++++++++++++++++++
> daemon/remote_dispatch_args.h | 2
> daemon/remote_dispatch_prototypes.h | 16 ++++++
> daemon/remote_dispatch_table.h | 10 ++++
> include/libvirt/libvirt.h.in | 3 +
> src/driver.h | 7 ++
> src/esx/esx_driver.c | 2
> src/libvirt.c | 88 ++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 2
> src/libxl/libxl_driver.c | 2
> src/lxc/lxc_driver.c | 2
> src/openvz/openvz_driver.c | 2
> src/phyp/phyp_driver.c | 4 +
> src/qemu/qemu_driver.c | 86 +++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.c | 27 +++++++++++
> src/qemu/qemu_monitor.h | 3 +
> src/qemu/qemu_monitor_json.c | 68 +++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 3 +
> src/qemu/qemu_monitor_text.c | 56 ++++++++++++++++++++++
> src/qemu/qemu_monitor_text.h | 2
> src/remote/remote_driver.c | 50 ++++++++++++++++++++
> src/remote/remote_protocol.c | 22 +++++++++
> src/remote/remote_protocol.h | 19 +++++++
> src/remote/remote_protocol.x | 14 +++++
> src/test/test_driver.c | 2
> src/uml/uml_driver.c | 2
> src/vbox/vbox_tmpl.c | 2
> src/vmware/vmware_driver.c | 2
> src/xen/xen_driver.c | 2
> src/xenapi/xenapi_driver.c | 2
> tools/virsh.c | 56 ++++++++++++++++++++++
> 31 files changed, 608 insertions(+), 2 deletions(-)
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 9bdb4c8..245247f 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -5245,6 +5245,94 @@ error:
> }
>
> /**
> + * virDomainSendEvnetNMI:
> + * @domain: pointer to domain object, or NULL for Domain0
> + * @vcpu: the virtual CPU id to send NMI to
> + *
> + * Send NMI to a special vcpu of the guest
> + *
> + * Returns 0 in case of success, -1 in case of failure.
> + */
> +
> +int virDomainSendEventNMI(virDomainPtr domain, unsigned int vcpu)
Your proposal to qemu-devel to add inject-nmi for QMP does not
include any CPU index parameter anymore. Instead it will automatically
inject the NMI to all present CPUs. This libvirt API would appear to
be incompatible with that QMP design. For Xen, it appears the API
also does not allow a CPU index to be given - it just injects to the
first CPU AFAICT.
So do we really need to have a 'unsigned int vcpu' parameter in the
libvirt API, or can we just leave it out and always inject to
CPU==0 for HMP ?
eg simplify to
int virDomainSendNMI(virDomainPtr domain)
> +{
> + virConnectPtr conn;
> + VIR_DOMAIN_DEBUG(domain, "vcpu=%u", vcpu);
> +
> + virResetLastError();
> +
> + if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virDispatchError(NULL);
> + return (-1);
> + }
> + if (domain->conn->flags & VIR_CONNECT_RO) {
> + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> + goto error;
> + }
> +
> + conn = domain->conn;
> +
> + if (conn->driver->domainSendEventNMI) {
> + int ret;
> + ret = conn->driver->domainSendEventNMI(domain, vcpu);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virLibConnError (VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> + virDispatchError(domain->conn);
> + return -1;
> +}
> +
> +/**
> + * virDomainSendEventKey:
> + * @domain: pointer to domain object, or NULL for Domain0
> + * @key: the string of key or key sequence
> + *
> + * Send a special key or key sequence to the guest
> + *
> + * Returns 0 in case of success, -1 in case of failure.
> + */
> +
> +int virDomainSendEventKey(virDomainPtr domain, const char *key)
I don't entirely like that the 'const char *key' is just directly
exposing QEMU key names and combining syntax eg "ctrl-alt-delete".
The names are symbolic names for XT scancodes, which QEMU uses
internally as its master scancode set (regardless of arch or
whether the keyboard is actually XT based).
The VirtualBox API, requires that the application send an array
of integer scancodes.
The QEMU 'sendkey' command also has another 'holdtime' parameter
which controls how long QEMU waits between the 'press' and
'release' of the keys, which our API does not have.
The core requirement is that our API be able to send any possible
key, to any hypervisor.
The problem with XT scancodes, is that while it can be made to
represent any possible scancode, it gets somewhat confusing for
apps because there are many different ways to represent extended
scancodes in use across apps.
One alternative would be to duplicate Linux's virtual scancode
constants, which are a superset of all known possible scancode
lists. Then we would do a conversion inside libvirt from Linux
to hypervisor specific scancode format (probably XT values, or
XT strings).
The interesting thing about using Linux virtual scancode constants
is that this includes mouse buttons. So the same API could be
used to inject mouse clicks. We'd still need an explicit API for
doing mouse events though, because we'd need to be able to press
buttons, then generate motion events, then release buttons.
So, my preference would be for an API
virDomainSendKey(virDomainPtr dom,
unsigned int nkeycodes,
unsigned int keycodes,
unsigned int holdtime);
If "holdtime" is zero, then the hypervisor default behaviour
would be used. This allows us to support VirtualBox where
we'd have to reject any non-zero "holdtime" value
Then I think I'd create a new header file include/libvirt/virtkeys.h
which contained a copy of only the KEY_* constants from:
/usr/include/linux/input.h
We'd just have a static array with a mapping from keys to the QEMU
keycode names.
> diff --git a/tools/virsh.c b/tools/virsh.c
> index faeaf47..0b78c6d 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2910,6 +2910,61 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
> }
>
> /*
> + * "sendevent" command
> + */
> +static const vshCmdInfo info_sendevent[] = {
> + {"help", N_("send events to the guest")},
> + {"desc", N_("Send events (NMI or Keys) to the guest domain.")},
> + {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_sendevent[] = {
> + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> + {"eventtype", VSH_OT_DATA, VSH_OFLAG_REQ, N_("the type of event (nmi or key)")},
> + {"eventcontent", VSH_OT_DATA, VSH_OFLAG_REQ, N_("content for the event.")},
> + {NULL, 0, 0, NULL}
> +};
> +
> +
> +static int
> +cmdSendEvent(vshControl *ctl, const vshCmd *cmd)
> +{
> + virDomainPtr dom;
> + const char *type;
> + int ret = TRUE;
> +
> + if (!vshConnectionUsability(ctl, ctl->conn))
> + return FALSE;
> +
> + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> + return FALSE;
> +
> + if (vshCommandOptString(cmd, "eventtype", &type) < 0)
> + return FALSE;
> +
> + if (STREQ(type, "nmi")) {
> + int cpu;
> +
> + if ((vshCommandOptInt(cmd, "eventcontent", &cpu) < 0)
> + || (virDomainSendEventNMI(dom, cpu) < 0))
> + ret = FALSE;
> + } else if (STREQ(type, "key")) {
> + const char *key;
> +
> + if ((vshCommandOptString(cmd, "eventcontent", &key) < 0)
> + || (virDomainSendEventKey(dom, key) < 0))
> + ret = FALSE;
> + } else {
> + virDomainFree(dom);
> + vshError(ctl, _("Invalid event type: %s, only \"nmi\" or \"key\" supported currently."), type);
> + return FALSE;
> + }
> +
> + virDomainFree(dom);
> + return ret;
> +}
I don't like the way two APIs are combined into one virsh command.
Particularly since I think we'll likely add another API 'virDomainSendPointer'
which will take as many as 4/5 parameters, instead of just one.
Thus I think it would be better to have
virsh send-nmi <domain>
virsh send-key [--holdtime millsec] <domain> <keycode1> [<keycode2> ...]
virsh send-pointer <domain> <buttonstate> <dx> <dy> <dz>...
virsh send-..... etc...
Regards,
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list