[collectd] [PATCH] New plugin - lpar

[Florian Forster]

Hi Aurélien,

thank you very much for your patch :)

Manuel has recently sent a patch for "Workload Partitioning" (WPAR),
also an AIX virtualization technique. Could you or Manuel enlighten me
how these things relate to one another? Would it make sense to combine
both plugins into one?

I have a couple of questions / comments regarding the data being
collected, too:

  * You are calculating the time difference yourself and calculating a
    rate from that. I'd prefer to use a DERIVE or COUNTER data source
    type for this kind of data rather than converting the counters to a
    "gauge" in the plugin.

    I have changed the generic CPU stuff to use the "cpu" type, which
    currently is of type COUNTER and will probably be converted to
    DERIVE before 5.0 is released. You can find the changed in my Git
    repository in the "ar/lpar" branch.

  * What's the deal with the minimum, entitled, maximum "proc capacity"?
    Is that something that actually does change often? It sounds more
    like a static configuration thing. Why do you divide that number by
    100? Is that some magical number required here?

  * Why do you use the chassis serial number as plugin instance? I'd
    expect that this information would be either assigned to the host
    name or that the partition's ID ("lpar_id") would be used as plugin
    instance. If the partition is moved to another system, the physical
    ID you're using changes, and this seems to be on purpose. I'd
    however expect that you'd look for something that *doesn't* change
    to identify the partition. Something like:
      hostname         = "lpar_pool-%x", pool_id
      plugin          = "lpar"
      plugin_instance = "partition-%x", lpar_id / "global"
    What am I missing?

  * Why not use the name included in the struct,
    (perfstat_partition_total_t *)->name?

  * What's this code doing?:
> +			dlt_pit = lparstats.pool_idle_time - last_pool_idle_time;
> +			total = (double)lparstats.phys_cpus_pool;
> +			idle  = (double)dlt_pit / XINTFRAC / (double)delta_time_base;
    Why don't you use the "pool_busy_time" member?

Regards,
—octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 481 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20100818/3412a784/attachment.pgp