[libvirt] [PATCH] Fix URI connect precedence

[Daniel P. Berrange]

On Wed, Aug 21, 2013 at 11:15:39AM +0200, Martin Kletzander wrote:
> Commit abfff210 changed the order of vshParseArgv() and vshInit() in
> order to make fix debugging of parameter parsing.  However, vshInit()
> did a vshReconnect() even though ctl->name wasn't set according to the
> '-c' parameter yet.  In order to keep both issues fixed, I've split
> the vshInit() into vshInitDebug() and vshInit().
> 
> One simple memleak of ctl->name is fixed as a part of this patch,
> since it is related to the issue it's fixing.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=999323
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  tools/virsh.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)

This looks like something we can usefully test for.

eg create a test suite that does something like this

 URI1=test:///$(top_srcdir)/examples/test/testnode.xml
 URI2=test:///$(top_srcdir)/examples/test/testnodeinline.xml

 export LIBVIRT_DEFAULT_URI=$URI1
 GOTURI=`virsh -c $URI2 uri`

 if $GOTURI == $URI2
  ....pass...
 else
   ...fail...
 fi

> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 15f529e..2ea44a6 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2294,16 +2294,13 @@ vshEventLoop(void *opaque)
> 
> 
>  /*
> - * Initialize connection.
> + * Initialize debug settings.
>   */
> -static bool
> -vshInit(vshControl *ctl)
> +static void
> +vshInitDebug(vshControl *ctl)
>  {
>      char *debugEnv;
> 
> -    if (ctl->conn)
> -        return false;
> -
>      if (ctl->debug == VSH_DEBUG_DEFAULT) {
>          /* log level not set from commandline, check env variable */
>          debugEnv = getenv("VIRSH_DEBUG");
> @@ -2328,6 +2325,16 @@ vshInit(vshControl *ctl)
>      }
> 
>      vshOpenLogFile(ctl);
> +}
> +
> +/*
> + * Initialize connection.
> + */
> +static bool
> +vshInit(vshControl *ctl)
> +{
> +    if (ctl->conn)
> +        return false;
> 
>      /* set up the library error handler */
>      virSetErrorFunc(NULL, virshErrorHandler);
> @@ -3017,6 +3024,7 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
>              ctl->timing = true;
>              break;
>          case 'c':
> +            VIR_FREE(ctl->name);
>              ctl->name = vshStrdup(ctl, optarg);
>              break;
>          case 'v':
> @@ -3192,12 +3200,10 @@ main(int argc, char **argv)
>          ctl->name = vshStrdup(ctl, defaultConn);
>      }
> 
> -    if (!vshInit(ctl)) {
> -        vshDeinit(ctl);
> -        exit(EXIT_FAILURE);
> -    }
> +    vshInitDebug(ctl);
> 
> -    if (!vshParseArgv(ctl, argc, argv)) {
> +    if (!vshParseArgv(ctl, argc, argv) ||
> +        !vshInit(ctl)) {
>          vshDeinit(ctl);
>          exit(EXIT_FAILURE);
>      }

ACK

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