Jim Meyering <jim@meyering.net> wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote: ...
The QEMU driver runs as non-root too. This is what the qemu:///session URI is used for. Likewise with the UML driver. The existing tests that invoke libvirtd fail quite frequently for me already due to them activating the QEMU / UML drivers. We really need a way to explicitly say what drivers should be allowed by the daemon, overriding what's compiled in. THis could in fact be useful even for production deployment, allowing site admins to guarentee that Xen driver is never used in the daemon even if it is compiled in by default.
So perhaps a couple of config params like
allowed_drivers = [ "qemu", "xen", "test" ] unix_sock_dir = "/var/run/libvirt/"
Not sure how best to hook the first one up to libvirt.so though - the virInitialize/virStateInitize calls always activate all of them, with no easy way to disable.
Sounds good. I'm deferring "allowed_drivers" for now, and preparing a patch to add support for a new configuration parameter
unix_sock_dir
and also for
log_dir
I'll add only unix_sock_dir for starters: The first patch adds the new parameter, and the second uses it in one of the two new libvirtd-running tests.
From 58a5957df136f5f38e5c400446301d28d1d2f80f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 12 Jan 2009 17:17:19 +0100 Subject: [PATCH 1/2] libvirtd: new config-file option: unix_sock_dir
Before this change, the unix socket directory was hard-coded to be e.g., /var/run/libvirt for euid==0 and ~/.libvirt otherwise. With this change, you may now specify that directory in libvirtd's config file via a line like this: unix_sock_dir = "/var/run/libvirt". This is essential for running tests that do not impinge on any existing libvirtd process, and in running tests in parallel. * qemud/libvirtd.conf (unix_sock_dir): Add comment and example. * qemud/qemud.c (unix_sock_dir): New global (remoteReadConfigFile): Set the global. (qemudInitPaths): Use the global, unix_sock_dir, if non-NULL. (main): Use the new global rather than hard-coding "/run/libvirt". * qemud/libvirtd.aug (sock_acl_entry): Add "unix_sock_dir". --- qemud/libvirtd.aug | 2 +- qemud/libvirtd.conf | 3 +- qemud/qemud.c | 78 ++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 58 insertions(+), 25 deletions(-) diff --git a/qemud/libvirtd.aug b/qemud/libvirtd.aug index 7cfd458..47e96b5 100644 --- a/qemud/libvirtd.aug +++ b/qemud/libvirtd.aug @@ -35,6 +35,7 @@ module Libvirtd = let sock_acl_entry = str_entry "unix_sock_group" | str_entry "unix_sock_ro_perms" | str_entry "unix_sock_rw_perms" + | str_entry "unix_sock_dir" let authentication_entry = str_entry "auth_unix_ro" | str_entry "auth_unix_rw" @@ -77,4 +78,3 @@ module Libvirtd = . Util.stdexcl let xfm = transform lns filter - diff --git a/qemud/libvirtd.conf b/qemud/libvirtd.conf index ecb28dc..a995012 100644 --- a/qemud/libvirtd.conf +++ b/qemud/libvirtd.conf @@ -97,7 +97,8 @@ # control then you may want to relax this to: #unix_sock_rw_perms = "0770" - +# Set the name of the directory in which sockets will be found/created. +#unix_sock_dir = "/var/run/libvirt" ################################################################# # diff --git a/qemud/qemud.c b/qemud/qemud.c index 336f0a9..62790fb 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -105,6 +105,7 @@ static char *tcp_port = (char *) LIBVIRTD_TCP_PORT; static gid_t unix_sock_gid = 0; /* Only root by default */ static int unix_sock_rw_mask = 0700; /* Allow user only */ static int unix_sock_ro_mask = 0777; /* Allow world */ +static char *unix_sock_dir = NULL; #if HAVE_POLKIT static int auth_unix_rw = REMOTE_AUTH_POLKIT; @@ -640,42 +641,72 @@ static int qemudInitPaths(struct qemud_server *server, char *roSockname, int maxlen) { uid_t uid = geteuid(); + struct passwd *pw = NULL; + char *sock_dir; - if (!uid) { - if (snprintf (sockname, maxlen, "%s/run/libvirt/libvirt-sock", - LOCAL_STATE_DIR) >= maxlen) - goto snprintf_error; - - unlink(sockname); + if (unix_sock_dir) + sock_dir = unix_sock_dir; + else { + sock_dir = sockname; + if (uid == 0) { + if (snprintf (sock_dir, maxlen, "%s/run/libvirt", + LOCAL_STATE_DIR) >= maxlen) + goto snprintf_error; + } else { + if (!(pw = getpwuid(uid))) { + VIR_ERROR(_("Failed to find user record for uid '%d': %s"), + uid, strerror(errno)); + return -1; + } - if (snprintf (roSockname, maxlen, "%s/run/libvirt/libvirt-sock-ro", - LOCAL_STATE_DIR) >= maxlen) - goto snprintf_error; + if (snprintf(sock_dir, maxlen, "%s/.libvirt", pw->pw_dir) >= maxlen) + goto snprintf_error; + } + } - unlink(roSockname); + char *dir_prefix = strdup (sock_dir); + if (!dir_prefix) { + VIR_ERROR(_("failed to allocate memory for socket dir %s"), sock_dir); + return -1; + } - if (snprintf(server->logDir, PATH_MAX, "%s/log/libvirt/", LOCAL_STATE_DIR) >= PATH_MAX) + if (!uid) { + if (snprintf (sockname, maxlen, "%s/libvirt-sock", dir_prefix) >= maxlen + || (snprintf (roSockname, maxlen, "%s/libvirt-sock-ro", dir_prefix) + >= maxlen)) goto snprintf_error; + unlink(sockname); + unlink(roSockname); } else { - struct passwd *pw; - if (!(pw = getpwuid(uid))) { VIR_ERROR(_("Failed to find user record for uid '%d': %s"), uid, strerror(errno)); return -1; } - - if (snprintf(sockname, maxlen, "@%s/.libvirt/libvirt-sock", pw->pw_dir) >= maxlen) + if (snprintf(sockname, maxlen, "@%s/libvirt-sock", dir_prefix) >= maxlen) goto snprintf_error; + } - if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log", pw->pw_dir) >= PATH_MAX) + if (!uid) { + if (snprintf(server->logDir, PATH_MAX, "%s/log/libvirt/", + LOCAL_STATE_DIR) >= PATH_MAX) goto snprintf_error; + } else { + if (pw == NULL && !(pw = getpwuid(uid))) { + VIR_ERROR(_("Failed to find user record for uid '%d': %s"), + uid, strerror(errno)); + return -1; + } + if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log", + pw->pw_dir) >= PATH_MAX) + goto snprintf_error; + } - } /* !remote */ - + free (dir_prefix); return 0; snprintf_error: + free (dir_prefix); VIR_ERROR("%s", _("Resulting path too long for buffer in qemudInitPaths()")); return -1; } @@ -2183,6 +2214,8 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) unix_sock_rw_perms = NULL; } + GET_CONF_STR (conf, filename, unix_sock_dir); + GET_CONF_INT (conf, filename, mdns_adv); GET_CONF_STR (conf, filename, mdns_name); @@ -2427,11 +2460,10 @@ int main(int argc, char **argv) { goto error2; /* Change the group ownership of /var/run/libvirt to unix_sock_gid */ - if (getuid() == 0) { - const char *sockdirname = LOCAL_STATE_DIR "/run/libvirt"; - - if (chown(sockdirname, -1, unix_sock_gid) < 0) - VIR_ERROR(_("Failed to change group ownership of %s"), sockdirname); + if (unix_sock_dir && geteuid() == 0) { + if (chown(unix_sock_dir, -1, unix_sock_gid) < 0) + VIR_ERROR(_("Failed to change group ownership of %s"), + unix_sock_dir); } if (virEventAddHandleImpl(sigpipe[0], -- 1.6.1.155.g1b01da
From a95478864c4d81a952082e95729ce2476aa4ea64 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Thu, 8 Jan 2009 20:18:17 +0100 Subject: [PATCH 2/2] add two tests
* tests/libvirtd-pool: New file. Exercise the new unix_sock_dir option * tests/libvirtd-fail: New file. * tests/Makefile.am (test_scripts): Add omitted backslash, so that the virsh-all test is run. Add libvirtd-fail and libvirtd-pool. --- tests/Makefile.am | 24 ++++++++++--------- tests/libvirtd-fail | 20 ++++++++++++++++ tests/libvirtd-pool | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 11 deletions(-) create mode 100755 tests/libvirtd-fail create mode 100755 tests/libvirtd-pool diff --git a/tests/Makefile.am b/tests/Makefile.am index 0486ee3..c735d62 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -57,17 +57,19 @@ endif test_scripts = domainschematest if WITH_LIBVIRTD -test_scripts += \ - test_conf.sh \ - cpuset \ - daemon-conf \ - int-overflow \ - read-bufsiz \ - read-non-seekable \ - start \ - undefine \ - vcpupin - virsh-all +test_scripts += \ + cpuset \ + daemon-conf \ + int-overflow \ + libvirtd-fail \ + libvirtd-pool \ + read-bufsiz \ + read-non-seekable \ + start \ + test_conf.sh \ + undefine \ + vcpupin \ + virsh-all \ virsh-synopsis endif diff --git a/tests/libvirtd-fail b/tests/libvirtd-fail new file mode 100755 index 0000000..724d7c6 --- /dev/null +++ b/tests/libvirtd-fail @@ -0,0 +1,20 @@ +#!/bin/sh +# Ensure that libvirt fails when given nonexistent --config=FILE + +if test "$VERBOSE" = yes; then + set -x + libvirtd --version +fi + +test -z "$srcdir" && srcdir=$(pwd) +test -z "$abs_top_srcdir" && abs_top_srcdir=$(pwd)/.. +. "$srcdir/test-lib.sh" + +fail=0 + +libvirtd --config=no-such-file > log 2>&1 && fail=1 +cat <<\EOF > exp +Failed to open file 'no-such-file': No such file or directory +EOF + +compare exp log diff --git a/tests/libvirtd-pool b/tests/libvirtd-pool new file mode 100755 index 0000000..37bff3b --- /dev/null +++ b/tests/libvirtd-pool @@ -0,0 +1,62 @@ +#!/bin/sh +# Get coverage of libvirtd's config-parsing code. + +if test "$VERBOSE" = yes; then + set -x + libvirtd --version + virsh --version +fi + +test -z "$srcdir" && srcdir=$(pwd) +test -z "$abs_top_srcdir" && abs_top_srcdir=$(pwd)/.. +. "$srcdir/test-lib.sh" + +fail=0 + +pwd=$(pwd) || fail=1 +sock_dir="$pwd" +cat > conf <<EOF || fail=1 +unix_sock_dir = "$sock_dir" +EOF + +libvirtd --config=conf > libvirtd-log 2>&1 & pid=$! +sleep 1 + +url="qemu:///session?socket=@$sock_dir/libvirt-sock" +virsh --connect "$url" \ + pool-define-as P dir src-host /src/path /src/dev S /target-path > out 2>&1 \ + || fail=1 +virsh --connect "$url" pool-dumpxml P >> out 2>&1 || fail=1 + +# remove random uuid +sed 's/<uuid>.*/-/' out > k && mv k out || fail=1 + +kill $pid + +cat <<EOF > pool-list-exp +Pool P defined + +<pool type='dir'> + <name>P</name> + - + <capacity>0</capacity> + <allocation>0</allocation> + <available>0</available> + <source> + </source> + <target> + <path>/target-path</path> + <permissions> + <mode>0700</mode> + <owner>500</owner> + <group>500</group> + </permissions> + </target> +</pool> + +EOF + +compare pool-list-exp out || fail=1 +compare /dev/null libvirtd-log || fail=1 + +exit $fail -- 1.6.1.155.g1b01da