On 01/27/2012 07:59 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> To assist people in verifying that their host is operating in an
> optimal manner, provide a 'virt-host-validate' command. For each
> type of hypervisor, it will check any pre-requisites, or other
> good recommendations and report what's working & what is not.
>
> eg
>
> # virt-host-validate
> QEMU: Checking for device /dev/kvm : FAIL (Check that the 'kvm-intel' or 'kvm-amd' modules are loaded & the BIOS has enabled virtualization)
> QEMU: Checking for device /dev/vhost : WARN (Load the 'vhost_net' module to improve performance of virtio networking)
> QEMU: Checking for device /dev/net/tun : PASS
> LXC: Checking for Linux >= 2.6.26 : PASS
>
> This warns people if they have vmx/svm, but don't have /dev/kvm. It
> also warns about missing /dev/vhost net.
>
> In version 2:
>
> - Add to RPM specs
> - Add man page
> - Use getopt to parse --help, --version & --quiet options
> - Don't use VT100 escape codes unless on a tty
> - Disable QEMU / LXC checks if not compiled into libvirt
Looks like you hit my v1 review comments.
> +++ b/tools/virt-host-validate-common.c
> @@ -0,0 +1,214 @@
> +/*
> + * virt-host-validate-common.c: Sanity check helper APis
s/APis/APIs/
> +int virHostValidateDevice(const char *hvname,
> + const char *devname,
> + virHostValidateLevel level,
> + const char *hint)
> +{
> + virHostMsgCheck(hvname, "for device %s", devname);
> +
> + if (access(devname, R_OK|W_OK) < 0) {
> + virHostMsgFail(level, hint);
This could have different failures, depending on whether it is called as
root or as an ordinary user; should we be trying to refine things if
/dev/kvm exists with 600 permissions but the current euid can't
read/write it?
> +int virHostValidateHasCPUFlag(const char *name)
> +{
> + FILE *fp = fopen("/proc/cpuinfo", "r");
> + int ret = 0;
You're using this like a bool, so maybe s/int/bool/ and s/0/false/ make
sense.
> +
> + if (virParseVersionString(uts.release, &thisversion, true) < 0) {
> + virHostMsgFail(level, hint);
> + return -1;
> + }
> +
> + micro = (thisversion & 0xff);
> + minor = ((thisversion >> 8) & 0xff);
> + major = ((thisversion >> 16) & 0xff);
> +
> + if (major > ((version >> 16) & 0xff)) {
> + virHostMsgPass();
> + return 0;
> + } else if (major < ((version >> 16) & 0xff)) {
> + virHostMsgFail(level, hint);
> + return -1;
> + }
Rather than break things down and check major/minor/micro independently,
why not just check if thisversion >= version and get all three checks
done at once?
> +++ b/tools/virt-host-validate-common.h
> @@ -0,0 +1,57 @@
> +/*
> + * virt-host-validate-common.h: Sanity check helper APis
s/APis/APIs/ (copy-paste strikes again :)
> +
> +int virHostValidateQEMU(void)
> +{
> + int ret = 0;
> +
> + if (virHostValidateHasCPUFlag("svm") ||
> + virHostValidateHasCPUFlag("vmx")) {
> + 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)
> + ret = -1;
> + }
Should we have an else clause with VIR_HOST_VALIDATE_WARN that hardware
lacks virtualization, therefore guests will run slower, when this is run
on older cpus?
> +
> + if (virHostValidateDevice("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)
> + ret = -1;
> +
> + return ret;
> +}
Cool idea for another qemu validation - if !HAVE_YAJL, but qemu version
is 0.15 or newer, then complain that the libvirt binary lacks support
for QMP.
> + while ((c = getopt_long(argc, argv, "hvq", argOptions, NULL)) != -1) {
> + switch (c) {
> + case 'v':
> + show_version(stdout, argv[0]);
> + return EXIT_SUCCESS;
Technically, we should still check for write errors to stdout, but
that's a bigger patch (gnulib provides a close-stdout module that lets
you add an atexit() hook to automate things, but it is not LGPLv2+), and
I'm okay if we overlook it for now.
> +
> +#if WITH_QEMU
> + if ((!hvname || STREQ(hvname, "qemu")) &&
> + virHostValidateQEMU() < 0)
> + ret = EXIT_FAILURE;
> +#endif
Needs:
#else
if (STREQ(hvname, "qemu"))
fail; this libvirt was not compiled with qemu support
> +
> +#if WITH_LXC
> + if ((!hvname || STREQ(hvname, "lxc")) &&
> + virHostValidateLXC() < 0)
> + ret = EXIT_FAILURE;
> +#endif
A similar #else complaining about no lxc support.
ACK - there are a bigger ideas for additional improvements that should
be followup patches, but what you have is reasonable for committing now
once you fix the minor typos.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list