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.
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list