[libvirt] Re: [PATCH] libvirtd: new config-file option: unix_sock_dir [was Re: adding tests....

[Daniel P. Berrange]

On Mon, Feb 09, 2009 at 11:25:24AM +0100, Jim Meyering wrote:
> I've rebased this and made some minor improvements,
> like calling virReportOOMError and having a single exit point.
> 
> From 907671319b056495eef1d146dc9260a1a2fcb64c Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Mon, 12 Jan 2009 17:17:19 +0100
> Subject: [PATCH] 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.
> One minor improvement: unlink both sockets or none, never
> just one of them.
> (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       |   98 +++++++++++++++++++++++++++++++++------------------
>  3 files changed, 67 insertions(+), 36 deletions(-)
> 
> diff --git a/qemud/libvirtd.aug b/qemud/libvirtd.aug
> index 40acd93..7406d23 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"
> @@ -79,4 +80,3 @@ module Libvirtd =
>                . Util.stdexcl
> 
>     let xfm = transform lns filter
> -
> diff --git a/qemud/libvirtd.conf b/qemud/libvirtd.conf
> index 4932084..1fd5918 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 a4add5a..f8c3c97 100644
> --- a/qemud/qemud.c
> +++ b/qemud/qemud.c
> @@ -51,6 +51,8 @@
>  #include "libvirt_internal.h"
>  #include "virterror_internal.h"
> 
> +#define VIR_FROM_THIS VIR_FROM_QEMU
> +
>  #include "qemud.h"
>  #include "util.h"
>  #include "remote_internal.h"
> @@ -136,6 +138,8 @@ static char *listen_addr  = (char *) LIBVIRTD_LISTEN_ADDR;
>  static char *tls_port = (char *) LIBVIRTD_TLS_PORT;
>  static char *tcp_port = (char *) LIBVIRTD_TCP_PORT;
> 
> +static char *unix_sock_dir = NULL;
> +
>  #if HAVE_POLKIT
>  static int auth_unix_rw = REMOTE_AUTH_POLKIT;
>  static int auth_unix_ro = REMOTE_AUTH_POLKIT;
> @@ -712,46 +716,71 @@ static int qemudInitPaths(struct qemud_server *server,
>                            int maxlen)
>  {
>      uid_t uid = geteuid();
> +    char *sock_dir;
> +    char *dir_prefix = NULL;
> +    int ret = -1;
> +    char *sock_dir_prefix = NULL;
> +
> +    if (unix_sock_dir)
> +        sock_dir = unix_sock_dir;
> +    else {
> +        sock_dir = sockname;
> +        if (uid == SYSTEM_UID) {
> +            dir_prefix = strdup (LOCAL_STATE_DIR);
> +            if (dir_prefix == NULL) {
> +                virReportOOMError(NULL);
> +                goto cleanup;
> +            }
> +            if (snprintf (sock_dir, maxlen, "%s/run/libvirt",
> +                          dir_prefix) >= maxlen)
> +                goto snprintf_error;
> +        } else {
> +            dir_prefix = virGetUserDirectory(NULL, uid);
> +            if (dir_prefix == NULL) {
> +                /* Do not diagnose here; virGetUserDirectory does that.  */
> +                goto snprintf_error;
> +            }

[snip]

> +    if (snprintf(server->logDir, PATH_MAX, "%s/.libvirt/log",
> +                 dir_prefix) >= PATH_MAX)
> +        goto snprintf_error;

If I'm reading correctly, this will cause system logs to get put in
the directory  /var/.libvirt/log   instead of /var/log/libvirt, since
this snprintf doesn't take account of uid == SYSTEM_UID as the old
code used todo.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|