Fix: lttng-destroy: missing newline on session destruction message

The lost packets/discarded events statistics are printed on the same
line as the session destruction progress message when the session is
stopped as part of the `destroy` command.

This is a consequence of printing the statistics as they are
retrieved; the statistics must be fetched before the destruction,
but the progress indicator is still being printed.

The statistics output is now formatted to a buffer and printed
after the session's destruction has completed.

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

diff --git a/src/bin/lttng/commands/destroy.c b/src/bin/lttng/commands/destroy.c
index 0a5489304e1bcae9cc7c792a9b91887fc3f486ed..eed1fe5c5878fc15679009c64b18e0788adf0cf9 100644 (file)
--- a/src/bin/lttng/commands/destroy.c
+++ b/src/bin/lttng/commands/destroy.c
@@ -59,18 +59,20 @@ static int destroy_session(struct lttng_session *session)
 {
        int ret;
        char *session_name = NULL;
-       bool session_was_stopped;
+       bool session_was_already_stopped;
        enum lttng_error_code ret_code;
        struct lttng_destruction_handle *handle = NULL;
-        enum lttng_destruction_handle_status status;
-       bool printed_wait_msg = false;
+       enum lttng_destruction_handle_status status;
+       bool newline_needed = false, printed_destroy_msg = false;
        enum lttng_rotation_state rotation_state;
+       char *stats_str = NULL;
 
        ret = lttng_stop_tracing_no_wait(session->name);
        if (ret < 0 && ret != -LTTNG_ERR_TRACE_ALREADY_STOPPED) {
                ERR("%s", lttng_strerror(ret));
        }
-       session_was_stopped = ret == -LTTNG_ERR_TRACE_ALREADY_STOPPED;
+
+       session_was_already_stopped = ret == -LTTNG_ERR_TRACE_ALREADY_STOPPED;
        if (!opt_no_wait) {
                do {
                        ret = lttng_data_pending(session->name);
@@ -80,16 +82,15 @@ static int destroy_session(struct lttng_session *session)
                        }
 
                        /*
-                        * Data sleep time before retrying (in usec). Don't sleep if the call
-                        * returned value indicates availability.
+                        * Data sleep time before retrying (in usec). Don't
+                        * sleep if the call returned value indicates
+                        * availability.
                         */
                        if (ret) {
-                               if (!printed_wait_msg) {
-                                       _MSG("Waiting for destruction of session \"%s\"",
-                                                       session->name);
-                                       printed_wait_msg = true;
-                                       fflush(stdout);
-                               }
+                               _MSG("Destroying session %s", session->name);
+                               newline_needed = true;
+                               printed_destroy_msg = true;
+                               fflush(stdout);
 
                                usleep(DEFAULT_DATA_AVAILABILITY_WAIT_TIME_US);
                                _MSG(".");
@@ -97,12 +98,16 @@ static int destroy_session(struct lttng_session *session)
                        }
                } while (ret != 0);
        }
