Re: [libvirt] [PATCH 08/41] remote: conditionalize daemon name in libvirtd daemon
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization