[ltt-dev] [UST PATCH] Fix libustctl_function_tests

[nils.carlson@ericsson.com (Nils Carlson)]

On Wed, 23 Feb 2011, Mathieu Desnoyers wrote:

> * Nils Carlson (nils.carlson at ericsson.com) wrote:
>> On Wed, 23 Feb 2011, Mathieu Desnoyers wrote:
>>
>>> * Nils Carlson (nils.carlson at ericsson.com) wrote:
>>>> On Wed, 23 Feb 2011, Yannick Brosseau wrote:
>>>>
>>>>> After discussions, we concluded that the enable a non existing marker is a valid case, so we
>>>>> move it to the working case section.
>>>>> While being there, check that the re-enable a marker set the right errno
>>>>>
>>>>
>>>> I don't really understand your reasoning. This is a question of libust
>>>> being broken and the test-case showing this. We should fix the test-case.
>>>> It should not be possible to enable a non-existent marker, the function
>>>> should return -1, not 0 as it currently does.
>>>
>>> Enabling a non-existent marker/tracepoint is a perfectly valid case. If
>>> the application later on loads a library with dlopen that contains this
>>> marker, it should be enabled as soon as the library is loaded. So this
>>> requires that the user can ask for enabling markers that do not appear
>>> in the marker list yet. I do that to handle module loading in LTTng
>>> (kernel).
>>
>> Ok. So we can pre-activate a marker... Interesting. Then I agree we can
>> change the test-case. Though maybe somebody should document this as it
>> doesn't make a lot of sense unless you consider the library case. Also,
>> I'm not really sure I like it. If a user expects a marker to be there
>> and the marker isn't it feels silly not to give any feedback. I could see
>> a lot of users expecting ust to complain when you activate non-existent
>> markers, following the rule of least-suprise I think that it should
>> complain.
>>
>> Have you considered adding a parameter for this? Like a FORCE_ACTIVATE
>> flag or something?
>
> As we move to the new ltt-sessiond scheme with integration of the kernel
> and userspace LTTng / LTTng-UST, enabling tracepoints in not-yet-running
> applications and not-yet-loaded libraries will become an even more
> common use-case. I don't plan on spending too much time polishing the
> usttrace interface, given the changes planned.
>

Maybe we should discuss these plans? I have a feeling there might be some 
duplication of work going on. Also activating trace-points in not yet 
started applications sounds risky as today everything is handled by pid.


/Nils

> Thanks,
>
> Mathieu
>
>>
>> /Nils
>>
>>> Thanks,
>>>
>>> Mathieu
>>>
>>>>
>>>> /Nils
>>>>
>>>>> Signed-off-by: Yannick Brosseau <yannick.brosseau at gmail.com>
>>>>> ---
>>>>> .../libustctl_function_tests.c                     |   10 ++++++----
>>>>> 1 files changed, 6 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c
>>>>> index 947028f..7c12695 100644
>>>>> --- a/tests/libustctl_function_tests/libustctl_function_tests.c
>>>>> +++ b/tests/libustctl_function_tests/libustctl_function_tests.c
>>>>> @@ -21,6 +21,7 @@
>>>>> #include <unistd.h>
>>>>> #include <sys/types.h>
>>>>> #include <sys/wait.h>
>>>>> +#include <errno.h>
>>>>>
>>>>> #include <ust/marker.h>
>>>>> #include <ust/ustctl.h>
>>>>> @@ -136,15 +137,16 @@ static void ustctl_function_tests(pid_t pid)
>>>>>
>>>>> 	tap_ok(!ustctl_destroy_trace(trace, pid), "ustctl_destroy_trace - without ever starting");
>>>>>
>>>>> +	tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid) == 0,
>>>>> +	       "Enable non-existent marker ustl blar");
>>>>>
>>>>> 	printf("##### Tests that definetly should work are completed #####\n");
>>>>> 	printf("############## Start expected failure cases ##############\n");
>>>>>
>>>>> 	tap_ok(ustctl_set_marker_state(trace, "ust","bar", 1, pid),
>>>>> 	       "Enable already enabled marker ust/bar");
>>>>> -
>>>>> -	tap_ok(ustctl_set_marker_state(trace, "ustl", "blar", 1, pid),
>>>>> -	       "Enable non-existent marker ustl blar");
>>>>> +	tap_ok(EEXIST == errno,
>>>>> +	       "Right error code for enabling an already enabled marker");
>>>>>
>>>>> 	tap_ok(ustctl_start_trace(trace, pid),
>>>>> 	       "Start a non-existent trace");
>>>>> @@ -161,7 +163,7 @@ int main(int argc, char **argv)
>>>>> 	int i, status;
>>>>> 	pid_t parent_pid, child_pid;
>>>>>
>>>>> -	tap_plan(27);
>>>>> +	tap_plan(28);
>>>>>
>>>>> 	printf("Function tests for ustctl\n");
>>>>>
>>>>> --
>>>>> 1.7.2.3
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> ltt-dev mailing list
>>>>> ltt-dev at lists.casi.polymtl.ca
>>>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>>>>
>>>>
>>>> _______________________________________________
>>>> ltt-dev mailing list
>>>> ltt-dev at lists.casi.polymtl.ca
>>>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>>>
>>>
>>> --
>>> Mathieu Desnoyers
>>> Operating System Efficiency R&D Consultant
>>> EfficiOS Inc.
>>> http://www.efficios.com
>>>
>>
>
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
>