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

2015-01-06 Thread Chun Yan Liu 


 On 12/23/2014 at 03:32 PM, in message 54991AA7.243 : 102 : 21807, Chun Yan
Liu wrote: 

  
 On 12/23/2014 at 11:42 AM, in message 5498e4ba.1000...@redhat.com, John 
 Ferlan jfer...@redhat.com wrote:  
  

 On 12/22/2014 09:55 PM, Chun Yan Liu wrote:  


  On 12/22/2014 at 08:17 PM, in message 54980bf2.1060...@redhat.com, 
  John  
  Ferlan jfer...@redhat.com wrote:   

 
  On 12/21/2014 10:15 PM, Chun Yan Liu wrote:   
 
 
  On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, 
  John   
  Ferlan jfer...@redhat.com wrote:
 
  
  On 12/19/2014 12:31 AM, Chun Yan Liu wrote:
   
   
  On 12/18/2014 at 01:00 PM, in message
  5492d008026600086...@soto.provo.novell.com, Chun Yan Liu
  cy...@suse.com wrote:
   
   
  On 12/17/2014 at 06:52 PM, in message 
  20141217105227.gq136...@orkuz.home,
  Jiri Denemark jdene...@redhat.com wrote:
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:
  Add public API virDomainSendSysrq for sending SysRequest key.
   
  Signed-off-by: Chunyan Liu cy...@suse.com
  ---
  changes:
 * add 'flags' to the new API
 * change parameter from 'const char *key' to 'char key'
 * change version number from 1.2.11 to 1.2.12
   
include/libvirt/libvirt-domain.h |  3 +++
src/driver-hypervisor.h  |  4 
src/libvirt-domain.c | 39
  +++
src/libvirt_public.syms  |  5 +
4 files changed, 51 insertions(+)
   
  diff --git a/include/libvirt/libvirt-domain.h
  b/include/libvirt/libvirt-domain.h
  index baef32d..5f72850 100644
  --- a/include/libvirt/libvirt-domain.h
  +++ b/include/libvirt/libvirt-domain.h
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
   virDomainFSInfoPtr **info,
   unsigned int flags);
   
  +/* virDomainSendSysrq */
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int 
  flags);
  +
   
  I think quite a few reviewers (Daniel, Eric, and I) agreed on using 
  an
  enum instead of char so that the API is more general.
   
  Sorry, I missed this part. I'll update. One left question:
  How about 'virsh sysrq' parameters? What would we expect users to 
  pass?
   
  Any thoughts on that?
libxl_send_sysrq
  
  Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what  
 
  you had in mind for syntax previously - although it looks like:
  
   virsh sysrq domain [key]
 
  Thanks for reply. The syntax I'm used previously is:
  #virsh sysrq domain key   
  key is required. It's just a letter, like 'h', 'c', etc. About which
  options can we   
  have, on can refer to the results on guest through sysrq help. (that is, 
 
  issue   
  'virsh sysrq domain h' and look at guest kernel message. I think on each 
 
  guest,   
  there must be 'h' option, it will print help message.)   
 
  h, c, etc. doesn't tell me enough about what to expect from the   
  perspective of this naive user...  Passing 'h' via virsh to a driver   
  to return some help string that gets displayed where?  

  Guest kernel message if guest is Linux. xen/libxl just passes the key  
  blindly to guest kernel, so to pass 'h' to guest kernel, it just like one  
   
 issue:  
  #echo 'h'  /proc/sysrq-trigger  
  in a Linux guest, you will see in /var/log/messages:  

  SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e)  
   memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j)  
   sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m)  
   nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q)  
   unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V)  
   show-blocked-tasks(w) dump-ftrace-buffer(z)  

   
 FWIW: My point on this was - by using 'virsh sysrq domain h' you don't  
 provide a mechanism to display this output.  
   
 Seems just from the descriptions some of those letters might return some  
 useful information... Some aren't one way commands.  Perhaps it would be  
 useful to capture output and be able to return it...  
  
 Right. But might be hard. 
 And I think of a problem when mapping enum to letter: 
 to different guests (e.g. Linux vs Windows), the letter to enum mapping  
 might 
 be different, even hypervisor may not precisely know that. At least for xen 
 hypervisor, I think it only blindly sends the key to guest, but has no idea  
 of 
 different key-letter meaning to different guests. 
  
 - Chunyan 

Hi, everyone, 

Happy New Year!

Pick up this thread again since I just could not follow your suggestions about
using 'enum' instead of 'char' as virDomainSendSysrq parameter and virsh sysrq
domain reboot|crash|... syntax. Summarize here so that you could remember it
and spot your

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

2014-12-22 Thread John Ferlan 


On 12/21/2014 10:15 PM, Chun Yan Liu wrote:
 
 
 On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, John
 Ferlan jfer...@redhat.com wrote: 
 
  
 On 12/19/2014 12:31 AM, Chun Yan Liu wrote: 


 On 12/18/2014 at 01:00 PM, in message 
 5492d008026600086...@soto.provo.novell.com, Chun Yan Liu 
 cy...@suse.com wrote: 


 On 12/17/2014 at 06:52 PM, in message 
 20141217105227.gq136...@orkuz.home, 
 Jiri Denemark jdene...@redhat.com wrote: 
 On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: 
 Add public API virDomainSendSysrq for sending SysRequest key. 

 Signed-off-by: Chunyan Liu cy...@suse.com 
 --- 
 changes: 
