4:06 p.m.
On 10/07/2015 12:50 PM, Daniel P. Berrange wrote:
Currently we just check that various devices are accessible.
This leads to inaccurate errors reported for /dev/kvm and
/dev/vhost-net if they exist but an unprivileged user lacks
access. Switch existing checks to look for file existance,
and add a separate check for accessibility of /dev/kvm
since some distros don't grant users access by default.
One problem with this is that the people with those distros probably
won't be running virt-host-validate under the same uid as used by
libvirt when running qemu, so the results won't necessarily tell you
what you need - if you run it as root it will say that /dev/kvm is
accessible, even though it may not be for the case of the "qemu user",
and if you run it as some unprivileged user, if may say that /dev/kvm
*isn't* accessible, even though it is in the case of the qemu user.
(This came to mind because someone in #virt recently was trying to
figure out why kvm wasn't working for them, showed that they had
/dev/kvm with mode 700, and someone else pointed out that this is how
it's shipped for [some other distro, I forget which].)
ACK other than that though.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
tools/virt-host-validate-common.c | 27 ++++++++++++++++++++++-----
tools/virt-host-validate-common.h | 13 +++++++++----
tools/virt-host-validate-qemu.c | 28 ++++++++++++++++------------
3 files changed, 47 insertions(+), 21 deletions(-)
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index 12a98f4..d646a79 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -115,12 +115,29 @@ void virHostMsgFail(virHostValidateLevel level,
}
-int virHostValidateDevice(const char *hvname,
- const char *dev_name,
- virHostValidateLevel level,
- const char *hint)
+int virHostValidateDeviceExists(const char *hvname,
+ const char *dev_name,
+ virHostValidateLevel level,
+ const char *hint)
{
- virHostMsgCheck(hvname, "for device %s", dev_name);
+ virHostMsgCheck(hvname, "if device %s exists", dev_name);
+
+ if (access(dev_name, F_OK) < 0) {
+ virHostMsgFail(level, hint);
+ return -1;
+ }
+
+ virHostMsgPass();
+ return 0;
+}
+
+
+int virHostValidateDeviceAccessible(const char *hvname,
+ const char *dev_name,
+ virHostValidateLevel level,
+ const char *hint)
+{
+ virHostMsgCheck(hvname, "if device %s is accessible", dev_name);
if (access(dev_name, R_OK|W_OK) < 0) {
virHostMsgFail(level, hint);
diff --git a/tools/virt-host-validate-common.h b/tools/virt-host-validate-common.h
index 9d8bcea..c25e69c 100644
--- a/tools/virt-host-validate-common.h
+++ b/tools/virt-host-validate-common.h
@@ -42,10 +42,15 @@ extern void virHostMsgPass(void);
extern void virHostMsgFail(virHostValidateLevel level,
const char *hint);
-extern int virHostValidateDevice(const char *hvname,
- const char *dev_name,
- virHostValidateLevel level,
- const char *hint);
+extern int virHostValidateDeviceExists(const char *hvname,
+ const char *dev_name,
+ virHostValidateLevel level,
+ const char *hint);
+
+extern int virHostValidateDeviceAccessible(const char *hvname,
+ const char *dev_name,
+ virHostValidateLevel level,
+ const char *hint);
extern bool virHostValidateHasCPUFlag(const char *name);
diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c
index 33a5738..9486064 100644
--- a/tools/virt-host-validate-qemu.c
+++ b/tools/virt-host-validate-qemu.c
@@ -20,7 +20,6 @@
*/
#include <config.h>
-
#include "virt-host-validate-qemu.h"
#include "virt-host-validate-common.h"
@@ -32,25 +31,30 @@ int virHostValidateQEMU(void)
if (virHostValidateHasCPUFlag("svm") ||
virHostValidateHasCPUFlag("vmx")) {
virHostMsgPass();
- if (virHostValidateDevice("QEMU", "/dev/kvm",
- VIR_HOST_VALIDATE_FAIL,
- _("Check that the 'kvm-intel' or
'kvm-amd' modules are "
- "loaded & the BIOS has enabled
virtualization")) < 0)
+ if (virHostValidateDeviceExists("QEMU", "/dev/kvm",
+ VIR_HOST_VALIDATE_FAIL,
+ _("Check that the 'kvm-intel' or
'kvm-amd' modules are "
+ "loaded & the BIOS has enabled
virtualization")) < 0)
+ ret = -1;
+ else if (virHostValidateDeviceAccessible("QEMU",
"/dev/kvm",
+ VIR_HOST_VALIDATE_FAIL,
+ _("Check /dev/kvm is world
writable or you are in "
+ "a group that is allowed to
access it")) < 0)
ret = -1;
} else {
virHostMsgFail(VIR_HOST_VALIDATE_WARN,
_("Only emulated CPUs are available, performance will be
significantly limited"));
}
- if (virHostValidateDevice("QEMU", "/dev/vhost-net",
- VIR_HOST_VALIDATE_WARN,
- _("Load the 'vhost_net' module to improve
performance "
- "of virtio networking")) < 0)
+ if (virHostValidateDeviceExists("QEMU", "/dev/vhost-net",
+ VIR_HOST_VALIDATE_WARN,
+ _("Load the 'vhost_net' module to
improve performance "
+ "of virtio networking")) < 0)
ret = -1;
- if (virHostValidateDevice("QEMU", "/dev/net/tun",
- VIR_HOST_VALIDATE_FAIL,
- _("Load the 'tun' module to enable networking
for QEMU guests")) < 0)
+ if (virHostValidateDeviceExists("QEMU", "/dev/net/tun",
+ VIR_HOST_VALIDATE_FAIL,
+ _("Load the 'tun' module to enable
networking for QEMU guests")) < 0)
ret = -1;
return ret;