<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 27, 2015 at 1:39 PM, Partha Pratim Mukherjee <span dir="ltr"><<a href="mailto:ppm.floss@gmail.com" target="_blank">ppm.floss@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Jérémie Galarneau <<a href="mailto:jeremie.galarneau@efficios.com">jeremie.galarneau@efficios.com</a>> writes:<br>
<br>
...<br>
<span class="">> This may not be an error.<br>
> get_session_name() will return NULL whenever a default session is not defined. This will happen, for instance, when loading a session<br>
> configuration.<br>
><br>
> The test_load script, under lttng-tools/tests/regression/tools/save-load will perform:<br>
><br>
> $ lttng load -i load-42.lttng<br>
> $ lttng destroy load-42<br>
> Session load-42 destroyed<br>
> Error: Can't find valid lttng config /home/jgalar/.lttngrc<br>
> Did you create a session? (lttng create <my_session>)<br>
> Error: Command error<br>
><br>
> (make sure you delete ~/.lttngrc beforehand to test this properly)<br>
><br>
>> + if (strncmp(session->name, session_name, NAME_MAX) == 0) {<br>
>> + config_destroy_default();<br>
>> + }<br>
>> +<br>
><br>
> We should skip this check if get_session_name() returned NULL with something like:<br>
><br>
> if (session_name && strncmp(session->name, session_name, NAME_MAX) == 0) { ...<br>
><br>
<br>
</span>I did not know about load option which may load single/multiple sessions<br>
from a file. So we can't set a default session. I agree on your<br>
suggestion. This may be handled like this.<br>
<br>
        session_name = get_session_name();<br>
<span class=""><br>
        if (session_name && strncmp(session->name, session_name, NAME_MAX) == 0) {<br>
</span>                config_destroy_default();<br>
        }<br>
<br>
        free(session_name);<br>
<span class=""><br>
><br>
> get_session_name() should not log this when an .lttngrc file can't be found; it might not be an error.<br>
> It should be handled on a case-by-case basis by its callers.<br>
><br>
<br>
</span>Now if we remove the error messages from get_session_name() as you have<br>
suggested, then we have to handle other commands which are looking for a<br>
default session like enable-event which now gives a very generic error<br>
message like this<br>
<br>
Error: Command error<br>
<br>
So we have to add error messages on all those commands which are expecting<br>
a default session. As per the help message the following commands need to<br>
be modified.<br>
<br>
destroy, disable-channel, disable-event, enable-channel, enable-event,<br>
save, snapshot, start, stop, track, untrack, view<br>
<br>
If yes, then I will create another patch and send.<br>
<br>
What do you suggest?<br>
<br></blockquote><div><br></div><div>Sounds good to me. Thanks!</div><div><br></div><div>Jérémie</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks,<br>
Partha<br>
<br>
> Thanks,<br>
> Jérémie<br>
><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Jérémie Galarneau<br>EfficiOS Inc.<br><a href="http://www.efficios.com" target="_blank">http://www.efficios.com</a></div>
</div></div>