8000 virt plugin: Open connection on init() · Pull Request #2101 · collectd/collectd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

virt plugin: Open connection on init() #2101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 13, 2016

Conversation

ghost
Copy link
@ghost ghost commented Dec 12, 2016

If we use more than one reader instance, we can spot errors like
this in the system logs:

Dec 12 09:59:24 $HOST collectd[19338]: reading number of
domains: invalid connection pointer in virConnectNumOfDomains
Dec 12 09:59:24 benji.rokugan.lan collectd[19338]: read-function of
plugin `virt-2' failed. Will suspend it for 20.000 seconds.

This causes unnecessary delay in the sampling of libvirt.
The reason for this is just one instance (always #0) takes care
of establishing the libvirt connection.

But this could be done safely in the plugin init callback: according
to doc, this function is called at least once before all the read
instances.

Signed-off-by: Francesco Romani fromani@redhat.com

If we use more than one reader instance, we can spot errors like
this in the system logs:

Dec 12 09:59:24 $HOST collectd[19338]: reading number of
domains: invalid connection pointer in virConnectNumOfDomains
Dec 12 09:59:24 benji.rokugan.lan collectd[19338]: read-function of
plugin `virt-2' failed. Will suspend it for 20.000 seconds.

This causes unnecessary delay in the sampling of libvirt.
The reason for this is just one instance (always #0) takes care
of establishing the libvirt connection.

But this could be done safely in the plugin init callback: according
to doc, this function is called at least once before all the read
instances.

Signed-off-by: Francesco Romani <fromani@redhat.com>
@@ -732,6 +740,8 @@ static int lv_init(void) {
if (virInitialize() != 0)
return -1;

lv_connect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you don't check for errors here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we also handle reconnection in lv_read, it is ok if we fail here. But probably deserves a WARNING as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right. A warning could certainly do no harm.

Even though we handle disconnection and reconnection
in the read() callback, we expect the libvirt connection
to be available most of the time, including the init()
stage.

Thus, let's fail init() if the connection is not available.

Signed-off-by: Francesco Romani <fromani@redhat.com>
@rubenk rubenk merged commit 27f2019 into collectd:master Dec 13, 2016
@octo octo added this to the 5.8 milestone Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0