All of lore.kernel.org
 help / color / mirror / Atom feed
From: Slavomir Kaslev <kaslevs@vmware.com>
To: "rostedt@goodmis.org" <rostedt@goodmis.org>
Cc: "linux-trace-devel@vger.kernel.org" <linux-trace-devel@vger.kernel.org>
Subject: Re: [PATCH] trace-cmd: Fix record --date flag when sending tracing data to a listener
Date: Tue, 27 Nov 2018 10:15:55 +0000	[thread overview]
Message-ID: <CAE0o1Ns+4UXU+UEgVT=sJvLujCOEPa78-UA7J_UVLTEK1gHoqQ@mail.gmail.com> (raw)
In-Reply-To: <20181126130530.63e8df2f@gandalf.local.home>

On Mon, Nov 26, 2018 at 8:06 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 14 Nov 2018 17:43:28 +0200
> kaslevs@vmware.com wrote:
>
> > From: Slavomir Kaslev <kaslevs@vmware.com>
> >
> > Currently the `trace-cmd record` --date is not taken into account when tracing
> > data is sent to a remote host with the -N flag.
> >
> > This patch fixes this by the writing output buffer options from the recording
> > side instead of on the listener side.
> >
> > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
> > ---
> >
>
> I don't see anything too wrong with it, accept the following test broke:
>
>  $ git checkout trace-cmd-v2.6
>  $ make
>  $ sudo cp trace-cmd /usr/local/bin/trace-cmd-v2.6
>  $ git checkout origin/master
>  $ patch -p1 < this.patch
>  $ make
>  $ trace-cmd-v2.6 listen -p 12345
>
> In another terminal:
>
>  $ sudo trace-cmd record -N 127.0.0.1:12345 -e sched sleep 1
> trace-cmd: No such file or directory
>   Cannot handle the protocol
>
>
> Remember, we need to remain backward compatible. We also need to test
> this code running as a listener, and the trace-cmd-v2.6 (and earlier)
> as the recorder.

This is a design bug (the best kind), metadata should really be written from the
recording side and not from the listener. A backward compatible fix should have
the newer recorder and listener detect they're talking to an older version and
fallback to broken behavior. This implies a new protocol version or extending
MSG_TINIT/MSG_RINIT so that we can infer the behavior on the other side and
fallback to being broken when necessary.

What would you suggest?

--
Slavi

  reply	other threads:[~2018-11-27 21:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-14 15:43 [PATCH] trace-cmd: Fix record --date flag when sending tracing data to a listener kaslevs
2018-11-26 18:05 ` Steven Rostedt
2018-11-27 10:15   ` Slavomir Kaslev [this message]
2018-11-27 13:54     ` Steven Rostedt
2018-11-28  3:19     ` Steven Rostedt
2018-11-30 12:33       ` Slavomir Kaslev
2018-11-30 13:19         ` Slavomir Kaslev
2018-11-28  3:21 ` Steven Rostedt

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='CAE0o1Ns+4UXU+UEgVT=sJvLujCOEPa78-UA7J_UVLTEK1gHoqQ@mail.gmail.com' \
    --to=kaslevs@vmware.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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.