* add 'flags' to the new API 
* change parameter from 'const char *key' to 'char key' 
* change version number from 1.2.11 to 1.2.12 

   include/libvirt/libvirt-domain.h |  3 +++ 
   src/driver-hypervisor.h  |  4  
   src/libvirt-domain.c | 39 
 +++ 
   src/libvirt_public.syms  |  5 + 
   4 files changed, 51 insertions(+) 

 diff --git a/include/libvirt/libvirt-domain.h 
 b/include/libvirt/libvirt-domain.h 
 index baef32d..5f72850 100644 
 --- a/include/libvirt/libvirt-domain.h 
 +++ b/include/libvirt/libvirt-domain.h 
 @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, 
  virDomainFSInfoPtr **info, 
  unsigned int flags); 

 +/* virDomainSendSysrq */ 
 +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags); 
 + 

 I think quite a few reviewers (Daniel, Eric, and I) agreed on using an 
 enum instead of char so that the API is more general. 

 Sorry, I missed this part. I'll update. One left question: 
 How about 'virsh sysrq' parameters? What would we expect users to pass? 

 Any thoughts on that? 
   libxl_send_sysrq 
  
 Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what  
 you had in mind for syntax previously - although it looks like: 
  
  virsh sysrq domain [key] 
 
 Thanks for reply. The syntax I'm used previously is: 
 #virsh sysrq domain key
 key is required. It's just a letter, like 'h', 'c', etc. About which options 
 can we
 have, on can refer to the results on guest through sysrq help. (that is, issue
 'virsh sysrq domain h' and look at guest kernel message. I think on each 
 guest,
 there must be 'h' option, it will print help message.)

h, c, etc. doesn't tell me enough about what to expect from the
perspective of this naive user...  Passing 'h' via virsh to a driver
to return some help string that gets displayed where?  Was there a
mechanism I missed to return and display that output? Do you have sample
output to show on a system with these changes applied?

 
  
 Where if not provided key would be NULL, which doesn't look good for how  
 the code reads now. 
 
 As said above, key is required, it couldn't be NULL, otherwise, it will 
 report error.
 

While the check in virsh because VSH_OFLAG_REQ is set for key is good,
what if someone calls the API directly? You have no check there for
key being non null - it just gets passed along.

 The description for key in virDomainSendSysrq is  
 still not sufficient to help me either: 
  
 + * @key:SysRq key, like h, c, ... 
  
 What does 'h', 'c', ... mean?  What are the options? What do they map to  
 functionality wise?  I assume it's hypervisor dependent, but that's all  
 stuff you need to describe somewhere. I don't want to guess or go  
 searching for the answer through numerous search engine hits. 
 
 I can add more description on how one could get those options, but the way
 I think is through 'sysrq help' and check guest message.
 
  
 Looking at the enum Jirka proposed: 
  
 typedef enum { 
  VIR_DOMAIN_SYSRQ_REBOOT, 
  VIR_DOMAIN_SYSRQ_CRASH, 
  VIR_DOMAIN_SYSRQ_OOM_KILL, 
  VIR_DOMAIN_SYSRQ_SYNC, 
  ... 
 } virDomainSysrqCommand; 
  
 It seems REBOOT would/could be the default. So if key wasn't provided  
 the incoming key would be 0 (zero)... If you didn't want a default,  
 then you'd have to force a style to be chosen. You're defining the API  
 so you show us how you want to handle that. Eventually, each hypervisor  
 would map that enum into a character. That is, you'll end up with a way  
 to map the enum to a letter for the types of sysrq's each hypervisor  
 could support. If a hypervisor doesn't support a specific type of sysrq,  
 then decide how to handle. 
  
 Anyway given the above enum list, I would think the virsh would be: 
  
  virsh sysrq domain reboot 
  virsh sysrq domain crash 
  virsh sysrq domain kill 
  virsh sysrq domain sync 
  ... 
 
 OK. That's what I'm concerned and why I hesitated to change API parameter
 from 'char key' to 'enum'. Personally I don't think this is a better user
 interface and has risk to miss some functionality, since we don't know
 which options those hypervisors can support.
 

If some other option is to be supported on some

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

2014-12-22 Thread Chun Yan Liu 


 On 12/22/2014 at 08:17 PM, in message 54980bf2.1060...@redhat.com, John
Ferlan jfer...@redhat.com wrote: 

  
 On 12/21/2014 10:15 PM, Chun Yan Liu wrote: 
   
   
  On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, 
  John 
  Ferlan jfer...@redhat.com wrote:  
   

  On 12/19/2014 12:31 AM, Chun Yan Liu wrote:  
  
  
  On 12/18/2014 at 01:00 PM, in message  
  5492d008026600086...@soto.provo.novell.com, Chun Yan Liu  
  cy...@suse.com wrote:  
  
  
  On 12/17/2014 at 06:52 PM, in message 
  20141217105227.gq136...@orkuz.home,  
  Jiri Denemark jdene...@redhat.com wrote:  
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:  
  Add public API virDomainSendSysrq for sending SysRequest key.  
  
  Signed-off-by: Chunyan Liu cy...@suse.com  
  ---  
  changes:  
 * add 'flags' to the new API  
 * change parameter from 'const char *key' to 'char key'  
 * change version number from 1.2.11 to 1.2.12  
  
include/libvirt/libvirt-domain.h |  3 +++  
src/driver-hypervisor.h  |  4   
src/libvirt-domain.c | 39  
  +++  
src/libvirt_public.syms  |  5 +  
4 files changed, 51 insertions(+)  
  
  diff --git a/include/libvirt/libvirt-domain.h  
  b/include/libvirt/libvirt-domain.h  
  index baef32d..5f72850 100644  
  --- a/include/libvirt/libvirt-domain.h  
  +++ b/include/libvirt/libvirt-domain.h  
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,  
   virDomainFSInfoPtr **info,  
   unsigned int flags);  
  
  +/* virDomainSendSysrq */  
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int 
  flags);  
  +  
  
  I think quite a few reviewers (Daniel, Eric, and I) agreed on using an  
  enum instead of char so that the API is more general.  
  
  Sorry, I missed this part. I'll update. One left question:  
  How about 'virsh sysrq' parameters? What would we expect users to pass?  
  
  Any thoughts on that?  
