Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [lttng-dev] [PATCH lttng-tools 1/3] Fix: disable kernel event based on name and event type
       [not found] ` <CA+jJMxscPwme95++cjrQU3eBPB8Tw88SDOQ0+rXrfvJS6t_gAw@mail.gmail.com>
@ 2015-09-21 16:26   ` Jérémie Galarneau
  0 siblings, 0 replies; 10+ messages in thread
From: Jérémie Galarneau @ 2015-09-21 16:26 UTC (permalink / raw)


Disregard this, it applies just fine!

J?r?mie

On Mon, Sep 21, 2015 at 12:10 PM, J?r?mie Galarneau
<jeremie.galarneau at efficios.com> wrote:
> This patch seems to no longer apply on master. On which commit was it based?
>
> Thanks,
> J?r?mie
>
> On Thu, Sep 17, 2015 at 4:37 PM, Jonathan Rajotte
> <jonathan.rajotte-julien at efficios.com> wrote:
>> The -a argument is interpreted as a zero-length event name
>> instead of '*' which is actually a valid wildcard event
>> name by itself. This simplify how a disable action is handled.
>>
>> The event type can now be passed as argument
>> and is a new criteria while disabling kernel events.
>> The default is to disable for all event type.
>>
>> UST and agent domain do not support yet disabling by event
>> type.
>>
>> e.g:
>>         # Only disable kernel event of type tracepoint.
>>         lttng disable -a -k --tracepoint
>>
>>         # Only disable the event with name '*' and type syscall.
>>         lttng disable -k '*' --syscall
>>
>>         # Disable all kernel event of all type.
>>         lttng disable -a -k
>>
>> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
>> ---
>>  src/bin/lttng-sessiond/cmd.c                       | 45 +++++-----
>>  src/bin/lttng-sessiond/event.c                     | 65 +++++---------
>>  src/bin/lttng-sessiond/event.h                     |  5 +-
>>  src/bin/lttng/commands/disable_events.c            | 98 ++++++++++++++++++----
>>  src/lib/lttng-ctl/lttng-ctl.c                      |  4 -
>>  .../ust/python-logging/test_python_logging.in      |  2 +-
>>  6 files changed, 121 insertions(+), 98 deletions(-)
>>
>> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
>> index 7878a05..5a1cc7a 100644
>> --- a/src/bin/lttng-sessiond/cmd.c
>> +++ b/src/bin/lttng-sessiond/cmd.c
>> @@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                 switch (event->type) {
>>                 case LTTNG_EVENT_ALL:
>> -                       ret = event_kernel_disable_event_all(kchan);
>> -                       if (ret != LTTNG_OK) {
>> -                               goto error_unlock;
>> -                       }
>> -                       break;
>> -               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
>> +               case LTTNG_EVENT_TRACEPOINT:
>>                 case LTTNG_EVENT_SYSCALL:
>> -                       if (!strcmp(event_name, "*")) {
>> -                               ret = event_kernel_disable_event_type(kchan,
>> -                                       event->type);
>> +               case LTTNG_EVENT_PROBE:
>> +               case LTTNG_EVENT_FUNCTION:
>> +               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
>> +                       if (event_name[0] == '\0') {
>> +                               ret = event_kernel_disable_event(kchan,
>> +                                       NULL, event->type);
>>                         } else {
>>                                 ret = event_kernel_disable_event(kchan,
>> -                                       event_name);
>> +                                       event_name, event->type);
>>                         }
>>                         if (ret != LTTNG_OK) {
>>                                 goto error_unlock;
>>                         }
>>                         break;
>> -               case LTTNG_EVENT_PROBE:
>> -               case LTTNG_EVENT_FUNCTION:
>> -               case LTTNG_EVENT_FUNCTION_ENTRY:
>> -                       ret = event_kernel_disable_event(kchan, event_name);
>> -                       if (ret != LTTNG_OK) {
>> -                               goto error_unlock;
>> -                       }
>> -                       break;
>>                 default:
>>                         ret = LTTNG_ERR_UNK;
>>                         goto error_unlock;
>> @@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                 /*
>>                  * If a non-default channel has been created in the
>> -                * session, explicitely require that -c chan_name needs
>> +                * session, explicitly require that -c chan_name needs
>>                  * to be provided.
>>                  */
>>                 if (usess->has_non_default_channel && channel_name[0] == '\0') {
>> @@ -1290,10 +1280,12 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                 switch (event->type) {
>>                 case LTTNG_EVENT_ALL:
>> -                       if (strlen(event->name) == 1 &&
>> -                                       !strncmp(event->name, "*", 1)) {
>> -                               ret = event_ust_disable_all_tracepoints(usess,
>> -                                               uchan);
>> +                       /*
>> +                        * An empty event name means that everything
>> +                        * should be disabled.
>> +                        */
>> +                       if (event->name[0] == '\0') {
>> +                               ret = event_ust_disable_all_tracepoints(usess, uchan);
>>                         } else {
>>                                 ret = event_ust_disable_tracepoint(usess, uchan,
>>                                                 event_name);
>> @@ -1333,8 +1325,11 @@ int cmd_disable_event(struct ltt_session *session,
>>                         ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
>>                         goto error_unlock;
>>                 }
>> -               /* The wild card * means that everything should be disabled. */
>> -               if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
>> +               /*
>> +                * An empty event name means that everything
>> +                * should be disabled.
>> +                */
>> +               if (event->name[0] == '\0') {
>>                         ret = event_agent_disable_all(usess, agt);
>>                 } else {
>>                         ret = event_agent_disable(usess, agt, event_name);
>> diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
>> index 13f09a5..a53d4e9 100644
>> --- a/src/bin/lttng-sessiond/event.c
>> +++ b/src/bin/lttng-sessiond/event.c
>> @@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
>>  }
>>
>>  /*
>> - * Disable kernel tracepoint event for a channel from the kernel session.
>> + * Disable kernel tracepoint events for a channel from the kernel session of
>> + * a specified event_name and event type.
>> + * On type LTTNG_EVENT_ALL all events with event_name are disabled.
>> + * If event_name if NULL all events of the specified type are disabled.
>>   */
>>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
>> -               char *event_name)
>> -{
>> -       int ret;
>> -       struct ltt_kernel_event *kevent;
>> -
>> -       assert(kchan);
>> -
>> -       kevent = trace_kernel_get_event_by_name(event_name, kchan,
>> -                       LTTNG_EVENT_ALL);
>> -       if (kevent == NULL) {
>> -               ret = LTTNG_ERR_NO_EVENT;
>> -               goto error;
>> -       }
>> -
>> -       ret = kernel_disable_event(kevent);
>> -       if (ret < 0) {
>> -               ret = LTTNG_ERR_KERN_DISABLE_FAIL;
>> -               goto error;
>> -       }
>> -
>> -       DBG("Kernel event %s disable for channel %s.",
>> -                       kevent->event->name, kchan->channel->name);
>> -
>> -       ret = LTTNG_OK;
>> -
>> -error:
>> -       return ret;
>> -}
>> -
>> -/*
>> - * Disable kernel tracepoint events for a channel from the kernel session.
>> - */
>> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>> -               enum lttng_event_type type)
>> +               char *event_name, enum lttng_event_type type)
>>  {
>> -       int ret;
>> +       int ret, error = 0, found = 0;
>>         struct ltt_kernel_event *kevent;
>>
>>         assert(kchan);
>> @@ -108,22 +78,25 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>>         cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
>>                 if (type != LTTNG_EVENT_ALL && kevent->type != type)
>>                         continue;
>> +               if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
>> +                       continue;
>> +               }
>> +               found++;
>>                 ret = kernel_disable_event(kevent);
>>                 if (ret < 0) {
>> -                       /* We continue disabling the rest */
>> +                       error = 1;
>>                         continue;
>>                 }
>>         }
>> -       ret = LTTNG_OK;
>> -       return ret;
>> -}
>> +       DBG("Disable kernel event: found %d events with name: %s and type: %d",
>> +                       found, event_name, type);
>>
>> -/*
>> - * Disable all kernel event for a channel from the kernel session.
>> - */
>> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
>> -{
>> -       return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
>> +       if (!found && event_name != NULL) {
>> +               error = 1;
>> +       }
>> +
>> +       ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
>> +       return ret;
>>  }
>>
>>  /*
>> diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
>> index 7c5231d..45dd1fc 100644
>> --- a/src/bin/lttng-sessiond/event.h
>> +++ b/src/bin/lttng-sessiond/event.h
>> @@ -23,10 +23,7 @@
>>  struct agent;
>>
>>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
>> -               char *event_name);
>> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>> -               enum lttng_event_type type);
>> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
>> +               char *event_name, enum lttng_event_type event_type);
>>
>>  int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
>>                 struct lttng_event *event, char *filter_expression,
>> diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
>> index c6ec85f..bcbc78d 100644
>> --- a/src/bin/lttng/commands/disable_events.c
>> +++ b/src/bin/lttng/commands/disable_events.c
>> @@ -44,7 +44,12 @@ static int opt_event_type;
>>  enum {
>>         OPT_HELP = 1,
>>         OPT_USERSPACE,
>> -       OPT_SYSCALL,
>> +       OPT_TYPE_SYSCALL,
>> +       OPT_TYPE_TRACEPOINT,
>> +       OPT_TYPE_PROBE,
>> +       OPT_TYPE_FUNCTION,
>> +       OPT_TYPE_FUNCTION_ENTRY,
>> +       OPT_TYPE_ALL,
>>         OPT_LIST_OPTIONS,
>>  };
>>
>> @@ -61,8 +66,24 @@ static struct poptOption long_options[] = {
>>         {"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
>>         {"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
>>         {"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
>> -       {"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
>> +       {"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
>> +       {"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
>> +       {"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
>> +       {"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
>> +       {"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
>> +#if 0
>> +       /*
>> +        * Function entry exits as a lttng_event_type but is never used on
>> +        * enable
>> +        */
>> +       {"function-entry",   0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION_ENTRY, 0, 0},
>> +
>> +       /* Not implemented yet */
>> +       {"userspace",      'u', POPT_ARG_STRING | POPT_ARGFLAG_OPTIONAL, &opt_cmd_name, OPT_USERSPACE, 0, 0},
>> +       {"pid",            'p', POPT_ARG_INT, &opt_pid, 0, 0, 0},
>> +#else
>>         {"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
>> +#endif
>>         {"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
>>         {0, 0, 0, 0, 0, 0, 0}
>>  };
>> @@ -86,8 +107,19 @@ static void usage(FILE *ofp)
>>         fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
>>         fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
>>         fprintf(ofp, "\n");
>> -       fprintf(ofp, "Event options:\n");
>> +       fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
>> +       fprintf(ofp, "      --all                All event type (default)\n");
>> +       fprintf(ofp, "      --tracepoint         Tracepoint event\n");
>>         fprintf(ofp, "      --syscall            System call event\n");
>> +       fprintf(ofp, "      --probe              Probe event\n");
>> +       fprintf(ofp, "      --function           Function event\n");
>> +#if 0
>> +       /*
>> +        * Function entry exits as a lttng_event_type but is never used on
>> +        * enable
>> +        */
>> +       fprintf(ofp, "      --function-entry     Function-entry event\n");
>> +#endif
>>         fprintf(ofp, "\n");
>>  }
>>
>> @@ -103,6 +135,27 @@ const char *print_raw_channel_name(const char *name)
>>         return name ? : "<default>";
>>  }
>>
>> +static
>> +const char *print_event_type(const enum lttng_event_type ev_type)
>> +{
>> +       switch (ev_type) {
>> +       case LTTNG_EVENT_ALL:
>> +               return "any";
>> +       case LTTNG_EVENT_TRACEPOINT:
>> +               return "tracepoint";
>> +       case LTTNG_EVENT_PROBE:
>> +               return "probe";
>> +       case LTTNG_EVENT_FUNCTION:
>> +               return "function";
>> +       case LTTNG_EVENT_FUNCTION_ENTRY:
>> +               return "function entry";
>> +       case LTTNG_EVENT_SYSCALL:
>> +               return "syscall";
>> +       default:
>> +               return "";
>> +       }
>> +}
>> +
>>  /* Mi print a partial event.
>>   * enabled is 0 or 1
>>   * success is 0 or 1
>> @@ -213,14 +266,8 @@ static int disable_events(char *session_name)
>>         /* Set default loglevel to any/unknown */
>>         event.loglevel = -1;
>>
>> -       switch (opt_event_type) {
>> -       case LTTNG_EVENT_SYSCALL:
>> -               event.type = LTTNG_EVENT_SYSCALL;
>> -               break;
>> -       default:
>> -               event.type = LTTNG_EVENT_ALL;
>> -               break;
>> -       }
>> +       /* opt_event_type contain the event type to disable at this point */
>> +       event.type = opt_event_type;
>>
>>         if (opt_disable_all) {
>>                 command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>> @@ -232,9 +279,9 @@ static int disable_events(char *session_name)
>>                 } else {
>>                         enabled = 0;
>>                         success = 1;
>> -                       MSG("All %s %s are disabled in channel %s",
>> +                       MSG("All %s events of type %s are disabled in channel %s",
>>                                         get_domain_str(dom.type),
>> -                                       opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
>> +                                       print_event_type(opt_event_type),
>>                                         print_channel_name(channel_name));
>>                 }
>>
>> @@ -255,9 +302,9 @@ static int disable_events(char *session_name)
>>                         event.name[sizeof(event.name) - 1] = '\0';
>>                         command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>>                         if (command_ret < 0) {
>> -                               ERR("%s %s: %s (channel %s, session %s)",
>> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
>> +                               ERR("%s of type %s : %s (channel %s, session %s)",
>>                                                 event_name,
>> +                                               print_event_type(opt_event_type),
>>                                                 lttng_strerror(command_ret),
>>                                                 command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
>>                                                         ? print_raw_channel_name(channel_name)
>> @@ -271,10 +318,10 @@ static int disable_events(char *session_name)
>>                                  */
>>                                 enabled = 1;
>>                         } else {
>> -                               MSG("%s %s %s disabled in channel %s for session %s",
>> +                               MSG("%s %s of type %s disabled in channel %s for session %s",
>>                                                 get_domain_str(dom.type),
>> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
>>                                                 event_name,
>> +                                               print_event_type(opt_event_type),
>>                                                 print_channel_name(channel_name),
>>                                                 session_name);
>>                                 success = 1;
>> @@ -341,9 +388,24 @@ int cmd_disable_events(int argc, const char **argv)
>>                 case OPT_USERSPACE:
>>                         opt_userspace = 1;
>>                         break;
>> -               case OPT_SYSCALL:
>> +               case OPT_TYPE_SYSCALL:
>>                         opt_event_type = LTTNG_EVENT_SYSCALL;
>>                         break;
>> +               case OPT_TYPE_TRACEPOINT:
>> +                       opt_event_type = LTTNG_EVENT_TRACEPOINT;
>> +                       break;
>> +               case OPT_TYPE_PROBE:
>> +                       opt_event_type = LTTNG_EVENT_PROBE;
>> +                       break;
>> +               case OPT_TYPE_FUNCTION:
>> +                       opt_event_type = LTTNG_EVENT_FUNCTION;
>> +                       break;
>> +               case OPT_TYPE_FUNCTION_ENTRY:
>> +                       opt_event_type = LTTNG_EVENT_FUNCTION_ENTRY;
>> +                       break;
>> +               case OPT_TYPE_ALL:
>> +                       opt_event_type = LTTNG_EVENT_ALL;
>> +                       break;
>>                 case OPT_LIST_OPTIONS:
>>                         list_cmd_options(stdout, long_options);
>>                         goto end;
>> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
>> index 9cbfef5..1007326 100644
>> --- a/src/lib/lttng-ctl/lttng-ctl.c
>> +++ b/src/lib/lttng-ctl/lttng-ctl.c
>> @@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
>>         }
>>
>>         lsm.cmd_type = LTTNG_DISABLE_EVENT;
>> -       if (ev->name[0] == '\0') {
>> -               /* Disable all events */
>> -               lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
>> -       }
>>
>>         lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
>>         /* FIXME: copying non-packed struct to packed struct. */
>> diff --git a/tests/regression/ust/python-logging/test_python_logging.in b/tests/regression/ust/python-logging/test_python_logging.in
>> index 8aff408..d5a9b80 100755
>> --- a/tests/regression/ust/python-logging/test_python_logging.in
>> +++ b/tests/regression/ust/python-logging/test_python_logging.in
>> @@ -351,7 +351,7 @@ function test_python_disable_all()
>>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME
>>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
>>
>> -       disable_python_lttng_event $SESSION_NAME '*'
>> +       disable_python_lttng_event $SESSION_NAME -a
>>
>>         start_lttng_tracing_ok $SESSION_NAME
>>
>> --
>> 2.1.4
>>
>
>
>
> --
> J?r?mie Galarneau
> EfficiOS Inc.
> http://www.efficios.com



-- 
J?r?mie Galarneau
EfficiOS Inc.
http://www.efficios.com



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

* [lttng-dev] [PATCH lttng-tools 1/3] Fix: disable kernel event based on name and event type
       [not found] <1442522277-31518-1-git-send-email-jonathan.rajotte-julien@efficios.com>
       [not found] ` <CA+jJMxscPwme95++cjrQU3eBPB8Tw88SDOQ0+rXrfvJS6t_gAw@mail.gmail.com>
@ 2015-09-21 19:00 ` Jérémie Galarneau
  2015-09-21 19:15   ` Jérémie Galarneau
  2015-09-21 22:32   ` Jonathan Rajotte Julien
  2015-09-21 19:29 ` Jérémie Galarneau
  2 siblings, 2 replies; 10+ messages in thread
From: Jérémie Galarneau @ 2015-09-21 19:00 UTC (permalink / raw)


On Thu, Sep 17, 2015 at 4:37 PM, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> The -a argument is interpreted as a zero-length event name
> instead of '*' which is actually a valid wildcard event
> name by itself. This simplify how a disable action is handled.
>
> The event type can now be passed as argument
> and is a new criteria while disabling kernel events.
> The default is to disable for all event type.
>
> UST and agent domain do not support yet disabling by event
> type.
>
> e.g:
>         # Only disable kernel event of type tracepoint.
>         lttng disable -a -k --tracepoint
>
>         # Only disable the event with name '*' and type syscall.
>         lttng disable -k '*' --syscall
>
>         # Disable all kernel event of all type.
>         lttng disable -a -k
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/cmd.c                       | 45 +++++-----
>  src/bin/lttng-sessiond/event.c                     | 65 +++++---------
>  src/bin/lttng-sessiond/event.h                     |  5 +-
>  src/bin/lttng/commands/disable_events.c            | 98 ++++++++++++++++++----
>  src/lib/lttng-ctl/lttng-ctl.c                      |  4 -
>  .../ust/python-logging/test_python_logging.in      |  2 +-
>  6 files changed, 121 insertions(+), 98 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
> index 7878a05..5a1cc7a 100644
> --- a/src/bin/lttng-sessiond/cmd.c
> +++ b/src/bin/lttng-sessiond/cmd.c
> @@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 switch (event->type) {
>                 case LTTNG_EVENT_ALL:
> -                       ret = event_kernel_disable_event_all(kchan);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
> -               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
> +               case LTTNG_EVENT_TRACEPOINT:
>                 case LTTNG_EVENT_SYSCALL:
> -                       if (!strcmp(event_name, "*")) {
> -                               ret = event_kernel_disable_event_type(kchan,
> -                                       event->type);
> +               case LTTNG_EVENT_PROBE:
> +               case LTTNG_EVENT_FUNCTION:
> +               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
> +                       if (event_name[0] == '\0') {
> +                               ret = event_kernel_disable_event(kchan,
> +                                       NULL, event->type);
>                         } else {
>                                 ret = event_kernel_disable_event(kchan,
> -                                       event_name);
> +                                       event_name, event->type);
>                         }
>                         if (ret != LTTNG_OK) {
>                                 goto error_unlock;
>                         }
>                         break;
> -               case LTTNG_EVENT_PROBE:
> -               case LTTNG_EVENT_FUNCTION:
> -               case LTTNG_EVENT_FUNCTION_ENTRY:
> -                       ret = event_kernel_disable_event(kchan, event_name);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
>                 default:
>                         ret = LTTNG_ERR_UNK;
>                         goto error_unlock;
> @@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 /*
>                  * If a non-default channel has been created in the
> -                * session, explicitely require that -c chan_name needs
> +                * session, explicitly require that -c chan_name needs
>                  * to be provided.
>                  */
>                 if (usess->has_non_default_channel && channel_name[0] == '\0') {
> @@ -1290,10 +1280,12 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 switch (event->type) {
>                 case LTTNG_EVENT_ALL:
> -                       if (strlen(event->name) == 1 &&
> -                                       !strncmp(event->name, "*", 1)) {
> -                               ret = event_ust_disable_all_tracepoints(usess,
> -                                               uchan);
> +                       /*
> +                        * An empty event name means that everything
> +                        * should be disabled.
> +                        */
> +                       if (event->name[0] == '\0') {
> +                               ret = event_ust_disable_all_tracepoints(usess, uchan);
>                         } else {
>                                 ret = event_ust_disable_tracepoint(usess, uchan,
>                                                 event_name);

Please move this hunk and the following one to separate patches as
they fix the same problem for user space and agent domains. This
commit's title implies that it only addresses the issue in the kernel
domain.

> @@ -1333,8 +1325,11 @@ int cmd_disable_event(struct ltt_session *session,
>                         ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
>                         goto error_unlock;
>                 }
> -               /* The wild card * means that everything should be disabled. */
> -               if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
> +               /*
> +                * An empty event name means that everything
> +                * should be disabled.
> +                */
> +               if (event->name[0] == '\0') {
>                         ret = event_agent_disable_all(usess, agt);
>                 } else {
>                         ret = event_agent_disable(usess, agt, event_name);
> diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
> index 13f09a5..a53d4e9 100644
> --- a/src/bin/lttng-sessiond/event.c
> +++ b/src/bin/lttng-sessiond/event.c
> @@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
>  }
>
>  /*
> - * Disable kernel tracepoint event for a channel from the kernel session.
> + * Disable kernel tracepoint events for a channel from the kernel session of
> + * a specified event_name and event type.
> + * On type LTTNG_EVENT_ALL all events with event_name are disabled.
> + * If event_name if NULL all events of the specified type are disabled.

if NULL -> is NULL

>   */
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name)
> -{
> -       int ret;
> -       struct ltt_kernel_event *kevent;
> -
> -       assert(kchan);
> -
> -       kevent = trace_kernel_get_event_by_name(event_name, kchan,
> -                       LTTNG_EVENT_ALL);
> -       if (kevent == NULL) {
> -               ret = LTTNG_ERR_NO_EVENT;
> -               goto error;
> -       }
> -
> -       ret = kernel_disable_event(kevent);
> -       if (ret < 0) {
> -               ret = LTTNG_ERR_KERN_DISABLE_FAIL;
> -               goto error;
> -       }
> -
> -       DBG("Kernel event %s disable for channel %s.",
> -                       kevent->event->name, kchan->channel->name);
> -
> -       ret = LTTNG_OK;
> -
> -error:
> -       return ret;
> -}
> -
> -/*
> - * Disable kernel tracepoint events for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type)
> +               char *event_name, enum lttng_event_type type)
>  {
> -       int ret;
> +       int ret, error = 0, found = 0;
>         struct ltt_kernel_event *kevent;
>
>         assert(kchan);
> @@ -108,22 +78,25 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>         cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
>                 if (type != LTTNG_EVENT_ALL && kevent->type != type)
>                         continue;
> +               if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
> +                       continue;
> +               }
> +               found++;
>                 ret = kernel_disable_event(kevent);
>                 if (ret < 0) {
> -                       /* We continue disabling the rest */
> +                       error = 1;
>                         continue;
>                 }
>         }
> -       ret = LTTNG_OK;
> -       return ret;
> -}
> +       DBG("Disable kernel event: found %d events with name: %s and type: %d",
> +                       found, event_name, type);

event_name could be NULL here.

event_name ? event_name : "NULL"

>
> -/*
> - * Disable all kernel event for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
> -{
> -       return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
> +       if (!found && event_name != NULL) {
> +               error = 1;
> +       }
> +
> +       ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
> +       return ret;

This should distinguish between:

1) event found, but we failed to disable it (LTTNG_ERR_KERN_DISABLE_FAIL)
2) event_name != NULL && !found (LTTNG_ERR_NO_EVENT, which it returned before)

>  }
>
>  /*
> diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
> index 7c5231d..45dd1fc 100644
> --- a/src/bin/lttng-sessiond/event.h
> +++ b/src/bin/lttng-sessiond/event.h
> @@ -23,10 +23,7 @@
>  struct agent;
>
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name);
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type);
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
> +               char *event_name, enum lttng_event_type event_type);
>
>  int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
>                 struct lttng_event *event, char *filter_expression,
> diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
> index c6ec85f..bcbc78d 100644
> --- a/src/bin/lttng/commands/disable_events.c
> +++ b/src/bin/lttng/commands/disable_events.c
> @@ -44,7 +44,12 @@ static int opt_event_type;
>  enum {
>         OPT_HELP = 1,
>         OPT_USERSPACE,
> -       OPT_SYSCALL,
> +       OPT_TYPE_SYSCALL,
> +       OPT_TYPE_TRACEPOINT,
> +       OPT_TYPE_PROBE,
> +       OPT_TYPE_FUNCTION,
> +       OPT_TYPE_FUNCTION_ENTRY,
> +       OPT_TYPE_ALL,
>         OPT_LIST_OPTIONS,
>  };
>
> @@ -61,8 +66,24 @@ static struct poptOption long_options[] = {
>         {"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
>         {"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
>         {"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
> -       {"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
> +       {"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
> +       {"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
> +       {"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
> +       {"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
> +       {"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
> +#if 0
> +       /*
> +        * Function entry exits as a lttng_event_type but is never used on
> +        * enable
> +        */
> +       {"function-entry",   0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION_ENTRY, 0, 0},
> +
> +       /* Not implemented yet */
> +       {"userspace",      'u', POPT_ARG_STRING | POPT_ARGFLAG_OPTIONAL, &opt_cmd_name, OPT_USERSPACE, 0, 0},
> +       {"pid",            'p', POPT_ARG_INT, &opt_pid, 0, 0, 0},
> +#else
>         {"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
> +#endif

I have removed #ifdef-ed out code from disable-event in

commit 3fd691fc09199be5a52009a391b22379477aa6e7
Author: J?r?mie Galarneau <jeremie.galarneau at efficios.com>
Date:   Tue Sep 15 13:00:40 2015 -0400

    Remove dead code from disable-event command

    Signed-off-by: J?r?mie Galarneau <jeremie.galarneau at efficios.com>

function-entry instrumentation is simply not supported, so let's
mention it in the switch case that handles the instrumentation type,
but not add compiled-out code.

>         {"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
>         {0, 0, 0, 0, 0, 0, 0}
>  };
> @@ -86,8 +107,19 @@ static void usage(FILE *ofp)
>         fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
>         fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
>         fprintf(ofp, "\n");
> -       fprintf(ofp, "Event options:\n");
> +       fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
> +       fprintf(ofp, "      --all                All event type (default)\n");

type -> types

> +       fprintf(ofp, "      --tracepoint         Tracepoint event\n");
>         fprintf(ofp, "      --syscall            System call event\n");
> +       fprintf(ofp, "      --probe              Probe event\n");
> +       fprintf(ofp, "      --function           Function event\n");
> +#if 0
> +       /*
> +        * Function entry exits as a lttng_event_type but is never used on
> +        * enable
> +        */
> +       fprintf(ofp, "      --function-entry     Function-entry event\n");
> +#endif

Same here; no ifdef-ed out code.

>         fprintf(ofp, "\n");
>  }
>
> @@ -103,6 +135,27 @@ const char *print_raw_channel_name(const char *name)
>         return name ? : "<default>";
>  }
>
> +static
> +const char *print_event_type(const enum lttng_event_type ev_type)
> +{
> +       switch (ev_type) {
> +       case LTTNG_EVENT_ALL:
> +               return "any";
> +       case LTTNG_EVENT_TRACEPOINT:
> +               return "tracepoint";
> +       case LTTNG_EVENT_PROBE:
> +               return "probe";
> +       case LTTNG_EVENT_FUNCTION:
> +               return "function";
> +       case LTTNG_EVENT_FUNCTION_ENTRY:
> +               return "function entry";
> +       case LTTNG_EVENT_SYSCALL:
> +               return "syscall";
> +       default:
> +               return "";
> +       }
> +}
> +
>  /* Mi print a partial event.
>   * enabled is 0 or 1
>   * success is 0 or 1
> @@ -213,14 +266,8 @@ static int disable_events(char *session_name)
>         /* Set default loglevel to any/unknown */
>         event.loglevel = -1;
>
> -       switch (opt_event_type) {
> -       case LTTNG_EVENT_SYSCALL:
> -               event.type = LTTNG_EVENT_SYSCALL;
> -               break;
> -       default:
> -               event.type = LTTNG_EVENT_ALL;
> -               break;
> -       }
> +       /* opt_event_type contain the event type to disable at this point */
> +       event.type = opt_event_type;
>
>         if (opt_disable_all) {
>                 command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
> @@ -232,9 +279,9 @@ static int disable_events(char *session_name)
>                 } else {
>                         enabled = 0;
>                         success = 1;
> -                       MSG("All %s %s are disabled in channel %s",
> +                       MSG("All %s events of type %s are disabled in channel %s",
>                                         get_domain_str(dom.type),
> -                                       opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
> +                                       print_event_type(opt_event_type),
>                                         print_channel_name(channel_name));
>                 }
>
> @@ -255,9 +302,9 @@ static int disable_events(char *session_name)
>                         event.name[sizeof(event.name) - 1] = '\0';
>                         command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>                         if (command_ret < 0) {
> -                               ERR("%s %s: %s (channel %s, session %s)",
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
> +                               ERR("%s of type %s : %s (channel %s, session %s)",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 lttng_strerror(command_ret),
>                                                 command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
>                                                         ? print_raw_channel_name(channel_name)
> @@ -271,10 +318,10 @@ static int disable_events(char *session_name)
>                                  */
>                                 enabled = 1;
>                         } else {
> -                               MSG("%s %s %s disabled in channel %s for session %s",
> +                               MSG("%s %s of type %s disabled in channel %s for session %s",
>                                                 get_domain_str(dom.type),
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 print_channel_name(channel_name),
>                                                 session_name);
>                                 success = 1;
> @@ -341,9 +388,24 @@ int cmd_disable_events(int argc, const char **argv)
>                 case OPT_USERSPACE:
>                         opt_userspace = 1;
>                         break;
> -               case OPT_SYSCALL:
> +               case OPT_TYPE_SYSCALL:
>                         opt_event_type = LTTNG_EVENT_SYSCALL;
>                         break;
> +               case OPT_TYPE_TRACEPOINT:
> +                       opt_event_type = LTTNG_EVENT_TRACEPOINT;
> +                       break;
> +               case OPT_TYPE_PROBE:
> +                       opt_event_type = LTTNG_EVENT_PROBE;
> +                       break;
> +               case OPT_TYPE_FUNCTION:
> +                       opt_event_type = LTTNG_EVENT_FUNCTION;
> +                       break;
> +               case OPT_TYPE_FUNCTION_ENTRY:
> +                       opt_event_type = LTTNG_EVENT_FUNCTION_ENTRY;
> +                       break;

We shouldn't allow this (FUNCTION and FUNCTION_ENTRY) for non-kernel
domains as their support is not implemented.

> +               case OPT_TYPE_ALL:
> +                       opt_event_type = LTTNG_EVENT_ALL;
> +                       break;
>                 case OPT_LIST_OPTIONS:
>                         list_cmd_options(stdout, long_options);
>                         goto end;
> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
> index 9cbfef5..1007326 100644
> --- a/src/lib/lttng-ctl/lttng-ctl.c
> +++ b/src/lib/lttng-ctl/lttng-ctl.c
> @@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
>         }
>
>         lsm.cmd_type = LTTNG_DISABLE_EVENT;
> -       if (ev->name[0] == '\0') {
> -               /* Disable all events */
> -               lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
> -       }
>
>         lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
>         /* FIXME: copying non-packed struct to packed struct. */
> diff --git a/tests/regression/ust/python-logging/test_python_logging.in b/tests/regression/ust/python-logging/test_python_logging.in
> index 8aff408..d5a9b80 100755
> --- a/tests/regression/ust/python-logging/test_python_logging.in
> +++ b/tests/regression/ust/python-logging/test_python_logging.in
> @@ -351,7 +351,7 @@ function test_python_disable_all()
>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME
>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
>
> -       disable_python_lttng_event $SESSION_NAME '*'
> +       disable_python_lttng_event $SESSION_NAME -a

Please submit a separate patch for this.

Thanks!
J?r?mie

>
>         start_lttng_tracing_ok $SESSION_NAME
>
> --
> 2.1.4
>



-- 
J?r?mie Galarneau
EfficiOS Inc.
http://www.efficios.com



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

* [lttng-dev] [PATCH lttng-tools 1/3] Fix: disable kernel event based on name and event type
  2015-09-21 19:00 ` Jérémie Galarneau
@ 2015-09-21 19:15   ` Jérémie Galarneau
  2015-09-21 22:32   ` Jonathan Rajotte Julien
  1 sibling, 0 replies; 10+ messages in thread
From: Jérémie Galarneau @ 2015-09-21 19:15 UTC (permalink / raw)


On Mon, Sep 21, 2015 at 3:00 PM, J?r?mie Galarneau
<jeremie.galarneau at efficios.com> wrote:
> On Thu, Sep 17, 2015 at 4:37 PM, Jonathan Rajotte
> <jonathan.rajotte-julien at efficios.com> wrote:
>> The -a argument is interpreted as a zero-length event name
>> instead of '*' which is actually a valid wildcard event
>> name by itself. This simplify how a disable action is handled.
>>
>> The event type can now be passed as argument
>> and is a new criteria while disabling kernel events.
>> The default is to disable for all event type.
>>
>> UST and agent domain do not support yet disabling by event
>> type.
>>
>> e.g:
>>         # Only disable kernel event of type tracepoint.
>>         lttng disable -a -k --tracepoint
>>
>>         # Only disable the event with name '*' and type syscall.
>>         lttng disable -k '*' --syscall
>>
>>         # Disable all kernel event of all type.
>>         lttng disable -a -k
>>
>> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
>> ---
>>  src/bin/lttng-sessiond/cmd.c                       | 45 +++++-----
>>  src/bin/lttng-sessiond/event.c                     | 65 +++++---------
>>  src/bin/lttng-sessiond/event.h                     |  5 +-
>>  src/bin/lttng/commands/disable_events.c            | 98 ++++++++++++++++++----
>>  src/lib/lttng-ctl/lttng-ctl.c                      |  4 -
>>  .../ust/python-logging/test_python_logging.in      |  2 +-
>>  6 files changed, 121 insertions(+), 98 deletions(-)
>>
>> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
>> index 7878a05..5a1cc7a 100644
>> --- a/src/bin/lttng-sessiond/cmd.c
>> +++ b/src/bin/lttng-sessiond/cmd.c
>> @@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                 switch (event->type) {
>>                 case LTTNG_EVENT_ALL:
>> -                       ret = event_kernel_disable_event_all(kchan);
>> -                       if (ret != LTTNG_OK) {
>> -                               goto error_unlock;
>> -                       }
>> -                       break;
>> -               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
>> +               case LTTNG_EVENT_TRACEPOINT:
>>                 case LTTNG_EVENT_SYSCALL:
>> -                       if (!strcmp(event_name, "*")) {
>> -                               ret = event_kernel_disable_event_type(kchan,
>> -                                       event->type);
>> +               case LTTNG_EVENT_PROBE:
>> +               case LTTNG_EVENT_FUNCTION:
>> +               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
>> +                       if (event_name[0] == '\0') {
>> +                               ret = event_kernel_disable_event(kchan,
>> +                                       NULL, event->type);
>>                         } else {
>>                                 ret = event_kernel_disable_event(kchan,
>> -                                       event_name);
>> +                                       event_name, event->type);
>>                         }
>>                         if (ret != LTTNG_OK) {
>>                                 goto error_unlock;
>>                         }
>>                         break;
>> -               case LTTNG_EVENT_PROBE:
>> -               case LTTNG_EVENT_FUNCTION:
>> -               case LTTNG_EVENT_FUNCTION_ENTRY:
>> -                       ret = event_kernel_disable_event(kchan, event_name);
>> -                       if (ret != LTTNG_OK) {
>> -                               goto error_unlock;
>> -                       }
>> -                       break;
>>                 default:
>>                         ret = LTTNG_ERR_UNK;
>>                         goto error_unlock;
>> @@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                 /*
>>                  * If a non-default channel has been created in the
>> -                * session, explicitely require that -c chan_name needs
>> +                * session, explicitly require that -c chan_name needs
>>                  * to be provided.
>>                  */
>>                 if (usess->has_non_default_channel && channel_name[0] == '\0') {
>> @@ -1290,10 +1280,12 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                 switch (event->type) {
>>                 case LTTNG_EVENT_ALL:
>> -                       if (strlen(event->name) == 1 &&
>> -                                       !strncmp(event->name, "*", 1)) {
>> -                               ret = event_ust_disable_all_tracepoints(usess,
>> -                                               uchan);
>> +                       /*
>> +                        * An empty event name means that everything
>> +                        * should be disabled.
>> +                        */
>> +                       if (event->name[0] == '\0') {
>> +                               ret = event_ust_disable_all_tracepoints(usess, uchan);
>>                         } else {
>>                                 ret = event_ust_disable_tracepoint(usess, uchan,
>>                                                 event_name);
>
> Please move this hunk and the following one to separate patches as
> they fix the same problem for user space and agent domains. This
> commit's title implies that it only addresses the issue in the kernel
> domain.
>
>> @@ -1333,8 +1325,11 @@ int cmd_disable_event(struct ltt_session *session,
>>                         ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
>>                         goto error_unlock;
>>                 }
>> -               /* The wild card * means that everything should be disabled. */
>> -               if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
>> +               /*
>> +                * An empty event name means that everything
>> +                * should be disabled.
>> +                */
>> +               if (event->name[0] == '\0') {
>>                         ret = event_agent_disable_all(usess, agt);
>>                 } else {
>>                         ret = event_agent_disable(usess, agt, event_name);
>> diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
>> index 13f09a5..a53d4e9 100644
>> --- a/src/bin/lttng-sessiond/event.c
>> +++ b/src/bin/lttng-sessiond/event.c
>> @@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
>>  }
>>
>>  /*
>> - * Disable kernel tracepoint event for a channel from the kernel session.
>> + * Disable kernel tracepoint events for a channel from the kernel session of
>> + * a specified event_name and event type.
>> + * On type LTTNG_EVENT_ALL all events with event_name are disabled.
>> + * If event_name if NULL all events of the specified type are disabled.
>
> if NULL -> is NULL
>
>>   */
>>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
>> -               char *event_name)
>> -{
>> -       int ret;
>> -       struct ltt_kernel_event *kevent;
>> -
>> -       assert(kchan);
>> -
>> -       kevent = trace_kernel_get_event_by_name(event_name, kchan,
>> -                       LTTNG_EVENT_ALL);
>> -       if (kevent == NULL) {
>> -               ret = LTTNG_ERR_NO_EVENT;
>> -               goto error;
>> -       }
>> -
>> -       ret = kernel_disable_event(kevent);
>> -       if (ret < 0) {
>> -               ret = LTTNG_ERR_KERN_DISABLE_FAIL;
>> -               goto error;
>> -       }
>> -
>> -       DBG("Kernel event %s disable for channel %s.",
>> -                       kevent->event->name, kchan->channel->name);
>> -
>> -       ret = LTTNG_OK;
>> -
>> -error:
>> -       return ret;
>> -}
>> -
>> -/*
>> - * Disable kernel tracepoint events for a channel from the kernel session.
>> - */
>> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>> -               enum lttng_event_type type)
>> +               char *event_name, enum lttng_event_type type)
>>  {
>> -       int ret;
>> +       int ret, error = 0, found = 0;
>>         struct ltt_kernel_event *kevent;
>>
>>         assert(kchan);
>> @@ -108,22 +78,25 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>>         cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
>>                 if (type != LTTNG_EVENT_ALL && kevent->type != type)
>>                         continue;
>> +               if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
>> +                       continue;
>> +               }
>> +               found++;
>>                 ret = kernel_disable_event(kevent);
>>                 if (ret < 0) {
>> -                       /* We continue disabling the rest */
>> +                       error = 1;
>>                         continue;
>>                 }
>>         }
>> -       ret = LTTNG_OK;
>> -       return ret;
>> -}
>> +       DBG("Disable kernel event: found %d events with name: %s and type: %d",
>> +                       found, event_name, type);
>
> event_name could be NULL here.
>
> event_name ? event_name : "NULL"
>
>>
>> -/*
>> - * Disable all kernel event for a channel from the kernel session.
>> - */
>> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
>> -{
>> -       return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
>> +       if (!found && event_name != NULL) {
>> +               error = 1;
>> +       }
>> +
>> +       ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
>> +       return ret;
>
> This should distinguish between:
>
> 1) event found, but we failed to disable it (LTTNG_ERR_KERN_DISABLE_FAIL)
> 2) event_name != NULL && !found (LTTNG_ERR_NO_EVENT, which it returned before)
>
>>  }
>>
>>  /*
>> diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
>> index 7c5231d..45dd1fc 100644
>> --- a/src/bin/lttng-sessiond/event.h
>> +++ b/src/bin/lttng-sessiond/event.h
>> @@ -23,10 +23,7 @@
>>  struct agent;
>>
>>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
>> -               char *event_name);
>> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>> -               enum lttng_event_type type);
>> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
>> +               char *event_name, enum lttng_event_type event_type);
>>
>>  int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
>>                 struct lttng_event *event, char *filter_expression,
>> diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
>> index c6ec85f..bcbc78d 100644
>> --- a/src/bin/lttng/commands/disable_events.c
>> +++ b/src/bin/lttng/commands/disable_events.c
>> @@ -44,7 +44,12 @@ static int opt_event_type;
>>  enum {
>>         OPT_HELP = 1,
>>         OPT_USERSPACE,
>> -       OPT_SYSCALL,
>> +       OPT_TYPE_SYSCALL,
>> +       OPT_TYPE_TRACEPOINT,
>> +       OPT_TYPE_PROBE,
>> +       OPT_TYPE_FUNCTION,
>> +       OPT_TYPE_FUNCTION_ENTRY,
>> +       OPT_TYPE_ALL,
>>         OPT_LIST_OPTIONS,
>>  };
>>
>> @@ -61,8 +66,24 @@ static struct poptOption long_options[] = {
>>         {"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
>>         {"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
>>         {"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
>> -       {"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
>> +       {"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
>> +       {"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
>> +       {"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
>> +       {"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
>> +       {"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
>> +#if 0
>> +       /*
>> +        * Function entry exits as a lttng_event_type but is never used on
>> +        * enable
>> +        */
>> +       {"function-entry",   0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION_ENTRY, 0, 0},
>> +
>> +       /* Not implemented yet */
>> +       {"userspace",      'u', POPT_ARG_STRING | POPT_ARGFLAG_OPTIONAL, &opt_cmd_name, OPT_USERSPACE, 0, 0},
>> +       {"pid",            'p', POPT_ARG_INT, &opt_pid, 0, 0, 0},
>> +#else
>>         {"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
>> +#endif
>
> I have removed #ifdef-ed out code from disable-event in
>
> commit 3fd691fc09199be5a52009a391b22379477aa6e7
> Author: J?r?mie Galarneau <jeremie.galarneau at efficios.com>
> Date:   Tue Sep 15 13:00:40 2015 -0400
>
>     Remove dead code from disable-event command
>
>     Signed-off-by: J?r?mie Galarneau <jeremie.galarneau at efficios.com>
>
> function-entry instrumentation is simply not supported, so let's
> mention it in the switch case that handles the instrumentation type,
> but not add compiled-out code.
>
>>         {"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
>>         {0, 0, 0, 0, 0, 0, 0}
>>  };
>> @@ -86,8 +107,19 @@ static void usage(FILE *ofp)
>>         fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
>>         fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
>>         fprintf(ofp, "\n");
>> -       fprintf(ofp, "Event options:\n");
>> +       fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
>> +       fprintf(ofp, "      --all                All event type (default)\n");
>
> type -> types
>
>> +       fprintf(ofp, "      --tracepoint         Tracepoint event\n");
>>         fprintf(ofp, "      --syscall            System call event\n");
>> +       fprintf(ofp, "      --probe              Probe event\n");
>> +       fprintf(ofp, "      --function           Function event\n");
>> +#if 0
>> +       /*
>> +        * Function entry exits as a lttng_event_type but is never used on
>> +        * enable
>> +        */
>> +       fprintf(ofp, "      --function-entry     Function-entry event\n");
>> +#endif
>
> Same here; no ifdef-ed out code.
>
>>         fprintf(ofp, "\n");
>>  }
>>
>> @@ -103,6 +135,27 @@ const char *print_raw_channel_name(const char *name)
>>         return name ? : "<default>";
>>  }
>>
>> +static
>> +const char *print_event_type(const enum lttng_event_type ev_type)
>> +{
>> +       switch (ev_type) {
>> +       case LTTNG_EVENT_ALL:
>> +               return "any";
>> +       case LTTNG_EVENT_TRACEPOINT:
>> +               return "tracepoint";
>> +       case LTTNG_EVENT_PROBE:
>> +               return "probe";
>> +       case LTTNG_EVENT_FUNCTION:
>> +               return "function";
>> +       case LTTNG_EVENT_FUNCTION_ENTRY:
>> +               return "function entry";
>> +       case LTTNG_EVENT_SYSCALL:
>> +               return "syscall";
>> +       default:
>> +               return "";
>> +       }
>> +}
>> +
>>  /* Mi print a partial event.
>>   * enabled is 0 or 1
>>   * success is 0 or 1
>> @@ -213,14 +266,8 @@ static int disable_events(char *session_name)
>>         /* Set default loglevel to any/unknown */
>>         event.loglevel = -1;
>>
>> -       switch (opt_event_type) {
>> -       case LTTNG_EVENT_SYSCALL:
>> -               event.type = LTTNG_EVENT_SYSCALL;
>> -               break;
>> -       default:
>> -               event.type = LTTNG_EVENT_ALL;
>> -               break;
>> -       }
>> +       /* opt_event_type contain the event type to disable at this point */
>> +       event.type = opt_event_type;
>>
>>         if (opt_disable_all) {
>>                 command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>> @@ -232,9 +279,9 @@ static int disable_events(char *session_name)
>>                 } else {
>>                         enabled = 0;
>>                         success = 1;
>> -                       MSG("All %s %s are disabled in channel %s",
>> +                       MSG("All %s events of type %s are disabled in channel %s",
>>                                         get_domain_str(dom.type),
>> -                                       opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
>> +                                       print_event_type(opt_event_type),
>>                                         print_channel_name(channel_name));
>>                 }
>>
>> @@ -255,9 +302,9 @@ static int disable_events(char *session_name)
>>                         event.name[sizeof(event.name) - 1] = '\0';
>>                         command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>>                         if (command_ret < 0) {
>> -                               ERR("%s %s: %s (channel %s, session %s)",
>> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
>> +                               ERR("%s of type %s : %s (channel %s, session %s)",
>>                                                 event_name,
>> +                                               print_event_type(opt_event_type),
>>                                                 lttng_strerror(command_ret),
>>                                                 command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
>>                                                         ? print_raw_channel_name(channel_name)
>> @@ -271,10 +318,10 @@ static int disable_events(char *session_name)
>>                                  */
>>                                 enabled = 1;
>>                         } else {
>> -                               MSG("%s %s %s disabled in channel %s for session %s",
>> +                               MSG("%s %s of type %s disabled in channel %s for session %s",
>>                                                 get_domain_str(dom.type),
>> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
>>                                                 event_name,
>> +                                               print_event_type(opt_event_type),
>>                                                 print_channel_name(channel_name),
>>                                                 session_name);
>>                                 success = 1;
>> @@ -341,9 +388,24 @@ int cmd_disable_events(int argc, const char **argv)
>>                 case OPT_USERSPACE:
>>                         opt_userspace = 1;
>>                         break;
>> -               case OPT_SYSCALL:
>> +               case OPT_TYPE_SYSCALL:
>>                         opt_event_type = LTTNG_EVENT_SYSCALL;
>>                         break;
>> +               case OPT_TYPE_TRACEPOINT:
>> +                       opt_event_type = LTTNG_EVENT_TRACEPOINT;
>> +                       break;
>> +               case OPT_TYPE_PROBE:
>> +                       opt_event_type = LTTNG_EVENT_PROBE;
>> +                       break;
>> +               case OPT_TYPE_FUNCTION:
>> +                       opt_event_type = LTTNG_EVENT_FUNCTION;
>> +                       break;
>> +               case OPT_TYPE_FUNCTION_ENTRY:
>> +                       opt_event_type = LTTNG_EVENT_FUNCTION_ENTRY;
>> +                       break;
>
> We shouldn't allow this (FUNCTION and FUNCTION_ENTRY) for non-kernel
> domains as their support is not implemented.

Just checked and since we simply don't accept this switch on the CLI,
let's remove it from the switch and error on default.

J?r?mie

>
>> +               case OPT_TYPE_ALL:
>> +                       opt_event_type = LTTNG_EVENT_ALL;
>> +                       break;
>>                 case OPT_LIST_OPTIONS:
>>                         list_cmd_options(stdout, long_options);
>>                         goto end;
>> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
>> index 9cbfef5..1007326 100644
>> --- a/src/lib/lttng-ctl/lttng-ctl.c
>> +++ b/src/lib/lttng-ctl/lttng-ctl.c
>> @@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
>>         }
>>
>>         lsm.cmd_type = LTTNG_DISABLE_EVENT;
>> -       if (ev->name[0] == '\0') {
>> -               /* Disable all events */
>> -               lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
>> -       }
>>
>>         lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
>>         /* FIXME: copying non-packed struct to packed struct. */
>> diff --git a/tests/regression/ust/python-logging/test_python_logging.in b/tests/regression/ust/python-logging/test_python_logging.in
>> index 8aff408..d5a9b80 100755
>> --- a/tests/regression/ust/python-logging/test_python_logging.in
>> +++ b/tests/regression/ust/python-logging/test_python_logging.in
>> @@ -351,7 +351,7 @@ function test_python_disable_all()
>>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME
>>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
>>
>> -       disable_python_lttng_event $SESSION_NAME '*'
>> +       disable_python_lttng_event $SESSION_NAME -a
>
> Please submit a separate patch for this.
>
> Thanks!
> J?r?mie
>
>>
>>         start_lttng_tracing_ok $SESSION_NAME
>>
>> --
>> 2.1.4
>>
>
>
>
> --
> J?r?mie Galarneau
> EfficiOS Inc.
> http://www.efficios.com



-- 
J?r?mie Galarneau
EfficiOS Inc.
http://www.efficios.com



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

* [lttng-dev] [PATCH lttng-tools 1/3] Fix: disable kernel event based on name and event type
       [not found] <1442522277-31518-1-git-send-email-jonathan.rajotte-julien@efficios.com>
       [not found] ` <CA+jJMxscPwme95++cjrQU3eBPB8Tw88SDOQ0+rXrfvJS6t_gAw@mail.gmail.com>
  2015-09-21 19:00 ` Jérémie Galarneau
@ 2015-09-21 19:29 ` Jérémie Galarneau
  2015-09-21 22:43   ` [lttng-dev] [PATCH lttng-tools 1/4 v2] " Jonathan Rajotte
  2 siblings, 1 reply; 10+ messages in thread
From: Jérémie Galarneau @ 2015-09-21 19:29 UTC (permalink / raw)


This seems to address bug #925. Can you add a mention to your commit
message in v2?

Thanks!
J?r?mie

On Thu, Sep 17, 2015 at 4:37 PM, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> The -a argument is interpreted as a zero-length event name
> instead of '*' which is actually a valid wildcard event
> name by itself. This simplify how a disable action is handled.
>
> The event type can now be passed as argument
> and is a new criteria while disabling kernel events.
> The default is to disable for all event type.
>
> UST and agent domain do not support yet disabling by event
> type.
>
> e.g:
>         # Only disable kernel event of type tracepoint.
>         lttng disable -a -k --tracepoint
>
>         # Only disable the event with name '*' and type syscall.
>         lttng disable -k '*' --syscall
>
>         # Disable all kernel event of all type.
>         lttng disable -a -k
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/cmd.c                       | 45 +++++-----
>  src/bin/lttng-sessiond/event.c                     | 65 +++++---------
>  src/bin/lttng-sessiond/event.h                     |  5 +-
>  src/bin/lttng/commands/disable_events.c            | 98 ++++++++++++++++++----
>  src/lib/lttng-ctl/lttng-ctl.c                      |  4 -
>  .../ust/python-logging/test_python_logging.in      |  2 +-
>  6 files changed, 121 insertions(+), 98 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
> index 7878a05..5a1cc7a 100644
> --- a/src/bin/lttng-sessiond/cmd.c
> +++ b/src/bin/lttng-sessiond/cmd.c
> @@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 switch (event->type) {
>                 case LTTNG_EVENT_ALL:
> -                       ret = event_kernel_disable_event_all(kchan);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
> -               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
> +               case LTTNG_EVENT_TRACEPOINT:
>                 case LTTNG_EVENT_SYSCALL:
> -                       if (!strcmp(event_name, "*")) {
> -                               ret = event_kernel_disable_event_type(kchan,
> -                                       event->type);
> +               case LTTNG_EVENT_PROBE:
> +               case LTTNG_EVENT_FUNCTION:
> +               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
> +                       if (event_name[0] == '\0') {
> +                               ret = event_kernel_disable_event(kchan,
> +                                       NULL, event->type);
>                         } else {
>                                 ret = event_kernel_disable_event(kchan,
> -                                       event_name);
> +                                       event_name, event->type);
>                         }
>                         if (ret != LTTNG_OK) {
>                                 goto error_unlock;
>                         }
>                         break;
> -               case LTTNG_EVENT_PROBE:
> -               case LTTNG_EVENT_FUNCTION:
> -               case LTTNG_EVENT_FUNCTION_ENTRY:
> -                       ret = event_kernel_disable_event(kchan, event_name);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
>                 default:
>                         ret = LTTNG_ERR_UNK;
>                         goto error_unlock;
> @@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 /*
>                  * If a non-default channel has been created in the
> -                * session, explicitely require that -c chan_name needs
> +                * session, explicitly require that -c chan_name needs
>                  * to be provided.
>                  */
>                 if (usess->has_non_default_channel && channel_name[0] == '\0') {
> @@ -1290,10 +1280,12 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 switch (event->type) {
>                 case LTTNG_EVENT_ALL:
> -                       if (strlen(event->name) == 1 &&
> -                                       !strncmp(event->name, "*", 1)) {
> -                               ret = event_ust_disable_all_tracepoints(usess,
> -                                               uchan);
> +                       /*
> +                        * An empty event name means that everything
> +                        * should be disabled.
> +                        */
> +                       if (event->name[0] == '\0') {
> +                               ret = event_ust_disable_all_tracepoints(usess, uchan);
>                         } else {
>                                 ret = event_ust_disable_tracepoint(usess, uchan,
>                                                 event_name);
> @@ -1333,8 +1325,11 @@ int cmd_disable_event(struct ltt_session *session,
>                         ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
>                         goto error_unlock;
>                 }
> -               /* The wild card * means that everything should be disabled. */
> -               if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
> +               /*
> +                * An empty event name means that everything
> +                * should be disabled.
> +                */
> +               if (event->name[0] == '\0') {
>                         ret = event_agent_disable_all(usess, agt);
>                 } else {
>                         ret = event_agent_disable(usess, agt, event_name);
> diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
> index 13f09a5..a53d4e9 100644
> --- a/src/bin/lttng-sessiond/event.c
> +++ b/src/bin/lttng-sessiond/event.c
> @@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
>  }
>
>  /*
> - * Disable kernel tracepoint event for a channel from the kernel session.
> + * Disable kernel tracepoint events for a channel from the kernel session of
> + * a specified event_name and event type.
> + * On type LTTNG_EVENT_ALL all events with event_name are disabled.
> + * If event_name if NULL all events of the specified type are disabled.
>   */
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name)
> -{
> -       int ret;
> -       struct ltt_kernel_event *kevent;
> -
> -       assert(kchan);
> -
> -       kevent = trace_kernel_get_event_by_name(event_name, kchan,
> -                       LTTNG_EVENT_ALL);
> -       if (kevent == NULL) {
> -               ret = LTTNG_ERR_NO_EVENT;
> -               goto error;
> -       }
> -
> -       ret = kernel_disable_event(kevent);
> -       if (ret < 0) {
> -               ret = LTTNG_ERR_KERN_DISABLE_FAIL;
> -               goto error;
> -       }
> -
> -       DBG("Kernel event %s disable for channel %s.",
> -                       kevent->event->name, kchan->channel->name);
> -
> -       ret = LTTNG_OK;
> -
> -error:
> -       return ret;
> -}
> -
> -/*
> - * Disable kernel tracepoint events for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type)
> +               char *event_name, enum lttng_event_type type)
>  {
> -       int ret;
> +       int ret, error = 0, found = 0;
>         struct ltt_kernel_event *kevent;
>
>         assert(kchan);
> @@ -108,22 +78,25 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>         cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
>                 if (type != LTTNG_EVENT_ALL && kevent->type != type)
>                         continue;
> +               if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
> +                       continue;
> +               }
> +               found++;
>                 ret = kernel_disable_event(kevent);
>                 if (ret < 0) {
> -                       /* We continue disabling the rest */
> +                       error = 1;
>                         continue;
>                 }
>         }
> -       ret = LTTNG_OK;
> -       return ret;
> -}
> +       DBG("Disable kernel event: found %d events with name: %s and type: %d",
> +                       found, event_name, type);
>
> -/*
> - * Disable all kernel event for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
> -{
> -       return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
> +       if (!found && event_name != NULL) {
> +               error = 1;
> +       }
> +
> +       ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
> +       return ret;
>  }
>
>  /*
> diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
> index 7c5231d..45dd1fc 100644
> --- a/src/bin/lttng-sessiond/event.h
> +++ b/src/bin/lttng-sessiond/event.h
> @@ -23,10 +23,7 @@
>  struct agent;
>
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name);
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type);
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
> +               char *event_name, enum lttng_event_type event_type);
>
>  int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
>                 struct lttng_event *event, char *filter_expression,
> diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
> index c6ec85f..bcbc78d 100644
> --- a/src/bin/lttng/commands/disable_events.c
> +++ b/src/bin/lttng/commands/disable_events.c
> @@ -44,7 +44,12 @@ static int opt_event_type;
>  enum {
>         OPT_HELP = 1,
>         OPT_USERSPACE,
> -       OPT_SYSCALL,
> +       OPT_TYPE_SYSCALL,
> +       OPT_TYPE_TRACEPOINT,
> +       OPT_TYPE_PROBE,
> +       OPT_TYPE_FUNCTION,
> +       OPT_TYPE_FUNCTION_ENTRY,
> +       OPT_TYPE_ALL,
>         OPT_LIST_OPTIONS,
>  };
>
> @@ -61,8 +66,24 @@ static struct poptOption long_options[] = {
>         {"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
>         {"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
>         {"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
> -       {"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
> +       {"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
> +       {"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
> +       {"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
> +       {"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
> +       {"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
> +#if 0
> +       /*
> +        * Function entry exits as a lttng_event_type but is never used on
> +        * enable
> +        */
> +       {"function-entry",   0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION_ENTRY, 0, 0},
> +
> +       /* Not implemented yet */
> +       {"userspace",      'u', POPT_ARG_STRING | POPT_ARGFLAG_OPTIONAL, &opt_cmd_name, OPT_USERSPACE, 0, 0},
> +       {"pid",            'p', POPT_ARG_INT, &opt_pid, 0, 0, 0},
> +#else
>         {"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
> +#endif
>         {"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
>         {0, 0, 0, 0, 0, 0, 0}
>  };
> @@ -86,8 +107,19 @@ static void usage(FILE *ofp)
>         fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
>         fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
>         fprintf(ofp, "\n");
> -       fprintf(ofp, "Event options:\n");
> +       fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
> +       fprintf(ofp, "      --all                All event type (default)\n");
> +       fprintf(ofp, "      --tracepoint         Tracepoint event\n");
>         fprintf(ofp, "      --syscall            System call event\n");
> +       fprintf(ofp, "      --probe              Probe event\n");
> +       fprintf(ofp, "      --function           Function event\n");
> +#if 0
> +       /*
> +        * Function entry exits as a lttng_event_type but is never used on
> +        * enable
> +        */
> +       fprintf(ofp, "      --function-entry     Function-entry event\n");
> +#endif
>         fprintf(ofp, "\n");
>  }
>
> @@ -103,6 +135,27 @@ const char *print_raw_channel_name(const char *name)
>         return name ? : "<default>";
>  }
>
> +static
> +const char *print_event_type(const enum lttng_event_type ev_type)
> +{
> +       switch (ev_type) {
> +       case LTTNG_EVENT_ALL:
> +               return "any";
> +       case LTTNG_EVENT_TRACEPOINT:
> +               return "tracepoint";
> +       case LTTNG_EVENT_PROBE:
> +               return "probe";
> +       case LTTNG_EVENT_FUNCTION:
> +               return "function";
> +       case LTTNG_EVENT_FUNCTION_ENTRY:
> +               return "function entry";
> +       case LTTNG_EVENT_SYSCALL:
> +               return "syscall";
> +       default:
> +               return "";
> +       }
> +}
> +
>  /* Mi print a partial event.
>   * enabled is 0 or 1
>   * success is 0 or 1
> @@ -213,14 +266,8 @@ static int disable_events(char *session_name)
>         /* Set default loglevel to any/unknown */
>         event.loglevel = -1;
>
> -       switch (opt_event_type) {
> -       case LTTNG_EVENT_SYSCALL:
> -               event.type = LTTNG_EVENT_SYSCALL;
> -               break;
> -       default:
> -               event.type = LTTNG_EVENT_ALL;
> -               break;
> -       }
> +       /* opt_event_type contain the event type to disable at this point */
> +       event.type = opt_event_type;
>
>         if (opt_disable_all) {
>                 command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
> @@ -232,9 +279,9 @@ static int disable_events(char *session_name)
>                 } else {
>                         enabled = 0;
>                         success = 1;
> -                       MSG("All %s %s are disabled in channel %s",
> +                       MSG("All %s events of type %s are disabled in channel %s",
>                                         get_domain_str(dom.type),
> -                                       opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
> +                                       print_event_type(opt_event_type),
>                                         print_channel_name(channel_name));
>                 }
>
> @@ -255,9 +302,9 @@ static int disable_events(char *session_name)
>                         event.name[sizeof(event.name) - 1] = '\0';
>                         command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>                         if (command_ret < 0) {
> -                               ERR("%s %s: %s (channel %s, session %s)",
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
> +                               ERR("%s of type %s : %s (channel %s, session %s)",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 lttng_strerror(command_ret),
>                                                 command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
>                                                         ? print_raw_channel_name(channel_name)
> @@ -271,10 +318,10 @@ static int disable_events(char *session_name)
>                                  */
>                                 enabled = 1;
>                         } else {
> -                               MSG("%s %s %s disabled in channel %s for session %s",
> +                               MSG("%s %s of type %s disabled in channel %s for session %s",
>                                                 get_domain_str(dom.type),
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 print_channel_name(channel_name),
>                                                 session_name);
>                                 success = 1;
> @@ -341,9 +388,24 @@ int cmd_disable_events(int argc, const char **argv)
>                 case OPT_USERSPACE:
>                         opt_userspace = 1;
>                         break;
> -               case OPT_SYSCALL:
> +               case OPT_TYPE_SYSCALL:
>                         opt_event_type = LTTNG_EVENT_SYSCALL;
>                         break;
> +               case OPT_TYPE_TRACEPOINT:
> +                       opt_event_type = LTTNG_EVENT_TRACEPOINT;
> +                       break;
> +               case OPT_TYPE_PROBE:
> +                       opt_event_type = LTTNG_EVENT_PROBE;
> +                       break;
> +               case OPT_TYPE_FUNCTION:
> +                       opt_event_type = LTTNG_EVENT_FUNCTION;
> +                       break;
> +               case OPT_TYPE_FUNCTION_ENTRY:
> +                       opt_event_type = LTTNG_EVENT_FUNCTION_ENTRY;
> +                       break;
> +               case OPT_TYPE_ALL:
> +                       opt_event_type = LTTNG_EVENT_ALL;
> +                       break;
>                 case OPT_LIST_OPTIONS:
>                         list_cmd_options(stdout, long_options);
>                         goto end;
> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
> index 9cbfef5..1007326 100644
> --- a/src/lib/lttng-ctl/lttng-ctl.c
> +++ b/src/lib/lttng-ctl/lttng-ctl.c
> @@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
>         }
>
>         lsm.cmd_type = LTTNG_DISABLE_EVENT;
> -       if (ev->name[0] == '\0') {
> -               /* Disable all events */
> -               lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
> -       }
>
>         lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
>         /* FIXME: copying non-packed struct to packed struct. */
> diff --git a/tests/regression/ust/python-logging/test_python_logging.in b/tests/regression/ust/python-logging/test_python_logging.in
> index 8aff408..d5a9b80 100755
> --- a/tests/regression/ust/python-logging/test_python_logging.in
> +++ b/tests/regression/ust/python-logging/test_python_logging.in
> @@ -351,7 +351,7 @@ function test_python_disable_all()
>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME
>         enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
>
> -       disable_python_lttng_event $SESSION_NAME '*'
> +       disable_python_lttng_event $SESSION_NAME -a
>
>         start_lttng_tracing_ok $SESSION_NAME
>
> --
> 2.1.4
>



-- 
J?r?mie Galarneau
EfficiOS Inc.
http://www.efficios.com



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

* [lttng-dev] [PATCH lttng-tools 1/3] Fix: disable kernel event based on name and event type
  2015-09-21 19:00 ` Jérémie Galarneau
  2015-09-21 19:15   ` Jérémie Galarneau
@ 2015-09-21 22:32   ` Jonathan Rajotte Julien
  1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Rajotte Julien @ 2015-09-21 22:32 UTC (permalink / raw)




On 2015-09-21 03:00 PM, J?r?mie Galarneau wrote:
> On Thu, Sep 17, 2015 at 4:37 PM, Jonathan Rajotte
> <jonathan.rajotte-julien at efficios.com> wrote:
>> The -a argument is interpreted as a zero-length event name
>> instead of '*' which is actually a valid wildcard event
>> name by itself. This simplify how a disable action is handled.
>>
>> The event type can now be passed as argument
>> and is a new criteria while disabling kernel events.
>> The default is to disable for all event type.
>>
>> UST and agent domain do not support yet disabling by event
>> type.
>>
>> e.g:
>>          # Only disable kernel event of type tracepoint.
>>          lttng disable -a -k --tracepoint
>>
>>          # Only disable the event with name '*' and type syscall.
>>          lttng disable -k '*' --syscall
>>
>>          # Disable all kernel event of all type.
>>          lttng disable -a -k
>>
>> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
>> ---
>>   src/bin/lttng-sessiond/cmd.c                       | 45 +++++-----
>>   src/bin/lttng-sessiond/event.c                     | 65 +++++---------
>>   src/bin/lttng-sessiond/event.h                     |  5 +-
>>   src/bin/lttng/commands/disable_events.c            | 98 ++++++++++++++++++----
>>   src/lib/lttng-ctl/lttng-ctl.c                      |  4 -
>>   .../ust/python-logging/test_python_logging.in      |  2 +-
>>   6 files changed, 121 insertions(+), 98 deletions(-)
>>
>> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
>> index 7878a05..5a1cc7a 100644
>> --- a/src/bin/lttng-sessiond/cmd.c
>> +++ b/src/bin/lttng-sessiond/cmd.c
>> @@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                  switch (event->type) {
>>                  case LTTNG_EVENT_ALL:
>> -                       ret = event_kernel_disable_event_all(kchan);
>> -                       if (ret != LTTNG_OK) {
>> -                               goto error_unlock;
>> -                       }
>> -                       break;
>> -               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
>> +               case LTTNG_EVENT_TRACEPOINT:
>>                  case LTTNG_EVENT_SYSCALL:
>> -                       if (!strcmp(event_name, "*")) {
>> -                               ret = event_kernel_disable_event_type(kchan,
>> -                                       event->type);
>> +               case LTTNG_EVENT_PROBE:
>> +               case LTTNG_EVENT_FUNCTION:
>> +               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
>> +                       if (event_name[0] == '\0') {
>> +                               ret = event_kernel_disable_event(kchan,
>> +                                       NULL, event->type);
>>                          } else {
>>                                  ret = event_kernel_disable_event(kchan,
>> -                                       event_name);
>> +                                       event_name, event->type);
>>                          }
>>                          if (ret != LTTNG_OK) {
>>                                  goto error_unlock;
>>                          }
>>                          break;
>> -               case LTTNG_EVENT_PROBE:
>> -               case LTTNG_EVENT_FUNCTION:
>> -               case LTTNG_EVENT_FUNCTION_ENTRY:
>> -                       ret = event_kernel_disable_event(kchan, event_name);
>> -                       if (ret != LTTNG_OK) {
>> -                               goto error_unlock;
>> -                       }
>> -                       break;
>>                  default:
>>                          ret = LTTNG_ERR_UNK;
>>                          goto error_unlock;
>> @@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                  /*
>>                   * If a non-default channel has been created in the
>> -                * session, explicitely require that -c chan_name needs
>> +                * session, explicitly require that -c chan_name needs
>>                   * to be provided.
>>                   */
>>                  if (usess->has_non_default_channel && channel_name[0] == '\0') {
>> @@ -1290,10 +1280,12 @@ int cmd_disable_event(struct ltt_session *session,
>>
>>                  switch (event->type) {
>>                  case LTTNG_EVENT_ALL:
>> -                       if (strlen(event->name) == 1 &&
>> -                                       !strncmp(event->name, "*", 1)) {
>> -                               ret = event_ust_disable_all_tracepoints(usess,
>> -                                               uchan);
>> +                       /*
>> +                        * An empty event name means that everything
>> +                        * should be disabled.
>> +                        */
>> +                       if (event->name[0] == '\0') {
>> +                               ret = event_ust_disable_all_tracepoints(usess, uchan);
>>                          } else {
>>                                  ret = event_ust_disable_tracepoint(usess, uchan,
>>                                                  event_name);
> Please move this hunk and the following one to separate patches as
> they fix the same problem for user space and agent domains. This
> commit's title implies that it only addresses the issue in the kernel
> domain.
>
>> @@ -1333,8 +1325,11 @@ int cmd_disable_event(struct ltt_session *session,
>>                          ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
>>                          goto error_unlock;
>>                  }
>> -               /* The wild card * means that everything should be disabled. */
>> -               if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
>> +               /*
>> +                * An empty event name means that everything
>> +                * should be disabled.
>> +                */
>> +               if (event->name[0] == '\0') {
>>                          ret = event_agent_disable_all(usess, agt);
>>                  } else {
>>                          ret = event_agent_disable(usess, agt, event_name);
>> diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
>> index 13f09a5..a53d4e9 100644
>> --- a/src/bin/lttng-sessiond/event.c
>> +++ b/src/bin/lttng-sessiond/event.c
>> @@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
>>   }
>>
>>   /*
>> - * Disable kernel tracepoint event for a channel from the kernel session.
>> + * Disable kernel tracepoint events for a channel from the kernel session of
>> + * a specified event_name and event type.
>> + * On type LTTNG_EVENT_ALL all events with event_name are disabled.
>> + * If event_name if NULL all events of the specified type are disabled.
> if NULL -> is NULL
>
>>    */
>>   int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
>> -               char *event_name)
>> -{
>> -       int ret;
>> -       struct ltt_kernel_event *kevent;
>> -
>> -       assert(kchan);
>> -
>> -       kevent = trace_kernel_get_event_by_name(event_name, kchan,
>> -                       LTTNG_EVENT_ALL);
>> -       if (kevent == NULL) {
>> -               ret = LTTNG_ERR_NO_EVENT;
>> -               goto error;
>> -       }
>> -
>> -       ret = kernel_disable_event(kevent);
>> -       if (ret < 0) {
>> -               ret = LTTNG_ERR_KERN_DISABLE_FAIL;
>> -               goto error;
>> -       }
>> -
>> -       DBG("Kernel event %s disable for channel %s.",
>> -                       kevent->event->name, kchan->channel->name);
>> -
>> -       ret = LTTNG_OK;
>> -
>> -error:
>> -       return ret;
>> -}
>> -
>> -/*
>> - * Disable kernel tracepoint events for a channel from the kernel session.
>> - */
>> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>> -               enum lttng_event_type type)
>> +               char *event_name, enum lttng_event_type type)
>>   {
>> -       int ret;
>> +       int ret, error = 0, found = 0;
>>          struct ltt_kernel_event *kevent;
>>
>>          assert(kchan);
>> @@ -108,22 +78,25 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>>          cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
>>                  if (type != LTTNG_EVENT_ALL && kevent->type != type)
>>                          continue;
>> +               if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
>> +                       continue;
>> +               }
>> +               found++;
>>                  ret = kernel_disable_event(kevent);
>>                  if (ret < 0) {
>> -                       /* We continue disabling the rest */
>> +                       error = 1;
>>                          continue;
>>                  }
>>          }
>> -       ret = LTTNG_OK;
>> -       return ret;
>> -}
>> +       DBG("Disable kernel event: found %d events with name: %s and type: %d",
>> +                       found, event_name, type);
> event_name could be NULL here.
>
> event_name ? event_name : "NULL"
Good catch !
>> -/*
>> - * Disable all kernel event for a channel from the kernel session.
>> - */
>> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
>> -{
>> -       return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
>> +       if (!found && event_name != NULL) {
>> +               error = 1;
>> +       }
>> +
>> +       ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
>> +       return ret;
> This should distinguish between:
>
> 1) event found, but we failed to disable it (LTTNG_ERR_KERN_DISABLE_FAIL)
> 2) event_name != NULL && !found (LTTNG_ERR_NO_EVENT, which it returned before)
You're right.
>
>>   }
>>
>>   /*
>> diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
>> index 7c5231d..45dd1fc 100644
>> --- a/src/bin/lttng-sessiond/event.h
>> +++ b/src/bin/lttng-sessiond/event.h
>> @@ -23,10 +23,7 @@
>>   struct agent;
>>
>>   int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
>> -               char *event_name);
>> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>> -               enum lttng_event_type type);
>> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
>> +               char *event_name, enum lttng_event_type event_type);
>>
>>   int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
>>                  struct lttng_event *event, char *filter_expression,
>> diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
>> index c6ec85f..bcbc78d 100644
>> --- a/src/bin/lttng/commands/disable_events.c
>> +++ b/src/bin/lttng/commands/disable_events.c
>> @@ -44,7 +44,12 @@ static int opt_event_type;
>>   enum {
>>          OPT_HELP = 1,
>>          OPT_USERSPACE,
>> -       OPT_SYSCALL,
>> +       OPT_TYPE_SYSCALL,
>> +       OPT_TYPE_TRACEPOINT,
>> +       OPT_TYPE_PROBE,
>> +       OPT_TYPE_FUNCTION,
>> +       OPT_TYPE_FUNCTION_ENTRY,
>> +       OPT_TYPE_ALL,
>>          OPT_LIST_OPTIONS,
>>   };
>>
>> @@ -61,8 +66,24 @@ static struct poptOption long_options[] = {
>>          {"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
>>          {"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
>>          {"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
>> -       {"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
>> +       {"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
>> +       {"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
>> +       {"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
>> +       {"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
>> +       {"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
>> +#if 0
>> +       /*
>> +        * Function entry exits as a lttng_event_type but is never used on
>> +        * enable
>> +        */
>> +       {"function-entry",   0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION_ENTRY, 0, 0},
>> +
>> +       /* Not implemented yet */
>> +       {"userspace",      'u', POPT_ARG_STRING | POPT_ARGFLAG_OPTIONAL, &opt_cmd_name, OPT_USERSPACE, 0, 0},
>> +       {"pid",            'p', POPT_ARG_INT, &opt_pid, 0, 0, 0},
>> +#else
>>          {"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
>> +#endif
> I have removed #ifdef-ed out code from disable-event in
>
> commit 3fd691fc09199be5a52009a391b22379477aa6e7
> Author: J?r?mie Galarneau <jeremie.galarneau at efficios.com>
> Date:   Tue Sep 15 13:00:40 2015 -0400
>
>      Remove dead code from disable-event command
>
>      Signed-off-by: J?r?mie Galarneau <jeremie.galarneau at efficios.com>
>
> function-entry instrumentation is simply not supported, so let's
> mention it in the switch case that handles the instrumentation type,
> but not add compiled-out code.
>
>>          {"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
>>          {0, 0, 0, 0, 0, 0, 0}
>>   };
>> @@ -86,8 +107,19 @@ static void usage(FILE *ofp)
>>          fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
>>          fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
>>          fprintf(ofp, "\n");
>> -       fprintf(ofp, "Event options:\n");
>> +       fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
>> +       fprintf(ofp, "      --all                All event type (default)\n");
> type -> types
>
>> +       fprintf(ofp, "      --tracepoint         Tracepoint event\n");
>>          fprintf(ofp, "      --syscall            System call event\n");
>> +       fprintf(ofp, "      --probe              Probe event\n");
>> +       fprintf(ofp, "      --function           Function event\n");
>> +#if 0
>> +       /*
>> +        * Function entry exits as a lttng_event_type but is never used on
>> +        * enable
>> +        */
>> +       fprintf(ofp, "      --function-entry     Function-entry event\n");
>> +#endif
> Same here; no ifdef-ed out code.
>
>>          fprintf(ofp, "\n");
>>   }
>>
>> @@ -103,6 +135,27 @@ const char *print_raw_channel_name(const char *name)
>>          return name ? : "<default>";
>>   }
>>
>> +static
>> +const char *print_event_type(const enum lttng_event_type ev_type)
>> +{
>> +       switch (ev_type) {
>> +       case LTTNG_EVENT_ALL:
>> +               return "any";
>> +       case LTTNG_EVENT_TRACEPOINT:
>> +               return "tracepoint";
>> +       case LTTNG_EVENT_PROBE:
>> +               return "probe";
>> +       case LTTNG_EVENT_FUNCTION:
>> +               return "function";
>> +       case LTTNG_EVENT_FUNCTION_ENTRY:
>> +               return "function entry";
>> +       case LTTNG_EVENT_SYSCALL:
>> +               return "syscall";
>> +       default:
>> +               return "";
>> +       }
>> +}
>> +
>>   /* Mi print a partial event.
>>    * enabled is 0 or 1
>>    * success is 0 or 1
>> @@ -213,14 +266,8 @@ static int disable_events(char *session_name)
>>          /* Set default loglevel to any/unknown */
>>          event.loglevel = -1;
>>
>> -       switch (opt_event_type) {
>> -       case LTTNG_EVENT_SYSCALL:
>> -               event.type = LTTNG_EVENT_SYSCALL;
>> -               break;
>> -       default:
>> -               event.type = LTTNG_EVENT_ALL;
>> -               break;
>> -       }
>> +       /* opt_event_type contain the event type to disable at this point */
>> +       event.type = opt_event_type;
>>
>>          if (opt_disable_all) {
>>                  command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>> @@ -232,9 +279,9 @@ static int disable_events(char *session_name)
>>                  } else {
>>                          enabled = 0;
>>                          success = 1;
>> -                       MSG("All %s %s are disabled in channel %s",
>> +                       MSG("All %s events of type %s are disabled in channel %s",
>>                                          get_domain_str(dom.type),
>> -                                       opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
>> +                                       print_event_type(opt_event_type),
>>                                          print_channel_name(channel_name));
>>                  }
>>
>> @@ -255,9 +302,9 @@ static int disable_events(char *session_name)
>>                          event.name[sizeof(event.name) - 1] = '\0';
>>                          command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>>                          if (command_ret < 0) {
>> -                               ERR("%s %s: %s (channel %s, session %s)",
>> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
>> +                               ERR("%s of type %s : %s (channel %s, session %s)",
>>                                                  event_name,
>> +                                               print_event_type(opt_event_type),
>>                                                  lttng_strerror(command_ret),
>>                                                  command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
>>                                                          ? print_raw_channel_name(channel_name)
>> @@ -271,10 +318,10 @@ static int disable_events(char *session_name)
>>                                   */
>>                                  enabled = 1;
>>                          } else {
>> -                               MSG("%s %s %s disabled in channel %s for session %s",
>> +                               MSG("%s %s of type %s disabled in channel %s for session %s",
>>                                                  get_domain_str(dom.type),
>> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
>>                                                  event_name,
>> +                                               print_event_type(opt_event_type),
>>                                                  print_channel_name(channel_name),
>>                                                  session_name);
>>                                  success = 1;
>> @@ -341,9 +388,24 @@ int cmd_disable_events(int argc, const char **argv)
>>                  case OPT_USERSPACE:
>>                          opt_userspace = 1;
>>                          break;
>> -               case OPT_SYSCALL:
>> +               case OPT_TYPE_SYSCALL:
>>                          opt_event_type = LTTNG_EVENT_SYSCALL;
>>                          break;
>> +               case OPT_TYPE_TRACEPOINT:
>> +                       opt_event_type = LTTNG_EVENT_TRACEPOINT;
>> +                       break;
>> +               case OPT_TYPE_PROBE:
>> +                       opt_event_type = LTTNG_EVENT_PROBE;
>> +                       break;
>> +               case OPT_TYPE_FUNCTION:
>> +                       opt_event_type = LTTNG_EVENT_FUNCTION;
>> +                       break;
>> +               case OPT_TYPE_FUNCTION_ENTRY:
>> +                       opt_event_type = LTTNG_EVENT_FUNCTION_ENTRY;
>> +                       break;
> We shouldn't allow this (FUNCTION and FUNCTION_ENTRY) for non-kernel
> domains as their support is not implemented.
>
>> +               case OPT_TYPE_ALL:
>> +                       opt_event_type = LTTNG_EVENT_ALL;
>> +                       break;
>>                  case OPT_LIST_OPTIONS:
>>                          list_cmd_options(stdout, long_options);
>>                          goto end;
>> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
>> index 9cbfef5..1007326 100644
>> --- a/src/lib/lttng-ctl/lttng-ctl.c
>> +++ b/src/lib/lttng-ctl/lttng-ctl.c
>> @@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
>>          }
>>
>>          lsm.cmd_type = LTTNG_DISABLE_EVENT;
>> -       if (ev->name[0] == '\0') {
>> -               /* Disable all events */
>> -               lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
>> -       }
>>
>>          lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
>>          /* FIXME: copying non-packed struct to packed struct. */
>> diff --git a/tests/regression/ust/python-logging/test_python_logging.in b/tests/regression/ust/python-logging/test_python_logging.in
>> index 8aff408..d5a9b80 100755
>> --- a/tests/regression/ust/python-logging/test_python_logging.in
>> +++ b/tests/regression/ust/python-logging/test_python_logging.in
>> @@ -351,7 +351,7 @@ function test_python_disable_all()
>>          enable_python_lttng_event $SESSION_NAME $EVENT_NAME
>>          enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
>>
>> -       disable_python_lttng_event $SESSION_NAME '*'
>> +       disable_python_lttng_event $SESSION_NAME -a
> Please submit a separate patch for this.

Since I added two more commits I'll simply reply to your message with 
the new patch set.

>
> Thanks!
> J?r?mie
>
>>          start_lttng_tracing_ok $SESSION_NAME
>>
>> --
>> 2.1.4
>>
>
>

-- 
Jonathan R. Julien
Efficios




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

* [lttng-dev] [PATCH lttng-tools 1/4 v2] Fix: disable kernel event based on name and event type
  2015-09-21 19:29 ` Jérémie Galarneau
@ 2015-09-21 22:43   ` Jonathan Rajotte
  2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 2/4 v2] Use empty event name on disable -a for ust and agent domain Jonathan Rajotte
                       ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Jonathan Rajotte @ 2015-09-21 22:43 UTC (permalink / raw)


The -a argument is interpreted as a zero-length event name
instead of '*' which is actually a valid wildcard event
name by itself. This simplify how a disable action is handled.

The event type can now be passed as argument
and is a new criteria while disabling kernel events.
The default is to disable for all event type.

UST and agent domain do not support yet disabling by event
type.

e.g:
	# Only disable kernel event of type tracepoint.
	lttng disable -a -k --tracepoint

	# Only disable the event with name '*' and type syscall.
	lttng disable -k '*' --syscall

	# Disable all kernel event of all type.
	lttng disable -a -k

Fixes #925

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng-sessiond/cmd.c            | 28 ++++------
 src/bin/lttng-sessiond/event.c          | 66 +++++++----------------
 src/bin/lttng-sessiond/event.h          |  5 +-
 src/bin/lttng/commands/disable_events.c | 94 +++++++++++++++++++++++++--------
 src/lib/lttng-ctl/lttng-ctl.c           |  4 --
 5 files changed, 101 insertions(+), 96 deletions(-)

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index 7878a05..0daeb9e 100644
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
 
 		switch (event->type) {
 		case LTTNG_EVENT_ALL:
-			ret = event_kernel_disable_event_all(kchan);
-			if (ret != LTTNG_OK) {
-				goto error_unlock;
-			}
-			break;
-		case LTTNG_EVENT_TRACEPOINT:	/* fall-through */
+		case LTTNG_EVENT_TRACEPOINT:
 		case LTTNG_EVENT_SYSCALL:
-			if (!strcmp(event_name, "*")) {
-				ret = event_kernel_disable_event_type(kchan,
-					event->type);
+		case LTTNG_EVENT_PROBE:
+		case LTTNG_EVENT_FUNCTION:
+		case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
+			if (event_name[0] == '\0') {
+				ret = event_kernel_disable_event(kchan,
+					NULL, event->type);
 			} else {
 				ret = event_kernel_disable_event(kchan,
-					event_name);
+					event_name, event->type);
 			}
 			if (ret != LTTNG_OK) {
 				goto error_unlock;
 			}
 			break;
-		case LTTNG_EVENT_PROBE:
-		case LTTNG_EVENT_FUNCTION:
-		case LTTNG_EVENT_FUNCTION_ENTRY:
-			ret = event_kernel_disable_event(kchan, event_name);
-			if (ret != LTTNG_OK) {
-				goto error_unlock;
-			}
-			break;
 		default:
 			ret = LTTNG_ERR_UNK;
 			goto error_unlock;
@@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
 
 		/*
 		 * If a non-default channel has been created in the
-		 * session, explicitely require that -c chan_name needs
+		 * session, explicitly require that -c chan_name needs
 		 * to be provided.
 		 */
 		if (usess->has_non_default_channel && channel_name[0] == '\0') {
diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
index 13f09a5..447b872 100644
--- a/src/bin/lttng-sessiond/event.c
+++ b/src/bin/lttng-sessiond/event.c
@@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
 }
 
 /*
- * Disable kernel tracepoint event for a channel from the kernel session.
+ * Disable kernel tracepoint events for a channel from the kernel session of
+ * a specified event_name and event type.
+ * On type LTTNG_EVENT_ALL all events with event_name are disabled.
+ * If event_name is NULL all events of the specified type are disabled.
  */
 int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
-		char *event_name)
+		char *event_name, enum lttng_event_type type)
 {
-	int ret;
-	struct ltt_kernel_event *kevent;
-
-	assert(kchan);
-
-	kevent = trace_kernel_get_event_by_name(event_name, kchan,
-			LTTNG_EVENT_ALL);
-	if (kevent == NULL) {
-		ret = LTTNG_ERR_NO_EVENT;
-		goto error;
-	}
-
-	ret = kernel_disable_event(kevent);
-	if (ret < 0) {
-		ret = LTTNG_ERR_KERN_DISABLE_FAIL;
-		goto error;
-	}
-
-	DBG("Kernel event %s disable for channel %s.",
-			kevent->event->name, kchan->channel->name);
-
-	ret = LTTNG_OK;
-
-error:
-	return ret;
-}
-
-/*
- * Disable kernel tracepoint events for a channel from the kernel session.
- */
-int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
-		enum lttng_event_type type)
-{
-	int ret;
+	int ret, error = 0, found = 0;
 	struct ltt_kernel_event *kevent;
 
 	assert(kchan);
@@ -108,22 +78,26 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
 	cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
 		if (type != LTTNG_EVENT_ALL && kevent->type != type)
 			continue;
+		if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
+			continue;
+		}
+		found++;
 		ret = kernel_disable_event(kevent);
 		if (ret < 0) {
-			/* We continue disabling the rest */
+			error = 1;
 			continue;
 		}
 	}
-	ret = LTTNG_OK;
-	return ret;
-}
+	DBG("Disable kernel event: found %d events with name: %s and type: %d",
+			found, event_name ? event_name : "NULL", type);
 
-/*
- * Disable all kernel event for a channel from the kernel session.
- */
-int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
-{
-	return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
+	if (event_name != NULL && !found) {
+		ret = LTTNG_ERR_NO_EVENT;
+	} else {
+		ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
+	}
+
+	return ret;
 }
 
 /*
diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
index 7c5231d..45dd1fc 100644
--- a/src/bin/lttng-sessiond/event.h
+++ b/src/bin/lttng-sessiond/event.h
@@ -23,10 +23,7 @@
 struct agent;
 
 int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
-		char *event_name);
-int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
-		enum lttng_event_type type);
-int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
+		char *event_name, enum lttng_event_type event_type);
 
 int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
 		struct lttng_event *event, char *filter_expression,
diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
index c6ec85f..440d107 100644
--- a/src/bin/lttng/commands/disable_events.c
+++ b/src/bin/lttng/commands/disable_events.c
@@ -43,8 +43,11 @@ static int opt_event_type;
 
 enum {
 	OPT_HELP = 1,
-	OPT_USERSPACE,
-	OPT_SYSCALL,
+	OPT_TYPE_SYSCALL,
+	OPT_TYPE_TRACEPOINT,
+	OPT_TYPE_PROBE,
+	OPT_TYPE_FUNCTION,
+	OPT_TYPE_ALL,
 	OPT_LIST_OPTIONS,
 };
 
@@ -61,8 +64,12 @@ static struct poptOption long_options[] = {
 	{"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
 	{"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
 	{"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
-	{"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
-	{"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
+	{"userspace",      'u', POPT_ARG_VAL, &opt_userspace, 1, 0, 0},
+	{"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
+	{"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
+	{"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
+	{"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
+	{"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
 	{"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
 	{0, 0, 0, 0, 0, 0, 0}
 };
@@ -86,8 +93,12 @@ static void usage(FILE *ofp)
 	fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
 	fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
 	fprintf(ofp, "\n");
-	fprintf(ofp, "Event options:\n");
+	fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
+	fprintf(ofp, "      --all                All event types (default)\n");
+	fprintf(ofp, "      --tracepoint         Tracepoint event\n");
 	fprintf(ofp, "      --syscall            System call event\n");
+	fprintf(ofp, "      --probe              Probe event\n");
+	fprintf(ofp, "      --function           Function event\n");
 	fprintf(ofp, "\n");
 }
 
@@ -103,6 +114,27 @@ const char *print_raw_channel_name(const char *name)
 	return name ? : "<default>";
 }
 
+static
+const char *print_event_type(const enum lttng_event_type ev_type)
+{
+	switch (ev_type) {
+	case LTTNG_EVENT_ALL:
+		return "any";
+	case LTTNG_EVENT_TRACEPOINT:
+		return "tracepoint";
+	case LTTNG_EVENT_PROBE:
+		return "probe";
+	case LTTNG_EVENT_FUNCTION:
+		return "function";
+	case LTTNG_EVENT_FUNCTION_ENTRY:
+		return "function entry";
+	case LTTNG_EVENT_SYSCALL:
+		return "syscall";
+	default:
+		return "";
+	}
+}
+
 /* Mi print a partial event.
  * enabled is 0 or 1
  * success is 0 or 1
@@ -213,14 +245,8 @@ static int disable_events(char *session_name)
 	/* Set default loglevel to any/unknown */
 	event.loglevel = -1;
 
-	switch (opt_event_type) {
-	case LTTNG_EVENT_SYSCALL:
-		event.type = LTTNG_EVENT_SYSCALL;
-		break;
-	default:
-		event.type = LTTNG_EVENT_ALL;
-		break;
-	}
+	/* opt_event_type contain the event type to disable at this point */
+	event.type = opt_event_type;
 
 	if (opt_disable_all) {
 		command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
@@ -232,9 +258,9 @@ static int disable_events(char *session_name)
 		} else {
 			enabled = 0;
 			success = 1;
-			MSG("All %s %s are disabled in channel %s",
+			MSG("All %s events of type %s are disabled in channel %s",
 					get_domain_str(dom.type),
-					opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
+					print_event_type(opt_event_type),
 					print_channel_name(channel_name));
 		}
 
@@ -255,9 +281,9 @@ static int disable_events(char *session_name)
 			event.name[sizeof(event.name) - 1] = '\0';
 			command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
 			if (command_ret < 0) {
-				ERR("%s %s: %s (channel %s, session %s)",
-						opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
+				ERR("%s of type %s : %s (channel %s, session %s)",
 						event_name,
+						print_event_type(opt_event_type),
 						lttng_strerror(command_ret),
 						command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
 							? print_raw_channel_name(channel_name)
@@ -271,10 +297,10 @@ static int disable_events(char *session_name)
 				 */
 				enabled = 1;
 			} else {
-				MSG("%s %s %s disabled in channel %s for session %s",
+				MSG("%s %s of type %s disabled in channel %s for session %s",
 						get_domain_str(dom.type),
-						opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
 						event_name,
+						print_event_type(opt_event_type),
 						print_channel_name(channel_name),
 						session_name);
 				success = 1;
@@ -338,12 +364,21 @@ int cmd_disable_events(int argc, const char **argv)
 		case OPT_HELP:
 			usage(stdout);
 			goto end;
-		case OPT_USERSPACE:
-			opt_userspace = 1;
-			break;
-		case OPT_SYSCALL:
+		case OPT_TYPE_SYSCALL:
 			opt_event_type = LTTNG_EVENT_SYSCALL;
 			break;
+		case OPT_TYPE_TRACEPOINT:
+			opt_event_type = LTTNG_EVENT_TRACEPOINT;
+			break;
+		case OPT_TYPE_PROBE:
+			opt_event_type = LTTNG_EVENT_PROBE;
+			break;
+		case OPT_TYPE_FUNCTION:
+			opt_event_type = LTTNG_EVENT_FUNCTION;
+			break;
+		case OPT_TYPE_ALL:
+			opt_event_type = LTTNG_EVENT_ALL;
+			break;
 		case OPT_LIST_OPTIONS:
 			list_cmd_options(stdout, long_options);
 			goto end;
@@ -372,6 +407,19 @@ int cmd_disable_events(int argc, const char **argv)
 		goto end;
 	}
 
+	/* Ust and agent only support ALL event type */
+	if ((opt_userspace || opt_jul || opt_log4j || opt_python)
+			&& opt_event_type != LTTNG_EVENT_ALL) {
+		ERR("UST and agent (-j | -l | -p) event(s) disabling based on event type is not supported.\n");
+		usage(stderr);
+		ret = CMD_ERROR;
+		goto end;
+	}
+
+	if (opt) {
+		/* code */
+	}
+
 	opt_event_list = (char*) poptGetArg(pc);
 	if (opt_event_list == NULL && opt_disable_all == 0) {
 		ERR("Missing event name(s).\n");
diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
index 9cbfef5..1007326 100644
--- a/src/lib/lttng-ctl/lttng-ctl.c
+++ b/src/lib/lttng-ctl/lttng-ctl.c
@@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
 	}
 
 	lsm.cmd_type = LTTNG_DISABLE_EVENT;
-	if (ev->name[0] == '\0') {
-		/* Disable all events */
-		lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
-	}
 
 	lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
 	/* FIXME: copying non-packed struct to packed struct. */
-- 
2.1.4




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

* [lttng-dev] [PATCH lttng-tools 2/4 v2] Use empty event name on disable -a for ust and agent domain
  2015-09-21 22:43   ` [lttng-dev] [PATCH lttng-tools 1/4 v2] " Jonathan Rajotte
@ 2015-09-21 22:43     ` Jonathan Rajotte
  2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 3/4 v2] man: update disable-event section Jonathan Rajotte
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Jonathan Rajotte @ 2015-09-21 22:43 UTC (permalink / raw)


Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng-sessiond/cmd.c                            | 17 +++++++++++------
 .../ust/python-logging/test_python_logging.in           |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index 0daeb9e..5a1cc7a 100644
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -1280,10 +1280,12 @@ int cmd_disable_event(struct ltt_session *session,
 
 		switch (event->type) {
 		case LTTNG_EVENT_ALL:
-			if (strlen(event->name) == 1 &&
-					!strncmp(event->name, "*", 1)) {
-				ret = event_ust_disable_all_tracepoints(usess,
-						uchan);
+			/*
+			 * An empty event name means that everything
+			 * should be disabled.
+			 */
+			if (event->name[0] == '\0') {
+				ret = event_ust_disable_all_tracepoints(usess, uchan);
 			} else {
 				ret = event_ust_disable_tracepoint(usess, uchan,
 						event_name);
@@ -1323,8 +1325,11 @@ int cmd_disable_event(struct ltt_session *session,
 			ret = -LTTNG_ERR_UST_EVENT_NOT_FOUND;
 			goto error_unlock;
 		}
-		/* The wild card * means that everything should be disabled. */
-		if (strncmp(event->name, "*", 1) == 0 && strlen(event->name) == 1) {
+		/*
+		 * An empty event name means that everything
+		 * should be disabled.
+		 */
+		if (event->name[0] == '\0') {
 			ret = event_agent_disable_all(usess, agt);
 		} else {
 			ret = event_agent_disable(usess, agt, event_name);
diff --git a/tests/regression/ust/python-logging/test_python_logging.in b/tests/regression/ust/python-logging/test_python_logging.in
index 8aff408..d5a9b80 100755
--- a/tests/regression/ust/python-logging/test_python_logging.in
+++ b/tests/regression/ust/python-logging/test_python_logging.in
@@ -351,7 +351,7 @@ function test_python_disable_all()
 	enable_python_lttng_event $SESSION_NAME $EVENT_NAME
 	enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
 
-	disable_python_lttng_event $SESSION_NAME '*'
+	disable_python_lttng_event $SESSION_NAME -a
 
 	start_lttng_tracing_ok $SESSION_NAME
 
-- 
2.1.4




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

* [lttng-dev] [PATCH lttng-tools 3/4 v2] man: update disable-event section
  2015-09-21 22:43   ` [lttng-dev] [PATCH lttng-tools 1/4 v2] " Jonathan Rajotte
  2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 2/4 v2] Use empty event name on disable -a for ust and agent domain Jonathan Rajotte
@ 2015-09-21 22:43     ` Jonathan Rajotte
  2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 4/4 v2] Help: add -j -l -p option to help string Jonathan Rajotte
  2015-09-22 17:47     ` [lttng-dev] [PATCH lttng-tools 1/4 v2] Fix: disable kernel event based on name and event type Jérémie Galarneau
  3 siblings, 0 replies; 10+ messages in thread
From: Jonathan Rajotte @ 2015-09-21 22:43 UTC (permalink / raw)


Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 doc/man/lttng.1 | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/doc/man/lttng.1 b/doc/man/lttng.1
index cdd816d..db5a7c9 100644
--- a/doc/man/lttng.1
+++ b/doc/man/lttng.1
@@ -590,11 +590,7 @@ Dynamic function entry/return probe. Addr and offset can be octal
 (0NNN...), decimal (NNN...) or hexadecimal (0xNNN...)
 .TP
 .BR "\-\-syscall"
-System call event. Enabling syscalls tracing (kernel tracer), you will
-not be able to disable them with disable-event. This is a known
-limitation. You can disable the entire channel to do the trick. Also note
-that per-syscall selection is not supported yet. Use with "-a" to enable
-all syscalls.
+System call event.
 .TP
 .BR "\-\-filter 'expression'"
 Set a filter on a newly enabled event. Filter expression on event
@@ -681,7 +677,7 @@ Apply for the user-space tracer
 .PP
 
 .PP
-\fBdisable-event\fP NAME[,NAME2,...] (\-k | \-u) [OPTIONS]
+\fBdisable-event\fP NAME[,NAME2,...] (\-k | \-u | \-j | \-l | \-p) [TYPE] [OPTIONS]
 .RS
 Disable tracing event
 
@@ -711,8 +707,8 @@ Apply on session name
 Apply on channel name
 .TP
 .BR "\-a, \-\-all-events"
-Disable all events. This does NOT disable "*" but rather every known
-events of the session.
+Disable all events. This does NOT ONLY disable "*" but rather every known
+events of the session
 .TP
 .BR "\-k, \-\-kernel"
 Apply for the kernel tracer
@@ -727,7 +723,26 @@ Apply for Java application using Java Util Logging interface (JUL)
 Apply for Java application using LOG4J
 .TP
 .BR "\-p, \-\-python"
-Apply for Python application using the logging module.
+Apply for Python application using the logging module
+
+.TP
+.B TYPE (kernel domain only):
+
+.TP
+.BR "\-\-all"
+Disable event of all type
+.TP
+.BR "\-\-tracepoint"
+Disable event of type tracepoint
+.TP
+.BR "\-\-syscall"
+Disable event of type syscall
+.TP
+.BR "\-\-probe"
+Disable event of type probe
+.TP
+.BR "\-\-function"
+Disable event of type function
 .RE
 .PP
 
-- 
2.1.4




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

* [lttng-dev] [PATCH lttng-tools 4/4 v2] Help: add -j -l -p option to help string
  2015-09-21 22:43   ` [lttng-dev] [PATCH lttng-tools 1/4 v2] " Jonathan Rajotte
  2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 2/4 v2] Use empty event name on disable -a for ust and agent domain Jonathan Rajotte
  2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 3/4 v2] man: update disable-event section Jonathan Rajotte
@ 2015-09-21 22:43     ` Jonathan Rajotte
  2015-09-22 17:47     ` [lttng-dev] [PATCH lttng-tools 1/4 v2] Fix: disable kernel event based on name and event type Jérémie Galarneau
  3 siblings, 0 replies; 10+ messages in thread
From: Jonathan Rajotte @ 2015-09-21 22:43 UTC (permalink / raw)


Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 src/bin/lttng/commands/disable_events.c | 6 +++---
 src/bin/lttng/commands/enable_events.c  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
index 440d107..9c2f1cc 100644
--- a/src/bin/lttng/commands/disable_events.c
+++ b/src/bin/lttng/commands/disable_events.c
@@ -79,7 +79,7 @@ static struct poptOption long_options[] = {
  */
 static void usage(FILE *ofp)
 {
-	fprintf(ofp, "usage: lttng disable-event NAME[,NAME2,...] (-k | -u) [OPTIONS]\n");
+	fprintf(ofp, "usage: lttng disable-event NAME[,NAME2,...] (-k | -u | -j | -l | -p) [OPTIONS]\n");
 	fprintf(ofp, "\n");
 	fprintf(ofp, "Options:\n");
 	fprintf(ofp, "  -h, --help               Show this help\n");
@@ -87,9 +87,9 @@ static void usage(FILE *ofp)
 	fprintf(ofp, "  -s, --session NAME       Apply to session name\n");
 	fprintf(ofp, "  -c, --channel NAME       Apply to this channel\n");
 	fprintf(ofp, "  -a, --all-events         Disable all tracepoints\n");
-	fprintf(ofp, "  -k, --kernel             Apply for the kernel tracer\n");
+	fprintf(ofp, "  -k, --kernel             Apply to the kernel tracer\n");
 	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
-	fprintf(ofp, "  -j, --jul                Apply for Java application using JUL\n");
+	fprintf(ofp, "  -j, --jul                Apply to Java application using JUL\n");
 	fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
 	fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
 	fprintf(ofp, "\n");
diff --git a/src/bin/lttng/commands/enable_events.c b/src/bin/lttng/commands/enable_events.c
index 972043e..5dcdd06 100644
--- a/src/bin/lttng/commands/enable_events.c
+++ b/src/bin/lttng/commands/enable_events.c
@@ -109,9 +109,9 @@ static void usage(FILE *ofp)
 	fprintf(ofp, "  -s, --session NAME       Apply to session name\n");
 	fprintf(ofp, "  -c, --channel NAME       Apply to this channel\n");
 	fprintf(ofp, "  -a, --all                Enable all tracepoints and syscalls\n");
-	fprintf(ofp, "  -k, --kernel             Apply for the kernel tracer\n");
+	fprintf(ofp, "  -k, --kernel             Apply to the kernel tracer\n");
 	fprintf(ofp, "  -u, --userspace          Apply to the user-space tracer\n");
-	fprintf(ofp, "  -j, --jul                Apply for Java application using JUL\n");
+	fprintf(ofp, "  -j, --jul                Apply to Java application using JUL\n");
 	fprintf(ofp, "  -l, --log4j              Apply for Java application using LOG4j\n");
 	fprintf(ofp, "  -p, --python             Apply for Python application\n");
 	fprintf(ofp, "\n");
-- 
2.1.4




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

* [lttng-dev] [PATCH lttng-tools 1/4 v2] Fix: disable kernel event based on name and event type
  2015-09-21 22:43   ` [lttng-dev] [PATCH lttng-tools 1/4 v2] " Jonathan Rajotte
                       ` (2 preceding siblings ...)
  2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 4/4 v2] Help: add -j -l -p option to help string Jonathan Rajotte
@ 2015-09-22 17:47     ` Jérémie Galarneau
  3 siblings, 0 replies; 10+ messages in thread
From: Jérémie Galarneau @ 2015-09-22 17:47 UTC (permalink / raw)


All merged in master and stable-2.7.

Thanks,
J?r?mie

On Mon, Sep 21, 2015 at 6:43 PM, Jonathan Rajotte
<jonathan.rajotte-julien at efficios.com> wrote:
> The -a argument is interpreted as a zero-length event name
> instead of '*' which is actually a valid wildcard event
> name by itself. This simplify how a disable action is handled.
>
> The event type can now be passed as argument
> and is a new criteria while disabling kernel events.
> The default is to disable for all event type.
>
> UST and agent domain do not support yet disabling by event
> type.
>
> e.g:
>         # Only disable kernel event of type tracepoint.
>         lttng disable -a -k --tracepoint
>
>         # Only disable the event with name '*' and type syscall.
>         lttng disable -k '*' --syscall
>
>         # Disable all kernel event of all type.
>         lttng disable -a -k
>
> Fixes #925
>
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  src/bin/lttng-sessiond/cmd.c            | 28 ++++------
>  src/bin/lttng-sessiond/event.c          | 66 +++++++----------------
>  src/bin/lttng-sessiond/event.h          |  5 +-
>  src/bin/lttng/commands/disable_events.c | 94 +++++++++++++++++++++++++--------
>  src/lib/lttng-ctl/lttng-ctl.c           |  4 --
>  5 files changed, 101 insertions(+), 96 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
> index 7878a05..0daeb9e 100644
> --- a/src/bin/lttng-sessiond/cmd.c
> +++ b/src/bin/lttng-sessiond/cmd.c
> @@ -1225,32 +1225,22 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 switch (event->type) {
>                 case LTTNG_EVENT_ALL:
> -                       ret = event_kernel_disable_event_all(kchan);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
> -               case LTTNG_EVENT_TRACEPOINT:    /* fall-through */
> +               case LTTNG_EVENT_TRACEPOINT:
>                 case LTTNG_EVENT_SYSCALL:
> -                       if (!strcmp(event_name, "*")) {
> -                               ret = event_kernel_disable_event_type(kchan,
> -                                       event->type);
> +               case LTTNG_EVENT_PROBE:
> +               case LTTNG_EVENT_FUNCTION:
> +               case LTTNG_EVENT_FUNCTION_ENTRY:/* fall-through */
> +                       if (event_name[0] == '\0') {
> +                               ret = event_kernel_disable_event(kchan,
> +                                       NULL, event->type);
>                         } else {
>                                 ret = event_kernel_disable_event(kchan,
> -                                       event_name);
> +                                       event_name, event->type);
>                         }
>                         if (ret != LTTNG_OK) {
>                                 goto error_unlock;
>                         }
>                         break;
> -               case LTTNG_EVENT_PROBE:
> -               case LTTNG_EVENT_FUNCTION:
> -               case LTTNG_EVENT_FUNCTION_ENTRY:
> -                       ret = event_kernel_disable_event(kchan, event_name);
> -                       if (ret != LTTNG_OK) {
> -                               goto error_unlock;
> -                       }
> -                       break;
>                 default:
>                         ret = LTTNG_ERR_UNK;
>                         goto error_unlock;
> @@ -1273,7 +1263,7 @@ int cmd_disable_event(struct ltt_session *session,
>
>                 /*
>                  * If a non-default channel has been created in the
> -                * session, explicitely require that -c chan_name needs
> +                * session, explicitly require that -c chan_name needs
>                  * to be provided.
>                  */
>                 if (usess->has_non_default_channel && channel_name[0] == '\0') {
> diff --git a/src/bin/lttng-sessiond/event.c b/src/bin/lttng-sessiond/event.c
> index 13f09a5..447b872 100644
> --- a/src/bin/lttng-sessiond/event.c
> +++ b/src/bin/lttng-sessiond/event.c
> @@ -61,45 +61,15 @@ static void add_unique_ust_event(struct lttng_ht *ht,
>  }
>
>  /*
> - * Disable kernel tracepoint event for a channel from the kernel session.
> + * Disable kernel tracepoint events for a channel from the kernel session of
> + * a specified event_name and event type.
> + * On type LTTNG_EVENT_ALL all events with event_name are disabled.
> + * If event_name is NULL all events of the specified type are disabled.
>   */
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name)
> +               char *event_name, enum lttng_event_type type)
>  {
> -       int ret;
> -       struct ltt_kernel_event *kevent;
> -
> -       assert(kchan);
> -
> -       kevent = trace_kernel_get_event_by_name(event_name, kchan,
> -                       LTTNG_EVENT_ALL);
> -       if (kevent == NULL) {
> -               ret = LTTNG_ERR_NO_EVENT;
> -               goto error;
> -       }
> -
> -       ret = kernel_disable_event(kevent);
> -       if (ret < 0) {
> -               ret = LTTNG_ERR_KERN_DISABLE_FAIL;
> -               goto error;
> -       }
> -
> -       DBG("Kernel event %s disable for channel %s.",
> -                       kevent->event->name, kchan->channel->name);
> -
> -       ret = LTTNG_OK;
> -
> -error:
> -       return ret;
> -}
> -
> -/*
> - * Disable kernel tracepoint events for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type)
> -{
> -       int ret;
> +       int ret, error = 0, found = 0;
>         struct ltt_kernel_event *kevent;
>
>         assert(kchan);
> @@ -108,22 +78,26 @@ int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
>         cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {
>                 if (type != LTTNG_EVENT_ALL && kevent->type != type)
>                         continue;
> +               if (event_name != NULL && strcmp(event_name, kevent->event->name)) {
> +                       continue;
> +               }
> +               found++;
>                 ret = kernel_disable_event(kevent);
>                 if (ret < 0) {
> -                       /* We continue disabling the rest */
> +                       error = 1;
>                         continue;
>                 }
>         }
> -       ret = LTTNG_OK;
> -       return ret;
> -}
> +       DBG("Disable kernel event: found %d events with name: %s and type: %d",
> +                       found, event_name ? event_name : "NULL", type);
>
> -/*
> - * Disable all kernel event for a channel from the kernel session.
> - */
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan)
> -{
> -       return event_kernel_disable_event_type(kchan, LTTNG_EVENT_ALL);
> +       if (event_name != NULL && !found) {
> +               ret = LTTNG_ERR_NO_EVENT;
> +       } else {
> +               ret = error ? LTTNG_ERR_KERN_DISABLE_FAIL : LTTNG_OK;
> +       }
> +
> +       return ret;
>  }
>
>  /*
> diff --git a/src/bin/lttng-sessiond/event.h b/src/bin/lttng-sessiond/event.h
> index 7c5231d..45dd1fc 100644
> --- a/src/bin/lttng-sessiond/event.h
> +++ b/src/bin/lttng-sessiond/event.h
> @@ -23,10 +23,7 @@
>  struct agent;
>
>  int event_kernel_disable_event(struct ltt_kernel_channel *kchan,
> -               char *event_name);
> -int event_kernel_disable_event_type(struct ltt_kernel_channel *kchan,
> -               enum lttng_event_type type);
> -int event_kernel_disable_event_all(struct ltt_kernel_channel *kchan);
> +               char *event_name, enum lttng_event_type event_type);
>
>  int event_kernel_enable_event(struct ltt_kernel_channel *kchan,
>                 struct lttng_event *event, char *filter_expression,
> diff --git a/src/bin/lttng/commands/disable_events.c b/src/bin/lttng/commands/disable_events.c
> index c6ec85f..440d107 100644
> --- a/src/bin/lttng/commands/disable_events.c
> +++ b/src/bin/lttng/commands/disable_events.c
> @@ -43,8 +43,11 @@ static int opt_event_type;
>
>  enum {
>         OPT_HELP = 1,
> -       OPT_USERSPACE,
> -       OPT_SYSCALL,
> +       OPT_TYPE_SYSCALL,
> +       OPT_TYPE_TRACEPOINT,
> +       OPT_TYPE_PROBE,
> +       OPT_TYPE_FUNCTION,
> +       OPT_TYPE_ALL,
>         OPT_LIST_OPTIONS,
>  };
>
> @@ -61,8 +64,12 @@ static struct poptOption long_options[] = {
>         {"log4j",          'l', POPT_ARG_VAL, &opt_log4j, 1, 0, 0},
>         {"python",         'p', POPT_ARG_VAL, &opt_python, 1, 0, 0},
>         {"kernel",         'k', POPT_ARG_VAL, &opt_kernel, 1, 0, 0},
> -       {"syscall",        0,   POPT_ARG_NONE, 0, OPT_SYSCALL, 0, 0},
> -       {"userspace",      'u', POPT_ARG_NONE, 0, OPT_USERSPACE, 0, 0},
> +       {"userspace",      'u', POPT_ARG_VAL, &opt_userspace, 1, 0, 0},
> +       {"syscall",          0, POPT_ARG_NONE, 0, OPT_TYPE_SYSCALL, 0, 0},
> +       {"probe",            0, POPT_ARG_NONE, 0, OPT_TYPE_PROBE, 0, 0},
> +       {"tracepoint",       0, POPT_ARG_NONE, 0, OPT_TYPE_TRACEPOINT, 0, 0},
> +       {"function",         0, POPT_ARG_NONE, 0, OPT_TYPE_FUNCTION, 0, 0},
> +       {"all",              0, POPT_ARG_NONE, 0, OPT_TYPE_ALL, 0, 0},
>         {"list-options", 0, POPT_ARG_NONE, NULL, OPT_LIST_OPTIONS, NULL, NULL},
>         {0, 0, 0, 0, 0, 0, 0}
>  };
> @@ -86,8 +93,12 @@ static void usage(FILE *ofp)
>         fprintf(ofp, "  -l, --log4j              Apply to Java application using LOG4j\n");
>         fprintf(ofp, "  -p, --python             Apply to Python application using logging\n");
>         fprintf(ofp, "\n");
> -       fprintf(ofp, "Event options:\n");
> +       fprintf(ofp, "Event type options (Only supported with kernel domain):\n");
> +       fprintf(ofp, "      --all                All event types (default)\n");
> +       fprintf(ofp, "      --tracepoint         Tracepoint event\n");
>         fprintf(ofp, "      --syscall            System call event\n");
> +       fprintf(ofp, "      --probe              Probe event\n");
> +       fprintf(ofp, "      --function           Function event\n");
>         fprintf(ofp, "\n");
>  }
>
> @@ -103,6 +114,27 @@ const char *print_raw_channel_name(const char *name)
>         return name ? : "<default>";
>  }
>
> +static
> +const char *print_event_type(const enum lttng_event_type ev_type)
> +{
> +       switch (ev_type) {
> +       case LTTNG_EVENT_ALL:
> +               return "any";
> +       case LTTNG_EVENT_TRACEPOINT:
> +               return "tracepoint";
> +       case LTTNG_EVENT_PROBE:
> +               return "probe";
> +       case LTTNG_EVENT_FUNCTION:
> +               return "function";
> +       case LTTNG_EVENT_FUNCTION_ENTRY:
> +               return "function entry";
> +       case LTTNG_EVENT_SYSCALL:
> +               return "syscall";
> +       default:
> +               return "";
> +       }
> +}
> +
>  /* Mi print a partial event.
>   * enabled is 0 or 1
>   * success is 0 or 1
> @@ -213,14 +245,8 @@ static int disable_events(char *session_name)
>         /* Set default loglevel to any/unknown */
>         event.loglevel = -1;
>
> -       switch (opt_event_type) {
> -       case LTTNG_EVENT_SYSCALL:
> -               event.type = LTTNG_EVENT_SYSCALL;
> -               break;
> -       default:
> -               event.type = LTTNG_EVENT_ALL;
> -               break;
> -       }
> +       /* opt_event_type contain the event type to disable at this point */
> +       event.type = opt_event_type;
>
>         if (opt_disable_all) {
>                 command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
> @@ -232,9 +258,9 @@ static int disable_events(char *session_name)
>                 } else {
>                         enabled = 0;
>                         success = 1;
> -                       MSG("All %s %s are disabled in channel %s",
> +                       MSG("All %s events of type %s are disabled in channel %s",
>                                         get_domain_str(dom.type),
> -                                       opt_event_type == LTTNG_EVENT_SYSCALL ? "system calls" : "events",
> +                                       print_event_type(opt_event_type),
>                                         print_channel_name(channel_name));
>                 }
>
> @@ -255,9 +281,9 @@ static int disable_events(char *session_name)
>                         event.name[sizeof(event.name) - 1] = '\0';
>                         command_ret = lttng_disable_event_ext(handle, &event, channel_name, NULL);
>                         if (command_ret < 0) {
> -                               ERR("%s %s: %s (channel %s, session %s)",
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "System call" : "Event",
> +                               ERR("%s of type %s : %s (channel %s, session %s)",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 lttng_strerror(command_ret),
>                                                 command_ret == -LTTNG_ERR_NEED_CHANNEL_NAME
>                                                         ? print_raw_channel_name(channel_name)
> @@ -271,10 +297,10 @@ static int disable_events(char *session_name)
>                                  */
>                                 enabled = 1;
>                         } else {
> -                               MSG("%s %s %s disabled in channel %s for session %s",
> +                               MSG("%s %s of type %s disabled in channel %s for session %s",
>                                                 get_domain_str(dom.type),
> -                                               opt_event_type == LTTNG_EVENT_SYSCALL ? "system call" : "event",
>                                                 event_name,
> +                                               print_event_type(opt_event_type),
>                                                 print_channel_name(channel_name),
>                                                 session_name);
>                                 success = 1;
> @@ -338,12 +364,21 @@ int cmd_disable_events(int argc, const char **argv)
>                 case OPT_HELP:
>                         usage(stdout);
>                         goto end;
> -               case OPT_USERSPACE:
> -                       opt_userspace = 1;
> -                       break;
> -               case OPT_SYSCALL:
> +               case OPT_TYPE_SYSCALL:
>                         opt_event_type = LTTNG_EVENT_SYSCALL;
>                         break;
> +               case OPT_TYPE_TRACEPOINT:
> +                       opt_event_type = LTTNG_EVENT_TRACEPOINT;
> +                       break;
> +               case OPT_TYPE_PROBE:
> +                       opt_event_type = LTTNG_EVENT_PROBE;
> +                       break;
> +               case OPT_TYPE_FUNCTION:
> +                       opt_event_type = LTTNG_EVENT_FUNCTION;
> +                       break;
> +               case OPT_TYPE_ALL:
> +                       opt_event_type = LTTNG_EVENT_ALL;
> +                       break;
>                 case OPT_LIST_OPTIONS:
>                         list_cmd_options(stdout, long_options);
>                         goto end;
> @@ -372,6 +407,19 @@ int cmd_disable_events(int argc, const char **argv)
>                 goto end;
>         }
>
> +       /* Ust and agent only support ALL event type */
> +       if ((opt_userspace || opt_jul || opt_log4j || opt_python)
> +                       && opt_event_type != LTTNG_EVENT_ALL) {
> +               ERR("UST and agent (-j | -l | -p) event(s) disabling based on event type is not supported.\n");
> +               usage(stderr);
> +               ret = CMD_ERROR;
> +               goto end;
> +       }
> +
> +       if (opt) {
> +               /* code */
> +       }
> +
>         opt_event_list = (char*) poptGetArg(pc);
>         if (opt_event_list == NULL && opt_disable_all == 0) {
>                 ERR("Missing event name(s).\n");
> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
> index 9cbfef5..1007326 100644
> --- a/src/lib/lttng-ctl/lttng-ctl.c
> +++ b/src/lib/lttng-ctl/lttng-ctl.c
> @@ -1091,10 +1091,6 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
>         }
>
>         lsm.cmd_type = LTTNG_DISABLE_EVENT;
> -       if (ev->name[0] == '\0') {
> -               /* Disable all events */
> -               lttng_ctl_copy_string(ev->name, "*", sizeof(ev->name));
> -       }
>
>         lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
>         /* FIXME: copying non-packed struct to packed struct. */
> --
> 2.1.4
>



-- 
J?r?mie Galarneau
EfficiOS Inc.
http://www.efficios.com



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

end of thread, other threads:[~2015-09-22 17:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1442522277-31518-1-git-send-email-jonathan.rajotte-julien@efficios.com>
     [not found] ` <CA+jJMxscPwme95++cjrQU3eBPB8Tw88SDOQ0+rXrfvJS6t_gAw@mail.gmail.com>
2015-09-21 16:26   ` [lttng-dev] [PATCH lttng-tools 1/3] Fix: disable kernel event based on name and event type Jérémie Galarneau
2015-09-21 19:00 ` Jérémie Galarneau
2015-09-21 19:15   ` Jérémie Galarneau
2015-09-21 22:32   ` Jonathan Rajotte Julien
2015-09-21 19:29 ` Jérémie Galarneau
2015-09-21 22:43   ` [lttng-dev] [PATCH lttng-tools 1/4 v2] " Jonathan Rajotte
2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 2/4 v2] Use empty event name on disable -a for ust and agent domain Jonathan Rajotte
2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 3/4 v2] man: update disable-event section Jonathan Rajotte
2015-09-21 22:43     ` [lttng-dev] [PATCH lttng-tools 4/4 v2] Help: add -j -l -p option to help string Jonathan Rajotte
2015-09-22 17:47     ` [lttng-dev] [PATCH lttng-tools 1/4 v2] Fix: disable kernel event based on name and event type Jérémie Galarneau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox