Added username support for libvirt over SSH #2728

Merged
merged 4 commits into from Jan 17, 2016

Projects

None yet

5 participants

@jviersel
Contributor
jviersel commented Jan 9, 2016

No description provided.

@jviersel jviersel Added username support for libvirt over SSH
93e76a2
@f0o
Member
f0o commented Jan 9, 2016

👍

@f0o
Member
f0o commented Jan 9, 2016

Can you sign the contributors agreement please?

@jviersel jviersel I agree to the conditions of the Contributor Agreement contained in d…
…oc/General/Contributing.md.
31243b2
@murrant murrant and 1 other commented on an outdated diff Jan 9, 2016
includes/discovery/libvirt-vminfo.inc.php
@@ -9,18 +9,23 @@
$ssh_ok = 0;
+ $userHostname = $device['hostname'];
+ if (!is_null($config['libvirt_username'])) {
@murrant
murrant Jan 9, 2016 Contributor

I think isset or !empty would be preferred here.
https://www.virendrachandak.com/techtalk/php-isset-vs-empty-vs-is_null/

@jviersel
jviersel Jan 9, 2016 Contributor

$config['libvirt_username'] is predefined in the defaults with a NULL-value. I personally do not like empty() because is also returns true on zero (which might be the value you want to use)

@murrant
murrant Jan 9, 2016 Contributor

That's true, as unlikely as a username of 0 might be ;D.
But I do think isset is more clear for readability and isset is only false if the variable is null or undefined.

@laf
Member
laf commented Jan 10, 2016

I'd say isset may be better as well.

@f0o f0o added the Blocker label Jan 12, 2016
@laf
Member
laf commented Jan 17, 2016

bump

jviersel added some commits Jan 17, 2016
@jviersel jviersel Update defaults.inc.php
d0f0e05
@jviersel jviersel Update libvirt-vminfo.inc.php
a1c5a9d
@jviersel
Contributor

Changed check to isset() and removed default setting from defaults.inc.php. It looks like the auto-deploy-check hangs for some reason.

@laf laf merged commit 1136365 into librenms:master Jan 17, 2016

1 of 2 checks passed

Auto-Deploy Triggered
Details
Scrutinizer No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment