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

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.