On Tue, 2019-07-23 at 17:02 +0100, Daniel P. Berrangé wrote: [...]
@@ -895,7 +899,7 @@ daemonUsage(const char *argv0, bool privileged) fprintf(stderr, "\n");
fprintf(stderr, " %s:\n", _("Configuration file (unless overridden by -f)")); - fprintf(stderr, " %s/libvirt/libvirtd.conf\n", + fprintf(stderr, " %s/libvirt/" DAEMON_NAME ".conf\n", privileged ? SYSCONFDIR : "$XDG_CONFIG_HOME");
Similarly to the previous commit, this should be fprintf(stderr, " %s/libvirt/%s.conf\n", DAEMON_NAME, privileged ? SYSCONFDIR : "$XDG_CONFIG_HOME"); [...]
@@ -922,9 +926,9 @@ daemonUsage(const char *argv0, bool privileged)
fprintf(stderr, " %s:\n", _("PID file (unless overridden by -p)")); - fprintf(stderr, " %s\n", - privileged ? LOCALSTATEDIR "/run/libvirtd.pid": - "$XDG_RUNTIME_DIR/libvirt/libvirtd.pid"); + fprintf(stderr, " %s/\n", + privileged ? LOCALSTATEDIR "/run/" DAEMON_NAME ".pid": + "$XDG_RUNTIME_DIR/libvirt/" DAEMON_NAME ".pid");
The pattern suggested above and in the previous patch make even more sense here. [...]
+++ b/src/remote/remote_daemon_config.c @@ -77,7 +77,8 @@ int daemonConfigFilePath(bool privileged, char **configfile) { if (privileged) { - if (VIR_STRDUP(*configfile, SYSCONFDIR "/libvirt/libvirtd.conf") < 0) + if (VIR_STRDUP(*configfile, + SYSCONFDIR "/libvirt/" DAEMON_NAME ".conf") < 0)
Same here - just use virAsprintf() instead of VIR_STRDUP(). [...]
@@ -85,7 +86,7 @@ daemonConfigFilePath(bool privileged, char **configfile) if (!(configdir = virGetUserConfigDirectory())) goto error;
- if (virAsprintf(configfile, "%s/libvirtd.conf", configdir) < 0) { + if (virAsprintf(configfile, "%s/%s.conf", configdir, DAEMON_NAME) < 0) {
This is what I'm talking about! ;) [...]
+++ b/src/remote/remote_driver.h @@ -34,7 +34,6 @@ unsigned long remoteVersion(void); #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-sock" -#define LIBVIRTD_CONFIGURATION_FILE SYSCONFDIR "/libvirt/libvirtd.conf"
Oh, this was unused even before your changes, wasn't it? You should drop it in a separate, trivial patch. Going through this patch only strenghtened my convintion that what I suggested in the previous patch is the way to go, so I urge you to implement that suggestion; however, for the sake of being coherent, even if you decide not to go through with it you can still consider this Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization