Fix: filter attach vs event enable race

In order to correctly handle the use-case where events are enabled
_after_ trace is started, and _after_ applications are already being
traced, the event should be created in a "disabled" state, so that it
does not trace events until its filter is attached.

This fix needs to be done both in lttng-tools and lttng-ust. In order to
keep ABI compatibility between tools and ust within a stable release
cycle, we introduce a new "disabled" within struct lttng_ust_event
padding (previously zeroed). Newer LTTng-UST checks this flag, and
fallback on the old racy behavior (enabling the event on creation) if it
is unset.

Therefore, old session daemon works with newer lttng-ust of the same
stable release, and vice-versa. However, building lttng-tools requires
an upgraded lttng-ust, which contains the communication protocol with
the new "disabled" field.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/include/lttng/ust-abi.h b/include/lttng/ust-abi.h
index 0287466b5746a4aa7a673ff8f2e3180334158f8b..1c1d961405bdd5f8352ac4f9d22b8757a3058906 100644 (file)
--- a/include/lttng/ust-abi.h
+++ b/include/lttng/ust-abi.h
@@ -98,7 +98,7 @@ struct lttng_ust_stream {
         */
 } LTTNG_PACKED;
 
-#define LTTNG_UST_EVENT_PADDING1       16
+#define LTTNG_UST_EVENT_PADDING1       15
 #define LTTNG_UST_EVENT_PADDING2       (LTTNG_UST_SYM_NAME_LEN + 32)
 struct lttng_ust_event {
        enum lttng_ust_instrumentation instrumentation;
@@ -106,6 +106,7 @@ struct lttng_ust_event {
 
        enum lttng_ust_loglevel_type loglevel_type;
        int loglevel;   /* value, -1: all */
+       char disabled;
        char padding[LTTNG_UST_EVENT_PADDING1];
 
        /* Per instrumentation type configuration */

diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
index e07ec926adb51b843c66b0e55207f01e5432b628..8a0ef0bfdd2ebfb580f5683c4a814893228ca1b6 100644 (file)
--- a/liblttng-ust-ctl/ustctl.c
+++ b/liblttng-ust-ctl/ustctl.c
@@ -200,6 +200,7 @@ int ustctl_create_event(int sock, struct lttng_ust_event *ev,
        lum.u.event.instrumentation = ev->instrumentation;
        lum.u.event.loglevel_type = ev->loglevel_type;
        lum.u.event.loglevel = ev->loglevel;
+       lum.u.event.disabled = ev->disabled;
        ret = ustcomm_send_app_cmd(sock, &lum, &lur);
        if (ret) {
                free(event_data);

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 7c5c05ed63955e89ba6c0bb104f49ae8c48c871e..68c024c4647e9206f6b9bec37d17f63e40425be8 100644 (file)
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -741,7 +741,16 @@ struct lttng_enabler *lttng_enabler_create(enum lttng_enabler_type type,
                sizeof(enabler->event_param));
        enabler->chan = chan;
        /* ctx left NULL */
-       enabler->enabled = 1;
+       /*
+        * The "disable" event create comm field has been added to fix a
+        * race between event creation (of a started trace) and enabling
+        * filtering. New session daemon always set the "disable" field
+        * to 1, and are aware that they need to explicitly enable the
+        * event. Older session daemon (within same ABI) leave it at 0,
+        * and therefore we need to enable it here, keeping the original
+        * racy behavior.
+        */
+       enabler->enabled = !event_param->disabled;
        cds_list_add(&enabler->node, &enabler->chan->session->enablers_head);
        lttng_session_lazy_sync_enablers(enabler->chan->session);
        return enabler;