[libvirt] [PATCH] virsh: Fix debugging

[Eric Blake]

On 08/27/2013 06:27 AM, Martin Kletzander wrote:
> On 08/27/2013 01:19 PM, Martin Kletzander wrote:
>> Commit a0b6a36f "fixed" what abfff210 broke (URI precedence), but
>> there was still one more thing missing to fix.  When using virsh
>> parameters to setup debugging, those weren't honored, because at the
>> time debugging was initializing, arguments weren't parsed yet.  To
>> make ewerything work as expected, we need to initialize the debugging
>> twice, once before debugging (so we can debug option parsing properly)
>> and then again after these options are parsed.
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
> 
> As Ján pointed out in person, this fix (apart from the leak it
> introduces) does use all the settings configured, i.e.:
>  1) it parses VIRSH_DEBUG and VIRSH_LOG_FILE
>  2) it parses command line arguments the output behaves according to (1)
>  3) and after that, it uses debug settings from command line.
> 
> But this was intended from my side as it is pretty reasonable IMHO.
> Anyway, feel free to express your opinions on how do we fix this (what's
> the desired behavior) and I'll cook up an appropriate patch.  If nobody
> replies (cares), I'll send v2 or in case of ACK mentioned here, I can
> push it with this squashed in:

I can live with the patch as amended by this squash.

ACK.

> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index ac77156..34f5c4a 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2321,10 +2321,9 @@ vshInitDebug(vshControl *ctl)
>          debugEnv = getenv("VIRSH_LOG_FILE");
>          if (debugEnv && *debugEnv) {
>              ctl->logfile = vshStrdup(ctl, debugEnv);
> +            vshOpenLogFile(ctl);
>          }
>      }
> -
> -    vshOpenLogFile(ctl);
>  }
> 
>  /*
> @@ -3044,7 +3043,9 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
>              ctl->readonly = true;
>              break;
>          case 'l':
> +            vshCloseLogFile(ctl);
>              ctl->logfile = vshStrdup(ctl, optarg);
> +            vshOpenLogFile(ctl);

Note that there's another leak here (pre-existing): If I call:
 virsh -l file1 -l file2, we leak "file1" by reassigning a malloc'd
string without first freeing the old version.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130827/971c1f03/attachment-0001.sig>