On 01/27/2012 07:59 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org