[libvirt] [PATCH V2 1/5] Add public API virDomainSendSysrq

[Daniel P. Berrange]

On Fri, Dec 12, 2014 at 10:35:45AM +0100, Jiri Denemark 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 at 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);

That would open up greater possibilities for implementing it differently
for different guest OS, rather than assuming Linux characters.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|