libxl_send_sysrq  

  Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what   
  you had in mind for syntax previously - although it looks like:  

   virsh sysrq domain [key]  
   
  Thanks for reply. The syntax I'm used previously is:  
  #virsh sysrq domain key 
  key is required. It's just a letter, like 'h', 'c', etc. About which  
 options can we 
  have, on can refer to the results on guest through sysrq help. (that is,  
 issue 
  'virsh sysrq domain h' and look at guest kernel message. I think on each  
 guest, 
  there must be 'h' option, it will print help message.) 
  
 h, c, etc. doesn't tell me enough about what to expect from the 
 perspective of this naive user...  Passing 'h' via virsh to a driver 
 to return some help string that gets displayed where?

Guest kernel message if guest is Linux. xen/libxl just passes the key
blindly to guest kernel, so to pass 'h' to guest kernel, it just like one issue:
#echo 'h'  /proc/sysrq-trigger
in a Linux guest, you will see in /var/log/messages:

SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e)
 memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j)
 sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m)
 nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q)
 unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V)
 show-blocked-tasks(w) dump-ftrace-buffer(z)

  Was there a 
 mechanism I missed to return and display that output? Do you have sample 
 output to show on a system with these changes applied? 

I don't know how if any other hypervisor behaves differently, for xen/libxl,
they just send sysrq key to guest blindly if I understand correctly. So, which 
letter
is corresponding to which option is all the same with guest sysrq key 
definition,
in other words, it depends on guest sysrq key definition.

  
   

  Where if not provided key would be NULL, which doesn't look good for how   
  the code reads now.  
   
  As said above, key is required, it couldn't be NULL, otherwise, it will  
 report error. 
   
  
 While the check in virsh because VSH_OFLAG_REQ is set for key is good, 
 what if someone calls the API directly? You have no check there for 
 key being non null - it just gets passed along. 
  
  The description for key in virDomainSendSysrq is   
  still not sufficient to help me either:  

  + * @key:SysRq key, like h, c, ...  

  What does 'h', 'c', ... mean?  What are the options? What do they map to   
  functionality wise?  I assume it's hypervisor dependent, but that's all   
  stuff you need to describe somewhere. I don't want to guess or go   
  searching for the answer through numerous search engine hits.  
   
  I can add more description on how one could get those options, but the way 
  I think is through 'sysrq help' and check guest message. 
   

  Looking at the enum Jirka proposed:  

  typedef enum {

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

2014-12-22 Thread John Ferlan 


On 12/22/2014 09:55 PM, Chun Yan Liu wrote:
 
 
 On 12/22/2014 at 08:17 PM, in message 54980bf2.1060...@redhat.com, John
 Ferlan jfer...@redhat.com wrote: 
 
  
 On 12/21/2014 10:15 PM, Chun Yan Liu wrote: 
  
  
 On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, 
 John 
 Ferlan jfer...@redhat.com wrote:  
  
   
 On 12/19/2014 12:31 AM, Chun Yan Liu wrote:  


 On 12/18/2014 at 01:00 PM, in message  
 5492d008026600086...@soto.provo.novell.com, Chun Yan Liu  
 cy...@suse.com wrote:  


 On 12/17/2014 at 06:52 PM, in message 
 20141217105227.gq136...@orkuz.home,  
 Jiri Denemark jdene...@redhat.com wrote:  
 On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:  
 Add public API virDomainSendSysrq for sending SysRequest key.  

 Signed-off-by: Chunyan Liu cy...@suse.com  
 ---  
 changes:  
* add 'flags' to the new API  
* change parameter from 'const char *key' to 'char key'  
* change version number from 1.2.11 to 1.2.12  

   include/libvirt/libvirt-domain.h |  3 +++  
   src/driver-hypervisor.h  |  4   
   src/libvirt-domain.c | 39  
 +++  
   src/libvirt_public.syms  |  5 +  
   4 files changed, 51 insertions(+)  

 diff --git a/include/libvirt/libvirt-domain.h  
 b/include/libvirt/libvirt-domain.h  
 index baef32d..5f72850 100644  
 --- a/include/libvirt/libvirt-domain.h  
 +++ b/include/libvirt/libvirt-domain.h  
 @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,  
  virDomainFSInfoPtr **info,  
  unsigned int flags);  

 +/* virDomainSendSysrq */  
 +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int 
 flags);  
 +  

 I think quite a few reviewers (Daniel, Eric, and I) agreed on using an  
 enum instead of char so that the API is more general.  

 Sorry, I missed this part. I'll update. One left question:  
 How about 'virsh sysrq' parameters? What would we expect users to pass?  

 Any thoughts on that?  
   libxl_send_sysrq  
   
 Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what   
 you had in mind for syntax previously - although it looks like:  
   
  virsh sysrq domain [key]  
  
 Thanks for reply. The syntax I'm used previously is:  
 #virsh sysrq domain key 
 key is required. It's just a letter, like 'h', 'c', etc. About which  
 options can we 
 have, on can refer to the results on guest through sysrq help. (that is,  
 issue 
 'virsh sysrq domain h' and look at guest kernel message. I think on each  
 guest, 
 there must be 'h' option, it will print help message.) 
  
 h, c, etc. doesn't tell me enough about what to expect from the 
 perspective of this naive user...  Passing 'h' via virsh to a driver 
 to return some help string that gets displayed where?
 
 Guest kernel message if guest is Linux. xen/libxl just passes the key
 blindly to guest kernel, so to pass 'h' to guest kernel, it just like one 
 issue:
 #echo 'h'  /proc/sysrq-trigger
 in a Linux guest, you will see in /var/log/messages:
 
 SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e)
  memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j)
  sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m)
  nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q)
  unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V)
  show-blocked-tasks(w) dump-ftrace-buffer(z)
 

FWIW: My point on this was - by using 'virsh sysrq domain h' you don't
provide a mechanism to display this output.

Seems just from the descriptions some of those letters might return some
useful information... Some aren't one way commands.  Perhaps it would be
useful to capture output and be able to return it...

