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

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



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.

So +1 for it.

-- 
Doug Goldstein


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