* [PATCH] trace/simple: Allow enabling simple traces from command line
@ 2020-07-23 5:33 Josh DuBois
2020-07-29 13:05 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Josh DuBois @ 2020-07-23 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Josh DuBois
The simple trace backend is enabled / disabled with a call
to st_set_trace_file_enabled(). When initializing tracing
from the command-line, this must be enabled on startup.
(Prior to db25d56c014aa1a9, command-line initialization of
simple trace worked because every call to st_set_trace_file
enabled tracing.)
Fixes: db25d56c014aa1a96319c663e0a60346a223b31e
Signed-off-by: Josh DuBois <josh@joshdubois.com>
---
trace/control.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/trace/control.c b/trace/control.c
index 2ffe000818..6558b5c906 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -225,6 +225,7 @@ void trace_init_file(const char *file)
{
#ifdef CONFIG_TRACE_SIMPLE
st_set_trace_file(file);
+ st_set_trace_file_enabled(true);
#elif defined CONFIG_TRACE_LOG
/*
* If both the simple and the log backends are enabled, "--trace file"
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] trace/simple: Allow enabling simple traces from command line
2020-07-23 5:33 [PATCH] trace/simple: Allow enabling simple traces from command line Josh DuBois
@ 2020-07-29 13:05 ` Stefan Hajnoczi
2020-07-30 22:50 ` Josh DuBois
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2020-07-29 13:05 UTC (permalink / raw)
To: Josh DuBois; +Cc: qemu-trivial, qemu-devel, Josh DuBois
[-- Attachment #1: Type: text/plain, Size: 665 bytes --]
On Thu, Jul 23, 2020 at 12:33:59AM -0500, Josh DuBois wrote:
> The simple trace backend is enabled / disabled with a call
> to st_set_trace_file_enabled(). When initializing tracing
> from the command-line, this must be enabled on startup.
> (Prior to db25d56c014aa1a9, command-line initialization of
> simple trace worked because every call to st_set_trace_file
> enabled tracing.)
>
> Fixes: db25d56c014aa1a96319c663e0a60346a223b31e
> Signed-off-by: Josh DuBois <josh@joshdubois.com>
> ---
> trace/control.c | 1 +
> 1 file changed, 1 insertion(+)
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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] trace/simple: Allow enabling simple traces from command line
2020-07-29 13:05 ` Stefan Hajnoczi
@ 2020-07-30 22:50 ` Josh DuBois
2020-08-03 9:08 ` Markus Armbruster
0 siblings, 1 reply; 6+ messages in thread
From: Josh DuBois @ 2020-07-30 22:50 UTC (permalink / raw)
To: Stefan Hajnoczi, Josh DuBois; +Cc: qemu-trivial, qemu-devel
Well, this is a bit embarrassing. The patch below simply re-introduced
the bug which the Fixes: line was trying to fix in the first place.
I.e, :
- with my patch (just committed as
1b7157be3a8c4300fc8044d40f4b2e64a152a1b4) applied, a QEMU built with
simple tracing will always produce a trace-<pid> file, regardless of
whether traces were asked for.
- after db25d56c014aa1a96319c663e0a60346a223b31e, which my patch was
supposed to "fix," QEMU will not produce a trace file unless asked, I
believe, via the monitor. Enabling traces is, near as I can tell,
simply impossible via the command-line in that case.
- prior to db25d56c014aa1a96319c663e0a60346a223b31e, just like today,
QEMU built with simple tracing will always produce a trace-<pid> file,
regardless of whether the user asks for traces at runtime.
I'm sorry for the mess. Having stepped in it already, I'm open to
trying to track it down and fix it properly. I imagine perhaps few
people truly care, since traces require a special build and are probably
only being done by developers anyway. (And the original message for
db25d56c014aa1a96319c663e0a60346a223b31e said it had been "broken" for
an unknown period of time).
I'm brand new around here so I'll leave it to others whether it's better
to revert and have traces impossible to enable from the cli (as I say, I
think they're only possible from the monitor prior to my "fix" ) or to
leave it be.
If I resubmit, I'll try to test a little more next time. I just wanted
my traces to work. ;)
On 7/29/20 8:05 AM, Stefan Hajnoczi wrote:
> On Thu, Jul 23, 2020 at 12:33:59AM -0500, Josh DuBois wrote:
>> The simple trace backend is enabled / disabled with a call
>> to st_set_trace_file_enabled(). When initializing tracing
>> from the command-line, this must be enabled on startup.
>> (Prior to db25d56c014aa1a9, command-line initialization of
>> simple trace worked because every call to st_set_trace_file
>> enabled tracing.)
>>
>> Fixes: db25d56c014aa1a96319c663e0a60346a223b31e
>> Signed-off-by: Josh DuBois <josh@joshdubois.com>
>> ---
>> trace/control.c | 1 +
>> 1 file changed, 1 insertion(+)
> Thanks, applied to my tracing tree:
> https://github.com/stefanha/qemu/commits/tracing
>
> Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] trace/simple: Allow enabling simple traces from command line
2020-07-30 22:50 ` Josh DuBois
@ 2020-08-03 9:08 ` Markus Armbruster
2020-08-04 17:41 ` Josh DuBois
0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2020-08-03 9:08 UTC (permalink / raw)
To: Josh DuBois; +Cc: qemu-trivial, Stefan Hajnoczi, Josh DuBois, qemu-devel
Josh DuBois <josh@joshdubois.com> writes:
> Well, this is a bit embarrassing. The patch below simply
> re-introduced the bug which the Fixes: line was trying to fix in the
> first place.
>
> I.e, :
>
> - with my patch (just committed as
> 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4) applied, a QEMU built with
> simple tracing will always produce a trace-<pid> file, regardless of
> whether traces were asked for.
>
> - after db25d56c014aa1a96319c663e0a60346a223b31e, which my patch was
> supposed to "fix," QEMU will not produce a trace file unless asked, I
> believe, via the monitor. Enabling traces is, near as I can tell,
> simply impossible via the command-line in that case.
>
> - prior to db25d56c014aa1a96319c663e0a60346a223b31e, just like today,
> QEMU built with simple tracing will always produce a trace-<pid> file,
> regardless of whether the user asks for traces at runtime.
When you send a patch with a Fixes: tag, consider cc'ing people involved
in the commit being fixed. I might have spotted the regression.
> I'm sorry for the mess. Having stepped in it already, I'm open to
> trying to track it down and fix it properly. I imagine perhaps few
> people truly care, since traces require a special build and are
> probably only being done by developers anyway. (And the original
> message for db25d56c014aa1a96319c663e0a60346a223b31e said it had been
> "broken" for an unknown period of time).
>
> I'm brand new around here so I'll leave it to others whether it's
> better to revert and have traces impossible to enable from the cli (as
> I say, I think they're only possible from the monitor prior to my
> "fix" ) or to leave it be.
>
> If I resubmit, I'll try to test a little more next time. I just
> wanted my traces to work. ;)
I missed the CLI issue. I just wanted my directories not littered with
trace files ;)
Stefan, what shall we do for 5.1?
If we keep littering, the annoyance will make me drop the trace backend
"simple" from my build tests. I might even remember to put it back when
the fix arrives.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] trace/simple: Allow enabling simple traces from command line
2020-08-03 9:08 ` Markus Armbruster
@ 2020-08-04 17:41 ` Josh DuBois
2020-08-05 4:38 ` Markus Armbruster
0 siblings, 1 reply; 6+ messages in thread
From: Josh DuBois @ 2020-08-04 17:41 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-trivial, Stefan Hajnoczi, Josh DuBois, qemu-devel
On Aug 3, 2020, at 4:08 AM, Markus Armbruster <armbru@redhat.com> wrote:
>
>>
>> - prior to db25d56c014aa1a96319c663e0a60346a223b31e, just like today,
>> QEMU built with simple tracing will always produce a trace-<pid> file,
>> regardless of whether the user asks for traces at runtime.
>
> When you send a patch with a Fixes: tag, consider cc'ing people involved
> in the commit being fixed. I might have spotted the regression.
Sure, this makes sense.
> I missed the CLI issue. I just wanted my directories not littered with
> trace files ;)
>
> Stefan, what shall we do for 5.1?
>
> If we keep littering, the annoyance will make me drop the trace backend
> "simple" from my build tests. I might even remember to put it back when
> the fix arrives.
I haven't seen another response, but I just noticed a 'last call' for 5.1. If this means something is going to get excluded from regular build tests, that seems important - I for one have no objection to simply reverting this - 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 <-- my "fix."
I will try to send a better fix along sometime soonish, but I probably won't get to that before 5.1. If the nuisance of the trace-<pid> files is causing real-world problems for someone actually doing regular development, that seems more important than the command line issue, to me. Just my $0.02.
Cheers, and sorry if your build dirs do end up littered again.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] trace/simple: Allow enabling simple traces from command line
2020-08-04 17:41 ` Josh DuBois
@ 2020-08-05 4:38 ` Markus Armbruster
0 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2020-08-05 4:38 UTC (permalink / raw)
To: Josh DuBois; +Cc: qemu-trivial, Stefan Hajnoczi, Josh DuBois, qemu-devel
Josh DuBois <josh@joshdubois.com> writes:
> On Aug 3, 2020, at 4:08 AM, Markus Armbruster <armbru@redhat.com> wrote:
>>
>>>
>>> - prior to db25d56c014aa1a96319c663e0a60346a223b31e, just like today,
>>> QEMU built with simple tracing will always produce a trace-<pid> file,
>>> regardless of whether the user asks for traces at runtime.
>>
>> When you send a patch with a Fixes: tag, consider cc'ing people involved
>> in the commit being fixed. I might have spotted the regression.
>
> Sure, this makes sense.
>
>> I missed the CLI issue. I just wanted my directories not littered with
>> trace files ;)
>>
>> Stefan, what shall we do for 5.1?
>>
>> If we keep littering, the annoyance will make me drop the trace backend
>> "simple" from my build tests. I might even remember to put it back when
>> the fix arrives.
>
> I haven't seen another response, but I just noticed a 'last call' for 5.1. If this means something is going to get excluded from regular build tests, that seems important - I for one have no objection to simply reverting this - 1b7157be3a8c4300fc8044d40f4b2e64a152a1b4 <-- my "fix."
These are just my local build test. Our CI is not affected.
Reverting is up to Stefan.
> I will try to send a better fix along sometime soonish, but I probably won't get to that before 5.1. If the nuisance of the trace-<pid> files is causing real-world problems for someone actually doing regular development, that seems more important than the command line issue, to me. Just my $0.02.
>
> Cheers, and sorry if your build dirs do end up littered again.
Sorry for breaking your use case, and looking forward to your fix for
your fix!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-05 4:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 5:33 [PATCH] trace/simple: Allow enabling simple traces from command line Josh DuBois
2020-07-29 13:05 ` Stefan Hajnoczi
2020-07-30 22:50 ` Josh DuBois
2020-08-03 9:08 ` Markus Armbruster
2020-08-04 17:41 ` Josh DuBois
2020-08-05 4:38 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).