[PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[duboisj]

From: Josh DuBois <josh@joshdubois.com>

Tracing can be enabled at the command line or via the
monitor. Command-line trace options are recorded during
trace_opt_parse(), but tracing is not enabled until the various
front-ends later call trace_init_file(). If the user passes a trace
option on the command-line, remember that and enable tracing during
trace_init_file().  Otherwise, trace_init_file() should record the
trace file specified by the frontend and avoid enabling traces
until the user requests them via the monitor.

This fixes 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 and also
db25d56c014aa1a96319c663e0a60346a223b31e, by allowing the user
to enable traces on the command line and also avoiding
unwanted trace-<pid> files when the user has not asked for them.

Fixes: 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4
Signed-off-by: Josh DuBois <josh@joshdubois.com>
---
 trace/control.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/trace/control.c b/trace/control.c
index 6558b5c906..8f94f09444 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -39,6 +39,7 @@ static TraceEventGroup *event_groups;
 static size_t nevent_groups;
 static uint32_t next_id;
 static uint32_t next_vcpu_id;
+static bool init_trace_on_startup;
 
 QemuOptsList qemu_trace_opts = {
     .name = "trace",
@@ -225,7 +226,9 @@ void trace_init_file(const char *file)
 {
 #ifdef CONFIG_TRACE_SIMPLE
     st_set_trace_file(file);
-    st_set_trace_file_enabled(true);
+    if (init_trace_on_startup) {
+        st_set_trace_file_enabled(true);
+    }
 #elif defined CONFIG_TRACE_LOG
     /*
      * If both the simple and the log backends are enabled, "--trace file"
@@ -299,6 +302,7 @@ char *trace_opt_parse(const char *optarg)
     }
     trace_init_events(qemu_opt_get(opts, "events"));
     trace_file = g_strdup(qemu_opt_get(opts, "file"));
+    init_trace_on_startup = true;
     qemu_opts_del(opts);
 
     return trace_file;
-- 
2.25.1

Re: [PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[Markus Armbruster]

duboisj@gmail.com writes:

> From: Josh DuBois <josh@joshdubois.com>
>
> Tracing can be enabled at the command line or via the
> monitor. Command-line trace options are recorded during
> trace_opt_parse(), but tracing is not enabled until the various
> front-ends later call trace_init_file(). If the user passes a trace
> option on the command-line, remember that and enable tracing during
> trace_init_file().  Otherwise, trace_init_file() should record the
> trace file specified by the frontend and avoid enabling traces
> until the user requests them via the monitor.
>
> This fixes 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 and also
> db25d56c014aa1a96319c663e0a60346a223b31e, by allowing the user
> to enable traces on the command line and also avoiding
> unwanted trace-<pid> files when the user has not asked for them.
>
> Fixes: 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4
> Signed-off-by: Josh DuBois <josh@joshdubois.com>
> ---
>  trace/control.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/trace/control.c b/trace/control.c
> index 6558b5c906..8f94f09444 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -39,6 +39,7 @@ static TraceEventGroup *event_groups;
>  static size_t nevent_groups;
>  static uint32_t next_id;
>  static uint32_t next_vcpu_id;
> +static bool init_trace_on_startup;
>  
>  QemuOptsList qemu_trace_opts = {
>      .name = "trace",
> @@ -225,7 +226,9 @@ void trace_init_file(const char *file)
>  {
>  #ifdef CONFIG_TRACE_SIMPLE
>      st_set_trace_file(file);
> -    st_set_trace_file_enabled(true);
> +    if (init_trace_on_startup) {
> +        st_set_trace_file_enabled(true);
> +    }
>  #elif defined CONFIG_TRACE_LOG
>      /*
>       * If both the simple and the log backends are enabled, "--trace file"
> @@ -299,6 +302,7 @@ char *trace_opt_parse(const char *optarg)
>      }
>      trace_init_events(qemu_opt_get(opts, "events"));
>      trace_file = g_strdup(qemu_opt_get(opts, "file"));
> +    init_trace_on_startup = true;
>      qemu_opts_del(opts);
>  
>      return trace_file;

Stops the littering for me.

Stefan, please have a closer look.

Re: [PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> duboisj@gmail.com writes:
>
>> From: Josh DuBois <josh@joshdubois.com>
>>
>> Tracing can be enabled at the command line or via the
>> monitor. Command-line trace options are recorded during
>> trace_opt_parse(), but tracing is not enabled until the various
>> front-ends later call trace_init_file(). If the user passes a trace
>> option on the command-line, remember that and enable tracing during
>> trace_init_file().  Otherwise, trace_init_file() should record the
>> trace file specified by the frontend and avoid enabling traces
>> until the user requests them via the monitor.
>>
>> This fixes 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 and also
>> db25d56c014aa1a96319c663e0a60346a223b31e, by allowing the user
>> to enable traces on the command line and also avoiding
>> unwanted trace-<pid> files when the user has not asked for them.
>>
>> Fixes: 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4
>> Signed-off-by: Josh DuBois <josh@joshdubois.com>
>> ---
>>  trace/control.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/trace/control.c b/trace/control.c
>> index 6558b5c906..8f94f09444 100644
>> --- a/trace/control.c
>> +++ b/trace/control.c
>> @@ -39,6 +39,7 @@ static TraceEventGroup *event_groups;
>>  static size_t nevent_groups;
>>  static uint32_t next_id;
>>  static uint32_t next_vcpu_id;
>> +static bool init_trace_on_startup;
>>  
>>  QemuOptsList qemu_trace_opts = {
>>      .name = "trace",
>> @@ -225,7 +226,9 @@ void trace_init_file(const char *file)
>>  {
>>  #ifdef CONFIG_TRACE_SIMPLE
>>      st_set_trace_file(file);
>> -    st_set_trace_file_enabled(true);
>> +    if (init_trace_on_startup) {
>> +        st_set_trace_file_enabled(true);
>> +    }
>>  #elif defined CONFIG_TRACE_LOG
>>      /*
>>       * If both the simple and the log backends are enabled, "--trace file"
>> @@ -299,6 +302,7 @@ char *trace_opt_parse(const char *optarg)
>>      }
>>      trace_init_events(qemu_opt_get(opts, "events"));
>>      trace_file = g_strdup(qemu_opt_get(opts, "file"));
>> +    init_trace_on_startup = true;
>>      qemu_opts_del(opts);
>>  
>>      return trace_file;
>
> Stops the littering for me.
>
> Stefan, please have a closer look.

Ping?

Re: [PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[Stefan Hajnoczi]

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

On Sun, Aug 16, 2020 at 12:46:10PM -0500, duboisj@gmail.com wrote:
> From: Josh DuBois <josh@joshdubois.com>
> 
> Tracing can be enabled at the command line or via the
> monitor. Command-line trace options are recorded during
> trace_opt_parse(), but tracing is not enabled until the various
> front-ends later call trace_init_file(). If the user passes a trace
> option on the command-line, remember that and enable tracing during
> trace_init_file().  Otherwise, trace_init_file() should record the
> trace file specified by the frontend and avoid enabling traces
> until the user requests them via the monitor.
> 
> This fixes 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 and also
> db25d56c014aa1a96319c663e0a60346a223b31e, by allowing the user
> to enable traces on the command line and also avoiding
> unwanted trace-<pid> files when the user has not asked for them.
> 
> Fixes: 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4
> Signed-off-by: Josh DuBois <josh@joshdubois.com>
> ---
>  trace/control.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

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

Stefan

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

Re: [PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[Markus Armbruster]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Sun, Aug 16, 2020 at 12:46:10PM -0500, duboisj@gmail.com wrote:
>> From: Josh DuBois <josh@joshdubois.com>
>> 
>> Tracing can be enabled at the command line or via the
>> monitor. Command-line trace options are recorded during
>> trace_opt_parse(), but tracing is not enabled until the various
>> front-ends later call trace_init_file(). If the user passes a trace
>> option on the command-line, remember that and enable tracing during
>> trace_init_file().  Otherwise, trace_init_file() should record the
>> trace file specified by the frontend and avoid enabling traces
>> until the user requests them via the monitor.
>> 
>> This fixes 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 and also
>> db25d56c014aa1a96319c663e0a60346a223b31e, by allowing the user
>> to enable traces on the command line and also avoiding
>> unwanted trace-<pid> files when the user has not asked for them.
>> 
>> Fixes: 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4
>> Signed-off-by: Josh DuBois <josh@joshdubois.com>
>> ---
>>  trace/control.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> Thanks, applied to my tracing-next tree:
> https://github.com/stefanha/qemu/commits/tracing-next
>
> Stefan

Pull request?

Re: [PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Stefan Hajnoczi <stefanha@gmail.com> writes:
>
>> On Sun, Aug 16, 2020 at 12:46:10PM -0500, duboisj@gmail.com wrote:
>>> From: Josh DuBois <josh@joshdubois.com>
>>> 
>>> Tracing can be enabled at the command line or via the
>>> monitor. Command-line trace options are recorded during
>>> trace_opt_parse(), but tracing is not enabled until the various
>>> front-ends later call trace_init_file(). If the user passes a trace
>>> option on the command-line, remember that and enable tracing during
>>> trace_init_file().  Otherwise, trace_init_file() should record the
>>> trace file specified by the frontend and avoid enabling traces
>>> until the user requests them via the monitor.
>>> 
>>> This fixes 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 and also
>>> db25d56c014aa1a96319c663e0a60346a223b31e, by allowing the user
>>> to enable traces on the command line and also avoiding
>>> unwanted trace-<pid> files when the user has not asked for them.
>>> 
>>> Fixes: 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4
>>> Signed-off-by: Josh DuBois <josh@joshdubois.com>
>>> ---
>>>  trace/control.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> Thanks, applied to my tracing-next tree:
>> https://github.com/stefanha/qemu/commits/tracing-next
>>
>> Stefan
>
> Pull request?

Pretty-please?

Re: [PATCH] trace/simple: Enable tracing on startup only if the user specifies a trace option

[Josh DuBois]

Thanks for keeping on following-up, Markus.  

I know it's a small one, but it's still nice to have a small patch feel loved ;)

> On Oct 20, 2020, at 4:19 AM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Stefan Hajnoczi <stefanha@gmail.com> writes:
>> 
>>> On Sun, Aug 16, 2020 at 12:46:10PM -0500, duboisj@gmail.com wrote:
>>>> From: Josh DuBois <josh@joshdubois.com>
>>>> 
>>>> Tracing can be enabled at the command line or via the
>>>> monitor. Command-line trace options are recorded during
>>>> trace_opt_parse(), but tracing is not enabled until the various
>>>> front-ends later call trace_init_file(). If the user passes a trace
>>>> option on the command-line, remember that and enable tracing during
>>>> trace_init_file().  Otherwise, trace_init_file() should record the
>>>> trace file specified by the frontend and avoid enabling traces
>>>> until the user requests them via the monitor.
>>>> 
>>>> This fixes 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 and also
>>>> db25d56c014aa1a96319c663e0a60346a223b31e, by allowing the user
>>>> to enable traces on the command line and also avoiding
>>>> unwanted trace-<pid> files when the user has not asked for them.
>>>> 
>>>> Fixes: 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4
>>>> Signed-off-by: Josh DuBois <josh@joshdubois.com>
>>>> ---
>>>> trace/control.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>> 
>>> Thanks, applied to my tracing-next tree:
>>> https://github.com/stefanha/qemu/commits/tracing-next
>>> 
>>> Stefan
>> 
>> Pull request?
> 
> Pretty-please?
>