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

Re: [libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'



Doug Goldstein wrote:
> On Mon, Nov 5, 2012 at 4:57 PM, Eric Blake <eblake redhat com> wrote:
>   
>> On 11/05/2012 03:53 PM, Peter Krempa wrote:
>>     
>>> On 11/05/12 20:59, Eric Blake wrote:
>>>       
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=873344 suggested that
>>>> the grouping 'boot', 'shutdown', 'reboot'; as well as the grouping
>>>> 'start', 'stop', 'restart'; might be easier to remember than the
>>>> current mix of 'start', 'shutdown', 'reboot'.
>>>>
>>>> * tools/virsh-domain.c (domManagementCmds): Add other command names.
>>>> * tools/virsh.pod (start, shutdown, reboot): Document the aliases.
>>>> ---
>>>>
>>>> This patch documents both spellings.  An alternative would be to
>>>> leave the alternate spellings as hidden aliases (virsh has support
>>>> for that), but still mention them in virsh.pod (see how we did an
>>>> alias for nodedev-dettach, for reference).
>>>>         
>>> I agree with Dave, we should have these visible.
>>>
>>>       
>>>>   tools/virsh-domain.c | 3 +++
>>>>   tools/virsh.pod      | 4 ++++
>>>>   2 files changed, 7 insertions(+)
>>>>
>>>> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>>>> index 393b67b..86ed4d3 100644
>>>> --- a/tools/virsh-domain.c
>>>> +++ b/tools/virsh-domain.c
>>>> @@ -8298,6 +8298,7 @@ const vshCmdDef domManagementCmds[] = {
>>>>       {"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0},
>>>>       {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0},
>>>>       {"blockresize", cmdBlockResize, opts_block_resize,
>>>> info_block_resize, 0},
>>>> +    {"boot", cmdStart, opts_start, info_start, 0},
>>>>         
>>> Hm, the boot command is a little bit awkward. But let's have it for the
>>> sake of consistence.
>>>
>>>       
>>>>       {"change-media", cmdChangeMedia, opts_change_media,
>>>> info_change_media, 0},
>>>>   #ifndef WIN32
>>>>       {"console", cmdConsole, opts_console, info_console, 0},
>>>> @@ -8351,6 +8352,7 @@ const vshCmdDef domManagementCmds[] = {
>>>>       {"numatune", cmdNumatune, opts_numatune, info_numatune, 0},
>>>>       {"reboot", cmdReboot, opts_reboot, info_reboot, 0},
>>>>       {"reset", cmdReset, opts_reset, info_reset, 0},
>>>> +    {"restart", cmdReboot, opts_reboot, info_reboot, 0},
>>>>       {"restore", cmdRestore, opts_restore, info_restore, 0},
>>>>       {"resume", cmdResume, opts_resume, info_resume, 0},
>>>>       {"save", cmdSave, opts_save, info_save, 0},
>>>> @@ -8367,6 +8369,7 @@ const vshCmdDef domManagementCmds[] = {
>>>>       {"setvcpus", cmdSetvcpus, opts_setvcpus, info_setvcpus, 0},
>>>>       {"shutdown", cmdShutdown, opts_shutdown, info_shutdown, 0},
>>>>       {"start", cmdStart, opts_start, info_start, 0},
>>>> +    {"stop", cmdShutdown, opts_shutdown, info_shutdown, 0},
>>>>       {"suspend", cmdSuspend, opts_suspend, info_suspend, 0},
>>>>       {"ttyconsole", cmdTTYConsole, opts_ttyconsole, info_ttyconsole, 0},
>>>>       {"undefine", cmdUndefine, opts_undefine, info_undefine, 0},
>>>>         
>>> ACK to the code changes, but I'm not 100% convinced if this is
>>> necessary. OTOH these changes are really trivial and some of those
>>> command names are awkward in the current state so if nobody speaks
>>> against in a reasonable amount of time, let's push it then.
>>>       
>> Okay, I'll wait 48 hours or so, then push if no one speaks to the contrary.
>>
>> --
>> Eric Blake   eblake redhat com    +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>>
>>     
>
> Honestly the patch seems reasonable to me and after reading the
> bugzilla request it seems reasonable as well. Sometimes our job is to
> make things easier on users, I'd say this is one of those times.
>   

Agreed.

Coincidently, having played with OpenStack for a bit now, I recently
typed 'virsh boot' :).

> So +1 for it.
>   

+1

Regards,
Jim


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