>>> On 12/21/2014 at 10:34 PM, in message <5496DA81.40708@xxxxxxxxxx>, Peter Krempa
<pkrempa@xxxxxxxxxx> 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
> >> <5492D0080200006600086404@xxxxxxxxxxxxxxxxxxxxx>, "Chun Yan Liu"
> >> <cyliu@xxxxxxxx> wrote:
> >>
> >>>
> >>>>>> On 12/17/2014 at 06:52 PM, in message
> >>>>>> <20141217105227.GQ136165@xxxxxxxxxx>,
> >>> Jiri Denemark <jdenemar@xxxxxxxxxx> 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 <cyliu@xxxxxxxx>
> >>>>> ---
> >>>>> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list