[Fusioninventory-devel] Proposed enhancements/corrections

[Guillaume Rousse]

Le 04/10/2011 15:32, Oliver Tuschhoff a écrit :
> Dear FusionInventory-Team,
>
> thank you for providing such a great piece of software. While deploying
> the Agent into our environment I had to make several
> enhancements/corrections which I wanted to give back (against Version
> 2.1.10):
Hello Olivier.

We're currently finalising 2.2.x branch, where some of the issues you 
are reporting seems already fixed.

> fusioninventory-win32-service.patch:
> Correcting the name of a utilized function of the Win32::Daemon package
> from LastControlMessage() to QueryLastMessage().
> The change in the underlying package seems to have happened in version
> 20101014 and is still current
> (http://cpansearch.perl.org/src/JDB/Win32-Daemon-20110117/Changes).
Seems fines for me.

> solaris-plugins-zonename.patch:
> While calling zonename to detect the zone-type, a newline-char
> (outputted by zonename) prevents the correct detection.
> The proposed patch sanitizes the output of zonename by piping its output
> into tr -d [[:cntrl:]].
> Affected solaris-plugins: CPU, Bios, Memory, Networks
Using external command is not a good idea. If that's just a newline, 
using chomp() should be enough.

BTW, that's already the case in 2.2.x branch, getZone() function in 
FusionInventory::Agent::Tools::Solaris relies on getFirstLine(), which 
automatically chomp its output.

> solaris-network-plugin.patch:
> While checking the autospeed-status of a bge network-adapter, the called
> system-app (ndd) seems to be given bogus variables, which prevents
> correct detection. The proposed patch aligns the variables of the
> affected call with the rest of the plugin.
I'm not 100% affirmative, but it seems to be already corrected in 2.2.x 
branch. Can you check ?

> CPU.pm:
> In our solaris-environment, the current CPU-plugin had severe problems
> detecting the correct values. The proposed patch exchanges the whole
> memconf-based plugin for a kstat-based plugin. As an added benefit, the
> plugin-size and complexity is severely shrunk.
I wholly agree the current code is ugly (even in current 2.2.x branch). 
Here is my understanding for it:
- ony all solaris versions, excepted solaris containers, memconf is used 
for extracting count, name and speed of processors
- on solaris containers, memconf is used for extracting count and speed 
of processors, while prctl is used for processor names
- if the previous method didn't work, prlctl is used as a fallback

As I have no clue about the solaris world, I wonder about the kstat 
command availability, and its potential usage as universal method (as in 
your proposal) or just as a partial replacement for memconf. I guess you 
have more experience than us on the subject.

Note to myself: change numeric solaris class values with symbolic constants.

So, considering the 2.2.x branch, my understanding is:
- 2 patches are not needed anymore
- 1 (the Win32 agent) is trivial enough to be ported
- 1 (the CPU.pm module for Solaris) need first some discussion, and some 
porting to the new API
-- 
Guillaume