All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Josh DuBois <josh@joshdubois.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>,
	Josh DuBois <duboisj@gmail.com>,
	qemu-trivial@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [PATCH] trace/simple: Allow enabling simple traces from command line
Date: Mon, 03 Aug 2020 11:08:08 +0200	[thread overview]
Message-ID: <87h7tkm70n.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <e08651bd-f775-eb85-817c-44d27ff072dc@joshdubois.com> (Josh DuBois's message of "Thu, 30 Jul 2020 17:50:09 -0500")

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.



WARNING: multiple messages have this Message-ID (diff)
From: Markus Armbruster <armbru@redhat.com>
To: Josh DuBois <josh@joshdubois.com>
Cc: qemu-trivial@nongnu.org, Stefan Hajnoczi <stefanha@gmail.com>,
	Josh DuBois <duboisj@gmail.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH] trace/simple: Allow enabling simple traces from command line
Date: Mon, 03 Aug 2020 11:08:08 +0200	[thread overview]
Message-ID: <87h7tkm70n.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <e08651bd-f775-eb85-817c-44d27ff072dc@joshdubois.com> (Josh DuBois's message of "Thu, 30 Jul 2020 17:50:09 -0500")

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.



  reply	other threads:[~2020-08-03  9:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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-29 13:05   ` Stefan Hajnoczi
2020-07-30 22:50   ` Josh DuBois
2020-08-03  9:08     ` Markus Armbruster [this message]
2020-08-03  9:08       ` Markus Armbruster
2020-08-04 17:41       ` Josh DuBois
2020-08-05  4:38         ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h7tkm70n.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=duboisj@gmail.com \
    --cc=josh@joshdubois.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.