John
  Was there a 
 mechanism I missed to return and display that output? Do you have sample 
 output to show on a system with these changes applied? 
 
 I don't know how if any other hypervisor behaves differently, for xen/libxl,
 they just send sysrq key to guest blindly if I understand correctly. So, 
 which letter
 is corresponding to which option is all the same with guest sysrq key 
 definition,
 in other words, it depends on guest sysrq key definition.
 
  
  
   
 Where if not provided key would be NULL, which doesn't look good for how   
 the code reads now.  
  
 As said above, key is required, it couldn't be NULL, otherwise, it will  
 report error. 
  
  
 While the check in virsh because VSH_OFLAG_REQ is set for key is good, 
 what if someone calls the API directly? You have no check there for 
 key being non null - it just gets passed along. 
  
 The description for key in virDomainSendSysrq is   
 still not sufficient to help me either:  
   
 + * @key:SysRq key, like h, c, ...  
   
 What does 'h', 'c', ... mean?  What are the options? What do they map to   
 functionality wise?  I assume it's hypervisor dependent, but that's all   
 stuff you need to describe somewhere. I don't

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

2014-12-22 Thread Chun Yan Liu 


 On 12/23/2014 at 11:42 AM, in message 5498e4ba.1000...@redhat.com, John
Ferlan jfer...@redhat.com wrote: 

  
 On 12/22/2014 09:55 PM, Chun Yan Liu wrote: 
   
   
  On 12/22/2014 at 08:17 PM, in message 54980bf2.1060...@redhat.com, 
  John 
  Ferlan jfer...@redhat.com wrote:  
   

  On 12/21/2014 10:15 PM, Chun Yan Liu wrote:  


  On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, 
  John  
  Ferlan jfer...@redhat.com wrote:   

 
  On 12/19/2014 12:31 AM, Chun Yan Liu wrote:   
  
  
  On 12/18/2014 at 01:00 PM, in message   
  5492d008026600086...@soto.provo.novell.com, Chun Yan Liu   
  cy...@suse.com wrote:   
  
  
  On 12/17/2014 at 06:52 PM, in message 
  20141217105227.gq136...@orkuz.home,   
  Jiri Denemark jdene...@redhat.com wrote:   
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:   
  Add public API virDomainSendSysrq for sending SysRequest key.   
  
  Signed-off-by: Chunyan Liu cy...@suse.com   
  ---   
  changes:   
 * add 'flags' to the new API   
 * change parameter from 'const char *key' to 'char key'   
 * change version number from 1.2.11 to 1.2.12   
  
include/libvirt/libvirt-domain.h |  3 +++   
src/driver-hypervisor.h  |  4    
src/libvirt-domain.c | 39   
  +++   
src/libvirt_public.syms  |  5 +   
4 files changed, 51 insertions(+)   
  
  diff --git a/include/libvirt/libvirt-domain.h   
  b/include/libvirt/libvirt-domain.h   
  index baef32d..5f72850 100644   
  --- a/include/libvirt/libvirt-domain.h   
  +++ b/include/libvirt/libvirt-domain.h   
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,   
   virDomainFSInfoPtr **info,   
   unsigned int flags);   
  
  +/* virDomainSendSysrq */   
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int 
  flags);   
  +   
  
  I think quite a few reviewers (Daniel, Eric, and I) agreed on using 
  an   
  enum instead of char so that the API is more general.   
  
  Sorry, I missed this part. I'll update. One left question:   
  How about 'virsh sysrq' parameters? What would we expect users to 
  pass?   
  
  Any thoughts on that?   
libxl_send_sysrq   
 
  Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what   
   
  you had in mind for syntax previously - although it looks like:   
 
   virsh sysrq domain [key]   

  Thanks for reply. The syntax I'm used previously is:   
  #virsh sysrq domain key  
  key is required. It's just a letter, like 'h', 'c', etc. About which   
  options can we  
  have, on can refer to the results on guest through sysrq help. (that is,  
   
  issue  
  'virsh sysrq domain h' and look at guest kernel message. I think on each  
   
  guest,  
  there must be 'h' option, it will print help message.)  

  h, c, etc. doesn't tell me enough about what to expect from the  
  perspective of this naive user...  Passing 'h' via virsh to a driver  
  to return some help string that gets displayed where? 
   
  Guest kernel message if guest is Linux. xen/libxl just passes the key 
  blindly to guest kernel, so to pass 'h' to guest kernel, it just like one  
 issue: 
  #echo 'h'  /proc/sysrq-trigger 
  in a Linux guest, you will see in /var/log/messages: 
   
  SysRq : HELP : loglevel(0-9) reboot(b) crash(c) terminate-all-tasks(e) 
   memory-full-oom-kill(f) kill-all-tasks(i) thaw-filesystems(j) 
   sak(k) show-backtrace-all-active-cpus(l) show-memory-usage(m) 
   nice-all-RT-tasks(n) poweroff(o) show-registers(p) show-all-timers(q) 
   unraw(r) sync(s) show-task-states(t) unmount(u) force-fb(V) 
   show-blocked-tasks(w) dump-ftrace-buffer(z) 
   
  
 FWIW: My point on this was - by using 'virsh sysrq domain h' you don't 
 provide a mechanism to display this output. 
  
 Seems just from the descriptions some of those letters might return some 
 useful information... Some aren't one way commands.  Perhaps it would be 
 useful to capture output and be able to return it... 

Right. But might be hard.
And I think of a problem when mapping enum to letter:
to different guests (e.g. Linux vs Windows), the letter to enum mapping might
be different, even hypervisor may not precisely know that. At least for xen
hypervisor, I think it only blindly sends the key to guest, but has no idea of
different key-letter meaning to different guests.

