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

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(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org