[lttng-dev] [PATCH lttng-tools] Fix: Improve the error reporting of the track/untrack command

[Jérémie Galarneau]

On Wed, Aug 12, 2015 at 4:08 PM, Michael Jeanson <mjeanson at efficios.com>
wrote:

> This patch adds feedback when a track/untrack command is successful and
> specific messages on errors. It also changes the behavior when tracking
> an already tracked PID or untracking a PID that is not tracked from an
> error to a warning with a successful exit code.
>
> Fixes: #898
>
> Signed-off-by: Michael Jeanson <mjeanson at efficios.com>
> ---
>  include/lttng/lttng-error.h            |  2 ++
>  src/bin/lttng-sessiond/trace-ust.c     |  4 ++--
>  src/bin/lttng/commands/track-untrack.c | 23 +++++++++++++++++++++--
>  src/common/error.c                     |  2 ++
>  4 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/include/lttng/lttng-error.h b/include/lttng/lttng-error.h
> index efdca65..a7746fa 100644
> --- a/include/lttng/lttng-error.h
> +++ b/include/lttng/lttng-error.h
> @@ -134,6 +134,8 @@ enum lttng_error_code {
>         LTTNG_ERR_EXCLUSION_NOMEM        = 111, /* Lack of memory while
> processing event exclusions */
>         LTTNG_ERR_INVALID_EVENT_NAME     = 112, /* Invalid event name */
>         LTTNG_ERR_INVALID_CHANNEL_NAME   = 113, /* Invalid channel name */
> +       LTTNG_ERR_PID_TRACKED            = 114, /* PID already tracked */
> +       LTTNG_ERR_PID_NOT_TRACKED        = 115, /* PID not tracked */
>
>         /* MUST be last element */
>         LTTNG_ERR_NR,                           /* Last element */
> diff --git a/src/bin/lttng-sessiond/trace-ust.c
> b/src/bin/lttng-sessiond/trace-ust.c
> index 2486696..3881084 100644
> --- a/src/bin/lttng-sessiond/trace-ust.c
> +++ b/src/bin/lttng-sessiond/trace-ust.c
> @@ -656,7 +656,7 @@ int pid_tracker_add_pid(struct ust_pid_tracker
> *pid_tracker, int pid)
>         tracker_node = pid_tracker_lookup(pid_tracker, pid, &iter);
>         if (tracker_node) {
>                 /* Already exists. */
> -               retval = LTTNG_ERR_INVALID;
> +               retval = LTTNG_ERR_PID_TRACKED;
>                 goto end;
>         }
>         tracker_node = zmalloc(sizeof(*tracker_node));
> @@ -684,7 +684,7 @@ int pid_tracker_del_pid(struct ust_pid_tracker
> *pid_tracker, int pid)
>         tracker_node = pid_tracker_lookup(pid_tracker, pid, &iter);
>         if (!tracker_node) {
>                 /* Not found */
> -               retval = LTTNG_ERR_INVALID;
> +               retval = LTTNG_ERR_PID_NOT_TRACKED;
>                 goto end;
>         }
>         ret = lttng_ht_del(pid_tracker->ht, &iter);
> diff --git a/src/bin/lttng/commands/track-untrack.c
> b/src/bin/lttng/commands/track-untrack.c
> index 9ce8504..b3139c6 100644
> --- a/src/bin/lttng/commands/track-untrack.c
> +++ b/src/bin/lttng/commands/track-untrack.c
> @@ -261,9 +261,28 @@ enum cmd_error_code track_untrack_pid(enum cmd_type
> cmd_type, const char *cmd_st
>                 DBG("%s PID %d", cmd_str, pid_list[i]);
>                 ret = lib_func(handle, pid_list[i]);
>                 if (ret) {
> -                       success = 0;
> -                       retval = CMD_ERROR;
> +                       switch (-ret) {
> +                       case LTTNG_ERR_PID_TRACKED:
> +                               WARN("PID %i already tracked in session
> %s",
> +                                               pid_list[i], session_name);
> +                               success = 1;
> +                               retval = CMD_SUCCESS;
> +                               break;
> +                       case LTTNG_ERR_PID_NOT_TRACKED:
> +                               WARN("PID %i not tracked in session %s",
> +                                               pid_list[i], session_name);
> +                               success = 1;
> +                               retval = CMD_SUCCESS;
>

I'm wondering if untracking a PID that was not being tracked should be
reported as an error... Thoughts anyone?

Jérémie


> +                               break;
> +                       default:
> +                               ERR("%s", lttng_strerror(ret));
> +                               success = 0;
> +                               retval = CMD_ERROR;
> +                               break;
> +                       }
>                 } else {
> +                       MSG("PID %i %sed in session %s",
> +                                       pid_list[i], cmd_str,
> session_name);
>                         success = 1;
>                 }
>
> diff --git a/src/common/error.c b/src/common/error.c
> index d3e3b94..ad0ac51 100644
> --- a/src/common/error.c
> +++ b/src/common/error.c
> @@ -167,6 +167,8 @@ static const char *error_string_array[] = {
>         [ ERROR_INDEX(LTTNG_ERR_MI_NOT_IMPLEMENTED) ] = "Mi feature not
> implemented",
>         [ ERROR_INDEX(LTTNG_ERR_INVALID_EVENT_NAME) ] = "Invalid event
> name",
>         [ ERROR_INDEX(LTTNG_ERR_INVALID_CHANNEL_NAME) ] = "Invalid channel
> name",
> +       [ ERROR_INDEX(LTTNG_ERR_PID_TRACKED) ] = "PID already tracked",
> +       [ ERROR_INDEX(LTTNG_ERR_PID_NOT_TRACKED) ] = "PID not tracked",
>
>         /* Last element */
>         [ ERROR_INDEX(LTTNG_ERR_NR) ] = "Unknown error code"
> --
> 1.9.1
>
>


-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20150812/f59d0990/attachment.html>