[collectd] Varnish plugin

[Florian Forster]

Hi Jérôme,

thank you very much for your patch :)

I have a couple of small suggestions / questios regarding the code,
maybe you could help me out:

* You forgot to add yourself as copyright holder at the beginning of the
  file. I guess you copied the "apache" plugin and modified it. Since
  there's basically nothing of the "apache" plugin left, I'd suggest you
  simply remove those other lines.

* The new "types" introduces by the patch, "varnish_cache" and
  "varnish_connections", have multiple "data sources" each. This is
  deprecated and only used when there's a good reason for it. I think
  the "cache_ratio" type could be used instead of "varnish_cache". Many
  other plugins define their own "foo_connections" type, for example
  "memcached_connections" and "nginx_connections". Feel free to copy one
  of those definitions to "varnish_connections". [*]

* The only argument passed to "VSL_OpenStats" is a NULL pointer called
  "varnish_instance_name". Would it be reasonable to let the user
  configure something else here and use NULL as a default?

* When "VSL_OpenStats" fails, is it possible to get a more detailed
  error description or at least an error code?

* The member (value_list_t *)->values_len is of type "int". You assign
  to it a "size_t" without cast, which may result in warnings (and hence
  problems) with some optimization settings.

Best regards,
—octo

[*] In the long run, I'd prefer to have *one* type for this purpose that
    is used by all plugins. I might change that in an attempt to unify
    the behavior eventually.
-- 
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: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20100601/e3bab140/attachment.pgp