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

Re: [libvirt] [PATCH] virsh: let 'connect' without args remember -c option



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


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