Re: [PATCH 2/2] Add test for target_set tapset.

["Przemysław Pawełczyk" <przemyslaw@pawelczyk.it>]

On Fri, Jun 19, 2009 at 03:55, Josh Stone<jistone@redhat.com> wrote:
> On 06/18/2009 03:33 PM, Przemyslaw Pawelczyk wrote:
>> +proc abort {} {
>> +     global test
>> +     fail $test
>> +     exit
>> +}
>
> exit should not be used, because that will stop the entire testing
> session.  We don't want failure here to prevent other tests from running.

At the time I was unaware of this.

>> +     set pid_it $stp_pid
>> +     while {[info exists pid_array($pid_it)]} {
>> +             if {[exec pgrep -P $pid_it] != $pid_array($pid_it)} {
>> +                     abort
>> +             }
>> +             set pid_it $pid_array($pid_it)
>> +     }
>
> There's a race here that the sleep process might finish before pgrep
> sees it.  Most of the time, one second will probably be plenty of time,
> but on a slow and/or loaded system we could have false failures.
>
> What if you just used an absurdly long timeout for sleep, and then "kill
> -INT" it after you've verified the chain?

Great idea. Thanks.

>> +probe nd_syscall.nanosleep
>> +{
>> +     if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "<linux/time.h>")->tv_sec == $1)
>> +             target_set_report()
>> +}
>
> Some systems have a 32-bit userspace with a 64-bit kernel, and in that
> case you would need to catch nd_syscall.compat_nanosleep as well.

Ok.

> I feel like you're going through somewhat heroic efforts to validate
> this in tcl, and you're not able to use any of the common infrastructure
> we have for other tests.  Maybe it would easier to check results within
> the script?  We couldn't check the report() that way, but
> target_set_pid() is what we really care about anyway, right?
>
> I'm imagining that in the nanosleep probe, you could recursively walk up
> task_parent() until you hit stp_pid() or 1 (init).  Then as the
> recursion unwinds, make sure that target_set_pid() matches.  You can use
> system() to also launch a sleep that's outside of the target_set.  Does
> that make sense?

IMO tested systemtap script should do as little as possible and the
testing is duty of the tester, here: expect/tcl script. Easiness of
implementing tester is secondary thing.

I'll send v2 of the patch right away.

> Josh

Regards.

-- 
Przemysław Pawełczyk