Fix: cmd_rotate_session() returns unexpected error codes

Certain paths in cmd_rotate_session() result in its return value not
obeying the convention for commands: return LTTNG_OK on success, and a
negative LTTNG_ERR_* code on error.

This patch separates the use of 'int ret' from a separate 'enum
lttng_error_code' value to ensure 'ret' values never bubble-up to the
caller.

Note that this patch assumes that ust_app_rotate_session() returns an
lttng_error_code, which is not the case right now (upcoming patch).

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

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index 4d4d0088cf8299ed7859c5b1558d4db0aac644f9..50bacf0c236dc3ea4917a6f4100a80aa39b98623 100644 (file)
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -4521,12 +4521,13 @@ int cmd_set_session_shm_path(struct ltt_session *session,
  * Ask the consumer to rotate the session output directory.
  * The session lock must be held.
  *
- * Return LTTNG_OK on success or else a LTTNG_ERR code.
+ * Returns LTTNG_OK on success or else a negative LTTng error code.
  */
 int cmd_rotate_session(struct ltt_session *session,
                struct lttng_rotate_session_return *rotate_return)
 {
        int ret;
+       enum lttng_error_code cmd_ret = LTTNG_OK;
        size_t strf_ret;
        struct tm *timeinfo;
        char datetime[21];
@@ -4540,13 +4541,13 @@ int cmd_rotate_session(struct ltt_session *session,
        assert(session);
 
        if (!session->has_been_started) {
-               ret = -LTTNG_ERR_START_SESSION_ONCE;
+               cmd_ret = LTTNG_ERR_START_SESSION_ONCE;
                goto end;
        }
 
        if (session->live_timer || session->snapshot_mode ||
                        !session->output_traces) {
-               ret = -LTTNG_ERR_ROTATION_NOT_AVAILABLE;
+               cmd_ret = LTTNG_ERR_ROTATION_NOT_AVAILABLE;
                goto end;
        }
 
@@ -4556,14 +4557,14 @@ int cmd_rotate_session(struct ltt_session *session,
        if (session->consumer->type == CONSUMER_DST_NET &&
                        (session->consumer->relay_major_version == 2 &&
                        session->consumer->relay_minor_version < 11)) {
-               ret = -LTTNG_ERR_ROTATION_NOT_AVAILABLE_RELAY;
+               cmd_ret = LTTNG_ERR_ROTATION_NOT_AVAILABLE_RELAY;
                goto end;
        }
 
        if (session->rotation_state == LTTNG_ROTATION_STATE_ONGOING) {
-               ret = -LTTNG_ERR_ROTATION_PENDING;
                DBG("Refusing to launch a rotation; a rotation is already in progress for session %s",
                                session->name);
+               cmd_ret = LTTNG_ERR_ROTATION_PENDING;
                goto end;
        }
 
@@ -4574,7 +4575,7 @@ int cmd_rotate_session(struct ltt_session *session,
        if (session->rotated_after_last_stop) {
                DBG("Session \"%s\" was already rotated after stop, refusing rotation",
                                session->name);
-               ret = -LTTNG_ERR_ROTATION_MULTIPLE_AFTER_STOP;
+               cmd_ret = LTTNG_ERR_ROTATION_MULTIPLE_AFTER_STOP;
                goto end;
        }
 
@@ -4592,7 +4593,7 @@ int cmd_rotate_session(struct ltt_session *session,
                                sizeof(session->rotation_chunk.current_rotate_path));
                if (ret) {
                        ERR("Failed to copy session base path to current rotation chunk path");
-                       ret = -LTTNG_ERR_UNK;
+                       cmd_ret = LTTNG_ERR_UNK;
                        goto end;
                }
        } else {
@@ -4605,7 +4606,7 @@ int cmd_rotate_session(struct ltt_session *session,
                                sizeof(session->rotation_chunk.current_rotate_path));
                if (ret) {
                        ERR("Failed to copy the active tracing path to the current rotate path");
-                       ret = -LTTNG_ERR_UNK;
+                       cmd_ret = LTTNG_ERR_UNK;
                        goto end;
                }
        }
@@ -4619,7 +4620,7 @@ int cmd_rotate_session(struct ltt_session *session,
 
        now = time(NULL);
        if (now == (time_t) -1) {
-               ret = -LTTNG_ERR_ROTATION_NOT_AVAILABLE;
+               cmd_ret = LTTNG_ERR_ROTATION_NOT_AVAILABLE;
                goto end;
        }
 
@@ -4633,14 +4634,14 @@ int cmd_rotate_session(struct ltt_session *session,
        timeinfo = localtime(&now);
        if (!timeinfo) {
                PERROR("Failed to sample local time in rotate session command");
-               ret = -LTTNG_ERR_UNK;
+               cmd_ret = LTTNG_ERR_UNK;
                goto end;
        }
        strf_ret = strftime(datetime, sizeof(datetime), "%Y%m%dT%H%M%S%z",
                        timeinfo);
        if (!strf_ret) {
                ERR("Failed to format local time timestamp in rotate session command");
-               ret = -LTTNG_ERR_UNK;
+               cmd_ret = LTTNG_ERR_UNK;
                goto end;
        }
 
@@ -4665,7 +4666,7 @@ int cmd_rotate_session(struct ltt_session *session,
                                datetime, session->current_archive_id + 1);
                if (ret < 0 || ret == sizeof(session->rotation_chunk.active_tracing_path)) {
                        ERR("Failed to format active kernel tracing path in rotate session command");
-                       ret = -LTTNG_ERR_UNK;
+                       cmd_ret = LTTNG_ERR_UNK;
                        goto error;
                }
                /*
@@ -4678,7 +4679,7 @@ int cmd_rotate_session(struct ltt_session *session,
                                session->current_archive_id + 1);
                if (ret < 0 || ret == sizeof(session->kernel_session->consumer->chunk_path)) {
                        ERR("Failed to format the kernel consumer's sub-directory in rotate session command");
-                       ret = -LTTNG_ERR_UNK;
+                       cmd_ret = LTTNG_ERR_UNK;
                        goto error;
                }
                /*
@@ -4691,12 +4692,11 @@ int cmd_rotate_session(struct ltt_session *session,
                if (ret) {
                        ERR("Failed to create kernel session tracing path at %s",
                                        session->kernel_session->consumer->chunk_path);
-                       ret = -LTTNG_ERR_CREATE_DIR_FAIL;
+                       cmd_ret = LTTNG_ERR_CREATE_DIR_FAIL;
                        goto error;
                }
-               ret = kernel_rotate_session(session);
-               if (ret != LTTNG_OK) {
-                       ret = -ret;
+               cmd_ret = kernel_rotate_session(session);
+               if (cmd_ret != LTTNG_OK) {
                        goto error;
                }
        }
@@ -4707,7 +4707,7 @@ int cmd_rotate_session(struct ltt_session *session,
                                datetime, session->current_archive_id + 1);
                if (ret < 0) {
                        ERR("Failed to format active UST tracing path in rotate session command");
-                       ret = -LTTNG_ERR_UNK;
+                       cmd_ret = LTTNG_ERR_UNK;
                        goto error;
                }
                ret = snprintf(session->ust_session->consumer->chunk_path,
@@ -4715,7 +4715,7 @@ int cmd_rotate_session(struct ltt_session *session,
                                session->current_archive_id + 1);
                if (ret < 0) {
                        ERR("Failed to format the UST consumer's sub-directory in rotate session command");
-                       ret = -LTTNG_ERR_UNK;
+                       cmd_ret = LTTNG_ERR_UNK;
                        goto error;
                }
                /*
@@ -4726,11 +4726,15 @@ int cmd_rotate_session(struct ltt_session *session,
                                session->ust_session->uid,
                                session->ust_session->gid);
                if (ret) {
-                       ret = -LTTNG_ERR_CREATE_DIR_FAIL;
+                       cmd_ret = LTTNG_ERR_CREATE_DIR_FAIL;
                        goto error;
                }
-               ret = ust_app_rotate_session(session);
-               if (ret != LTTNG_OK) {
+               /*
+                * TODO: ust_app_rotate_session must be adapted to return
+                * an lttng_error_code, like its kernel counterpart.
+                */
+               cmd_ret = ust_app_rotate_session(session);
+               if (cmd_ret != LTTNG_OK) {
                        goto error;
                }
        }
@@ -4738,6 +4742,7 @@ int cmd_rotate_session(struct ltt_session *session,
        ret = timer_session_rotation_pending_check_start(session,
                        DEFAULT_ROTATE_PENDING_TIMER);
        if (ret) {
+               cmd_ret = LTTNG_ERR_UNK;
                goto error;
        }
 
@@ -4756,13 +4761,13 @@ int cmd_rotate_session(struct ltt_session *session,
        if (ret != LTTNG_OK) {
                ERR("Failed to notify notification thread that a session rotation is ongoing for session %s",
                                session->name);
+               cmd_ret = ret;
        }
 
        DBG("Cmd rotate session %s, archive_id %" PRIu64 " sent",
                        session->name, session->current_archive_id - 1);
-       ret = LTTNG_OK;
-
 end:
+       ret = (cmd_ret == LTTNG_OK) ? cmd_ret : -((int) cmd_ret);
        return ret;
 error:
        session->last_chunk_start_ts = original_last_chunk_start_ts;

diff --git a/src/bin/lttng-sessiond/kernel.c b/src/bin/lttng-sessiond/kernel.c
index be263943f0211a9cffdb563f0a22ddfda9e9653c..4327db24bdb3291f575da0d90c143e280c7bdbfa 100644 (file)
--- a/src/bin/lttng-sessiond/kernel.c
+++ b/src/bin/lttng-sessiond/kernel.c
@@ -1401,11 +1401,12 @@ error:
 /*
  * Rotate a kernel session.
  *
- * Return 0 on success or else return a LTTNG_ERR code.
+ * Return LTTNG_OK on success or else an LTTng error code.
  */
-int kernel_rotate_session(struct ltt_session *session)
+enum lttng_error_code kernel_rotate_session(struct ltt_session *session)
 {
        int ret;
+       enum lttng_error_code status = LTTNG_OK;
        struct consumer_socket *socket;
        struct lttng_ht_iter iter;
        struct ltt_kernel_session *ksess = session->kernel_session;
@@ -1436,7 +1437,7 @@ int kernel_rotate_session(struct ltt_session *session)
                                        /* is_metadata_channel */ false,
                                        session->current_archive_id);
                        if (ret < 0) {
-                               ret = LTTNG_ERR_KERN_CONSUMER_FAIL;
+                               status = LTTNG_ERR_KERN_CONSUMER_FAIL;
                                goto error;
                        }
                }
@@ -1450,14 +1451,12 @@ int kernel_rotate_session(struct ltt_session *session)
                                /* is_metadata_channel */ true,
                                session->current_archive_id);
                if (ret < 0) {
-                       ret = LTTNG_ERR_KERN_CONSUMER_FAIL;
+                       status = LTTNG_ERR_KERN_CONSUMER_FAIL;
                        goto error;
                }
        }
 
-       ret = LTTNG_OK;
-
 error:
        rcu_read_unlock();
-       return ret;
+       return status;
 }

diff --git a/src/bin/lttng-sessiond/kernel.h b/src/bin/lttng-sessiond/kernel.h
index 8d5ddb6687f8ff696cc3196c9a2082c2f84efb0d..e9024a83e975290b53b9f624d8e8cc8060984f0d 100644 (file)
--- a/src/bin/lttng-sessiond/kernel.h
+++ b/src/bin/lttng-sessiond/kernel.h
@@ -63,7 +63,7 @@ int kernel_snapshot_record(struct ltt_kernel_session *ksess,
                struct snapshot_output *output, int wait,
                uint64_t nb_packets_per_stream);
 int kernel_syscall_mask(int chan_fd, char **syscall_mask, uint32_t *nr_bits);
-int kernel_rotate_session(struct ltt_session *session);
+enum lttng_error_code kernel_rotate_session(struct ltt_session *session);
 
 int init_kernel_workarounds(void);
 ssize_t kernel_list_tracker_pids(struct ltt_kernel_session *session,

diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h
index 5aef9e9519879cfa463188b42d94687d23462010..a5dc0d300c51a790bd762d85307829bd50389a44 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.h
+++ b/src/bin/lttng-sessiond/ust-app.h
@@ -595,7 +595,7 @@ int ust_app_regenerate_statedump_all(struct ltt_ust_session *usess)
 }
 
 static inline
-int ust_app_rotate_session(struct ltt_session *session)
+enum lttng_error_code ust_app_rotate_session(struct ltt_session *session)
 {
        return 0;
 }