On Sat, 2010-03-27 at 08:23 +1100, nathans@xxxxxxxxxx wrote:
> ----- "Ken McDonell" <kenj@xxxxxxxxxxxxxxxx> wrote:
>
...
> > So if we are to keep PM_CONTEXT_LOCAL, then I support Corneliu's
> > suggestion that we should define new API support to add entries to
> > dsotbl[] at run-time ... but how would this work with something like
> > pminfo, where the metrics are arbitrary and the PCP client does not
> > really know _which_ PMDAs need to be loaded into the local context?
>
> I'd like to see this API cleaned up. We have the unused argument to
> pmNewContext which could be used to give, perhaps, a comma separated
> list of dso PMDAs... (pretty sure we audited this a year or two back
> and checked all callers were passing NULL in this argument atm, if it
> helps).
Don't you just hate it when the mail arrives after you've written the
code?
I don't think overloading the name argument to pmNewContext for the type
== PM_CONTEXT_LOCAL case is going to work ...
1. In general there may be more than one DSO PMDA that a client would
like to use via a local context
2. For each DSO PMDA, you need to specify
- a domain number
- a path (albeit relative) to the DSO
- the name of the initialization routine to call after dlopen()
3. So 1. and 2. mean you'd need a syntax like
sts = pmNewContext(PM_CONTEXT_LOCAL, "104|foo/pmdafoo.so|
init_foo,903|/home/me/secret/place/blah.so|blah_pmda_init")
which is getting a little ugly!
4. Independent of how this is done, we need to preserve the existing
behaviour (including the weird environment variables).
5. Any client using additional DSO PMDAs already needs to know a lot
about them, specifically their domain number, path to the DSO and
initialization routine name, so we're probably talking about a very
small set of targeted clients, like the cluster PMDA, standalone
foostat, etc.
The solution I've coded uses a new routine to register additional DSO
PMDAs for use with PM_CONTEXT_LOCAL. The example above would become:
if ((sts = __pmAddLocalPMDA(104, "foo/pmdafoo.so", "init_foo")) < 0) {
// exercise for reader
}
if ((sts = __pmAddLocalPMDA(903, "/home/me/secret/place/blah.so",
"blah_pmda_init")) < 0) {
// exercise for reader
}
...
sts = pmNewContext(PM_CONTEXT_NULL, NULL);
Before starting to QA this, we probably need consensus on the general
approach.
The only down-side I can see of the new libpcp routine is that it is
hard to properly control making this change and the corresponding change
the cluster PMDA (which triggered all of this) ... since they are in
different packages.
I have looked at the versions DSO hooks provided by gcc via
-Wl,--version-script=mylib.map (as suggested much earlier by Nathan in
another context) but unfortunately this does not come close to the weak
symbols and run-time symbol interrogation that the IRIX toolchain
supported which would have allowed the cluster PMDA to start even if the
symbol __pmAddLocalPMDA was undefined and to _discover_ if
__pmAddLocalPMDA was indeed defined before trying to call it.
The gcc approach addresses some other versioning issues but not this
particular one unfortunately.
|