On Mon, 2010-03-29 at 10:53 +1100, nathans@xxxxxxxxxx wrote:
...
> > ...
> > sts = pmNewContext(PM_CONTEXT_NULL, NULL);
> >
>
> Seems better than overloading new context as an API (assume you meant
> PM_CONTEXT_LOCAL at the end there?). A few other considerations:
Yes, PM_CONTEXT_LOCAL was the intent before the fat fingers got in the
way.
> - hard coding the domain numbers doesn't really make for readable code,
> wonder if we should install stdpmid as a header? or perhaps figure out
> the domain number from stdpmid on the fly and remove that 1st argument?
If the PMDA is not one of the "standard ones" or has been renamed for
some reason (I found a good reason to do that in the QA test when
calling sample_init() twice for the _same_ DSO produces something less
than desirable results), then I don't see how you could deduce the
domain number from the name of the DSO ... and unless the PMDA is using
the newer-than-new dynamic metrics services, the metrics for the PMDA
have to already be in the PMNS (or a local PMNS via the -n option) ...
so knowledge about the domain number needs to be pretty pervasive
already. Using stdpmid with #include is one possible solution, but the
symbols #defined therein are likely to cause serious clashes when you
compile the client application ... perhaps we need stdpmid.h to be built
from stdpmid by prefixing each name with PMDA_ ... but this would mean
writing into /usr/include (or the Windows equivalent) each time stdpmid
is rebuilt (which happens after software installation, not during a PCP
build).
Since this is such a low frequency usage, I'd probably opt for the
simpler
#define PMDA_BLAH 903
#define PMDA_FOO 104
in the source code that calls __pmAddLocalPMDA().
> - .so is platform specific (dylib on mac, dll on win) ... it would be
> a good idea to hide that aspect (platform differences) from the caller.
Would not be hard to have the code check for the existence of name,
name.so, name.dylib, name.dll, ... and use the first found file ...
would this work?
> - should there be a way to remove entries from the table too? Or at
> least start from a clean slate?
Yeah, I toyed with that but laziness won out ... I can revisit the
issue, but rather than a 3 or 4 more routines, I'd probably go the
ioctl() path and change to __pmLocalPMDA() with an additional first
argument being an opcode for one of ...
- ADD
- DEL (match domain number _or_ dso name)
- CLEAR (remove all entries)
Should I do this?
> > 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.
> >
>
> Sounded earlier like the current cluster PMDA source (in oss tree, not
> released by SGI from that tree yet, I guess?) is completely broken, so
> simply adding a configure-and-fail check there would be a good start.
> Guess we could bump the libpcp shared library soname too (ouch).
I'd suggest not bumping the soname version this for this particular
change, it just ain't worth the pain of many for the gain of so very
few. ... 8^)>
|