Re: [libvirt] [PATCH] virsh: Fix debugging

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(a)redhat.com&gt; > --- 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:

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);