Fix: no-output sessions do not enforce snapshot constraints

A number of scenarios can lead to failures to record snapshots when a
session is created as "no-output" (not snapshot) and has snapshot
outputs added later-on.

These changes prevent a user from adding snapshot outputs after the
creation of a kernel channel that makes use of the 'splice' output
type since those do not allow the capture of snapshots.

Moreover, the output type of kernel channels is overriden when a
snapshot output is present in a session to behave like a snapshot
session would.

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

diff --git a/include/lttng/lttng-error.h b/include/lttng/lttng-error.h
index 0ea77aadbcf9b5475497df2bd8c7a5fbe8cc7dbf..3f645fe1bc33bc9ad68044ed2a1111428fbc6a41 100644 (file)
--- a/include/lttng/lttng-error.h
+++ b/include/lttng/lttng-error.h
@@ -167,7 +167,7 @@ enum lttng_error_code {
        LTTNG_ERR_ROTATION_PENDING_RELAY_FAIL_CONSUMER = 144, /* Rotation pending check (relay) failure on consumer */
        LTTNG_ERR_MKDIR_FAIL_CONSUMER    = 145, /* mkdir failure on consumer */
        LTTNG_ERR_CHAN_NOT_FOUND         = 146, /* Channel not found */
-
+       LTTNG_ERR_SNAPSHOT_UNSUPPORTED   = 147, /* Session configuration does not allow the use of snapshots */
 
        /* MUST be last element */
        LTTNG_ERR_NR,                           /* Last element */

diff --git a/src/bin/lttng-sessiond/channel.c b/src/bin/lttng-sessiond/channel.c
index dc2c91ddc5bde29d07a14b8759a74671c924b51b..b270a43ca3a29118a1218c6d32308ec1496fdef0 100644 (file)
--- a/src/bin/lttng-sessiond/channel.c
+++ b/src/bin/lttng-sessiond/channel.c
@@ -259,11 +259,6 @@ int channel_kernel_create(struct ltt_kernel_session *ksession,
                attr->attr.overwrite = !!ksession->snapshot_mode;
        }
 
-       /* Enforce mmap output for snapshot sessions. */
-       if (ksession->snapshot_mode) {
-               attr->attr.output = LTTNG_EVENT_MMAP;
-       }
-
        /* Validate common channel properties. */
        if (channel_validate(attr) < 0) {
                ret = LTTNG_ERR_INVALID;

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index 0330186209268cb63b49175e08d4a4102d07e96a..9cbadc6dde53fd16afde8b7a31cd6c4b15ac39cf 100644 (file)
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -1523,6 +1523,11 @@ int cmd_enable_channel(struct ltt_session *session,
                kchan = trace_kernel_get_channel_by_name(attr->name,
                                session->kernel_session);
                if (kchan == NULL) {
+                       if (session->snapshot.nb_output > 0 ||
+                                       session->snapshot_mode) {
+                               /* Enforce mmap output for snapshot sessions. */
+                               attr->attr.output = LTTNG_EVENT_MMAP;
+                       }
                        ret = channel_kernel_create(session->kernel_session, attr, wpipe);
                        if (attr->name[0] != '\0') {
                                session->kernel_session->has_non_default_channel = 1;
@@ -1591,6 +1596,9 @@ int cmd_enable_channel(struct ltt_session *session,
                goto error;
        }
 
+       if (ret == LTTNG_OK && attr->attr.output != LTTNG_EVENT_MMAP) {
+               session->has_non_mmap_channel = true;
+       }
 error:
        rcu_read_unlock();
 end:
@@ -3548,6 +3556,11 @@ int cmd_snapshot_add_output(struct ltt_session *session,
                goto error;
        }
 
+       if (session->has_non_mmap_channel) {
+               ret = LTTNG_ERR_SNAPSHOT_UNSUPPORTED;
+               goto error;
+       }
+
        /* Only one output is allowed until we have the "tee" feature. */
        if (session->snapshot.nb_output == 1) {
                ret = LTTNG_ERR_SNAPSHOT_OUTPUT_EXIST;

diff --git a/src/bin/lttng-sessiond/session.h b/src/bin/lttng-sessiond/session.h
index 82e409521c440c621f06d5d46a86d8cc380e6d6c..8d41c38aa60c911174f37ba92909f1c7d4ab345b 100644 (file)
--- a/src/bin/lttng-sessiond/session.h
+++ b/src/bin/lttng-sessiond/session.h
@@ -121,6 +121,12 @@ struct ltt_session {
         * creation defaults.
         */
        unsigned int snapshot_mode;
+       /*
+        * A session that has channels that don't use 'mmap' output can't be
+        * used to capture snapshots. This is set to true whenever a
+        * 'splice' kernel channel is enabled.
+        */
+       bool has_non_mmap_channel;
        /*
         * Timer set when the session is created for live reading.
         */

diff --git a/src/bin/lttng/commands/snapshot.c b/src/bin/lttng/commands/snapshot.c
index b6b9faa59be0e3412c18a0b56d7a284819b525b9..33ab5d99516186814987f251df01a952eb2b9a29 100644 (file)
--- a/src/bin/lttng/commands/snapshot.c
+++ b/src/bin/lttng/commands/snapshot.c
@@ -346,11 +346,23 @@ static int cmd_add_output(int argc, const char **argv)
        int ret;
 
        if (argc < 2 && (!opt_data_url || !opt_ctrl_url)) {
+               ERR("An output destination must be specified to add a snapshot output.");
                ret = CMD_ERROR;
                goto end;
        }
 
        ret = add_output(argv[1]);
+       if (ret < 0) {
+               switch (-ret) {
+               case LTTNG_ERR_SNAPSHOT_UNSUPPORTED:
+                       ERR("Session \"%s\" contains a channel that is incompatible with the snapshot functionality.\nMake sure all channels are configured in 'mmap' output mode.",
+                                       current_session_name);
+                       ret = CMD_ERROR;
+                       break;
+               default:
+                       break;
+               }
+       }
 
 end:
        return ret;
@@ -506,8 +518,22 @@ static enum cmd_error_code handle_command(const char **argv)
 
                        result = cmd->func(argc, argv);
                        if (result) {
-                               switch (-result) {
-                               case LTTNG_ERR_SNAPSHOT_NODATA:
+                               switch (result) {
+                               case CMD_ERROR:
+                               case CMD_UNDEFINED:
+                               case CMD_FATAL:
+                               case CMD_WARNING:
+                               case CMD_UNSUPPORTED:
+                                       /*
+                                        * Sub-commands mix lttng_error_codes
+                                        * and cmd_error_codes. This should be
+                                        * cleaned-up, but in the meantime this
+                                        * hack works since the values of the
+                                        * two enums do not intersect.
+                                        */
+                                       cmd_ret = result;
+                                       break;
+                               case -LTTNG_ERR_SNAPSHOT_NODATA:
                                        WARN("%s", lttng_strerror(result));
 
                                        /*  A warning is fine since the user has no control on

diff --git a/src/common/error.c b/src/common/error.c
index dd1735dd4779094f35dd85c028ce156e488f6754..11cdb0ed8d61968384e369be43ce61cbd320297f 100644 (file)
--- a/src/common/error.c
+++ b/src/common/error.c
@@ -208,6 +208,7 @@ static const char *error_string_array[] = {
        [ ERROR_INDEX(LTTNG_ERR_ROTATION_PENDING_RELAY_FAIL_CONSUMER) ] = "Rotation pending check (relay) failure on consumer",
        [ ERROR_INDEX(LTTNG_ERR_MKDIR_FAIL_CONSUMER) ] = "mkdir failure on consumer",
        [ ERROR_INDEX(LTTNG_ERR_CHAN_NOT_FOUND) ] = "Channel not found",
+       [ ERROR_INDEX(LTTNG_ERR_SNAPSHOT_UNSUPPORTED) ] = "Session configuration does not allow the use of snapshots",
 
        /* Last element */
        [ ERROR_INDEX(LTTNG_ERR_NR) ] = "Unknown error code"