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@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
