On 06/29/2017 03:24 AM, Jiri Denemark wrote:
On Wed, Jun 28, 2017 at 15:30:28 -0400, John Ferlan wrote:
>
https://bugzilla.redhat.com/show_bug.cgi?id=1458630
>
> Introduce virQEMUDriverConfigSetCertDir which will handle reading the
> qemu.conf config file specific setting for default, vnc, spice, chardev,
> and migrate. Then if a setting was provided, validating the existence of
> the directory and overwriting the default set by virQEMUDriverConfigNew.
>
> Also update the qemu.conf description for default to indicate the consequences
> if the default directory does not exist.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu.conf | 9 ++++++++-
> src/qemu/qemu_conf.c | 42 ++++++++++++++++++++++++++++++++++--------
> 2 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index e6c0832..737fa46 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -3,7 +3,7 @@
> # defaults are used.
>
> # Use of TLS requires that x509 certificates be issued. The default is
> -# to keep them in /etc/pki/qemu. This directory must contain
> +# to keep them in /etc/pki/qemu. This directory must exist and contain:
> #
> # ca-cert.pem - the CA master certificate
> # server-cert.pem - the server certificate signed with ca-cert.pem
> @@ -13,6 +13,13 @@
> #
> # dh-params.pem - the DH params configuration file
> #
> +# If the directory does not exist or does not contain the necessary files,
> +# QEMU domains will fail to start if they are configured to use TLS.
> +#
> +# In order to overwrite the default directory alter the following. If the
> +# provided directory does not exist, then the setting reverts back to the
> +# default /etc/pki/qemu.
> +#
I don't think this is a good idea. We should use the directory a user
specified in qemu.conf. If it doesn't exist, well things won't work.
Sure, we can complain about it in the logs, but we should not fallback
to any magic default in that case. Anyone setting a custom directory for
TLS certificates does this because they want to use it. If the directory
does not exist, it's either because they forgot to create it or they
made a typo somewhere. It's very unlikely someone actually wants to use
a default directory even though they set a custom one.
NACK
Jirka
It's simple enough to fail, but that's in code which you snipped and not
in text description which essentially matched what the code is/was
doing. Your feeling is then that :
+ if (!virFileExists(tlsCertDir)) {
+ VIR_INFO("%s, directory '%s' does not exist, retain default",
+ setting, tlsCertDir);
+ VIR_FREE(tlsCertDir);
should fail at libvirtd startup instead of a VIR_INFO, true? Is that
true for default, vnc, spice, chardev, and migrate?
That's different from the current environment which only would fail for
domains that use TLS which is why I was hesitant to make it fail. Also
note that if /etc/pki/qemu doesn't exist and someone used a TLS marker,
then even without changing any of the *cert_dir values, only the domain
will fail to start. That is, /etc/pki/qemu is not checked for existence
and startup failure.
Altering text in qemu.conf to match what really happens will be simple,
but ensuring the approach is agreed upon is what I need to clear up.
Tks -
John