Hi Martins,
----- Original Message -----
> ----- Original Message -----
> > Attached is a nvidia/nvml pmda for general review and suggestions.
It sparked my interest & I hacked on it a bit further (and merged the
second update you sent me, thanks for that!), and have merged it into
the dev branch now. There's a couple of small things left...
> > I still need to do some error checking on metrics that may not be
> > [...]
I added the error checking, long option support, some QA testing, and
the dso independence discussed earlier. Oh - also work a closer look,
I added a fetch PDU handler to accompany your existing fetch callback.
This means when we get a fetch request, we only refresh the values at
the start (once), not once for each metric/instance pair. This is a
small efficiency improvement with how we interact with the cards, the
functionality is exactly the same as before.
Not essential, but it'd also be nice to have a man page as well, if you
wouldn't mind writing one? I didn't tackle that side of things, though
I did update the help text a bit based on my readings of the API docs
(please review? that, and all the code changes :) - heh, thanks!).
I've tested this only on "faked-out" hardware - see qa/744 - and not
using the real nvidia libraries (I have neither). Any chance you could
hack on those aspects before the next release Martins? (after the IB
updates you mentioned privately would be perfect, if you can). We aim
for a point release in the middle of each month, so there's a couple of
weeks until then. Especially the "using real hardware" side of testing
would be really good, since I can't do that here. :)
The other aspect I could look at is the spec file - you sent through an
initial version, which would provide a separate pcp-pmda-nvidia package.
However with the way the code is now, we could include this PMDA in the
main pcp package if you'd prefer that? (I would, it makes life easier
for users - this can be done since the PMDA now functions gracefully
with and without the nvidia library, even if the library appears while
its already running, it should handle that nicely too.
The only change we would need would be a one-line spec update for your
existing installations, to ensure existing pcp-pmda-nvidia RPMs you have
are replaced by pcp-3.9.7 or later (I'll go and hack on that when I hear
back re your packaging preference).
cheers.
--
Nathan
|