[libvirt] [resend][PATCH] deamon: use default value if ca_file, cert_file or key_file not set

[Jiri Denemark]

On Fri, Jan 12, 2018 at 17:09:10 +0800, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at gmail.com>
> 
> As the description of daemon/libvirtd.conf, setting
> key_file, cert_file or key_file will override the default value.
> But if we set any one of them, we need to set all the rest of them.

I think this is a reasonable behavior. If a default value is not usable
for one of them, the other will likely need to be changed too.

Although ca_file could be separated. In other words, I can imagine
someone wants to change ca_file but keep default values for
cert_file/key_file or keep default ca_file and override
cert_file/key_file. Overriding cert_file or key_file only without also
changing the other one doesn't make a lot of sense.

Anyway, the patch is incorrect...

> This patch set default value to them as daemon/libvirtd.conf
> described.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at gmail.com>
> ---
>  daemon/libvirtd.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 6d3b83355..93983f63b 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -493,19 +493,28 @@ daemonSetupNetworking(virNetServerPtr srv,
>                  config->cert_file ||
>                  config->key_file) {
>                  if (!config->ca_file) {
> -                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                                   _("No CA certificate path set to match server key/cert"));
> -                    goto cleanup;
> +                    VIR_WARN("Using default path for ca_file");
> +                    if (VIR_STRDUP(config->ca_file, LIBVIRT_CACERT) < 0) {
> +                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                       _("No CA certificate path set to match server key/cert"));

This error message doesn't make any sense now. Not to mention you're
overriding the error which was already set by VIR_STRDUP.

> +                        goto cleanup;
> +                    }
>                  }
>                  if (!config->cert_file) {
> -                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                                   _("No server certificate path set to match server key"));
> -                    goto cleanup;
> +                    VIR_WARN("Using default path for cert_file");
> +                    if (VIR_STRDUP(config->cert_file, LIBVIRT_SERVERCERT) < 0) {
> +                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                       _("No server certificate path set to match server key"));

Dtto.

> +                        goto cleanup;
> +                    }
>                  }
>                  if (!config->key_file) {
> -                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                                   _("No server key path set to match server cert"));
> -                    goto cleanup;
> +                    VIR_WARN("Using default path for key_file");
> +                    if (VIR_STRDUP(config->key_file, LIBVIRT_SERVERKEY) < 0) {
> +                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                       _("No server key path set to match server cert"));

Dtto.

> +                        goto cleanup;
> +                    }
>                  }
>                  VIR_DEBUG("Using CA='%s' cert='%s' key='%s'",
>                            config->ca_file, config->cert_file, config->key_file);

Jirka