[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 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

Attachment: signature.asc
Description: OpenPGP digital signature


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