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


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