[libvirt] [PATCH] virsh: let 'connect' without args remember -c option
Daniel P. Berrange
berrange at redhat.com
Mon Jun 11 13:01:20 UTC 2012
On Mon, Jun 11, 2012 at 06:35:10AM -0600, Eric Blake wrote:
> On 06/10/2012 10:35 PM, Osier Yang wrote:
> > On 2012年06月09日 07:42, Eric Blake wrote:
> >> If a user invokes 'virsh -c $URI', then within that batch shell,
> >> they probably want 'connect' to revert to $URI rather than the
> >> normal default URI you get for passing in NULL.
>
> >> if ((defaultConn = getenv("VIRSH_DEFAULT_CONNECT_URI"))) {
> >> - ctl->name = vshStrdup(ctl, defaultConn);
> >> + ctl->default_name = vshStrdup(ctl, defaultConn);
> >> }
> >>
> >> if (!vshParseArgv(ctl, argc, argv)) {
> >
> > Isn't the following patch enough for the fix? The problem is just
> > caused by ctl->name is freed even if no --name is specified for
> > command 'connect'. Or I missed something obviously?
>
> Your proposal also has merit, if for no other reason than that we don't
> lose the current connection on failure to connect elsewhere, but I still
> think we want a combination of both approaches. Consider:
>
> virsh -c test:///default 'list; connect qemu:///session; list; connect'
>
> Under my proposal, the second 'conect' would return us to
> test:///default, since we used -c to establish that as the default
> connection when 'connect' is used without args. Under your patch in
> isolation, the second 'connect' would leave us stuck in qemu:///session,
> and there is no way to return to our initial default connection except
> to type it out in full.
Personally I think we should return a fatal error if the user
attempts to use 'connect' in non-interactive mode, and not try
to hack it to behave the same as --connect/-c.
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 :|
More information about the libvir-list
mailing list