Re: [PATCH 29/66] tools lib traceevent: Use str_error_r()

[Arnaldo Carvalho de Melo]

Em Wed, Jul 13, 2016 at 03:17:00PM -0400, Steven Rostedt escreveu:
> On Wed, 13 Jul 2016 10:51:25 -0300
> Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
> 
> > Em Wed, Jul 13, 2016 at 08:30:01AM -0400, Steven Rostedt escreveu:
> > > On Tue, 12 Jul 2016 20:32:18 -0300
> > > Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:  
> > > > > > Forgot about the out of tree copy :-\    
> > 
> > > > > Yeah, we really need to make this into a real library. I haven't had
> > > > > the time to do that. Hopefully in August I can talk with some people at    
> > 
> > > > What exactly do you mean by that? To grab a copy of what is in tools/
> > > > and have it turned into a library somewhere else?  
> > 
> > > > Or to freeze its interfaces and create an .so with a committed to ABI?  
> > 
> > > Yes, this is what we need to do.  
> > 
> > > > I kind of like the way it is now... :-)  
> > 
> > > You like the current ABI? Or the fact that we have 4 tools with each  
> > 
> > I like the lack of an ABI, i.e. I like to use the same rationale for
> > when a out of tree driver breaks because we improve kernel internals. As
> > soon as we commit to an ABI for those libraries, that becomes way more
> > difficult.
> 
> Yes, ABI is hard. But it can be done. Let's not get lazy. The code came

Hey, that is what I call enthusiasm, expecting the patches!

> from external sources to begin with. I've been working hard to try to

What code "that come from external sources" are you talking about?

> keep the ABI fixed. There's still a few things I would like to change
> before doing it officially, and that's because this code is quite
> mature now, and I have an idea of what to change.

Ah, you're talking about just tools/lib/traceevent/ ?
 
> One thing I want to do is change the event_format structure to
> "pevent_event" and the format_field to "pevent_field" just to be more
> consistent. There's probably a few other changes as well. But once they
> are done, I think we should get this out into a library.

> > And since we're still adding more and more stuff (write_backward, eBPF
> > for kernel and userspace, etc, etc) I fear lots are still very much in
> > flux to commit to something like that.
 
> We can always add new functions. The old ones will have to be
> maintained. Anyway, I'm only looking to what is being used externally
> from perf. We can slowly add functionality. The .so releases should
> only increment the version if something new is added. I refuse to
> follow the gtk crap that they find ways to remove or modify existing
> functions every single release.

> > So if we know what are the needs of those out of the tree tools (can't
> > they be brought in tree? I'm digressing, couldn't resist, sorry ;-)),
 
> Well, I Cc'd some of the maintainers. We can see what they think.
 
> > then we could, if absolutely required, try, without much enthusiasm,
> > relutanctly, see what could be done besides what has been already
> > underway (moving stuff out of tools/perf and into tools/lib).
> 
> Note, I'm only looking at making a library out of what is currently in
> tools/lib. We don't need to move stuff out from perf unless you want to.

Hey, if it is only rom tools/lib/traceevent, I have no worries, you're
the one proposing turning it into a committed to ABI and you're the main
author, also willing to do the work of turning it into a .so, fair enough.

As time goes by and the changes take place that requires we to write
tools/build/feature/ entries to cope with changes in its ABI, we'll just
do it, it will be just another external library we need and can't do
changes in tandem with tools/perf/, we'll have to suport old versions,
do feature checks to see if the one present in a particular
distro/release has some new feature, etc, etc.
 
> There's a trace-cmd library I want to have as well, which I would love
> to have in the kernel tools/lib directory too. I'd call it
> libftrace.so. This would include the ways to parse the ftrace ring
> buffer, as well as parsing and creating trace.dat files.
 
> > It would add more burden to me for perhaps reducing the burden to who
> > would use those libraries.
 
> Well the event-parse.* code has already been pretty stable. I do want
> to do the renames as I mentioned above before making it official. But
> once that is done, I would want us to get a libtraceevent.so that
> distros can provide.

I was confused, I was thinking you were talking about code other than
tools/lib/traceevent/.

> > > their own copies of the library? (and maybe even more)  

> > Which ones?
 
> perf, trace-cmd/kernelshark, powertop and ras-daemon

Cool, so this is all just about tools/lib/traceevent/ after all, sure,
go ahead, you're the "external source" after all :-)

- Arnaldo