Re: [PATCH lttng-tools 00/28] UST Event exclusions

* Re: [PATCH lttng-tools 00/28] UST Event exclusions
       [not found] <1383819403-6386-1-git-send-email-jp_ikaheimonen@mentor.com>
@ 2013-11-13 21:07 ` David Goulet
       [not found] ` <20131113210758.GB7694@dalia>
  1 sibling, 0 replies; 6+ messages in thread
From: David Goulet @ 2013-11-13 21:07 UTC (permalink / raw)
  To: JP Ikaheimonen; +Cc: lttng-dev

[-- Attachment #1.1: Type: text/plain, Size: 5216 bytes --]

Hey JP!

So here is the deal with this big patch set and the 2.4-rc ;).

I've checked the patches and they are OK in terms of code and structure.
The feature seems to work fine also! Which is a good news. However, we
are a couple hours before the release candidate! and there is still
issues with all this :S.

We know that you guys have been putting a lot of efforts in this and the
code looks good so what we are going to do is merge this in the next RC
release (meaning 2.4-rc2) considering that right now the feature works
and thus is freezed for the next version.

For that to happen, I'll ask you to fix the following issues as soon as
you can so we can merge it quickly thus having more testing. Not getting
this in time, unfortunately we won't be able to merge that for 2.4.
Usually, the RC process cycle is around 1 month maximum.

Issues:

1) "make check" fails here so it *has* to pass in order to accept this
patch set. I didn't investiguate thoroughly but it might be that you
broke some existing feature or small adjustement to the tests.

2) For this kind of big feature, we *need* at least one test that makes
sure that in further release there is no regression for that exclusion
feature.

3) A patch for the man page (lttng.1) would be *very* appreciated.

4) Commit id: 78bef43205f5296cb6cae81b6e264b849fe94988
Please add a PERROR() if the zmalloc returns NULL so we can at least
report it somewhere.

I've fixed small issues also so please look at my dev branch for the two
small commits on top of the 28 patches.

https://github.com/dgoulet/lttng-tools-dev/tree/ev-exclusion

Considering that, either you send back the patches *ON TOP* of that dev
branch or you can also provide me with a pull request because with that
amount of patches, by email is kind of a pain :).

Hope this seems fair enough with you guys. It's the best we can do to
make sure this feature gets in 2.4 :).

Cheers!
David

On 07 Nov (12:16:43), JP Ikaheimonen wrote:
> This patch set, along with a patch set for lttng-ust, implements
> a new option for lttng enable-event command.
> 
> The new option makes it possible to exclude events from a wildcarded
> event definition.
> 
> That is, running command
> 
> lttng enable-event -u -c c1 'myevents*' -x 'myevents:dbg*'
> 
> would enable all UST tracepoint events whose names start with 'myevents'
> except those that start with 'myevents:dbg'.
> 
> The lttng list command is also updated, so that it points out the
> fact that an event has some exclusions.
> 
> JP Ikaheimonen (28):
>   Add exclusion data to enable-command
>   Define a structure for event exclusion data
>   Add exclusion data to cmd_enable_event
>   Add exclusions to event_ust_enable_tracepoint
>   Add event exclusion data to ltt_ust_event structure
>   Add exclusion matching logic to trace_ust_ht_match_event
>   Add exclusion data to trace_ust_create_event
>   Add exclusion data to trace_ust_find_event
>   Copy exclusion data in add_unique_ust_event.
>   Add new error codes for exclusion processing
>   Add handling of LTTNG_ENABLE_EVENT_WITH_EXCLUSION
>   Collate handling of LTTNG_ENABLE_EVENT_WITH_FILTER
>   Add exclusion data to ust_app_event structure
>   Compare also exclusions in ht_match_ust_app_event
>   Copy event exclusion data in add_unique_ust_app_event
>   Free exclusion data when ust_app_event is deleted
>   Add exclusion data to find_ust_app_event
>   Copy exclusion data in shadow_copy_event
>   Set event exclusions in the target when event is created
>   Send enable-event commands with exclusion data
>   Collate lttng_enable_event* code
>   Add -exclude option to enable-event command
>   Add usage for --exclude option
>   Add a function to check for legal exclusions
>   Parse exclusions and forward them to the control handler
>   Detect and report exclusion option errors
>   Add exclusion names to diagnostic printouts
>   When listing events, show exclusions if they exist
> 
>  include/lttng/lttng-error.h              |   2 +
>  include/lttng/lttng.h                    |  20 ++-
>  src/bin/lttng-sessiond/cmd.c             |  13 +-
>  src/bin/lttng-sessiond/cmd.h             |   4 +-
>  src/bin/lttng-sessiond/event.c           |  12 +-
>  src/bin/lttng-sessiond/event.h           |   3 +-
>  src/bin/lttng-sessiond/main.c            | 110 +++++++-----
>  src/bin/lttng-sessiond/trace-ust.c       |  23 ++-
>  src/bin/lttng-sessiond/trace-ust.h       |  14 +-
>  src/bin/lttng-sessiond/ust-app.c         |  95 +++++++++-
>  src/bin/lttng-sessiond/ust-app.h         |   2 +
>  src/bin/lttng/commands/enable_events.c   | 242 +++++++++++++++++++++++--
>  src/bin/lttng/commands/list.c            |  15 +-
>  src/common/sessiond-comm/sessiond-comm.h |  22 ++-
>  src/lib/lttng-ctl/lttng-ctl.c            | 291 ++++++++++++++++++-------------
>  tests/unit/test_ust_data.c               |   2 +-
>  16 files changed, 657 insertions(+), 213 deletions(-)
> 
> -- 
> 1.8.1.2
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

[-- Attachment #2: Type: text/plain, Size: 155 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 6+ messages in thread