- Chunyan

  
 John 
   Was there a  
  mechanism I missed to return and display that output? Do you have sample  
  output to show on a system with these changes applied?  
   
  I don't know how if any other hypervisor behaves differently, for  
 xen/libxl, 
  they just send sysrq key to guest blindly if I understand correctly. So,  
 which letter 
  is corresponding to which option is all the same with guest sysrq key  
 definition, 
  in other words, it depends on guest sysrq key definition.

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

2014-12-21 Thread Peter Krempa 
On 12/19/14 13:03, John Ferlan wrote:
 
 
 On 12/19/2014 12:31 AM, Chun Yan Liu wrote:


 On 12/18/2014 at 01:00 PM, in message
 5492d008026600086...@soto.provo.novell.com, Chun Yan Liu
 cy...@suse.com wrote:


 On 12/17/2014 at 06:52 PM, in message
 20141217105227.gq136...@orkuz.home,
 Jiri Denemark jdene...@redhat.com wrote:
 On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:
 Add public API virDomainSendSysrq for sending SysRequest key.

 Signed-off-by: Chunyan Liu cy...@suse.com
 ---
 changes:
* add 'flags' to the new API
* change parameter from 'const char *key' to 'char key'
* change version number from 1.2.11 to 1.2.12

   include/libvirt/libvirt-domain.h |  3 +++
   src/driver-hypervisor.h  |  4 
   src/libvirt-domain.c | 39
 +++
   src/libvirt_public.syms  |  5 +
   4 files changed, 51 insertions(+)

 diff --git a/include/libvirt/libvirt-domain.h
 b/include/libvirt/libvirt-domain.h
 index baef32d..5f72850 100644
 --- a/include/libvirt/libvirt-domain.h
 +++ b/include/libvirt/libvirt-domain.h
 @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
  virDomainFSInfoPtr **info,
  unsigned int flags);

 +/* virDomainSendSysrq */
 +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int
 flags);
 +

 I think quite a few reviewers (Daniel, Eric, and I) agreed on using an
 enum instead of char so that the API is more general.

 Sorry, I missed this part. I'll update. One left question:
 How about 'virsh sysrq' parameters? What would we expect users to pass?

 Any thoughts on that?
   libxl_send_sysrq
 
 Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what
 you had in mind for syntax previously - although it looks like:
 
 virsh sysrq domain [key]
 
 Where if not provided key would be NULL, which doesn't look good for how
 the code reads now. The description for key in virDomainSendSysrq is
 still not sufficient to help me either:
 
 + * @key:SysRq key, like h, c, ...
 
 What does 'h', 'c', ... mean?  What are the options? What do they map to
 functionality wise?  I assume it's hypervisor dependent, but that's all
 stuff you need to describe somewhere. I don't want to guess or go
 searching for the answer through numerous search engine hits.
 
 Looking at the enum Jirka proposed:
 
 typedef enum {
 VIR_DOMAIN_SYSRQ_REBOOT,
 VIR_DOMAIN_SYSRQ_CRASH,
 VIR_DOMAIN_SYSRQ_OOM_KILL,
 VIR_DOMAIN_SYSRQ_SYNC,
 ...
 } virDomainSysrqCommand;
 
 It seems REBOOT would/could be the default. So if key wasn't provided
 the incoming key would be 0 (zero)... If you didn't want a default,
 then you'd have to force a style to be chosen. You're defining the API
 so you show us how you want to handle that. Eventually, each hypervisor
 would map that enum into a character. That is, you'll end up with a way
 to map the enum to a letter for the types of sysrq's each hypervisor
 could support. If a hypervisor doesn't support a specific type of sysrq,
 then decide how to handle.
 
 Anyway given the above enum list, I would think the virsh would be:
 
 virsh sysrq domain reboot
 virsh sysrq domain crash
 virsh sysrq domain kill
 virsh sysrq domain sync
 ...
 
 And key goes from optional to required unless you want to allow 'virsh
 sysrq domain' to mean reboot by default (e.g., if not provided the
 default is to reboot).
 

This still can be implemented using the existing API for sending general
keystrokes to the guest. I still don't see a reason to add a new API as
a special case of an existing one.

Peter



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-12-21 Thread Chun Yan Liu 


 On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, John
Ferlan jfer...@redhat.com wrote: 

  
 On 12/19/2014 12:31 AM, Chun Yan Liu wrote: 
  
  
  On 12/18/2014 at 01:00 PM, in message 
  5492d008026600086...@soto.provo.novell.com, Chun Yan Liu 
  cy...@suse.com wrote: 
  
  
  On 12/17/2014 at 06:52 PM, in message 
  20141217105227.gq136...@orkuz.home, 
  Jiri Denemark jdene...@redhat.com wrote: 
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: 
  Add public API virDomainSendSysrq for sending SysRequest key. 
  
  Signed-off-by: Chunyan Liu cy...@suse.com 
  --- 
  changes: 
 * add 'flags' to the new API 
 * change parameter from 'const char *key' to 'char key' 
 * change version number from 1.2.11 to 1.2.12 
  
include/libvirt/libvirt-domain.h |  3 +++ 
src/driver-hypervisor.h  |  4  
src/libvirt-domain.c | 39 
  +++ 
src/libvirt_public.syms  |  5 + 
4 files changed, 51 insertions(+) 
  
  diff --git a/include/libvirt/libvirt-domain.h 
  b/include/libvirt/libvirt-domain.h 
  index baef32d..5f72850 100644 
  --- a/include/libvirt/libvirt-domain.h 
  +++ b/include/libvirt/libvirt-domain.h 
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, 
   virDomainFSInfoPtr **info, 
   unsigned int flags); 
  
  +/* virDomainSendSysrq */ 
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags); 
  + 
  
  I think quite a few reviewers (Daniel, Eric, and I) agreed on using an 
  enum instead of char so that the API is more general. 
  
  Sorry, I missed this part. I'll update. One left question: 
  How about 'virsh sysrq' parameters? What would we expect users to pass? 
  
  Any thoughts on that? 
libxl_send_sysrq 
  
 Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what  
 you had in mind for syntax previously - although it looks like: 
  
  virsh sysrq domain [key] 

