[libvirt] [PATCH] virsh: add aliases 'boot', 'stop', and 'restart'
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). 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}, {"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}, diff --git a/tools/virsh.pod b/tools/virsh.pod index e0c6b42..7a3835b 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1124,6 +1124,7 @@ If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. =item B<reboot> I<domain> [I<--mode acpi|agent>] +=item B<restart> I<domain> [I<--mode acpi|agent>] Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has @@ -1523,6 +1524,7 @@ be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag. =item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<stop> I<domain> [I<--mode acpi|agent>] Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will @@ -1543,6 +1545,8 @@ can specify C<acpi> or C<agent>. =item B<start> I<domain-name-or-uuid> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] +=item B<boot> I<domain-name-or-uuid> [I<--console>] [I<--paused>] +[I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] Start a (previously defined) inactive domain, either from the last B<managedsave> state, or via a fresh boot if no managedsave state is -- 1.7.11.7
On Mon, Nov 05, 2012 at 12:59:16PM -0700, 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 don't see any reason to hide them; as long as we're being backward compatible, I think we're doing the right thing, and we should let people know that these options exist. Dave
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}, {"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}, diff --git a/tools/virsh.pod b/tools/virsh.pod index e0c6b42..7a3835b 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1124,6 +1124,7 @@ If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state.
=item B<reboot> I<domain> [I<--mode acpi|agent>] +=item B<restart> I<domain> [I<--mode acpi|agent>]
Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has @@ -1523,6 +1524,7 @@ be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag.
=item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<stop> I<domain> [I<--mode acpi|agent>]
Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will @@ -1543,6 +1545,8 @@ can specify C<acpi> or C<agent>.
=item B<start> I<domain-name-or-uuid> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] +=item B<boot> I<domain-name-or-uuid> [I<--console>] [I<--paused>] +[I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>]
Start a (previously defined) inactive domain, either from the last B<managedsave> state, or via a fresh boot if no managedsave state is -- 1.7.11.7
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. Peter
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
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
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
On 11/05/2012 08:59 PM, Eric Blake wrote:
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).
In my opinion start, stop, restart is better than the mix before and semantically more accurate than boot, shutdown, reboot. The latter uses terms that apply to the operating system inside the domain rather than the domain (virtual machine) itself. So I would even go so far to mark the latter group as legacy and recommend the usage of the former.
=item B<reboot> I<domain> [I<--mode acpi|agent>] +=item B<restart> I<domain> [I<--mode acpi|agent>]
Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has
Now it would be a good time to correct the documentation. In reality the restart/reboot is a combination of stop/shutdown followed by an in-place reset of the domain (at least for QEMU/KVM). Which can behave the same as a reboot but doesn't have to. This is one area of confusion for users I have observed. Restart seems to raise a lower expectation level ;-). For that reason I would also vote to exchange the order of appearance (restart first).
@@ -1523,6 +1524,7 @@ be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag.
=item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<stop> I<domain> [I<--mode acpi|agent>]
Same suggestion: first line stop, second shutdown. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 11/06/12 09:30, Viktor Mihajlovski wrote:
On 11/05/2012 08:59 PM, Eric Blake wrote:
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).
In my opinion start, stop, restart is better than the mix before and semantically more accurate than boot, shutdown, reboot. The latter uses terms that apply to the operating system inside the domain rather than the domain (virtual machine) itself. So I would even go so far to mark the latter group as legacy and recommend the usage of the former.
=item B<reboot> I<domain> [I<--mode acpi|agent>] +=item B<restart> I<domain> [I<--mode acpi|agent>]
Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has
Now it would be a good time to correct the documentation. In reality the restart/reboot is a combination of stop/shutdown followed by an in-place reset of the domain (at least for QEMU/KVM). Which can behave the same as a reboot but doesn't have to. This is one area of confusion for users I have observed. Restart seems to raise a lower expectation level ;-). For that reason I would also vote to exchange the order of appearance (restart first).
I agree on this point.
@@ -1523,6 +1524,7 @@ be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag.
=item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<stop> I<domain> [I<--mode acpi|agent>]
Same suggestion: first line stop, second shutdown.
But here I think stop is less appropriate. For me stop sounds semantically more like what destroy does.
Giving the number of voices wanting this, I don't object any more. Peter
On 11/06/2012 02:01 AM, Peter Krempa wrote:
=item B<reboot> I<domain> [I<--mode acpi|agent>] +=item B<restart> I<domain> [I<--mode acpi|agent>]
Reboot a domain. This acts just as if the domain had the B<reboot> command run from the console. The command returns as soon as it has Now it would be a good time to correct the documentation. In reality the restart/reboot is a combination of stop/shutdown followed by an in-place reset of the domain (at least for QEMU/KVM). Which can behave the same as a reboot but doesn't have to. This is one area of confusion for users I have observed. Restart seems to raise a lower expectation level ;-). For that reason I would also vote to exchange the order of appearance (restart first).
I agree on this point.
I'm not going to switch the order of the synopsis (the older command should be listed first, since that is the more portable version if you are coding for older virsh); but I do agree that the help text needs improvement (it is NOT the same as 'reboot' from within the guest). This is what I squashed in before pushing: diff --git i/tools/virsh.pod w/tools/virsh.pod index 7a3835b..7a46ffd 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -1126,10 +1126,12 @@ If I<--current> is specified, affect the current guest state. =item B<reboot> I<domain> [I<--mode acpi|agent>] =item B<restart> I<domain> [I<--mode acpi|agent>] -Reboot a domain. This acts just as if the domain had the B<reboot> -command run from the console. The command returns as soon as it has -executed the reboot action, which may be significantly before the -domain actually reboots. +Restart a domain. Depending on the hypervisor, this may request a +shutdown followed by a fresh boot, rather than triggering a software +reboot. The command returns as soon as it has requested the reboot +action, and depending on the guest, there may be a significant delay +before the domain actually reboots, or the request might even be +ignored. The exact behavior of a domain when it reboots is set by the I<on_reboot> parameter in the domain's XML definition. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/06/2012 03:30 AM, Viktor Mihajlovski wrote:
On 11/05/2012 08:59 PM, Eric Blake wrote:
@@ -1523,6 +1524,7 @@ be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag.
=item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<stop> I<domain> [I<--mode acpi|agent>]
Same suggestion: first line stop, second shutdown.
And one final thing I didn't notice if anybody had mentioned yet (and don't have time to look for now, since I'm off to wait in line to vote): it's *very* important that every mention of the "old" verbs note that it is a deprecated synonym for the new verb (or at least that it's a synonym). We don't want people thinking that there are two different commands because they are slightly different in behavior, then getting confused about which to use.
On 11/06/2012 07:31 AM, Laine Stump wrote:
On 11/06/2012 03:30 AM, Viktor Mihajlovski wrote:
On 11/05/2012 08:59 PM, Eric Blake wrote:
@@ -1523,6 +1524,7 @@ be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag.
=item B<shutdown> I<domain> [I<--mode acpi|agent>] +=item B<stop> I<domain> [I<--mode acpi|agent>]
Same suggestion: first line stop, second shutdown.
And one final thing I didn't notice if anybody had mentioned yet (and don't have time to look for now, since I'm off to wait in line to vote): it's *very* important that every mention of the "old" verbs note that it is a deprecated synonym for the new verb (or at least that it's a synonym). We don't want people thinking that there are two different commands because they are slightly different in behavior, then getting confused about which to use.
Oops, just pushed before seeing this mail; I'll add this as a followup patch: diff --git i/tools/virsh.pod w/tools/virsh.pod index 7a46ffd..d507455 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -1140,6 +1140,8 @@ By default the hypervisor will try to pick a suitable shutdown method. To specify an alternative method, the I<--mode> parameter can specify C<acpi> or C<agent>. +The command B<restart> is an alias for the older B<reboot>. + =item B<reset> I<domain> Reset a domain immediately without any guest shutdown. B<reset> @@ -1545,6 +1547,8 @@ By default the hypervisor will try to pick a suitable shutdown method. To specify an alternative method, the I<--mode> parameter can specify C<acpi> or C<agent>. +The command B<stop> is an alias for the older B<shutdown>. + =item B<start> I<domain-name-or-uuid> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] [I<--force-boot>] =item B<boot> I<domain-name-or-uuid> [I<--console>] [I<--paused>] @@ -1562,6 +1566,8 @@ the restore will avoid the file system cache, although this may slow down the operation. If I<--force-boot> is specified, then any managedsave state is discarded and a fresh boot occurs. +The command B<boot> is an alias for the older B<start>. + =item B<suspend> I<domain> Suspend a running domain. It is kept in memory but won't be scheduled -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Nov 06, 2012 at 09:30:15AM +0100, Viktor Mihajlovski wrote:
On 11/05/2012 08:59 PM, Eric Blake wrote:
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).
In my opinion start, stop, restart is better than the mix before and semantically more accurate than boot, shutdown, reboot. The latter uses terms that apply to the operating system inside the domain rather than the domain (virtual machine) itself.
The 'shutdown' and 'reboot' commands use terms that apply to the operating system, because they *are* controlling the operating system, not the virtual machine. They directly result in the guest OS commands 'shutdown' and 'reboot' being invoked. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 11/08/2012 03:25 PM, Daniel P. Berrange wrote:
The 'shutdown' and 'reboot' commands use terms that apply to the operating system, because they *are* controlling the operating system, not the virtual machine. They directly result in the guest OS commands 'shutdown' and 'reboot' being invoked.
At least for QEMU this is only true for agent mode. Otherwise (default) both virsh shutdown and virsh reboot are the same from the OS perspective, namely a system powerdown request and will typically result in the shutdown script to be run. That's what can confuse users without insight to libvirt code. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On Mon, Nov 05, 2012 at 12:59:16PM -0700, 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).
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt. I actually think that shutdown & reboot are *better* names than restart and stop. If we wanted to replace any existing names, then the 'create' and 'destroy' names are the ones to replace, and for those I would expect to use 'boot' and 'stop'. I still don't thin we should do that either, due to creating inconsistency with other commands. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, November 8, 2012 09:24, Daniel P. Berrange wrote:
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
Duplicates (aliases) will make life worse for whom? In what way? On what evidence? The present nomenclature is idiomatic to virsh and is a variance with how many people think of managing a host whether virtual or not. On the other hand, one does not customarily speak of rebooting a network or a storage array, at least not in my experience.
I actually think that shutdown & reboot are *better* names than restart and stop.
Then change start to boot and be done with it. But, the issue really is what English words are commonly associated with each other in the context we are dealing with. I submit that 'start' is not intuitively associated with 'shutdown' by the vast majority of English speakers and hardly associated with 'reboot' by any. Consider the syntax of 'initctl' and 'service'. Initctl uses start, stop and restart. Service scripts virtually without exception use start, stop and restart, including that for libvirtd. Operators are far more likely to be familiar with this combination of terms than any other. Why force them to learn yet one more variant? What is the advantage for the users? . -- *** E-Mail is NOT a SECURE channel *** James B. Byrne mailto:ByrneJB@Harte-Lyne.ca Harte & Lyne Limited http://www.harte-lyne.ca 9 Brockley Drive vox: +1 905 561 1241 Hamilton, Ontario fax: +1 905 561 0757 Canada L8E 3C3
On Thu, Nov 08, 2012 at 10:30:55AM -0500, James B. Byrne wrote:
On Thu, November 8, 2012 09:24, Daniel P. Berrange wrote:
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
Duplicates (aliases) will make life worse for whom? In what way? On what evidence? The present nomenclature is idiomatic to virsh and is a variance with how many people think of managing a host whether virtual or not. On the other hand, one does not customarily speak of rebooting a network or a storage array, at least not in my experience.
I actually think that shutdown & reboot are *better* names than restart and stop.
Then change start to boot and be done with it. But, the issue really is what English words are commonly associated with each other in the context we are dealing with. I submit that 'start' is not intuitively associated with 'shutdown' by the vast majority of English speakers and hardly associated with 'reboot' by any.
Consider the syntax of 'initctl' and 'service'. Initctl uses start, stop and restart. Service scripts virtually without exception use start, stop and restart, including that for libvirtd. Operators are far more likely to be familiar with this combination of terms than any other. Why force them to learn yet one more variant? What is the advantage for the users?
If you are comparing to init services, then the 'stop' verb is semanticaly equivalent to libvirt's 'destory' verb, not the shutdown verb. 'stop' does a synchronous termination of the service. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
On Mon, Nov 05, 2012 at 12:59:16PM -0700, 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).
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
I actually think that shutdown & reboot are *better* names than restart and stop.
If we wanted to replace any existing names, then the 'create' and 'destroy' names are the ones to replace, and for those I would expect to use 'boot' and 'stop'. I still don't thin we should do that either, due to creating inconsistency with other commands.
IMO this is fundamentally an aesthetic question here that's not going to be solved by debate, however, from the comments so far, the patch seems to have a fair amount of support. DV, perhaps you could weigh in with your opinion? Dave
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Nov 08, 2012 at 10:51:14AM -0500, Dave Allan wrote:
On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
On Mon, Nov 05, 2012 at 12:59:16PM -0700, 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).
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
I actually think that shutdown & reboot are *better* names than restart and stop.
If we wanted to replace any existing names, then the 'create' and 'destroy' names are the ones to replace, and for those I would expect to use 'boot' and 'stop'. I still don't thin we should do that either, due to creating inconsistency with other commands.
IMO this is fundamentally an aesthetic question here that's not going to be solved by debate, however, from the comments so far, the patch seems to have a fair amount of support. DV, perhaps you could weigh in with your opinion?
Since this is an aesthetic question, even adding this patch is not going to solve it. It is a fact of life that there are a huge variety of names that could be used - adding more & more aliases is not a solution. You can never satisfy everyone's desired name choice. You just have to pick one name and stick with it consistently. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 11/08/2012 11:24 AM, Daniel P. Berrange wrote:
On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange 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). NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates
On Mon, Nov 05, 2012 at 12:59:16PM -0700, Eric Blake wrote: divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
I actually think that shutdown & reboot are *better* names than restart and stop.
If we wanted to replace any existing names, then the 'create' and 'destroy' names are the ones to replace, and for those I would expect to use 'boot' and 'stop'. I still don't thin we should do that either, due to creating inconsistency with other commands. IMO this is fundamentally an aesthetic question here that's not going to be solved by debate, however, from the comments so far, the patch seems to have a fair amount of support. DV, perhaps you could weigh in with your opinion? Since this is an aesthetic question, even adding this patch is not going to solve it. It is a fact of life that there are a huge variety of names
On Thu, Nov 08, 2012 at 10:51:14AM -0500, Dave Allan wrote: that could be used - adding more & more aliases is not a solution. You can never satisfy everyone's desired name choice. You just have to pick one name and stick with it consistently.
I'm putting my vote with Dan on this (Just to be clear, my original message in this thread was written in the spirit of "well, if you're going to do this, then at least make sure what you're doing is explicitly documented"). It's often a painful exercise to try and come up with the "best" name for any particular operation, but once you've got a name, you've got a name. After that, the truths are: 1) Multiple names for the same thing leads to confusion (which is what prompted my earlier comment saying that the new names needed to be clearly marked as synonyms). 2) There is no end to the new names that could be thought of as "better" when taken in some particular context, or by some particular person. 3) If some action is named "foo-x" for foo objects, then a similar action on fi objects should be named "fi-x" if at all possible. Maybe "x" isn't the most appropriate name in the context of foo or fi, but following a single model makes it easier to remember. I know virsh isn't considered to be as strict of an environment as the libvirt API proper, but we still need to be mindful of both backward compatibility and of namespace pollution leading to confusion. (If the current names bother you, you could just think of it from the point of view of someone who has absolutely no knowledge of English, and sees the command names as opaque cookies that produce the desired result.)
On Thu, Nov 08, 2012 at 12:39:45 -0500, Laine Stump wrote:
On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
I'm putting my vote with Dan on this (Just to be clear, my original message in this thread was written in the spirit of "well, if you're going to do this, then at least make sure what you're doing is explicitly documented").
Well, I'm with Dan and Laine on this. I think adding the alias for mistyped commands was good since it made life easier for those who know what command they want to use keep forgetting to mistype it in the same way. However, I don't like the idea of adding synonyms for commands just because it seems easier for someone to remember the synonyms. It may lead to confusion when people are talking about the same thing but are used to different synonyms. It also destroys consistency which we have now. And we are not going to translate various commands into different languages either, are we? However, if we wanted to still be nice to end users, we could implement user-defined aliases for virsh. It would give users more power since commonly used options could be aliased as well, e.g., someone always doing p2p live migrations could setup an alias "lm" to be "migrate --p2p --live". And anyone could make aliases to fit their needs. And I'm suggesting just a simple text substitution that would just replace a recognized aliase at the beginning of a command with the definition of the alias and just parse the result as if that was what the user typed. Jirka
On 11/16/2012 03:51 AM, Jiri Denemark wrote:
On Thu, Nov 08, 2012 at 12:39:45 -0500, Laine Stump wrote:
On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
I'm putting my vote with Dan on this (Just to be clear, my original message in this thread was written in the spirit of "well, if you're going to do this, then at least make sure what you're doing is explicitly documented").
Well, I'm with Dan and Laine on this. I think adding the alias for mistyped commands was good since it made life easier for those who know what command they want to use keep forgetting to mistype it in the same way. However, I don't like the idea of adding synonyms for commands just because it seems easier for someone to remember the synonyms. It may lead to confusion when people are talking about the same thing but are used to different synonyms. It also destroys consistency which we have now. And we are not going to translate various commands into different languages either, are we?
However, if we wanted to still be nice to end users, we could implement user-defined aliases for virsh. It would give users more power since commonly used options could be aliased as well, e.g., someone always doing p2p live migrations could setup an alias "lm" to be "migrate --p2p --live". And anyone could make aliases to fit their needs. And I'm suggesting just a simple text substitution that would just replace a recognized aliase at the beginning of a command with the definition of the alias and just parse the result as if that was what the user typed.
Indeed, allowing user-defined aliases would be nicer. I'll go ahead and revert my patch, and think about what it would take to let the user customize things instead. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Nov 16, 2012 at 9:21 AM, Eric Blake <eblake@redhat.com> wrote:
On 11/16/2012 03:51 AM, Jiri Denemark wrote:
On Thu, Nov 08, 2012 at 12:39:45 -0500, Laine Stump wrote:
On Thu, Nov 08, 2012 at 03:24:07PM +0100, Daniel P. Berrange wrote:
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
I'm putting my vote with Dan on this (Just to be clear, my original message in this thread was written in the spirit of "well, if you're going to do this, then at least make sure what you're doing is explicitly documented").
Well, I'm with Dan and Laine on this. I think adding the alias for mistyped commands was good since it made life easier for those who know what command they want to use keep forgetting to mistype it in the same way. However, I don't like the idea of adding synonyms for commands just because it seems easier for someone to remember the synonyms. It may lead to confusion when people are talking about the same thing but are used to different synonyms. It also destroys consistency which we have now. And we are not going to translate various commands into different languages either, are we?
However, if we wanted to still be nice to end users, we could implement user-defined aliases for virsh. It would give users more power since commonly used options could be aliased as well, e.g., someone always doing p2p live migrations could setup an alias "lm" to be "migrate --p2p --live". And anyone could make aliases to fit their needs. And I'm suggesting just a simple text substitution that would just replace a recognized aliase at the beginning of a command with the definition of the alias and just parse the result as if that was what the user typed.
Jiri, That's actually an outstanding idea and I think would solve a lot of this in the future.
Indeed, allowing user-defined aliases would be nicer. I'll go ahead and revert my patch, and think about what it would take to let the user customize things instead.
Eric, For your design it might be worth while to have a global place for aliases like /etc/libvirt while also allowing one in $HOME as well. And both would be applied for system and session libvirt's. With the one in $HOME being able to override or at least taking precedence to the one in /etc/libvirt. Just my 2 cents. -- Doug Goldstein
On 11/08/2012 07:24 AM, Daniel P. Berrange wrote:
On Mon, Nov 05, 2012 at 12:59:16PM -0700, 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).
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
The patch is already in, but we also took care to explicitly document that the new aliases are just that, and that portable scripts should use the old name. I don't see how this is any different to having both 'quit' and 'exit' as aliases, nor even from 'detach-device' being documented as having an older misspelled 'dettach-device' counterpart. In other words, I don't see that adding aliases is a problem. On the other hand, I do agree that we cannot remove any alias, once released, so we need to resolve this situation prior to the release of 1.0.1.
I actually think that shutdown & reboot are *better* names than restart and stop.
If we wanted to replace any existing names, then the 'create' and 'destroy' names are the ones to replace, and for those I would expect to use 'boot' and 'stop'. I still don't thin we should do that either, due to creating inconsistency with other commands.
Although I have argued that aliases are helpful, I can understand your counter-argument that too many aliases, or aliases that are unrelated to the task at hand, are not wise. If you would like to propose a counter-patch that removes the just-added 'restart' alias, and repurposes 'stop' to be an alias to 'destroy' rather than 'shutdown', then that would leave us with: 'start' has alias 'boot' 'reboot' has no alias 'shutdown' has no alias 'destroy' has alias 'stop' and I would feel comfortable ACK'ing such a patch before 1.0.1. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Nov 08, 2012 at 08:55:27AM -0700, Eric Blake wrote:
On 11/08/2012 07:24 AM, Daniel P. Berrange wrote:
On Mon, Nov 05, 2012 at 12:59:16PM -0700, 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).
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects. It also means scripts written again the new commands will not work with existing libvirt.
The patch is already in, but we also took care to explicitly document that the new aliases are just that, and that portable scripts should use the old name.
I really want to see that patch reverted from GIT.
I don't see how this is any different to having both 'quit' and 'exit' as aliases, nor even from 'detach-device' being documented as having an older misspelled 'dettach-device' counterpart. In other words, I don't see that adding aliases is a problem. On the other hand, I do agree that we cannot remove any alias, once released, so we need to resolve this situation prior to the release of 1.0.1.
FWIW, I was not in favour of aliases in the first place, but I defer to others in the case of previous aliases. In this case though, I am really against the new proposed names since I think 'stop' and 'restart' and a bad choice of names as compared to our existing 'shutdown' and 'reboot'.
I actually think that shutdown & reboot are *better* names than restart and stop.
If we wanted to replace any existing names, then the 'create' and 'destroy' names are the ones to replace, and for those I would expect to use 'boot' and 'stop'. I still don't thin we should do that either, due to creating inconsistency with other commands.
Although I have argued that aliases are helpful, I can understand your counter-argument that too many aliases, or aliases that are unrelated to the task at hand, are not wise. If you would like to propose a counter-patch that removes the just-added 'restart' alias, and repurposes 'stop' to be an alias to 'destroy' rather than 'shutdown', then that would leave us with:
'start' has alias 'boot' 'reboot' has no alias 'shutdown' has no alias 'destroy' has alias 'stop'
and I would feel comfortable ACK'ing such a patch before 1.0.1.
If we add an alias for 'destroy' we must do so for all instances of 'destroy', including for storage, network. I'm not really in favour of using 'boot' as an alias, since I don't think 'boot' is a good term in the context of non-machine based virtualization. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Daniel P. Berrange wrote:
On Mon, Nov 05, 2012 at 12:59:16PM -0700, 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).
NACK to this patch. I think the current command names are good. Creating duplicates will make life worse. First, it creates divergance from the similarly named commands for networks, storage and other objects.
Hmm, that is a very good observation, one I hadn't considered. Agreed that consistency with other commands is certainly important, where 'booting' a network e.g. is not very intuitive. Regards, Jim
It also means scripts written again the new commands will not work with existing libvirt.
I actually think that shutdown & reboot are *better* names than restart and stop.
If we wanted to replace any existing names, then the 'create' and 'destroy' names are the ones to replace, and for those I would expect to use 'boot' and 'stop'. I still don't thin we should do that either, due to creating inconsistency with other commands.
Daniel
participants (10)
-
Daniel P. Berrange -
Dave Allan -
Doug Goldstein -
Eric Blake -
James B. Byrne -
Jim Fehlig -
Jiri Denemark -
Laine Stump -
Peter Krempa -
Viktor Mihajlovski