[PATCH] tracing: only allow -trace to override -D if set

[PATCH] tracing: only allow -trace to override -D if set

[Alex Bennée]

Otherwise any -D settings the user may have made get ignored.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 trace/control.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/trace/control.c b/trace/control.c
index 6c775e68eba..2ffe0008184 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -226,10 +226,15 @@ void trace_init_file(const char *file)
 #ifdef CONFIG_TRACE_SIMPLE
     st_set_trace_file(file);
 #elif defined CONFIG_TRACE_LOG
-    /* If both the simple and the log backends are enabled, "--trace file"
-     * only applies to the simple backend; use "-D" for the log backend.
+    /*
+     * If both the simple and the log backends are enabled, "--trace file"
+     * only applies to the simple backend; use "-D" for the log
+     * backend. However we should only override -D if we actually have
+     * something to override it with.
      */
-    qemu_set_log_filename(file, &error_fatal);
+    if (file) {
+        qemu_set_log_filename(file, &error_fatal);
+    }
 #else
     if (file) {
         fprintf(stderr, "error: --trace file=...: "
-- 
2.20.1

Re: [PATCH] tracing: only allow -trace to override -D if set

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

On Tue, Feb 11, 2020 at 11:10:54AM +0000, Alex Bennée wrote:
> Otherwise any -D settings the user may have made get ignored.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  trace/control.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] tracing: only allow -trace to override -D if set

[Philippe Mathieu-Daudé]

On 2/12/20 4:34 PM, Stefan Hajnoczi wrote:
> On Tue, Feb 11, 2020 at 11:10:54AM +0000, Alex Bennée wrote:
>> Otherwise any -D settings the user may have made get ignored.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   trace/control.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> Thanks, applied to my tracing tree:
> https://github.com/stefanha/qemu/commits/tracing

If possible, please add 'Fixes: e144a605a'.

Re: [PATCH] tracing: only allow -trace to override -D if set

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Wed, Feb 12, 2020 at 11:31:00PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/12/20 4:34 PM, Stefan Hajnoczi wrote:
> > On Tue, Feb 11, 2020 at 11:10:54AM +0000, Alex Bennée wrote:
> > > Otherwise any -D settings the user may have made get ignored.
> > > 
> > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > > ---
> > >   trace/control.c | 11 ++++++++---
> > >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > Thanks, applied to my tracing tree:
> > https://github.com/stefanha/qemu/commits/tracing
> 
> If possible, please add 'Fixes: e144a605a'.

Done!

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] tracing: only allow -trace to override -D if set

[Laurent Vivier]

On 11/02/2020 12:10, Alex Bennée wrote:
> Otherwise any -D settings the user may have made get ignored.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  trace/control.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/trace/control.c b/trace/control.c
> index 6c775e68eba..2ffe0008184 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -226,10 +226,15 @@ void trace_init_file(const char *file)
>  #ifdef CONFIG_TRACE_SIMPLE
>      st_set_trace_file(file);
>  #elif defined CONFIG_TRACE_LOG
> -    /* If both the simple and the log backends are enabled, "--trace file"
> -     * only applies to the simple backend; use "-D" for the log backend.
> +    /*
> +     * If both the simple and the log backends are enabled, "--trace file"
> +     * only applies to the simple backend; use "-D" for the log
> +     * backend. However we should only override -D if we actually have
> +     * something to override it with.
>       */
> -    qemu_set_log_filename(file, &error_fatal);
> +    if (file) {
> +        qemu_set_log_filename(file, &error_fatal);
> +    }
>  #else
>      if (file) {
>          fprintf(stderr, "error: --trace file=...: "
> 

Tested-by: Laurent Vivier <laurent@vivier.eu>