Thanks for reply. The syntax I'm used previously is: 
#virsh sysrq domain key
key is required. It's just a letter, like 'h', 'c', etc. About which options 
can we
have, on can refer to the results on guest through sysrq help. (that is, issue
'virsh sysrq domain h' and look at guest kernel message. I think on each guest,
there must be 'h' option, it will print help message.)

  
 Where if not provided key would be NULL, which doesn't look good for how  
 the code reads now. 

As said above, key is required, it couldn't be NULL, otherwise, it will report 
error.

 The description for key in virDomainSendSysrq is  
 still not sufficient to help me either: 
  
 + * @key:SysRq key, like h, c, ... 
  
 What does 'h', 'c', ... mean?  What are the options? What do they map to  
 functionality wise?  I assume it's hypervisor dependent, but that's all  
 stuff you need to describe somewhere. I don't want to guess or go  
 searching for the answer through numerous search engine hits. 

I can add more description on how one could get those options, but the way
I think is through 'sysrq help' and check guest message.

  
 Looking at the enum Jirka proposed: 
  
 typedef enum { 
  VIR_DOMAIN_SYSRQ_REBOOT, 
  VIR_DOMAIN_SYSRQ_CRASH, 
  VIR_DOMAIN_SYSRQ_OOM_KILL, 
  VIR_DOMAIN_SYSRQ_SYNC, 
  ... 
 } virDomainSysrqCommand; 
  
 It seems REBOOT would/could be the default. So if key wasn't provided  
 the incoming key would be 0 (zero)... If you didn't want a default,  
 then you'd have to force a style to be chosen. You're defining the API  
 so you show us how you want to handle that. Eventually, each hypervisor  
 would map that enum into a character. That is, you'll end up with a way  
 to map the enum to a letter for the types of sysrq's each hypervisor  
 could support. If a hypervisor doesn't support a specific type of sysrq,  
 then decide how to handle. 
  
 Anyway given the above enum list, I would think the virsh would be: 
  
  virsh sysrq domain reboot 
  virsh sysrq domain crash 
  virsh sysrq domain kill 
  virsh sysrq domain sync 
  ... 

OK. That's what I'm concerned and why I hesitated to change API parameter
from 'char key' to 'enum'. Personally I don't think this is a better user
interface and has risk to miss some functionality, since we don't know
which options those hypervisors can support.

I still prefer: 
#virsh sysrq domain key_letter
One can first issue 'virsh sysrq domain h', and check guest kernel message
for all sysrq options. Then send option as he need.
And as a result, I still think I don't see benefit of changing the API parameter
from 'char key' to 'enum'.

How do you think?

Chunyan

 And key goes from optional to required unless you want to allow 'virsh  
 sysrq domain' to mean reboot by default (e.g., if not provided the  
 default is to reboot). 
  
 The string for key would be passed to the virDomainSendSysrq which would  
 then convert or map

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

2014-12-21 Thread Chun Yan Liu 


 On 12/21/2014 at 10:34 PM, in message 5496da81.40...@redhat.com, Peter 
 Krempa
pkre...@redhat.com wrote: 
 On 12/19/14 13:03, John Ferlan wrote: 
   
   
  On 12/19/2014 12:31 AM, Chun Yan Liu wrote: 
  
  
  On 12/18/2014 at 01:00 PM, in message 
  5492d008026600086...@soto.provo.novell.com, Chun Yan Liu 
  cy...@suse.com wrote: 
  
  
  On 12/17/2014 at 06:52 PM, in message 
  20141217105227.gq136...@orkuz.home, 
  Jiri Denemark jdene...@redhat.com wrote: 
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: 
  Add public API virDomainSendSysrq for sending SysRequest key. 
  
  Signed-off-by: Chunyan Liu cy...@suse.com 
  --- 
  changes: 
 * add 'flags' to the new API 
 * change parameter from 'const char *key' to 'char key' 
 * change version number from 1.2.11 to 1.2.12 
  
include/libvirt/libvirt-domain.h |  3 +++ 
src/driver-hypervisor.h  |  4  
src/libvirt-domain.c | 39 
  +++ 
src/libvirt_public.syms  |  5 + 
4 files changed, 51 insertions(+) 
  
  diff --git a/include/libvirt/libvirt-domain.h 
  b/include/libvirt/libvirt-domain.h 
  index baef32d..5f72850 100644 
  --- a/include/libvirt/libvirt-domain.h 
  +++ b/include/libvirt/libvirt-domain.h 
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, 
   virDomainFSInfoPtr **info, 
   unsigned int flags); 
  
  +/* virDomainSendSysrq */ 
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int 
  flags); 
  + 
  
  I think quite a few reviewers (Daniel, Eric, and I) agreed on using an 
  enum instead of char so that the API is more general. 
  
  Sorry, I missed this part. I'll update. One left question: 
  How about 'virsh sysrq' parameters? What would we expect users to pass? 
  
  Any thoughts on that? 
