From mboxrd@z Thu Jan 1 00:00:00 1970 From: nils.carlson@ericsson.com (Nils Carlson) Date: Wed, 23 Feb 2011 22:20:31 +0100 Subject: [ltt-dev] [UST PATCH] Fix libustctl_function_tests In-Reply-To: References: <1298483182-18994-1-git-send-email-yannick.brosseau@gmail.com> Message-ID: 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 >>>>> --- >>>>> .../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 >>>>> #include >>>>> #include >>>>> +#include >>>>> >>>>> #include >>>>> #include >>>>> @@ -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 >