[libvirt] [PATCH RFC] Add a virt-host-validate command to sanity check HV config
Eric Blake
eblake at redhat.com
Tue Jan 10 21:48:18 UTC 2012
On 01/10/2012 10:33 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at 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 at redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120110/61136873/attachment-0001.sig>
More information about the libvir-list
mailing list