libxl_send_sysrq 
   
  Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what 
  you had in mind for syntax previously - although it looks like: 
   
  virsh sysrq domain [key] 
   
  Where if not provided key would be NULL, which doesn't look good for how 
  the code reads now. The description for key in virDomainSendSysrq is 
  still not sufficient to help me either: 
   
  + * @key:SysRq key, like h, c, ... 
   
  What does 'h', 'c', ... mean?  What are the options? What do they map to 
  functionality wise?  I assume it's hypervisor dependent, but that's all 
  stuff you need to describe somewhere. I don't want to guess or go 
  searching for the answer through numerous search engine hits. 
   
  Looking at the enum Jirka proposed: 
   
  typedef enum { 
  VIR_DOMAIN_SYSRQ_REBOOT, 
  VIR_DOMAIN_SYSRQ_CRASH, 
  VIR_DOMAIN_SYSRQ_OOM_KILL, 
  VIR_DOMAIN_SYSRQ_SYNC, 
  ... 
  } virDomainSysrqCommand; 
   
  It seems REBOOT would/could be the default. So if key wasn't provided 
  the incoming key would be 0 (zero)... If you didn't want a default, 
  then you'd have to force a style to be chosen. You're defining the API 
  so you show us how you want to handle that. Eventually, each hypervisor 
  would map that enum into a character. That is, you'll end up with a way 
  to map the enum to a letter for the types of sysrq's each hypervisor 
  could support. If a hypervisor doesn't support a specific type of sysrq, 
  then decide how to handle. 
   
  Anyway given the above enum list, I would think the virsh would be: 
   
  virsh sysrq domain reboot 
  virsh sysrq domain crash 
  virsh sysrq domain kill 
  virsh sysrq domain sync 
  ... 
   
  And key goes from optional to required unless you want to allow 'virsh 
  sysrq domain' to mean reboot by default (e.g., if not provided the 
  default is to reboot). 
   
  
 This still can be implemented using the existing API for sending general 
 keystrokes to the guest. I still don't see a reason to add a new API as 
 a special case of an existing one. 

First version is implemented by using .domainSendKey but objected. See:
https://www.redhat.com/archives/libvir-list/2014-December/msg00480.html

Thanks.

  
 Peter 
  
  


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

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

2014-12-19 Thread John Ferlan 



On 12/19/2014 12:31 AM, Chun Yan Liu wrote:




On 12/18/2014 at 01:00 PM, in message

5492d008026600086...@soto.provo.novell.com, Chun Yan Liu
cy...@suse.com wrote:




On 12/17/2014 at 06:52 PM, in message 20141217105227.gq136...@orkuz.home,

Jiri Denemark jdene...@redhat.com wrote:

On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:

Add public API virDomainSendSysrq for sending SysRequest key.

Signed-off-by: Chunyan Liu cy...@suse.com
---
changes:
   * add 'flags' to the new API
   * change parameter from 'const char *key' to 'char key'
   * change version number from 1.2.11 to 1.2.12

  include/libvirt/libvirt-domain.h |  3 +++
  src/driver-hypervisor.h  |  4 
  src/libvirt-domain.c | 39

+++

  src/libvirt_public.syms  |  5 +
  4 files changed, 51 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h

b/include/libvirt/libvirt-domain.h

index baef32d..5f72850 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
 virDomainFSInfoPtr **info,
 unsigned int flags);

+/* virDomainSendSysrq */
+int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags);
+


I think quite a few reviewers (Daniel, Eric, and I) agreed on using an
enum instead of char so that the API is more general.


Sorry, I missed this part. I'll update. One left question:
How about 'virsh sysrq' parameters? What would we expect users to pass?


Any thoughts on that?
  libxl_send_sysrq


Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what 
you had in mind for syntax previously - although it looks like:


virsh sysrq domain [key]

Where if not provided key would be NULL, which doesn't look good for how 
the code reads now. The description for key in virDomainSendSysrq is 
still not sufficient to help me either:


+ * @key:SysRq key, like h, c, ...

What does 'h', 'c', ... mean?  What are the options? What do they map to 
functionality wise?  I assume it's hypervisor dependent, but that's all 
stuff you need to describe somewhere. I don't want to guess or go 
searching for the answer through numerous search engine hits.


Looking at the enum Jirka proposed:

typedef enum {
VIR_DOMAIN_SYSRQ_REBOOT,
VIR_DOMAIN_SYSRQ_CRASH,
VIR_DOMAIN_SYSRQ_OOM_KILL,
VIR_DOMAIN_SYSRQ_SYNC,
...
} virDomainSysrqCommand;

It seems REBOOT would/could be the default. So if key wasn't provided 
the incoming key would be 0 (zero)... If you didn't want a default, 
then you'd have to force a style to be chosen. You're defining the API 
so you show us how you want to handle that. Eventually, each hypervisor 
would map that enum into a character. That is, you'll end up with a way 
to map the enum to a letter for the types of sysrq's each hypervisor 
could support. If a hypervisor doesn't support a specific type of sysrq, 
then decide how to handle.


Anyway given the above enum list, I would think the virsh would be:

virsh sysrq domain reboot
virsh sysrq domain crash
virsh sysrq domain kill
virsh sysrq domain sync
...

And key goes from optional to required unless you want to allow 'virsh 
sysrq domain' to mean reboot by default (e.g., if not provided the 
default is to reboot).


The string for key would be passed to the virDomainSendSysrq which would 
then convert or map that string into an enum. Check out the 
VIR_ENUM_{IMPL|DECL} usage and how they generate TypeToString and 
TypeFromString API's to perform the string - enum mapping.


So there's some thoughts for you - hopefully it gives you some ideas.

John

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

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

2014-12-18 Thread Chun Yan Liu 


 On 12/18/2014 at 01:00 PM, in message
5492d008026600086...@soto.provo.novell.com, Chun Yan Liu
cy...@suse.com wrote: 

  
 On 12/17/2014 at 06:52 PM, in message 
 20141217105227.gq136...@orkuz.home, 
 Jiri Denemark jdene...@redhat.com wrote:  
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:  
   Add public API virDomainSendSysrq for sending SysRequest key.  
 
   Signed-off-by: Chunyan Liu cy...@suse.com  
   ---  
   changes:  
 * add 'flags' to the new API  
 * change parameter from 'const char *key' to 'char key'  
 * change version number from 1.2.11 to 1.2.12  
 
include/libvirt/libvirt-domain.h |  3 +++  
src/driver-hypervisor.h  |  4   
src/libvirt-domain.c | 39   
  +++  
