[lttng-dev] [PATCH lttng-tools] Fix: destroy session removes the default config file

Partha Pratim Mukherjee ppm.floss at gmail.com
Wed May 27 13:39:05 EDT 2015


Jérémie Galarneau <jeremie.galarneau at efficios.com> writes:

...
> This may not be an error.
> get_session_name() will return NULL whenever a default session is not defined. This will happen, for instance, when loading a session
> configuration.
>
> The test_load script, under lttng-tools/tests/regression/tools/save-load will perform:
>
> $ lttng load -i load-42.lttng
> $ lttng destroy load-42
> Session load-42 destroyed
> Error: Can't find valid lttng config /home/jgalar/.lttngrc
> Did you create a session? (lttng create <my_session>)
> Error: Command error
>
> (make sure you delete ~/.lttngrc beforehand to test this properly)
>
>> + if (strncmp(session->name, session_name, NAME_MAX) == 0) {
>> + config_destroy_default();
>> + }
>> +
>
> We should skip this check if get_session_name() returned NULL with something like:
>
> if (session_name && strncmp(session->name, session_name, NAME_MAX) == 0) { ...
>

I did not know about load option which may load single/multiple sessions
from a file. So we can't set a default session. I agree on your
suggestion. This may be handled like this.

        session_name = get_session_name();

        if (session_name && strncmp(session->name, session_name, NAME_MAX) == 0) {
                config_destroy_default();
        }

        free(session_name);

>
> get_session_name() should not log this when an .lttngrc file can't be found; it might not be an error.
> It should be handled on a case-by-case basis by its callers.
>

Now if we remove the error messages from get_session_name() as you have
suggested, then we have to handle other commands which are looking for a
default session like enable-event which now gives a very generic error
message like this

Error: Command error

So we have to add error messages on all those commands which are expecting
a default session. As per the help message the following commands need to
be modified.

destroy, disable-channel, disable-event, enable-channel, enable-event,
save, snapshot, start, stop, track, untrack, view

If yes, then I will create another patch and send.

What do you suggest?

Thanks,
Partha

> Thanks,
> Jérémie
>



More information about the lttng-dev mailing list