Re: [libvirt] [PATCH V2 1/5] Add public API virDomainSendSysrq
On Fri, Dec 12, 2014 at 10:18:36 +0100, Peter Krempa wrote:
On 12/12/14 10:04, Chunyan Liu wrote:
> Add public API virDomainSendSysrq for sending SysRequest key.
>
> Signed-off-by: Chunyan Liu <cyliu(a)suse.com>
> ---
> include/libvirt/libvirt-domain.h | 3 +++
> src/driver-hypervisor.h | 4 ++++
> src/libvirt-domain.c | 38 ++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 1 +
> 4 files changed, 46 insertions(+)
>
[...]
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index cb76d8c..4658fd7 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11192,3 +11192,41 @@ virDomainFSInfoFree(virDomainFSInfoPtr info)
> VIR_FREE(info->devAlias[i]);
> VIR_FREE(info->devAlias);
> }
> +
> +
> +/**
> + * virDomainSendSysrq:
> + * @domain: pointer to domain object, or NULL for Domain0
> + * @key: SysRq key, like h, c, ...
> + *
> + * Send SysRq key to the guest.
> + *
> + * Returns 0 in case of success, -1 in case of failure.
> + */
> +int
> +virDomainSendSysrq(virDomainPtr domain, const char *key)
The new API should definitely have a 'flags' argument although it may be
unused for now.
Moreover, passing a single character by reference sounds pretty strange,
why not just "char key"? Moreover, wouldn't it be better to provide an
enum of possible values with meaningful names (rather than keys), such
as
typedef enum {
VIR_DOMAIN_SYSRQ_REBOOT,
VIR_DOMAIN_SYSRQ_CRASH,
VIR_DOMAIN_SYSRQ_OOM_KILL,
VIR_DOMAIN_SYSRQ_SYNC,
...
} virDomainSysrqCommand;
This way, virDomainSendSysrq(dom, VIR_DOMAIN_SYSRQ_OOM_KILL, 0) would be
pretty self-explaining. The prototype would have to change to
int virDomainSendSysrq(virDomainPtr domain,
int command, /* one of virDomainSysrqCommand */
unsigned int flags);
Jirka