src/libvirt_public.syms  |  5 +  
4 files changed, 51 insertions(+)  
 
   diff --git a/include/libvirt/libvirt-domain.h   
  b/include/libvirt/libvirt-domain.h  
   index baef32d..5f72850 100644  
   --- a/include/libvirt/libvirt-domain.h  
   +++ b/include/libvirt/libvirt-domain.h  
   @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,  
   virDomainFSInfoPtr **info,  
   unsigned int flags);  
  
   +/* virDomainSendSysrq */  
   +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags);  
   +  

  I think quite a few reviewers (Daniel, Eric, and I) agreed on using an  
  enum instead of char so that the API is more general.  
  
 Sorry, I missed this part. I'll update. One left question: 
 How about 'virsh sysrq' parameters? What would we expect users to pass?

Any thoughts on that?
 
  

  Jirka  


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



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

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

2014-12-17 Thread Chunyan Liu 
Add public API virDomainSendSysrq for sending SysRequest key.

Signed-off-by: Chunyan Liu cy...@suse.com
---
changes:
  * add 'flags' to the new API
  * change parameter from 'const char *key' to 'char key'
  * change version number from 1.2.11 to 1.2.12

 include/libvirt/libvirt-domain.h |  3 +++
 src/driver-hypervisor.h  |  4 
 src/libvirt-domain.c | 39 +++
 src/libvirt_public.syms  |  5 +
 4 files changed, 51 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index baef32d..5f72850 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
virDomainFSInfoPtr **info,
unsigned int flags);
 
+/* virDomainSendSysrq */
+int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags);
+
 int virDomainGetTime(virDomainPtr dom,
  long long *seconds,
  unsigned int *nseconds,
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 9f26b13..d260d29 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1170,6 +1170,9 @@ typedef int
 unsigned int cellCount,
 unsigned int flags);
 
+typedef int
+(*virDrvDomainSendSysrq)(virDomainPtr dom, char key, unsigned int flags);
+
 
 typedef struct _virHypervisorDriver virHypervisorDriver;
 typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1396,6 +1399,7 @@ struct _virHypervisorDriver {
 virDrvConnectGetAllDomainStats connectGetAllDomainStats;
 virDrvNodeAllocPages nodeAllocPages;
 virDrvDomainGetFSInfo domainGetFSInfo;
+virDrvDomainSendSysrq domainSendSysrq;
 };
 
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cb76d8c..d58ec87 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11192,3 +11192,42 @@ 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, ...
+ * @flags:  extra flags; not used yet, so callers should always pass 0
+ *
+ * Send SysRq key to the guest.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+int
+virDomainSendSysrq(virDomainPtr domain, char key, unsigned int flags)
+{
+virConnectPtr conn;
+VIR_DOMAIN_DEBUG(domain, key=%c, flags=%x, key, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+conn = domain-conn;
+
+virCheckReadOnlyGoto(conn-flags, error);
+
+if (conn-driver-domainSendSysrq) {
+int ret;
+ret = conn-driver-domainSendSysrq(domain, key, flags);
+if (ret  0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain-conn);
+return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index e4c2df1..5d4999a 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -690,4 +690,9 @@ LIBVIRT_1.2.11 {
 virDomainGetFSInfo;
 } LIBVIRT_1.2.9;
 
+LIBVIRT_1.2.12 {
+global:
+virDomainSendSysrq;
+} LIBVIRT_1.2.11;
+
 #  define new API here using predicted next version number 
-- 
1.8.4.5

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

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

2014-12-17 Thread Jiri Denemark 
On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:
 Add public API virDomainSendSysrq for sending SysRequest key.
 
 Signed-off-by: Chunyan Liu cy...@suse.com
 ---
 changes:
   * add 'flags' to the new API
   * change parameter from 'const char *key' to 'char key'
   * change version number from 1.2.11 to 1.2.12
 
  include/libvirt/libvirt-domain.h |  3 +++
  src/driver-hypervisor.h  |  4 
  src/libvirt-domain.c | 39 +++
  src/libvirt_public.syms  |  5 +
  4 files changed, 51 insertions(+)
 
 diff --git a/include/libvirt/libvirt-domain.h 
 b/include/libvirt/libvirt-domain.h
 index baef32d..5f72850 100644
 --- a/include/libvirt/libvirt-domain.h
 +++ b/include/libvirt/libvirt-domain.h
 @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
 virDomainFSInfoPtr **info,
 unsigned int flags);
  
 +/* virDomainSendSysrq */
 +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags);
 +

I think quite a few reviewers (Daniel, Eric, and I) agreed on using an
enum instead of char so that the API is more general.

Jirka

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

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

2014-12-17 Thread Chun Yan Liu 


 On 12/17/2014 at 06:52 PM, in message 20141217105227.gq136...@orkuz.home,
Jiri Denemark jdene...@redhat.com wrote: 
 On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: 
  Add public API virDomainSendSysrq for sending SysRequest key. 
   
  Signed-off-by: Chunyan Liu cy...@suse.com 
  --- 
  changes: 
* add 'flags' to the new API 
* change parameter from 'const char *key' to 'char key' 
* change version number from 1.2.11 to 1.2.12 
   
   include/libvirt/libvirt-domain.h |  3 +++ 
   src/driver-hypervisor.h  |  4  
   src/libvirt-domain.c | 39  
 +++ 
   src/libvirt_public.syms  |  5 + 
   4 files changed, 51 insertions(+) 
   
  diff --git a/include/libvirt/libvirt-domain.h  
 b/include/libvirt/libvirt-domain.h 
  index baef32d..5f72850 100644 
  --- a/include/libvirt/libvirt-domain.h 
  +++ b/include/libvirt/libvirt-domain.h 
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, 
  virDomainFSInfoPtr **info, 
  unsigned int flags); 

  +/* virDomainSendSysrq */ 
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags); 
  + 
  
 I think quite a few reviewers (Daniel, Eric, and I) agreed on using an 
 enum instead of char so that the API is more general. 

Sorry, I missed this part. I'll update. One left question:
* How about 'virsh sysrq' parameters? What would we expect users to pass?

  
 Jirka 
  
  



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