Re: [PATCH V2 1/5] Add public API virDomainSendSysrq

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

 




>>> On 12/12/2014 at 05:35 PM, in message <20141212093545.GD136165@xxxxxxxxxx>,
Jiri Denemark <jdenemar@xxxxxxxxxx> 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@xxxxxxxx> 
> > > --- 
> > >  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 
>  
>  



--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]