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