[libvirt] [PATCH 2/4] virt-host-validate: distinguish exists vs accessible for devices
Laine Stump
laine at laine.org
Thu Oct 8 21:06:29 UTC 2015
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 at 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;
More information about the libvir-list
mailing list