[lttng-dev] [[RFC PATCH lttng-tools]] Fix: Disable all UST events matching the given name

[Jérémie Galarneau]

As discussed with Jonathan, while the original proposed patch would be
the "correct" fix, the disable-event command does not accept a filter,
loglevel and exclusion list. This makes it impossible to achieve a
perfect match such as what find_ust_app_event() would provide.
However, the current code is still incorrect as it only disables the
first event matching the provided name.

This patch iterates on all matches and disables all events matching
the name provided in the UST domain.

Jia, any chance you can test this on your end?

Thanks,
Jérémie

On Tue, Sep 15, 2015 at 4:59 PM, Jérémie Galarneau
<jeremie.galarneau at efficios.com> wrote:
> The session daemon will only disable the first event
> matching the name provided to the disable-event command.
> This fix iterates on all events matching the name, ignoring
> their filter, loglevel and exclusion list.
>
> Signed-off-by: Jérémie Galarneau <jeremie.galarneau at efficios.com>
> ---
>  src/bin/lttng-sessiond/ust-app.c | 78 +++++++++++++++++++++++++++++-----------
>  1 file changed, 57 insertions(+), 21 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
> index 4066b06..02f6153 100644
> --- a/src/bin/lttng-sessiond/ust-app.c
> +++ b/src/bin/lttng-sessiond/ust-app.c
> @@ -170,6 +170,34 @@ no_match:
>  }
>
>  /*
> + * Match function for the hash table lookup.
> + *
> + * Matches an ust app event based on its name only.
> + */
> +static int ht_match_ust_app_event_by_name(struct cds_lfht_node *node,
> +               const void *_key)
> +{
> +       struct ust_app_event *event;
> +       const char *name;
> +
> +       assert(node);
> +       assert(_key);
> +
> +       event = caa_container_of(node, struct ust_app_event, node.node);
> +       name = _key;
> +
> +       if (strncmp(event->attr.name, name, sizeof(event->attr.name))) {
> +               goto no_match;
> +       }
> +
> +       /* Match. */
> +       return 1;
> +
> +no_match:
> +       return 0;
> +}
> +
> +/*
>   * Unique add of an ust app event in the given ht. This uses the custom
>   * ht_match_ust_app_event match function and the event name as hash.
>   */
> @@ -3873,11 +3901,9 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>  {
>         int ret = 0;
>         struct lttng_ht_iter iter, uiter;
> -       struct lttng_ht_node_str *ua_chan_node, *ua_event_node;
> +       struct lttng_ht_node_str *ua_chan_node;
>         struct ust_app *app;
> -       struct ust_app_session *ua_sess;
>         struct ust_app_channel *ua_chan;
> -       struct ust_app_event *ua_event;
>
>         DBG("UST app disabling event %s for all apps in channel "
>                         "%s for session id %" PRIu64,
> @@ -3887,10 +3913,13 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>
>         /* For all registered applications */
>         cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) {
> +               struct ust_app_session *ua_sess;
> +               struct cds_lfht_node *ua_event_node;
> +
>                 if (!app->compatible) {
>                         /*
> -                        * TODO: In time, we should notice the caller of this error by
> -                        * telling him that this is a version error.
> +                        * TODO: In time, we should notice the caller of this
> +                        * error by telling him that this is a version error.
>                          */
>                         continue;
>                 }
> @@ -3901,28 +3930,35 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
>                 }
>
>                 /* Lookup channel in the ust app session */
> -               lttng_ht_lookup(ua_sess->channels, (void *)uchan->name, &uiter);
> +               lttng_ht_lookup(ua_sess->channels, (void *) uchan->name,
> +                               &uiter);
>                 ua_chan_node = lttng_ht_iter_get_node_str(&uiter);
> -               if (ua_chan_node == NULL) {
> +               if (!ua_chan_node) {
>                         DBG2("Channel %s not found in session id %" PRIu64 " for app pid %d."
> -                                       "Skipping", uchan->name, usess->id, app->pid);
> +                                       "Skipping", uchan->name, usess->id,
> +                                       app->pid);
>                         continue;
>                 }
>                 ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
>
> -               lttng_ht_lookup(ua_chan->events, (void *)uevent->attr.name, &uiter);
> -               ua_event_node = lttng_ht_iter_get_node_str(&uiter);
> -               if (ua_event_node == NULL) {
> -                       DBG2("Event %s not found in channel %s for app pid %d."
> -                                       "Skipping", uevent->attr.name, uchan->name, app->pid);
> -                       continue;
> -               }
> -               ua_event = caa_container_of(ua_event_node, struct ust_app_event, node);
> -
> -               ret = disable_ust_app_event(ua_sess, ua_event, app);
> -               if (ret < 0) {
> -                       /* XXX: Report error someday... */
> -                       continue;
> +               cds_lfht_for_each_duplicate(ua_chan->events->ht,
> +                               ua_chan->events->hash_fct(uevent->attr.name,
> +                                       lttng_ht_seed),
> +                               ht_match_ust_app_event_by_name,
> +                               (void *) uevent->attr.name, &uiter.iter,
> +                               ua_event_node) {
> +                       struct ust_app_event *ua_event;
> +
> +                       ua_event = caa_container_of(ua_event_node,
> +                                       struct ust_app_event, node);
> +                       ret = disable_ust_app_event(ua_sess, ua_event, app);
> +                       if (ret < 0) {
> +                               DBG("Failed to disable event %s in channel %s "
> +                                               "of app pid %d.",
> +                                               uevent->attr.name,
> +                                               uchan->name, app->pid);
> +                               continue;
> +                       }
>                 }
>         }
>
> --
> 2.5.2
>



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