On Mon, Apr 30, 2012 at 02:55:06PM -0400, William Jon McCann wrote:

Hi,

New to the list so hopefully I'm following the correct protocol.

Almost - we tend to like the commit messages for patches to be fairly verbose about the change, so your description of advantages here is actually better being put in the commit message itself. The patch also came through as an attachment with application/octet-stream, text/plain is preferrred, or even better just inline text. The simplest way to achieve this would have been to use git send-email eg git send-email --to libvir-list@redhat.com --no-chain-reply-to -1 60da72d7d502bfbb4cbde97898b98811a2221d4f Anyway, on with the real patch review....

Attached is a patch (mostly untested at the moment) to change the default storage location from .libvirt to the locations defined in the XDG Base Directory Specification (http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html).

This has a number of advantages for us:

It allows sharing a home directory between different machines, or sessions (eg. using NFS) Cleanly separates cache, runtime (eg. sockets), or app data from user settings

Yes, this is something we do a particularly bad job of with the current layout of $HOME/.libvirt. I have never liked the way the logs & lib directories are actually sub-dirs of the place where we store the XML configs. A shame this spec wasn't around back when I wrote this code originally.

Supports performing smart or selective migration of settings between different OS versions Supports reseting settings without breaking things Makes it possible to clear cache data to make room when the disk is filling up Allows us to write a robust and efficient backup solution Allows an admin flexibility to change where data and settings are stored It dramatically reduces the complexity and incoherence of the system for administrators

This is a pretty important set of features for both enterprise and individual use cases.

Yep, I think the advantages of using XDG directories is pretty compelling. The only point in favour of using $HOME/.libvirt is that fact that it is what we have always done. I think we can deal with that by auto-matically migrating the content to the new location, and leaving in a symlink or two from the old location to the new.

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b098f6a..f98ba6f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -722,6 +731,59 @@ static int daemonStateInit(virNetServerPtr srv) return 0; }

+static void migrateProfile(void) +{ + char *old_base = NULL; + char *updated = NULL; + char *home = NULL; + char *xdg_dir = NULL; + char *config_dir = NULL; + + home = virGetUserDirectory(geteuid());

Needs a check for home != NULL.

+ if (virAsprintf(&old_base, "%s/.libvirt", home) < 0) { + goto error; + } + + /* if the new directory is there or the old one is not: do nothing */ + config_dir = virGetUserConfigDirectory(geteuid());

Check config_dir != NULL

+ if (!virFileIsDir(old_base) || virFileExists(config_dir)) { + goto error; + } + + /* test if we already attempted to migrate first */ + if (virAsprintf(&updated, "%s/DEPRECATED-DIRECTORY", old_base) < 0) { + goto error; + } + if (virFileExists(updated)) { + goto error; + }

I'm not sure that this is actually needed - shouldn't the test for existance of 'config_dir' catch this. In any case, instead of using such a file, I think we should just add a symlink from $HOME/.libvirt to $HOME/.config/libvirt. So you could replace this with a check to see if old_base is a symlink

+ + xdg_dir = strdup(getenv("XDG_CONFIG_HOME")); + if (!xdg_dir || xdg_dir[0] == '\0') { + if (virAsprintf(&xdg_dir, "%s/.config", home) < 0) { + goto error; + } + } + + if (virFileMakePathWithParents(xdg_dir, 0700) != 0) { + goto error; + } + + if (rename (old_base, config_dir) != 0) {

We prefer to avoid ' ' between the function name & opening '(' and use '< 0' rather than '!= 0' for syscall checks.

+ int fd = creat(updated, 0600); + close(fd);

In light of above recommendation, I think you should replace this with symlink(old_base, config_dir). NB, you'd need to use 'VIR_FORCE_CLOSE(fd)' here - if you run 'make syntax-check' it should tell you about this coding style mistake (and any others)

+ goto error; + } + + error:

When the label is for a shared success/failure path our standard is to use 'cleanup'. Only use 'error' if it is a dedicated/exclusive failure path

+ VIR_FREE(home); + VIR_FREE(old_base); + VIR_FREE(xdg_dir); + VIR_FREE(config_dir); + VIR_FREE(updated);

We should also propagate a success/failure code back to the caller and exit as needed.

+ +} + /* Print command-line usage. */

diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h index 1504eec..aca9412 100644 --- a/src/remote/remote_driver.h +++ b/src/remote/remote_driver.h @@ -37,7 +37,7 @@ unsigned long remoteVersion(void); # define LIBVIRTD_TCP_PORT "16509" # define LIBVIRTD_PRIV_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt/libvirt-sock" # define LIBVIRTD_PRIV_UNIX_SOCKET_RO LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro" -# define LIBVIRTD_USER_UNIX_SOCKET "/.libvirt/libvirt-sock" +# define LIBVIRTD_USER_UNIX_SOCKET "libvirt-sock" # define LIBVIRTD_CONFIGURATION_FILE SYSCONFDIR "/libvirtd.conf"

Reminds me that since we moving the paths anyway, we should take this opportunity to change from the socket from the abstract namespace, into the real filesystem namespace. This would allow us to remotely connect to the session sockets over ssh and solve our OS-X portability problem. No need for you todo this, I'll write a separate patch.

@@ -1271,6 +1277,67 @@ static int virFileMakePathHelper(char *path) }

/** + * Creates the given directory with mode if it's not already existing. + * + * Returns 0 on success, or -1 if an error occurred (in which case, errno + * is set appropriately). + */ +int virFileMakePathWithParents (const char *pathname, + int mode) +{

Hmm, our virFileMakePath already takes care of creating parents as needed. The only difference is we don't have a 'mode' parameter for the existing function. Rather than introduce this new function, I'd rather you just used the existing one. If you really need the 'mode' parameter, then we should update the existing one to have that (ought to be done as a separate patch to this though if required)

+char *virGetUserConfigDirectory(uid_t uid) +{ + char *path = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (uid == getuid ()) + path = getenv("XDG_CONFIG_HOME"); + + if (path && path[0]) { + virBufferAsprintf(&buf, "%s/libvirt/", path); + } else { + char *home; + home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY); + virBufferAsprintf(&buf, "%s/.config/libvirt/", home); + VIR_FREE(home); + } + VIR_FREE(path); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); +} +

+char *virGetUserCacheDirectory(uid_t uid) +{ + char *path = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (uid == getuid ()) + path = getenv("XDG_CACHE_HOME"); + + if (path && path[0]) { + virBufferAsprintf(&buf, "%s/libvirt/", path); + } else { + char *home; + home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY); + virBufferAsprintf(&buf, "%s/.cache/libvirt/", home); + VIR_FREE(home); + } + VIR_FREE(path); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); +}

These two functions could be made to share a common impl eg static char *virGetXDGDirectory(uid_t uid, const gchar *xdgenvname, const gchar *xdgdefdir) { char *path = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; if (uid == getuid ()) path = getenv(xdgenvname); if (path && path[0]) { virBufferAsprintf(&buf, "%s/libvirt/", path); } else { char *home; home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY); virBufferAsprintf(&buf, "%s/%s/libvirt/", xdgdefdir, home); VIR_FREE(home); } VIR_FREE(path); if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); return NULL; } return virBufferContentAndReset(&buf); } char *virGetUserConfigDirectory(uid_t uid) { return virGetXDGDirectory(uid, "XDG_CONFIG_HOME", ".config"); } char *virGetUserCacheDirectory(uid_t uid) { return virGetXDGDirectory(uid, "XDG_CACHE_HOME", ".cache"); }

+ +char *virGetUserRuntimeDirectory(uid_t uid) +{ + char *path = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (uid == getuid ()) + path = getenv("XDG_RUNTIME_DIR"); + + if (!path || !path[0]) { + return virGetUserCacheDirectory(uid); + } else { + virBufferAsprintf(&buf, "%s/libvirt/", path); + VIR_FREE(path); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); + } +}

