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

[Jérémie Galarneau]

On Wed, May 27, 2015 at 1:39 PM, Partha Pratim Mukherjee <
ppm.floss at gmail.com> wrote:

> 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?
>
>
Sounds good to me. Thanks!

Jérémie


> Thanks,
> Partha
>
> > Thanks,
> > Jérémie
> >
>



-- 
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/20150527/ab09deaf/attachment-0001.html>