Summary: | setting module arguments needs to be tested/documented | ||
---|---|---|---|
Product: | systemtap | Reporter: | David Smith <dsmith> |
Component: | documentation | Assignee: | Charley <chwang> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: | ||
Attachments: |
Short test framework for module arguments
Sample stp file for modargs testing Sample stp file for modargs testing Brief module arguments test Module arguments test exp file Short script Documentation patch |
Description
David Smith
2010-01-26 22:15:02 UTC
Now that I think about it a little more, it makes more and more sense to have the translator detect that globals are initialized at the script level, and to block potential setting via module options (by omitting the MODULE_PARAM goo for those variables). This would remove any ambiguity. Further, it would help with a background security concern, giving script authors the ability to *block* staprun users (such as restricted-privileged staprun people) from overriding selected globals that could change the precompiled module's behavior. (In reply to comment #1) > Now that I think about it a little more, it makes more and more sense to > have the translator detect that globals are initialized at the script > level, and to block potential setting via module options (by omitting > the MODULE_PARAM goo for those variables). This would remove any ambiguity. I see your point, but I also wonder if we lose something here. For instance, assume the following pseudo-script: global debug=0 probe PROBE_POINT { if (debug) { # print interesting debug information } } This allows a default 'debug' value of 0, which we then can override from the command line using module arguments when we want the extra debug output. If we omit the MODULE_PARAM goo for 'debug', we wouldn't be able to override the default value from the command line (without recompiling).
> I see your point, but I also wonder if we lose something here. For instance,
> assume the following pseudo-script:
>
> global debug=0
> probe PROBE_POINT {
> if (debug) {
> # print interesting debug information
> }
> }
>
> This allows a default 'debug' value of 0, which we then can override from the
> command line using module arguments when we want the extra debug output.
One doesn't have to explicitly initialize anything to 0. By leaving out
the =0, in my scheme one would permit runtime overrides.
Created attachment 4570 [details]
Short test framework for module arguments
Wrote a quick little test framework for module arguments.
Workflow:
Create testsuite/modargs/test.stp (see attached one.stp for example)
In the .stp file:
Echo expected result to modargs/result
Echo arguments to modargs/args
Use stap -p4 -m test to create a module called test.ko
Then just run modargs.exp and it should handle the rest. Will check that module
can be created and then that the module returns the expected result.
Created attachment 4571 [details]
Sample stp file for modargs testing
Does very, very, very basic testing of modargs -- setting a number/string and
printing it.
WRT to the discussion about default values and overwriting them, I think it's
fine either way. But there should be an easy way to mark values as
'no-overwrite,' and it may be a good idea to make globals 'no-overwrite' by
default (to avoid accidental or intentional tampering with values)? I'm not
sure, but it seems like if we are going to make this feature optional it would
be safer as opt-in rather than opt-out.
Also, it would be nice from a user/scripting perspective if staprun one.ko
nonexistant_variable=a would run with a warning instead of terminating with an
error :)
Created attachment 4572 [details]
Sample stp file for modargs testing
Does very, very, very basic testing of modargs -- setting a number/string and
printing it.
WRT to the discussion about default values and overwriting them, I think it's
fine either way. But there should be an easy way to mark values as
'no-overwrite,' and it may be a good idea to make globals 'no-overwrite' by
default (to avoid accidental or intentional tampering with values)? I'm not
sure, but it seems like if we are going to make this feature optional it would
be safer as opt-in rather than opt-out.
Also, it would be nice from a user/scripting perspective if staprun one.ko
nonexistant_variable=a would run with a warning instead of terminating with an
error :)
(In reply to comment #6) > [...] it seems like if we are going to make this feature optional it would > be safer as opt-in rather than opt-out. I agree, and I would prefer for this to be more explicit than just whether it was initialized or not. One possibility is a new keyword to augment or replace the global keyword for parameterized variables. (e.g. extern/param/...) > Also, it would be nice from a user/scripting perspective if staprun one.ko > nonexistant_variable=a would run with a warning instead of terminating with an > error :) I disagree, because then you get the class of bug reports like "Why doesn't setting this parameter change anything? Oh, because I used the wrong name..." I think this is not under our control anyway, because we're using the kernel's module-parameter facility. We could perhaps improve the error message though. (In reply to comment #4) > Created an attachment (id=4570) > Short test framework for module arguments Consider skipping the framework and just running stap -p4 / staprun on some one-liner script, right in the .exp file. Something like the pr10854 test case, but even smaller. As for documentation, a good start would be a few sentences in staprun.8.in regarding MODULE-OPTIONS, and a blurb in stap.1.in to identify this aspect of global variables. Created attachment 4625 [details]
Brief module arguments test
I made the result string "name=value number=value", so on a successful run the
return value should be the same as the module arguments. See also modargs.stp.
Created attachment 4626 [details]
Module arguments test exp file
Expanded test slightly to test setting initialized/uninitialized values
(passing -w in the stap -p4 command to ignore warnings about uninitialized
variables). Also added a negative number.
Sets these values:
initializedname=foo initializednumber=999 name=charley number=-1
Created attachment 4627 [details]
Short script
Short script for testing module argument setting. Has 2 initialized and 2
uninitialized variables.
Charley, this test case is fine to commit. FWIW, I'd make the .exp file less verbose, and simplify the pass / fail messages to the bare essentials: pass "$test compilation" fail "$test execution" etc.. Likewise, there is little need to log the expected/actual data with so much frill. If they don't already show up in the logs automatically, just print the strings. (The expected one isn't really necessary since one can simply inspect the test case.) So basically KISS. Created attachment 4630 [details]
Documentation patch
Blurb in Variables section of stap.1.in to briefly state that globals can be
changed on the command line as a module argument. Added to ARGUMENTS and
OPTIONS sections of staprun.8.in. If there are no major objections I will go
ahead and push :)
thanks |