Unfortunately this one is slightly different so can't share Basically I think this patch's code is pretty sound and a good change overall. Look forward to an updated version. Don't forget to run 'make syntax-check' on it to validate coding style. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- daemon/libvirtd-config.c | 10 +-- daemon/libvirtd.c | 110 +++++++++++++++++++++++------- daemon/libvirtd.pod.in | 2 +- docs/auth.html.in | 2 +- docs/uri.html.in | 2 +- src/libvirt.c | 4 +- src/libvirt_private.syms | 5 ++ src/network/bridge_driver.c | 8 ++- src/nwfilter/nwfilter_driver.c | 11 +-- src/qemu/qemu_driver.c | 32 ++++++--- src/remote/remote_driver.c | 4 +- src/remote/remote_driver.h | 2 +- src/secret/secret_driver.c | 11 +-- src/storage/storage_driver.c | 13 +--- src/uml/uml_driver.c | 10 +-- src/util/util.c | 145 ++++++++++++++++++++++++++++++++++++++++ src/util/util.h | 6 ++ src/util/virauth.c | 4 +- tools/virsh.c | 6 +- 19 files changed, 298 insertions(+), 89 deletions(-) diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index 471236c..8b95969 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -205,16 +205,16 @@ daemonConfigFilePath(bool privileged, char **configfile) if (!(*configfile = strdup(SYSCONFDIR "/libvirt/libvirtd.conf"))) goto no_memory; } else { - char *userdir = NULL; + char *configdir = NULL; - if (!(userdir = virGetUserDirectory(geteuid()))) + if (!(configdir = virGetUserConfigDirectory(geteuid()))) goto error; - if (virAsprintf(configfile, "%s/.libvirt/libvirtd.conf", userdir) < 0) { - VIR_FREE(userdir); + if (virAsprintf(configfile, "%s/libvirtd.conf", configdir) < 0) { + VIR_FREE(configdir); goto no_memory; } - VIR_FREE(userdir); + VIR_FREE(configdir); } return 0; diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b098f6a..f98ba6f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -239,17 +239,20 @@ daemonPidFilePath(bool privileged, if (!(*pidfile = strdup(LOCALSTATEDIR "/run/libvirtd.pid"))) goto no_memory; } else { - char *userdir = NULL; + char *rundir = NULL; - if (!(userdir = virGetUserDirectory(geteuid()))) + if (!(rundir = virGetUserRuntimeDirectory(geteuid()))) goto error; - if (virAsprintf(pidfile, "%s/.libvirt/libvirtd.pid", userdir) < 0) { - VIR_FREE(userdir); + if (virFileMakePathWithParents (rundir, 0700) != 0) + goto error; + + if (virAsprintf(pidfile, "%s/libvirtd.pid", rundir) < 0) { + VIR_FREE(rundir); goto no_memory; } - VIR_FREE(userdir); + VIR_FREE(rundir); } return 0; @@ -279,17 +282,20 @@ daemonUnixSocketPaths(struct daemonConfig *config, if (!(*rosockfile = strdup(LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro"))) goto no_memory; } else { - char *userdir = NULL; + char *rundir = NULL; + + if (!(rundir = virGetUserRuntimeDirectory(geteuid()))) + goto error; - if (!(userdir = virGetUserDirectory(geteuid()))) + if (virFileMakePathWithParents (rundir, 0700) != 0) goto error; - if (virAsprintf(sockfile, "@%s/.libvirt/libvirt-sock", userdir) < 0) { - VIR_FREE(userdir); + if (virAsprintf(sockfile, "@%s/libvirt-sock", rundir) < 0) { + VIR_FREE(rundir); goto no_memory; } - VIR_FREE(userdir); + VIR_FREE(rundir); } } return 0; @@ -593,16 +599,19 @@ daemonSetupLogging(struct daemonConfig *config, LOCALSTATEDIR) == -1) goto no_memory; } else { - char *userdir = virGetUserDirectory(geteuid()); - if (!userdir) + char *logdir = virGetUserCacheDirectory(geteuid()); + if (!logdir) goto error; - if (virAsprintf(&tmp, "%d:file:%s/.libvirt/libvirtd.log", - virLogGetDefaultPriority(), userdir) == -1) { - VIR_FREE(userdir); + if (virFileMakePathWithParents (logdir, 0700) != 0) + goto error; + + if (virAsprintf(&tmp, "%d:file:%s/libvirtd.log", + virLogGetDefaultPriority(), logdir) == -1) { + VIR_FREE(logdir); goto no_memory; } - VIR_FREE(userdir); + VIR_FREE(logdir); } } else { if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) @@ -722,6 +731,59 @@ static int daemonStateInit(virNetServerPtr srv) return 0; } +static void migrateProfile(void) +{ + char *old_base = NULL; + char *updated = NULL; + char *home = NULL; + char *xdg_dir = NULL; + char *config_dir = NULL; + + home = virGetUserDirectory(geteuid()); + if (virAsprintf(&old_base, "%s/.libvirt", home) < 0) { + goto error; + } + + /* if the new directory is there or the old one is not: do nothing */ + config_dir = virGetUserConfigDirectory(geteuid()); + if (!virFileIsDir(old_base) || virFileExists(config_dir)) { + goto error; + } + + /* test if we already attempted to migrate first */ + if (virAsprintf(&updated, "%s/DEPRECATED-DIRECTORY", old_base) < 0) { + goto error; + } + if (virFileExists(updated)) { + goto error; + } + + xdg_dir = strdup(getenv("XDG_CONFIG_HOME")); + if (!xdg_dir || xdg_dir[0] == '\0') { + if (virAsprintf(&xdg_dir, "%s/.config", home) < 0) { + goto error; + } + } + + if (virFileMakePathWithParents(xdg_dir, 0700) != 0) { + goto error; + } + + if (rename (old_base, config_dir) != 0) { + int fd = creat(updated, 0600); + close(fd); + goto error; + } + + error: + VIR_FREE(home); + VIR_FREE(old_base); + VIR_FREE(xdg_dir); + VIR_FREE(config_dir); + VIR_FREE(updated); + +} + /* Print command-line usage. */ static void daemonUsage(const char *argv0, bool privileged) @@ -775,10 +837,10 @@ libvirt management daemon:\n"), argv0); Default paths:\n\ \n\ Configuration file (unless overridden by -f):\n\ - $HOME/.libvirt/libvirtd.conf\n\ + $XDG_CONFIG_HOME/libvirt/libvirtd.conf\n\ \n\ Sockets:\n\ - $HOME/.libvirt/libvirt-sock (in UNIX abstract namespace)\n\ + $XDG_RUNTIME_HOME/libvirt/libvirt-sock (in UNIX abstract namespace)\n\ \n\ TLS:\n\ CA certificate: $HOME/.pki/libvirt/cacert.pem\n\ @@ -786,7 +848,7 @@ libvirt management daemon:\n"), argv0); Server private key: $HOME/.pki/libvirt/serverkey.pem\n\ \n\ PID file:\n\ - $HOME/.libvirt/libvirtd.pid\n\ + $XDG_RUNTIME_HOME/libvirt/libvirtd.pid\n\ \n")); } } @@ -931,6 +993,8 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } + migrateProfile (); + if (config->host_uuid && virSetHostUUIDStr(config->host_uuid) < 0) { VIR_ERROR(_("invalid host UUID: %s"), config->host_uuid); @@ -977,14 +1041,12 @@ int main(int argc, char **argv) { if (privileged) { run_dir = strdup(LOCALSTATEDIR "/run/libvirt"); } else { - char *user_dir = virGetUserDirectory(geteuid()); + run_dir = virGetUserRuntimeDirectory(geteuid()); - if (!user_dir) { + if (!run_dir) { VIR_ERROR(_("Can't determine user directory")); goto cleanup; } - ignore_value(virAsprintf(&run_dir, "%s/.libvirt/", user_dir)); - VIR_FREE(user_dir); } if (!run_dir) { virReportOOMError(); @@ -992,7 +1054,7 @@ int main(int argc, char **argv) { } old_umask = umask(022); - if (virFileMakePath(run_dir) < 0) { + if (virFileMakePathWithParents(run_dir, 0700) < 0) { char ebuf[1024]; VIR_ERROR(_("unable to create rundir %s: %s"), run_dir, virStrerror(errno, ebuf, sizeof(ebuf))); diff --git a/daemon/libvirtd.pod.in b/daemon/libvirtd.pod.in index 6e699b8..ea6c37d 100644 --- a/daemon/libvirtd.pod.in +++ b/daemon/libvirtd.pod.in @@ -85,7 +85,7 @@ command line using the B<-f>|B<--config> option. The sockets libvirtd will use when B<run as root>. -=item F<$HOME/.libvirt/libvirt-sock> +=item F<$XDG_RUNTIME_DIR/libvirt/libvirt-sock> The socket libvirtd will use when run as a B<non-root> user. diff --git a/docs/auth.html.in b/docs/auth.html.in index ecff0fc..60e4f11 100644 --- a/docs/auth.html.in +++ b/docs/auth.html.in @@ -25,7 +25,7 @@ for the authentication file using the following sequence: variable.</li> <li>The file path specified by the "authfile=/some/file" URI query parameter</li> - <li>The file $HOME/.libvirt/auth.conf</li> + <li>The file $XDG_CONFIG_DIR/libvirt/auth.conf</li> <li>The file /etc/libvirt/auth.conf</li> </ol> diff --git a/docs/uri.html.in b/docs/uri.html.in index 2f76e8f..5812ca9 100644 --- a/docs/uri.html.in +++ b/docs/uri.html.in @@ -30,7 +30,7 @@ virConnectPtr conn = virConnectOpenReadOnly (<b>"test:///default"</b>); <p> To simplify life for administrators, it is possible to setup URI aliases in a libvirt client configuration file. The configuration file is <code>/etc/libvirt/libvirt.conf</code> -for the root user, or <code>$HOME/.libvirt/libvirt.conf</code> for any unprivileged user. +for the root user, or <code>$XDG_CONFIG_DIR/libvirt/libvirt.conf</code> for any unprivileged user. In this file, the following syntax can be used to setup aliases </p> diff --git a/src/libvirt.c b/src/libvirt.c index b01ebba..f467290 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -969,11 +969,11 @@ virConnectGetConfigFilePath(void) SYSCONFDIR) < 0) goto no_memory; } else { - char *userdir = virGetUserDirectory(geteuid()); + char *userdir = virGetUserConfigDirectory(geteuid()); if (!userdir) goto error; - if (virAsprintf(&path, "%s/.libvirt/libvirt.conf", + if (virAsprintf(&path, "%s/libvirt.conf", userdir) < 0) { VIR_FREE(userdir); goto no_memory; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 025816a..c198e36 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1109,8 +1109,10 @@ virFileFindMountPoint; virFileHasSuffix; virFileIsExecutable; virFileIsLink; +virFileIsDir; virFileLinkPointsTo; virFileLock; +virFileMakePathWithParents; virFileMakePath; virFileMatchesNameSuffix; virFileOpenAs; @@ -1129,6 +1131,9 @@ virGetGroupID; virGetGroupName; virGetHostname; virGetUserDirectory; +virGetUserConfigDirectory; +virGetUserCacheDirectory; +virGetUserRuntimeDirectory; virGetUserID; virGetUserName; virHexToBin; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d82212f..cea87c2 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -280,18 +280,20 @@ networkStartup(int privileged) { if ((base = strdup (SYSCONFDIR "/libvirt")) == NULL) goto out_of_memory; } else { - char *userdir = virGetUserDirectory(uid); + char *userdir = virGetUserCacheDirectory(uid); if (!userdir) goto error; if (virAsprintf(&driverState->logDir, - "%s/.libvirt/qemu/log", userdir) == -1) { + "%s/qemu/log", userdir) == -1) { VIR_FREE(userdir); goto out_of_memory; } + VIR_FREE(userdir); - if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { + userdir = virGetUserConfigDirectory(uid); + if (virAsprintf(&base, "%s", userdir) == -1) { VIR_FREE(userdir); goto out_of_memory; } diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index ffb4b5d..3d732ab 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -87,16 +87,9 @@ nwfilterDriverStartup(int privileged) { goto out_of_memory; } else { uid_t uid = geteuid(); - char *userdir = virGetUserDirectory(uid); - - if (!userdir) + base = virGetUserConfigDirectory(uid); + if (!base) goto error; - - if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { - VIR_FREE(userdir); - goto out_of_memory; - } - VIR_FREE(userdir); } if (virAsprintf(&driverState->configDir, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ce31e09..8293a19 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -518,28 +518,38 @@ qemudStartup(int privileged) { goto out_of_memory; } else { uid_t uid = geteuid(); - char *userdir = virGetUserDirectory(uid); - if (!userdir) + char *rundir; + char *cachedir; + + cachedir = virGetUserCacheDirectory(uid); + if (!cachedir) goto error; if (virAsprintf(&qemu_driver->logDir, - "%s/.libvirt/qemu/log", userdir) == -1) { - VIR_FREE(userdir); + "%s/qemu/log", cachedir) == -1) { + VIR_FREE(cachedir); goto out_of_memory; } - - if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { - VIR_FREE(userdir); + if (virAsprintf(&qemu_driver->cacheDir, "%s/qemu/cache", cachedir) == -1) { + VIR_FREE(cachedir); goto out_of_memory; } - VIR_FREE(userdir); + VIR_FREE(cachedir); - if (virAsprintf(&qemu_driver->stateDir, "%s/qemu/run", base) == -1) + rundir = virGetUserRuntimeDirectory(uid); + if (!rundir) + goto error; + if (virAsprintf(&qemu_driver->stateDir, "%s/qemu/run", rundir) == -1) { + VIR_FREE(rundir); goto out_of_memory; + } + VIR_FREE(rundir); + + base = virGetUserConfigDirectory(uid); + if (!base) + goto error; if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", base) == -1) goto out_of_memory; - if (virAsprintf(&qemu_driver->cacheDir, "%s/qemu/cache", base) == -1) - goto out_of_memory; if (virAsprintf(&qemu_driver->saveDir, "%s/qemu/save", base) == -1) goto out_of_memory; if (virAsprintf(&qemu_driver->snapshotDir, "%s/qemu/snapshot", base) == -1) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7863b73..4a9299a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -578,12 +578,12 @@ doRemoteOpen (virConnectPtr conn, case trans_unix: if (!sockname) { if (flags & VIR_DRV_OPEN_REMOTE_USER) { - char *userdir = virGetUserDirectory(getuid()); + char *userdir = virGetUserRuntimeDirectory(getuid()); if (!userdir) goto failed; - if (virAsprintf(&sockname, "@%s" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0) { + if (virAsprintf(&sockname, "@%s/" LIBVIRTD_USER_UNIX_SOCKET, userdir) < 0) { VIR_FREE(userdir); goto out_of_memory; } diff --git a/src/remote/remote_driver.h b/src/remote/remote_driver.h index 1504eec..aca9412 100644 --- a/src/remote/remote_driver.h +++ b/src/remote/remote_driver.h @@ -37,7 +37,7 @@ unsigned long remoteVersion(void); # define LIBVIRTD_TCP_PORT "16509" # define LIBVIRTD_PRIV_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt/libvirt-sock" # define LIBVIRTD_PRIV_UNIX_SOCKET_RO LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro" -# define LIBVIRTD_USER_UNIX_SOCKET "/.libvirt/libvirt-sock" +# define LIBVIRTD_USER_UNIX_SOCKET "libvirt-sock" # define LIBVIRTD_CONFIGURATION_FILE SYSCONFDIR "/libvirtd.conf" /* Defaults for PKI directory. */ diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 088a243..f3fcce2 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -1013,16 +1013,9 @@ secretDriverStartup(int privileged) goto out_of_memory; } else { uid_t uid = geteuid(); - char *userdir = virGetUserDirectory(uid); - - if (!userdir) + base = virGetUserConfigDirectory(uid); + if (!base) goto error; - - if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { - VIR_FREE(userdir); - goto out_of_memory; - } - VIR_FREE(userdir); } if (virAsprintf(&driverState->directory, "%s/secrets", base) == -1) goto out_of_memory; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f23ec7e..fd762c0 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -146,19 +146,12 @@ storageDriverStartup(int privileged) goto out_of_memory; } else { uid_t uid = geteuid(); - char *userdir = virGetUserDirectory(uid); - - if (!userdir) + base = virGetUserConfigDirectory(uid); + if (!base) goto error; - - if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) { - VIR_FREE(userdir); - goto out_of_memory; - } - VIR_FREE(userdir); } - /* Configuration paths are either ~/.libvirt/storage/... (session) or + /* Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/... (session) or * /etc/libvirt/storage/... (system). */ if (virAsprintf(&driverState->configDir, diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 4e640ff..8a39d73 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -434,12 +434,12 @@ umlStartup(int privileged) "%s/run/libvirt/uml-guest", LOCALSTATEDIR) == -1) goto out_of_memory; } else { + base = virGetUserConfigDirectory(uid); + if (!base) + goto error; if (virAsprintf(¨_driver->logDir, - "%s/.libvirt/uml/log", userdir) == -1) - goto out_of_memory; - - if (virAsprintf(&base, "%s/.libvirt", userdir) == -1) + "%s/uml/log", base) == -1) goto out_of_memory; if (virAsprintf(¨_driver->monitorDir, @@ -447,7 +447,7 @@ umlStartup(int privileged) goto out_of_memory; } - /* Configuration paths are either ~/.libvirt/uml/... (session) or + /* Configuration paths are either $XDG_CONFIG_HOME/libvirt/uml/... (session) or * /etc/libvirt/uml/... (system). */ if (virAsprintf(¨_driver->configDir, "%s/uml", base) == -1) diff --git a/src/util/util.c b/src/util/util.c index 48358b2..bb3703c 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -665,6 +665,12 @@ char *virFindFileInPath(const char *file) return fullpath; } +bool virFileIsDir(const char *path) +{ + struct stat s; + return (stat (path, &s) == 0) && S_ISDIR (s.st_mode); +} + bool virFileExists(const char *path) { return access(path, F_OK) == 0; @@ -1271,6 +1277,67 @@ static int virFileMakePathHelper(char *path) } /** + * Creates the given directory with mode if it's not already existing. + * + * Returns 0 on success, or -1 if an error occurred (in which case, errno + * is set appropriately). + */ +int virFileMakePathWithParents (const char *pathname, + int mode) +{ + char *fn, *p; + + if (pathname == NULL || *pathname == '\0') { + errno = EINVAL; + return -1; + } + + fn = strdup(pathname); + + if (fn[0] == '/') { + /* Skip initial slashes */ + p = fn; + while (p[0] == '/') + p++; + } else { + p = fn; + } + + do { + while (*p && *p != '/') + p++; + + if (!*p) + p = NULL; + else + *p = '\0'; + + if (access(fn, F_OK) != 0) { + if (mkdir(fn, mode) == -1 && errno != EEXIST) { + int errno_save = errno; + free(fn); + errno = errno_save; + return -1; + } + } else if (!virFileIsDir(fn)) { + free(fn); + errno = ENOTDIR; + return -1; + } + + if (p) { + *p++ = '/'; + while (*p && *p == '/') + p++; + } + } while (p); + + free(fn); + + return 0; +} + +/** * Creates the given directory with mode 0777 if it's not already existing. * * Returns 0 on success, or -1 if an error occurred (in which case, errno @@ -2304,6 +2371,84 @@ char *virGetUserDirectory(uid_t uid) return virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY); } +char *virGetUserConfigDirectory(uid_t uid) +{ + char *path = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (uid == getuid ()) + path = getenv("XDG_CONFIG_HOME"); + + if (path && path[0]) { + virBufferAsprintf(&buf, "%s/libvirt/", path); + } else { + char *home; + home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY); + virBufferAsprintf(&buf, "%s/.config/libvirt/", home); + VIR_FREE(home); + } + VIR_FREE(path); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); +} + +char *virGetUserCacheDirectory(uid_t uid) +{ + char *path = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (uid == getuid ()) + path = getenv("XDG_CACHE_HOME"); + + if (path && path[0]) { + virBufferAsprintf(&buf, "%s/libvirt/", path); + } else { + char *home; + home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY); + virBufferAsprintf(&buf, "%s/.cache/libvirt/", home); + VIR_FREE(home); + } + VIR_FREE(path); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); +} + +char *virGetUserRuntimeDirectory(uid_t uid) +{ + char *path = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (uid == getuid ()) + path = getenv("XDG_RUNTIME_DIR"); + + if (!path || !path[0]) { + return virGetUserCacheDirectory(uid); + } else { + virBufferAsprintf(&buf, "%s/libvirt/", path); + VIR_FREE(path); + + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return NULL; + } + + return virBufferContentAndReset(&buf); + } +} + char *virGetUserName(uid_t uid) { return virGetUserEnt(uid, VIR_USER_ENT_NAME); diff --git a/src/util/util.h b/src/util/util.h index 85e8bd6..5c87a49 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -85,6 +85,7 @@ int virFileIsLink(const char *linkpath) char *virFindFileInPath(const char *file); +bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1); bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1); bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1); @@ -114,6 +115,8 @@ enum { int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid, unsigned int flags) ATTRIBUTE_RETURN_CHECK; int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK; +int virFileMakePathWithParents (const char *pathname, + int mode) ATTRIBUTE_RETURN_CHECK; char *virFileBuildPath(const char *dir, const char *name, @@ -228,6 +231,9 @@ char *virGetHostname(virConnectPtr conn); int virKillProcess(pid_t pid, int sig); char *virGetUserDirectory(uid_t uid); +char *virGetUserConfigDirectory(uid_t uid); +char *virGetUserCacheDirectory(uid_t uid); +char *virGetUserRuntimeDirectory(uid_t uid); char *virGetUserName(uid_t uid); char *virGetGroupName(gid_t gid); int virGetUserID(const char *name, diff --git a/src/util/virauth.c b/src/util/virauth.c index c59c55a..92ecb7c 100644 --- a/src/util/virauth.c +++ b/src/util/virauth.c @@ -65,10 +65,10 @@ int virAuthGetConfigFilePath(virConnectPtr conn, } } - if (!(userdir = virGetUserDirectory(geteuid()))) + if (!(userdir = virGetUserConfigDirectory(geteuid()))) goto cleanup; - if (virAsprintf(path, "%s/.libvirt/auth.conf", userdir) < 0) + if (virAsprintf(path, "%s/auth.conf", userdir) < 0) goto no_memory; VIR_DEBUG("Checking for readability of '%s'", *path); diff --git a/tools/virsh.c b/tools/virsh.c index e177684..ee14c02 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -19863,15 +19863,15 @@ vshReadlineInit(vshControl *ctl) /* Limit the total size of the history buffer */ stifle_history(500); - /* Prepare to read/write history from/to the ~/.virsh/history file */ - userdir = virGetUserDirectory(getuid()); + /* Prepare to read/write history from/to the $XDG_CACHE_HOME/virsh/history file */ + userdir = virGetUserCacheDirectory(getuid()); if (userdir == NULL) { vshError(ctl, "%s", _("Could not determine home directory")); return -1; } - if (virAsprintf(&ctl->historydir, "%s/.virsh", userdir) < 0) { + if (virAsprintf(&ctl->historydir, "%s/virsh", userdir) < 0) { vshError(ctl, "%s", _("Out of memory")); VIR_FREE(userdir); return -1; -- 1.7.10