From: Jérémie Galarneau Date: Wed, 27 Mar 2019 22:36:15 +0000 (-0400) Subject: Fix: no-output sessions do not enforce snapshot constraints X-Git-Url: http://git.efficios.com/?p=lttng-tools.git;a=commitdiff_plain;h=54213acc35b30fa5e95dd275c31cfb584d4e4abf 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 --- diff --git a/include/lttng/lttng-error.h b/include/lttng/lttng-error.h index 0ea77aadb..3f645fe1b 100644 --- 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 dc2c91ddc..b270a43ca 100644 --- 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 033018620..9cbadc6dd 100644 --- 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 82e409521..8d41c38aa 100644 --- 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 b6b9faa59..33ab5d995 100644 --- 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 dd1735dd4..11cdb0ed8 100644 --- 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"