-       if (!session_was_stopped) {
+
+       if (!session_was_already_stopped) {
                /*
                 * Don't print the event and packet loss warnings since the user
                 * already saw them when stopping the trace.
                 */
-               print_session_stats(session->name);
+               ret = get_session_stats_str(session->name, &stats_str);
+               if (ret < 0) {
+                       goto error;
+               }
        }
 
        ret_code = lttng_destroy_session_ext(session->name, &handle);
@@ -116,14 +121,15 @@ static int destroy_session(struct lttng_session *session)
        }
 
        do {
-               status = lttng_destruction_handle_wait_for_completion(handle,
-                               DEFAULT_DATA_AVAILABILITY_WAIT_TIME_US / USEC_PER_MSEC);
+               status = lttng_destruction_handle_wait_for_completion(
+                               handle, DEFAULT_DATA_AVAILABILITY_WAIT_TIME_US /
+                                                       USEC_PER_MSEC);
                switch (status) {
                case LTTNG_DESTRUCTION_HANDLE_STATUS_TIMEOUT:
-                       if (!printed_wait_msg) {
-                               _MSG("Waiting for destruction of session \"%s\"",
-                                               session->name);
-                               printed_wait_msg = true;
+                       if (!printed_destroy_msg) {
+                               _MSG("Destroying session %s", session->name);
+                               newline_needed = true;
+                               printed_destroy_msg = true;
                        }
                        _MSG(".");
                        fflush(stdout);
@@ -131,8 +137,10 @@ static int destroy_session(struct lttng_session *session)
                case LTTNG_DESTRUCTION_HANDLE_STATUS_COMPLETED:
                        break;
                default:
-                       ERR("Failed to wait for the completion of the destruction of session \"%s\"",
+                       ERR("%sFailed to wait for the completion of the destruction of session \"%s\"",
+                                       newline_needed ? "\n" : "",
                                        session->name);
+                       newline_needed = false;
                        ret = -1;
                        goto error;
                }
@@ -140,8 +148,10 @@ static int destroy_session(struct lttng_session *session)
 
        status = lttng_destruction_handle_get_result(handle, &ret_code);
        if (status != LTTNG_DESTRUCTION_HANDLE_STATUS_OK) {
-               ERR("Failed to get the result of session destruction");
+               ERR("%sFailed to get the result of session destruction",
+                               newline_needed ? "\n" : "");
                ret = -1;
+               newline_needed = false;
                goto error;
        }
        if (ret_code != LTTNG_OK) {
@@ -149,44 +159,50 @@ static int destroy_session(struct lttng_session *session)
                goto error;
        }
 
-       status = lttng_destruction_handle_get_rotation_state(handle,
-                       &rotation_state);
+       status = lttng_destruction_handle_get_rotation_state(
+                       handle, &rotation_state);
        if (status != LTTNG_DESTRUCTION_HANDLE_STATUS_OK) {
-               ERR("Failed to get rotation state from destruction handle");
+               ERR("%sFailed to get rotation state from destruction handle",
+                               newline_needed ? "\n" : "");
+               newline_needed = false;
                goto skip_wait_rotation;
        }
+
        switch (rotation_state) {
        case LTTNG_ROTATION_STATE_NO_ROTATION:
                break;
-        case LTTNG_ROTATION_STATE_COMPLETED:
+       case LTTNG_ROTATION_STATE_COMPLETED:
        {
                const struct lttng_trace_archive_location *location;
 
-               status = lttng_destruction_handle_get_archive_location(handle,
-                               &location);
+               status = lttng_destruction_handle_get_archive_location(
+                               handle, &location);
                if (status == LTTNG_DESTRUCTION_HANDLE_STATUS_OK) {
-                       if (printed_wait_msg) {
-                               MSG("");
-                               printed_wait_msg = false;
-                       }
-                       ret = print_trace_archive_location(location,
-                                       session->name);
+                       ret = print_trace_archive_location(
+                                       location, session->name);
                        if (ret) {
-                               ERR("Failed to print the location of trace archive");
+                               ERR("%sFailed to print the location of trace archive",
+                                               newline_needed ? "\n" : "");
+                               newline_needed = false;
                                goto skip_wait_rotation;
                        }
                        break;
                }
                /* fall-through. */
-        }
-        default:
-               ERR("Failed to get the location of the rotation performed during the session's destruction");
+       }
+       default:
+               ERR("%sFailed to get the location of the rotation performed during the session's destruction",
+                               newline_needed ? "\n" : "");
+               newline_needed = false;
                goto skip_wait_rotation;
        }
 skip_wait_rotation:
-       MSG("%sSession \"%s\" destroyed", printed_wait_msg ? "\n" : "",
+       MSG("%sSession %s destroyed", newline_needed ? "\n" : "",
                        session->name);
-       printed_wait_msg = false;
+       newline_needed = false;
+       if (stats_str) {
+               MSG("%s", stats_str);
+       }
 
        session_name = get_session_name_quiet();
        if (session_name && !strncmp(session->name, session_name, NAME_MAX)) {
@@ -203,11 +219,12 @@ skip_wait_rotation:
 
        ret = CMD_SUCCESS;
 error:
-       if (printed_wait_msg) {
+       if (newline_needed) {
                MSG("");
        }
        lttng_destruction_handle_destroy(handle);
        free(session_name);
+       free(stats_str);
        return ret;
 }
 

diff --git a/src/bin/lttng/utils.c b/src/bin/lttng/utils.c
index 61718e54d2414ace10097f5d3b558fddc086a4ad..28b007b2402889d8782473033eee8119258ebc73 100644 (file)
--- a/src/bin/lttng/utils.c
+++ b/src/bin/lttng/utils.c
@@ -456,16 +456,30 @@ int print_missing_or_multiple_domains(unsigned int domain_count,
  */
 void print_session_stats(const char *session_name)
 {
-       int count, nb_domains, domain_idx, channel_idx, session_idx;
+       char *str;
+       const int ret = get_session_stats_str(session_name, &str);
+
+       if (ret >= 0 && str) {
+               MSG("%s", str);
+               free(str);
+       }
+}
+
+int get_session_stats_str(const char *session_name, char **out_str)
+{
+       int count, nb_domains, domain_idx, channel_idx, session_idx, ret;
        struct lttng_domain *domains;
        struct lttng_channel *channels;
        uint64_t discarded_events_total = 0, lost_packets_total = 0;
        struct lttng_session *sessions = NULL;
        const struct lttng_session *selected_session = NULL;
+       char *stats_str = NULL;
+       bool print_discarded_events = false, print_lost_packets = false;
 
        count = lttng_list_sessions(&sessions);
        if (count < 1) {
                ERR("Failed to retrieve session descriptions while printing session statistics.");
+               ret = -1;
                goto end;
        }
 
@@ -478,11 +492,13 @@ void print_session_stats(const char *session_name)
        }
        if (!selected_session) {
                ERR("Failed to retrieve session \"%s\" description while printing session statistics.", session_name);
+               ret = -1;
                goto end;
        }
 
        nb_domains = lttng_list_domains(session_name, &domains);
        if (nb_domains < 0) {
+               ret = -1;
                goto end;
        }
        for (domain_idx = 0; domain_idx < nb_domains; domain_idx++) {
@@ -491,12 +507,12 @@ void print_session_stats(const char *session_name)
 
                if (!handle) {
                        ERR("Failed to create session handle while printing session statistics.");
+                       ret = -1;
                        goto end;
                }
 
                count = lttng_list_channels(handle, &channels);
                for (channel_idx = 0; channel_idx < count; channel_idx++) {
-                       int ret;
                        uint64_t discarded_events = 0, lost_packets = 0;
                        struct lttng_channel *channel = &channels[channel_idx];
 
@@ -519,19 +535,44 @@ void print_session_stats(const char *session_name)
                }
                lttng_destroy_handle(handle);
        }
-       if (discarded_events_total > 0 && !selected_session->snapshot_mode) {
-               MSG("[warning] %" PRIu64 " events discarded, please refer to "
+
+       print_discarded_events = discarded_events_total > 0 &&
+                                !selected_session->snapshot_mode;
+       print_lost_packets = lost_packets_total > 0 &&
+                            !selected_session->snapshot_mode;
+
+       if (print_discarded_events && print_lost_packets) {
+               ret = asprintf(&stats_str,
+                               "Warning: %" PRIu64
+                               " events were discarded and %" PRIu64
+                               " packets were lost, please refer to "
+                               "the documentation on channel configuration.",
+                               discarded_events_total, lost_packets_total);
+       } else if (print_discarded_events) {
+               ret = asprintf(&stats_str,
+                               "Warning: %" PRIu64
+                               " events were discarded, please refer to "
                                "the documentation on channel configuration.",
                                discarded_events_total);
-       }
-       if (lost_packets_total > 0 && !selected_session->snapshot_mode) {
-               MSG("[warning] %" PRIu64 " packets lost, please refer to "
+       } else if (print_lost_packets) {
+               ret = asprintf(&stats_str,
+                               "Warning: %" PRIu64
+                               " packets were lost, please refer to "
                                "the documentation on channel configuration.",
                                lost_packets_total);
+       } else {
+               ret = 0;
        }
 
+       if (ret < 0) {
+               ERR("Failed to format lost packet and discarded events statistics");
+       } else {
+               *out_str = stats_str;
+               ret = 0;
+       }
 end:
        free(sessions);
+       return ret;
 }
 
 int show_cmd_help(const char *cmd_name, const char *help_msg)

diff --git a/src/bin/lttng/utils.h b/src/bin/lttng/utils.h
index cc7d7afcac7f5b2412be9eafc74448422fe95ef8..28ebc51a8fe5c829e37dfcbb033c9fdb3f37cd04 100644 (file)
--- a/src/bin/lttng/utils.h
+++ b/src/bin/lttng/utils.h
@@ -51,6 +51,7 @@ int print_missing_or_multiple_domains(unsigned int domain_count,
 int spawn_relayd(const char *pathname, int port);
 int check_relayd(void);
 void print_session_stats(const char *session_name);
+int get_session_stats_str(const char *session_name, char **str);
 int show_cmd_help(const char *cmd_name, const char *help_msg);
 
 int print_trace_archive_location(