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

[Lai Jiangshan <laijs cn fujitsu com>]

On 05/26/2011 12:43 AM, Daniel P. Berrange wrote:
> On Wed, May 25, 2011 at 05:37:47PM +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 fujitsu com>
>> ---
>>  include/libvirt/libvirt.h.in |    7 +++++++
>>  include/libvirt/virtkeys.h   |   23 +++++++++++++++++++++++
>>  python/generator.py          |    1 +
>>  src/libvirt_public.syms      |    1 +
>>  4 files changed, 32 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
>> index 7cd6e13..9167dbc 100644
>> --- a/include/libvirt/libvirt.h.in
>> +++ b/include/libvirt/libvirt.h.in
>> @@ -2617,6 +2617,13 @@ int virDomainOpenConsole(virDomainPtr dom,
>>  
>>  int virDomainInjectNMI(virDomainPtr domain, unsigned int flags);
>>  
>> +int virDomainSendKey(virDomainPtr domain,
>> +                     unsigned int codeset,
>> +                     unsigned int holdtime,
>> +                     unsigned int nkeycodes,
>> +                     unsigned int *keycodes,
>> +                     unsigned int flags);
>> +
> 
> This looks fine. As mentioned earlier we might like to *also*
> have a variant which takes strings, to make life easier for
> virsh, or similar use cases
> 
>  +int virDomainSendKeyStr(virDomainPtr domain,
>  +                       unsigned int codeset,
>  +                       unsigned int holdtime,
>  +                       unsigned int nkeycodes,
>  +                       unsigned char **keycodestrs,
>  +                       unsigned int flags);
>  +
> 
> 
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> diff --git a/include/libvirt/virtkeys.h b/include/libvirt/virtkeys.h
>> new file mode 100644
>> index 0000000..eb07129
>> --- /dev/null
>> +++ b/include/libvirt/virtkeys.h
>> @@ -0,0 +1,23 @@
>> +#ifndef _LIBVIRT_VIRTKEYS_H
>> +#define _LIBVIRT_VIRTKEYS_H
>> +
>> +/*
>> + * Copyright (c) 2011 Lai Jiangshan
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + */
>> +
>> +enum libvirt_keycode_set {
>> +    LIBVIRT_KEYCODE_LINUX          = 0,
>> +    LIBVIRT_KEYCODE_DRIVER_DEFAULT = 1,
>> +    LIBVIRT_KEYCODE_XT             = 2,
>> +    LIBVIRT_KEYCODE_ATSET1         = LIBVIRT_KEYCODE_XT,
>> +    LIBVIRT_KEYCODE_ATSET2         = 3,
>> +    LIBVIRT_KEYCODE_ATSET3         = 4,
>> +};
> 
> If we're going to have constants for XT and ATSET1 then we
> probably want to have them separate values. If not then
> we should just kill one of them. As mentioned in the initial
> message, I think 'driver default' isn't useful, so I'd go
> with:
> 
>  +enum libvirt_keycode_set {
>  +    LIBVIRT_KEYCODE_LINUX          = 0,
>  +    LIBVIRT_KEYCODE_XT             = 1,
>  +    LIBVIRT_KEYCODE_ATSET1         = 2
>  +    LIBVIRT_KEYCODE_ATSET2         = 3,
>  +    LIBVIRT_KEYCODE_ATSET3         = 4,
>  +};
> 


Sorry, That's my fault, I thought xt and atset1 are the same codeset.
Thank you for correcting me.

Thanks,
Lai.

> 
>> +
>> +#define MAX_SEND_KEY  16
>> +
>> +#endif
>> diff --git a/python/generator.py b/python/generator.py
>> index 1741bba..3d57bf9 100755
>> --- a/python/generator.py
>> +++ b/python/generator.py
>> @@ -355,6 +355,7 @@ skip_impl = (
>>      'virNodeDeviceListCaps',
>>      'virConnectBaselineCPU',
>>      'virDomainRevertToSnapshot',
>> +    'virDomainSendKey',
>>  )
>>  
>>  
>> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
>> index 0590535..9e8a37c 100644
>> --- a/src/libvirt_public.syms
>> +++ b/src/libvirt_public.syms
>> @@ -442,6 +442,7 @@ LIBVIRT_0.9.2 {
>>          virDomainInjectNMI;
>>          virDomainScreenshot;
>>          virDomainSetSchedulerParametersFlags;
>> +        virDomainSendKey;
>>  } LIBVIRT_0.9.0;
>>  
>>  # .... define new API here using predicted next version number ....
> 
> Daniel