3:48 p.m.
On 01/10/2012 10:33 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)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.
---
tools/Makefile.am | 25 ++++++-
tools/virt-host-validate-common.c | 166 +++++++++++++++++++++++++++++++++++++
tools/virt-host-validate-common.h | 53 ++++++++++++
tools/virt-host-validate-lxc.c | 37 ++++++++
tools/virt-host-validate-lxc.h | 27 ++++++
tools/virt-host-validate-qemu.c | 50 +++++++++++
tools/virt-host-validate-qemu.h | 27 ++++++
tools/virt-host-validate.c | 77 +++++++++++++++++
8 files changed, 461 insertions(+), 1 deletions(-)
Missing changes to libvirt.spec.in to actually install it as part of the
appropriate rpm (and probably to mingw32-libvirt.spec.in to exclude it,
since neither kvm nor LXC is supported natively on mingw, leaving
nothing to check for).
@@ -64,6 +64,29 @@ virt-sanlock-cleanup: virt-sanlock-cleanup.in
Makefile
virt-sanlock-cleanup.8: virt-sanlock-cleanup.in
$(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
+virt_host_validate_SOURCES = \
+ virt-host-validate.c \
+ virt-host-validate-common.c virt-host-validate-common.h \
+ virt-host-validate-qemu.c virt-host-validate-qemu.h \
+ virt-host-validate-lxc.c virt-host-validate-lxc.h \
+ $(NULL)
I know you are copying from existing patterns, but there was an
interesting bug report raised recently where the use of
backslash-newline followed by a macro that expands to the empty string
triggers a bug in older bash:
https://lists.gnu.org/archive/html/bug-automake/2012-01/msg00046.html
I don't think it matters in our situation (particularly since we are
feeding these macros into longer command lines, so that the overall
command isn't ending in a backslash), but thought I'd mention it.
+
+virt_host_validate_LDFLAGS = \
+ $(WARN_LDFLAGS) \
+ $(COVERAGE_LDFLAGS) \
+ $(NULL)
+
+virt_host_validate_LDADD = \
+ $(WARN_CFLAGS) \
We shouldn't need $(WARN_CFLAGS) for LDADD, especially since...
+ ../src/libvirt.la \
+ ../gnulib/lib/libgnu.la \
+ $(NULL)
+
+virt_host_validate_CFLAGS = \
+ $(WARN_CFLAGS) \
you have them also listed here, in the correct location.
+++ b/tools/virt-host-validate-common.c
@@ -0,0 +1,166 @@
+/*
+ * virt-host-validate-common.c: Sanity check helper APis
+ *
+ * Copyright (C) 2011 Red Hat, Inc.
s/2011/2012/
+
+void virHostMsgCheck(const char *prefix,
+ const char *format,
+ ...)
+{
+ va_list args;
+ char *msg;
+
+ va_start(args, format);
+ if (virVasprintf(&msg, format, args) < 0) {
+ perror("malloc");
+ abort();
A bit harsh, but I suppose it's reasonable since OOM is unlikely for a
program so simple (we aren't doing arbitrary actions based on user
input, so our memory usage should be reasonably bounded).
+ }
+ va_end(args);
+
+ fprintf(stdout, "%6s: Checking %-60s: ", prefix, msg);
Do we want to provide translation of these messages? Or are you okay
with hard-coding it to the English language, regardless of locale?
+ VIR_FREE(msg);
+}
+
+void virHostMsgPass(void)
+{
+ fprintf(stdout, "\033[32mPASS\033[0m\n");
Are we sure that these particular terminal escape sequences will work
everywhere, or should we be making things conditional? And if
conditional, what do we depend on? checking isatty(1), linking against
ncurses, ...?
+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);
+ return -1;
+ }
Is this going to cause us grief if virt-host-validate is run as ordinary
users instead of root?
+int virHostValidateHasCPUFlag(const char *name)
+{
+ FILE *fp = fopen("/proc/cpuinfo", "r");
+ int ret = 0;
+
+ if (!fp)
+ return 0;
+
+ do {
+ char line[1024];
+
+ if (!fgets(line, sizeof(line), fp))
+ break;
+
+ if (strstr(line, name)) {
+ ret = 1;
+ break;
+ }
+ } while (1);
+
+ fclose(fp);
Doesn't 'make syntax-check' force us to use VIR_[FORCE_]FCLOSE here?
+int virHostValidateLinuxKernel(const char *hvname,
+ int version,
+ virHostValidateLevel level,
+ const char *hint)
+{
It sounds like this is Linux-only. Should we be conditionally compiling
things so that this helper app is only built and installed on Linux?
+
+ if (sscanf(uts.release, "%d.%d.%d", &major, &minor, µ) !=
3) {
+ micro = 0;
+ if (sscanf(uts.release, "%d.%d", &major, &minor) != 2) {
+ virHostMsgFail(level, hint);
+ return -1;
+ }
+ }
Rather than hand-parse things, can't you just use util.c's
virParseVersionString?
+++ b/tools/virt-host-validate-common.h
@@ -0,0 +1,53 @@
+/*
+ * virt-host-validate-common.h: Sanity check helper APis
+ *
+ * Copyright (C) 2011 Red Hat, Inc.
s/2011/2012/, throughout the rest of the patch.
+#include <config.h>
+
+#include "virt-host-validate-lxc.h"
+#include "virt-host-validate-common.h"
+
+int virHostValidateLXC(void)
Should this file (and virt-host-validate-qemu) only be compiled when
their respective hypervisor drivers are also being compiled? That is,
you can build libvirtd with qemu but no lxc support, in which case, this
helper app checking for lxc situations seems odd.
+
+static void
+show_help(FILE *out, const char *argv0)
+{
+ fprintf(out, "syntax: %s [OPTIONS] [HVTYPE]\n", argv0);
What options? I don't see any support for --help or --version.
+ if (!textdomain(PACKAGE)) {
+ perror("textdomain");
+ return EXIT_FAILURE;
+ }
+
+ if (argc > 2) {
+ fprintf(stderr, "too many command line arguments\n");
+ show_help(stderr, argv[0]);
+ return EXIT_FAILURE;
+ }
Should we be parsing options before this argc > 2 check?
I like the overall idea, but there are probably some things to fix first.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org