Re: [PATCH] virconf: Fix config file path construction

[Andrea Bolognani <abologna@xxxxxxxxxx>]

On Wed, 2016-07-27 at 08:50 +0200, Erik Skultety wrote:
> Since commit c4bdff19, the path to the configuration file has been constructed
> in the following manner:
>  - if no config filename was passed to virConfLoadConfigPath, libvirt.conf was
>  used as default
>  - otherwise the filename was concatenated with
>  "<config_dir>/libvirt/libvirt%s%s.conf" which in admin case resulted in
>  "libvirt-libvirt-admin.conf.conf". Obviously, this non-existent config led to
>  ignoring  all user settings in libvirt-admin.conf. This patch requires the
>  config filename to be always provided as an argument with the concatenation
>  being simplified.
> 
>  Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357364
> 
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
>  src/libvirt.c      |  2 +-
>  src/util/virconf.c | 12 ++++--------
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 68c8317..52462e3 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -969,7 +969,7 @@ virConnectOpenInternal(const char *name,
>      if (ret == NULL)
>          return NULL;
>  
> -    if (virConfLoadConfig(&conf, NULL) < 0)
> +    if (virConfLoadConfig(&conf, "libvirt.conf") < 0)
>          goto failed;
>  
>      if (name && name[0] == '\0')
> diff --git a/src/util/virconf.c b/src/util/virconf.c
> index ee54072..3e49f41 100644
> --- a/src/util/virconf.c
> +++ b/src/util/virconf.c
> @@ -1566,20 +1566,16 @@ virConfLoadConfigPath(const char *name)
>  {
>      char *path;
>      if (geteuid() == 0) {
> -        if (virAsprintf(&path, "%s/libvirt/libvirt%s%s.conf",
> -                        SYSCONFDIR,
> -                        name ? "-" : "",
> -                        name ? name : "") < 0)
> +        if (virAsprintf(&path, "%s/libvirt/%s",
> +                        SYSCONFDIR, name) < 0)
>              return NULL;
>      } else {
>          char *userdir = virGetUserConfigDirectory();
>          if (!userdir)
>              return NULL;
>  
> -        if (virAsprintf(&path, "%s/libvirt%s%s.conf",
> -                        userdir,
> -                        name ? "-" : "",
> -                        name ? name : "") < 0) {
> +        if (virAsprintf(&path, "%s/%s",
> +                        userdir, name) < 0) {
>              VIR_FREE(userdir);
>              return NULL;
>          }

You could also have changed src/libvirt-admin.c to call

    virConfLoadConfig(&conf, "admin")

instead, but your solution is clearer.

ACK

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list