Re: [PATCH] virsh: Fix debugging

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

 



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@xxxxxxxxxx>
> ---

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);
             break;
         case 'e':
             len = strlen(optarg);
---

Martin

>  tools/virsh.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 2ea44a6..ac77156 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2333,6 +2333,10 @@ vshInitDebug(vshControl *ctl)
>  static bool
>  vshInit(vshControl *ctl)
>  {
> +    /* Since we have the commandline arguments parsed, we need to
> +     * re-initialize all the debugging to make it work properly */
> +    vshInitDebug(ctl);
> +
>      if (ctl->conn)
>          return false;
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]