Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
From: compudj@krystal.dyndns.org (Mathieu Desnoyers)
Subject: [ltt-dev] [UST PATCH] Fix libustctl_function_tests
Date: Wed, 23 Feb 2011 15:56:28 -0500	[thread overview]
Message-ID: <BLU0-SMTP8934055010E140C658C24E96DB0@phx.gbl> (raw)
In-Reply-To: <alpine.DEB.2.00.1102232148240.22724@as68123.uab.ericsson.se>

* 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.

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



  reply	other threads:[~2011-02-23 20:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-23 17:46 Yannick Brosseau
2011-02-23 18:51 ` Nils Carlson
2011-02-23 18:43   ` Mathieu Desnoyers
2011-02-23 18:56   ` Nils Carlson
     [not found]   ` <BLU0-SMTP641C88D30CC26BE35858F96DB0@phx.gbl>
2011-02-23 20:54     ` Nils Carlson
2011-02-23 20:56       ` Mathieu Desnoyers [this message]
     [not found]       ` <BLU0-SMTP5860CC88B2AC6EDE7D2AEE96DB0@phx.gbl>
2011-02-23 21:20         ` Nils Carlson
2011-02-24  8:56 ` Nils Carlson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BLU0-SMTP8934055010E140C658C24E96DB0@phx.gbl \
    --to=compudj@krystal.dyndns.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox