[libvirt] [PATCH 08/41] remote: conditionalize daemon name in libvirtd daemon

[Andrea Bolognani]

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 at redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization