On 04/01/2011 03:51 AM, 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> Would you mind resending this split into several patches? For example, see how https://www.redhat.com/archives/libvir-list/2011-April/msg00052.html divided a new API into 7 patches: Michal Privoznik (7): screenshot: Defining the public API screenshot: Defining the internal API screenshot: Implementing the public API screenshot: Implementing the remote protocol screenshot: Expose the new API in virsh qemu: Implement the driver methods vbox: Implement the driver methods By separating definition, public access, virsh use, and driver implementation into separate patches, it becomes easier to review. Each division should still cleanly compile. See also http://libvirt.org/api_extension.html (although it predates some of the recent changes such as adding libxl and dropping OpenNebula, it is still a good reference). > +++ b/src/libvirt.c > @@ -5245,6 +5245,94 @@ error: > } > > /** > + * virDomainSendEvnetNMI: s/Evnet/Event/ > + * @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 s/special/specified/ > + * > + * Returns 0 in case of success, -1 in case of failure. > + */ > + > +int virDomainSendEventNMI(virDomainPtr domain, unsigned int vcpu) Absolutely must have a flags argument for future extension (even if the only supported flags value is 0 for now). > + > +/** > + * 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) Needs a flags argument. Also, what is the format of a key sequence? There's several layers of key events, from what a keyboard sends, to what the OS driver encodes, to what X encodes, and so forth. If the format ever needs to include a NUL byte, then you need a length parameter. I'm worried that this is a bit underspecified. Do we know in advance which keys are acceptable (I'm assuming this is for SysRq sequences), in which case an enum value might be better than a raw char*. > +++ b/src/libvirt_public.syms > @@ -434,6 +434,8 @@ LIBVIRT_0.9.0 { > virEventRunDefaultImpl; > virStorageVolDownload; > virStorageVolUpload; > + virDomainSendEventNMI; > + virDomainSendEventKey; > } LIBVIRT_0.8.8; This is too late for the 0.9.0 feature freeze cutoff; when you split this into multiple patches, can you rebase it to be targetting 0.9.1? I haven't closely reviewed the rest of the patch yet, but think that the overall idea of being able to inject NMI and SysRq input to the virtual hardware of the guest is worth adding. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list