Re: [libvirt] [PATCH V2 1/5] Add public API virDomainSendSysrq
>> On 12/12/2014 at 05:35 PM, in message
<20141212093545.GD136165(a)orkuz.home>,
Jiri Denemark
<jdenemar(a)redhat.com> wrote:
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"?
I tried to define as 'char key', but meet some trouble in remote protocol,
+struct remote_domain_send_sysrq_args {
+ remote_nonnull_domain dom;
+ char key;
+};
will report 'char key' as unsupported type.
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
Mainly because xen/libxl API both accept letter as parameter directly, so define
enum will need to map enum to letter before calling xen/libxl API.
Chunyan
int virDomainSendSysrq(virDomainPtr domain,
int command, /* one of virDomainSysrqCommand */
unsigned int flags);
Jirka