[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 05/10 V2] [PATCH 05/13] send-key: Defining the public API



On 06/13/2011 04:52 PM, Daniel P. Berrange wrote:
> On Tue, Jun 07, 2011 at 05:11:12PM +0800, Lai Jiangshan wrote:
>> Add public virDomainSendKey() and enum libvirt_keycode_set
>> for the @codeset.
>>
>> Python version of virDomainSendKey() has not been implemented yet,
>> it will be done soon.
>>
>> Signed-off-by: Lai Jiangshan <laijs cn fujitsu com>
>> ---
>>  include/libvirt/libvirt.h.in |    7 +++++++
>>  include/libvirt/virtkeys.h   |   22 ++++++++++++++++++++++
>>  python/generator.py          |    1 +
>>  src/libvirt_public.syms      |    5 +++++
>>  4 files changed, 35 insertions(+), 0 deletions(-)
>>  create mode 100644 include/libvirt/virtkeys.h
>>

>> +++ b/include/libvirt/virtkeys.h
>> @@ -0,0 +1,22 @@
> 
> I think this enum + #define should be in the main libvirt.h
> header file, alongside the virDomainSendKey API definition.
> Keep the separate 'virtkeys.h' file to be used only for the
> large list of keycode names.

Well, we don't have a large list of keycode names yet, so for now there
is no reason to have virtkeys.h.  Not to mention that you can't add a
new header without also touching Makefile.am to make sure it gets into
the tarball.  So I just removed that file.

> Also, using 'libvirt_' or LIBVIRT_ isn't our normal namespace,
> and enums use capital letters for separation, rather than
> underscores. Any enum should have a typedef, and finally the
> constant should have a VIR_DOMAIN prefix
> 
> So should be more like
> 
>  typedef enum {
>        VIR_KEYCODE_LINUX = 0,
>        ...
>  } virKeycodeSet;
> 
>  #define VIR_DOMAIN_SEND_KEY_MAX_KEYS 16

ACK to Daniel's approach, and needs documentation.  Also, this should be
added prior to deprecated interfaces.  Here's what I squashed in before
pushing:

diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in
index 63c0582..f0b6d9b 100644
--- i/include/libvirt/libvirt.h.in
+++ w/include/libvirt/libvirt.h.in
@@ -1566,6 +1566,33 @@ char *                  virStorageVolGetXMLDesc
       (virStorageVolPtr pool,

 char *                  virStorageVolGetPath
(virStorageVolPtr vol);

+/**
+ * virKeycodeSet:
+ *
+ * Enum to specify which keycode mapping is in use for virDomainSendKey().
+ */
+typedef enum {
+    VIR_KEYCODE_SET_LINUX          = 0,
+    VIR_KEYCODE_SET_XT             = 1,
+    VIR_KEYCODE_SET_ATSET1         = 2,
+    VIR_KEYCODE_SET_ATSET2         = 3,
+    VIR_KEYCODE_SET_ATSET3         = 4,
+} virKeycodeSet;
+
+/**
+ * VIR_DOMAIN_SEND_KEY_MAX_KEYS:
+ *
+ * Maximum number of keycodes that can be sent in one
virDomainSendKey() call.
+ */
+#define VIR_DOMAIN_SEND_KEY_MAX_KEYS  16
+
+int virDomainSendKey(virDomainPtr domain,
+                     unsigned int codeset,
+                     unsigned int holdtime,
+                     unsigned int *keycodes,
+                     unsigned int nkeycodes,
+                     unsigned int flags);
+
 /*
  * Deprecated calls
  */
@@ -2691,13 +2718,6 @@ typedef struct _virTypedParameter virMemoryParameter;
  */
 typedef virMemoryParameter *virMemoryParameterPtr;

-int virDomainSendKey(virDomainPtr domain,
-                     unsigned int codeset,
-                     unsigned int holdtime,
-                     unsigned int *keycodes,
-                     unsigned int nkeycodes,
-                     unsigned int flags);
-
 #ifdef __cplusplus
 }
 #endif


-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]