[libvirt] [PATCH 0/8] qemu: VNC: support encrypted server TLS keys
Applies on top of my qemu.conf cleanups: https://www.redhat.com/archives/libvir-list/2019-January/msg00401.html https://bugzilla.redhat.com/show_bug.cgi?id=1602418 Ján Tomko (8): conf: introduce virDomainGraphicsNew conf: add privateData to virDomainGraphicsDef qemu: add qemuDomainGraphicsPrivate data with a tlsAlias qemu: prepare secret for the graphics upfront qemu_process: fix debug message qemu.conf: add vnc_tls_x509_secret_uuid qemu: add support for encrypted VNC TLS keys qemu: error out when vnc vncTLSx509secretUUID is unsupported src/conf/domain_conf.c | 27 ++++- src/conf/domain_conf.h | 5 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 ++ src/qemu/qemu_command.c | 19 +++- src/qemu/qemu_conf.c | 3 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_domain.c | 102 ++++++++++++++++++ src/qemu/qemu_domain.h | 13 +++ src/qemu/qemu_process.c | 2 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + ...graphics-vnc-tls-secret.x86_64-latest.args | 36 +++++++ .../graphics-vnc-tls-secret.xml | 30 ++++++ tests/qemuxml2argvtest.c | 5 + 14 files changed, 241 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml -- 2.20.1
A helper function for allocating the virDomainGraphicsDef structure. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 21 +++++++++++++++++---- src/conf/domain_conf.h | 2 ++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 222bb8c482..82672c6493 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14114,10 +14114,21 @@ virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDefPtr def, return 0; } +virDomainGraphicsDefPtr +virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED) +{ + virDomainGraphicsDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + return def; +} /* Parse the XML definition for a graphics device */ static virDomainGraphicsDefPtr -virDomainGraphicsDefParseXML(xmlNodePtr node, +virDomainGraphicsDefParseXML(virDomainXMLOptionPtr xmlopt, + xmlNodePtr node, xmlXPathContextPtr ctxt, unsigned int flags) { @@ -14125,7 +14136,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, char *type = NULL; int typeVal; - if (VIR_ALLOC(def) < 0) + if (!(def = virDomainGraphicsDefNew(xmlopt))) return NULL; type = virXMLPropString(node, "type"); @@ -16237,7 +16248,8 @@ virDomainDeviceDefParse(const char *xmlStr, goto error; break; case VIR_DOMAIN_DEVICE_GRAPHICS: - if (!(dev->data.graphics = virDomainGraphicsDefParseXML(node, ctxt, flags))) + if (!(dev->data.graphics = virDomainGraphicsDefParseXML(xmlopt, node, + ctxt, flags))) goto error; break; case VIR_DOMAIN_DEVICE_HUB: @@ -20847,7 +20859,8 @@ virDomainDefParseXML(xmlDocPtr xml, if (n && VIR_ALLOC_N(def->graphics, n) < 0) goto error; for (i = 0; i < n; i++) { - virDomainGraphicsDefPtr graphics = virDomainGraphicsDefParseXML(nodes[i], + virDomainGraphicsDefPtr graphics = virDomainGraphicsDefParseXML(xmlopt, + nodes[i], ctxt, flags); if (!graphics) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fae130668f..2a97ad8ab3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2954,6 +2954,8 @@ virDomainChrSourceDefNew(virDomainXMLOptionPtr xmlopt); virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt); +virDomainGraphicsDefPtr +virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt); virDomainDefPtr virDomainDefNew(void); void virDomainObjAssignDef(virDomainObjPtr domain, -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
A helper function for allocating the virDomainGraphicsDef structure.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 21 +++++++++++++++++---- src/conf/domain_conf.h | 2 ++ 2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 222bb8c482..82672c6493 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14114,10 +14114,21 @@ virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDefPtr def, return 0; }
blank line
+virDomainGraphicsDefPtr +virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED) +{ + virDomainGraphicsDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + return def; +}
blank line Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 8 +++++++- src/conf/domain_conf.h | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 82672c6493..21112c6336 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14115,13 +14115,19 @@ virDomainGraphicsDefParseXMLEGLHeadless(virDomainGraphicsDefPtr def, } virDomainGraphicsDefPtr -virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED) +virDomainGraphicsDefNew(virDomainXMLOptionPtr xmlopt) { virDomainGraphicsDefPtr def = NULL; if (VIR_ALLOC(def) < 0) return NULL; + if (xmlopt && xmlopt->privateData.graphicsNew && + !(def->privateData = xmlopt->privateData.graphicsNew())) { + VIR_FREE(def); + def = NULL; + } + return def; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2a97ad8ab3..7776a3afb2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1603,6 +1603,8 @@ struct _virDomainGraphicsListenDef { }; struct _virDomainGraphicsDef { + virObjectPtr privateData; + /* Port value discipline: * Value -1 is legacy syntax indicating that it should be auto-allocated. * Value 0 means port wasn't specified in XML at all. @@ -2783,6 +2785,7 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataNewFunc vcpuNew; virDomainXMLPrivateDataNewFunc chrSourceNew; virDomainXMLPrivateDataNewFunc vsockNew; + virDomainXMLPrivateDataNewFunc graphicsNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; /* following function shall return a pointer which will be used as the -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 8 +++++++- src/conf/domain_conf.h | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
On 1/16/19 2:41 AM, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 8 +++++++- src/conf/domain_conf.h | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-)
An addendum because I've read further... and it became clearer. virDomainGraphicsDefFree Needs to have a virObjectUnref(def->privateData); John
Also introduce the necessary callbacks. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 12 ++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ec6b340308..63e739b778 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1234,6 +1234,44 @@ qemuDomainVsockPrivateDispose(void *obj ATTRIBUTE_UNUSED) } +static virClassPtr qemuDomainGraphicsPrivateClass; +static void qemuDomainGraphicsPrivateDispose(void *obj); + +static int +qemuDomainGraphicsPrivateOnceInit(void) +{ + if (!VIR_CLASS_NEW(qemuDomainGraphicsPrivate, virClassForObject())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(qemuDomainGraphicsPrivate) + +static virObjectPtr +qemuDomainGraphicsPrivateNew(void) +{ + qemuDomainGraphicsPrivatePtr priv; + + if (qemuDomainGraphicsPrivateInitialize() < 0) + return NULL; + + if (!(priv = virObjectNew(qemuDomainGraphicsPrivateClass))) + return NULL; + + return (virObjectPtr) priv; +} + + +static void +qemuDomainGraphicsPrivateDispose(void *obj) +{ + qemuDomainGraphicsPrivatePtr priv = obj; + + VIR_FREE(priv->tlsAlias); +} + + /* qemuDomainSecretPlainSetup: * @secinfo: Pointer to secret info * @usageType: The virSecretUsageType @@ -3035,6 +3073,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .vcpuNew = qemuDomainVcpuPrivateNew, .chrSourceNew = qemuDomainChrSourcePrivateNew, .vsockNew = qemuDomainVsockPrivateNew, + .graphicsNew = qemuDomainGraphicsPrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, .getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a837b8a731..01e47996f5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -481,6 +481,18 @@ struct _qemuDomainVsockPrivate { }; +# define QEMU_DOMAIN_GRAPHICS_PRIVATE(dev) \ + ((qemuDomainGraphicsPrivatePtr) (dev)->privateData) + +typedef struct _qemuDomainGraphicsPrivate qemuDomainGraphicsPrivate; +typedef qemuDomainGraphicsPrivate *qemuDomainGraphicsPrivatePtr; +struct _qemuDomainGraphicsPrivate { + virObject parent; + + char *tlsAlias; +}; + + typedef enum { QEMU_PROCESS_EVENT_WATCHDOG = 0, QEMU_PROCESS_EVENT_GUESTPANIC, -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
Also introduce the necessary callbacks.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 12 ++++++++++++ 2 files changed, 51 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
Instead of hardcoding the TLS creds alias in qemuBuildGraphicsVNCCommandLine, store it in the domain private data. Given that we only support one VNC graphics and thus have only one alias per-domain, this is overengineered, but it will allow us to prepare the secret upfront when we start supporting encrypted server TLS keys. Note that the alias is not formatted anywhere since we won't need to access it after domain startup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8669..d130d0463c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8035,18 +8035,18 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",password"); if (cfg->vncTLS) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { - const char *alias = "vnc-tls-creds0"; + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + if (gfxPriv->tlsAlias) { if (qemuBuildTLSx509CommandLine(cmd, cfg->vncTLSx509certdir, true, cfg->vncTLSx509verify, NULL, - alias, + gfxPriv->tlsAlias, qemuCaps) < 0) goto error; - virBufferAsprintf(&opt, ",tls-creds=%s", alias); + virBufferAsprintf(&opt, ",tls-creds=%s", gfxPriv->tlsAlias); } else { virBufferAddLit(&opt, ",tls"); if (cfg->vncTLSx509verify) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63e739b778..6960f0569b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1741,6 +1741,42 @@ qemuDomainSecretChardevPrepare(virQEMUDriverConfigPtr cfg, } +static void +qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics) +{ + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + + if (!gfxPriv) + return; + + VIR_FREE(gfxPriv->tlsAlias); +} + + +static int +qemuDomainSecretGraphicsPrepare(virQEMUDriverConfigPtr cfg, + qemuDomainObjPrivatePtr priv, + virDomainGraphicsDefPtr graphics) +{ + virQEMUCapsPtr qemuCaps = priv->qemuCaps; + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) + return 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) + return 0; + + if (!cfg->vncTLS) + return 0; + + if (VIR_STRDUP(gfxPriv->tlsAlias, "vnc-tls-creds0") < 0) + return -1; + + return 0; +} + + /* qemuDomainSecretDestroy: * @vm: Domain object * @@ -1782,6 +1818,9 @@ qemuDomainSecretDestroy(virDomainObjPtr vm) for (i = 0; i < vm->def->nredirdevs; i++) qemuDomainSecretChardevDestroy(vm->def->redirdevs[i]->source); + + for (i = 0; i < vm->def->ngraphics; i++) + qemuDomainSecretGraphicsDestroy(vm->def->graphics[i]); } @@ -1865,6 +1904,11 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver, goto cleanup; } + for (i = 0; i < vm->def->ngraphics; i++) { + if (qemuDomainSecretGraphicsPrepare(cfg, priv, vm->def->graphics[i]) < 0) + goto cleanup; + } + ret = 0; cleanup: -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
Instead of hardcoding the TLS creds alias in qemuBuildGraphicsVNCCommandLine, store it in the domain private data.
Given that we only support one VNC graphics and thus have only one alias per-domain, this is overengineered, but it will allow us to prepare the secret upfront when we start supporting encrypted server TLS keys.
Note that the alias is not formatted anywhere since we won't need to access it after domain startup.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8669..d130d0463c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8035,18 +8035,18 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",password");
if (cfg->vncTLS) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { - const char *alias = "vnc-tls-creds0"; + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + if (gfxPriv->tlsAlias) { if (qemuBuildTLSx509CommandLine(cmd, cfg->vncTLSx509certdir, true, cfg->vncTLSx509verify, NULL, - alias, + gfxPriv->tlsAlias, qemuCaps) < 0) goto error;
- virBufferAsprintf(&opt, ",tls-creds=%s", alias); + virBufferAsprintf(&opt, ",tls-creds=%s", gfxPriv->tlsAlias); } else { virBufferAddLit(&opt, ",tls"); if (cfg->vncTLSx509verify) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63e739b778..6960f0569b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1741,6 +1741,42 @@ qemuDomainSecretChardevPrepare(virQEMUDriverConfigPtr cfg, }
+static void +qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics) +{ + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + + if (!gfxPriv) + return; + + VIR_FREE(gfxPriv->tlsAlias);
Free'ing of tlsAlias is handled by qemuDomainGraphicsPrivateDispose, so this would be virObjectUnref(gfxPriv); QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics) = NULL; The second to avoid the virDomainGraphicsDefFree doing this as well. Still this is "unusual" (so to speak) for a qemuDomainSecret*Destroy function. Typically they just clear/destroy the *Secinfo data. Since you don't have that yet until patch 7, this could wait until then. On the other hand, I don't "foresee" anything wrong with properly free'ing the graphics def privateData now.
+} + + +static int +qemuDomainSecretGraphicsPrepare(virQEMUDriverConfigPtr cfg, + qemuDomainObjPrivatePtr priv, + virDomainGraphicsDefPtr graphics) +{ + virQEMUCapsPtr qemuCaps = priv->qemuCaps; + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) + return 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) + return 0; + + if (!cfg->vncTLS) + return 0; +
Just a note/thought while reviewing... nothing all that important... Seems to be a bit of overkill w/ graphics->privateData only being used for VNC private data in this very specialized case. Still weighed vs the then need for "gfxPriv && gfxPriv->...", who am I to complain ;-)
+ if (VIR_STRDUP(gfxPriv->tlsAlias, "vnc-tls-creds0") < 0) + return -1; + + return 0; +} + + /* qemuDomainSecretDestroy: * @vm: Domain object * @@ -1782,6 +1818,9 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
for (i = 0; i < vm->def->nredirdevs; i++) qemuDomainSecretChardevDestroy(vm->def->redirdevs[i]->source); + + for (i = 0; i < vm->def->ngraphics; i++) + qemuDomainSecretGraphicsDestroy(vm->def->graphics[i]);
Interesting placement until one reads patch 7. I think if patch 5 and 6 were placed before this one, then it'd be clearer what the steps are. I'm OK with this here now since eventually it'd be added. With a couple of adjustments... Reviewed-by: John Ferlan <jferlan@redhat.com> John
}
@@ -1865,6 +1904,11 @@ qemuDomainSecretPrepare(virQEMUDriverPtr driver, goto cleanup; }
+ for (i = 0; i < vm->def->ngraphics; i++) { + if (qemuDomainSecretGraphicsPrepare(cfg, priv, vm->def->graphics[i]) < 0) + goto cleanup; + } + ret = 0;
cleanup:
On Thu, Jan 17, 2019 at 11:08:03AM -0500, John Ferlan wrote:
On 1/16/19 2:41 AM, Ján Tomko wrote:
Instead of hardcoding the TLS creds alias in qemuBuildGraphicsVNCCommandLine, store it in the domain private data.
Given that we only support one VNC graphics and thus have only one alias per-domain, this is overengineered, but it will allow us to prepare the secret upfront when we start supporting encrypted server TLS keys.
Note that the alias is not formatted anywhere since we won't need to access it after domain startup.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 822d5f8669..d130d0463c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8035,18 +8035,18 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAddLit(&opt, ",password");
if (cfg->vncTLS) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { - const char *alias = "vnc-tls-creds0"; + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + if (gfxPriv->tlsAlias) { if (qemuBuildTLSx509CommandLine(cmd, cfg->vncTLSx509certdir, true, cfg->vncTLSx509verify, NULL, - alias, + gfxPriv->tlsAlias, qemuCaps) < 0) goto error;
- virBufferAsprintf(&opt, ",tls-creds=%s", alias); + virBufferAsprintf(&opt, ",tls-creds=%s", gfxPriv->tlsAlias); } else { virBufferAddLit(&opt, ",tls"); if (cfg->vncTLSx509verify) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63e739b778..6960f0569b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1741,6 +1741,42 @@ qemuDomainSecretChardevPrepare(virQEMUDriverConfigPtr cfg, }
+static void +qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics) +{ + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + + if (!gfxPriv) + return; + + VIR_FREE(gfxPriv->tlsAlias);
Free'ing of tlsAlias is handled by qemuDomainGraphicsPrivateDispose, so this would be
virObjectUnref(gfxPriv); QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics) = NULL;
The second to avoid the virDomainGraphicsDefFree doing this as well.
It would be more unusual for a Secret*Destroy function to free the private data. IIUC the point is to not have the temporary data allocated during the whole time the domain is running.
Still this is "unusual" (so to speak) for a qemuDomainSecret*Destroy function. Typically they just clear/destroy the *Secinfo data.
Since you don't have that yet until patch 7, this could wait until then. On the other hand, I don't "foresee" anything wrong with properly free'ing the graphics def privateData now.
+} + + +static int +qemuDomainSecretGraphicsPrepare(virQEMUDriverConfigPtr cfg, + qemuDomainObjPrivatePtr priv, + virDomainGraphicsDefPtr graphics) +{ + virQEMUCapsPtr qemuCaps = priv->qemuCaps; + qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); + + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) + return 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) + return 0; + + if (!cfg->vncTLS) + return 0; +
Just a note/thought while reviewing... nothing all that important...
Seems to be a bit of overkill w/ graphics->privateData only being used for VNC private data in this very specialized case. Still weighed vs the then need for "gfxPriv && gfxPriv->...", who am I to complain ;-)
I could convert it so that privateData is only allocated when needed, but I considered the marginal memory usage increase worth the considency and not forgetting to allocate/clean it up in some other case (especially if we decide to use privateData for something else too). So I'd rather not Unref the whole privateData in the Secret.*Dispose function. Jano
Be generic instead of trying to enumerate all the involved device types. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index aad6c12552..f4af673cdb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6050,7 +6050,7 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, VIR_DEBUG("Prepare chardev source backends for TLS"); qemuDomainPrepareChardevSource(vm->def, cfg); - VIR_DEBUG("Add secrets to hostdevs and chardevs"); + VIR_DEBUG("Prepare device secrets"); if (qemuDomainSecretPrepare(driver, vm) < 0) goto cleanup; -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
Be generic instead of trying to enumerate all the involved device types.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Trivial, Reviewed-by: John Ferlan <jferlan@redhat.com> John
Add an option that lets the user specify the secret that unlocks the server TLS key. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 5 files changed, 12 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 28bd851411..b311f02da6 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -35,6 +35,7 @@ module Libvirtd_qemu = | bool_entry "vnc_auto_unix_socket" | bool_entry "vnc_tls" | str_entry "vnc_tls_x509_cert_dir" + | str_entry "vnc_tls_x509_secret_uuid" | bool_entry "vnc_tls_x509_verify" | str_entry "vnc_password" | bool_entry "vnc_sasl" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 28e51b2c59..c1f1201134 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -95,6 +95,12 @@ #vnc_tls_x509_cert_dir = "/etc/pki/libvirt-vnc" +# Uncomment and use the following option to override the default secret +# UUID provided in the default_tls_x509_secret_uuid parameter. +# +#vnc_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" + + # The default TLS configuration only uses certificates for the server # allowing the client to verify the server's identity and establish # an encrypted channel. diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 18ad99c173..0f74fd1716 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -457,6 +457,8 @@ virQEMUDriverConfigLoadVNCEntry(virQEMUDriverConfigPtr cfg, cfg->vncTLSx509verifyPresent = true; if (virConfGetValueString(conf, "vnc_tls_x509_cert_dir", &cfg->vncTLSx509certdir) < 0) return -1; + if (virConfGetValueString(conf, "vnc_tls_x509_secret_uuid", &cfg->vncTLSx509secretUUID) < 0) + return -1; if (virConfGetValueString(conf, "vnc_listen", &cfg->vncListen) < 0) return -1; if (virConfGetValueString(conf, "vnc_password", &cfg->vncPassword) < 0) @@ -1184,6 +1186,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) } \ } while (0) + SET_TLS_SECRET_UUID_DEFAULT(vnc); SET_TLS_SECRET_UUID_DEFAULT(chardev); SET_TLS_SECRET_UUID_DEFAULT(migrate); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index bce8364c5a..14c9d15a72 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -125,6 +125,7 @@ struct _virQEMUDriverConfig { bool vncTLSx509verifyPresent; bool vncSASL; char *vncTLSx509certdir; + char *vncTLSx509secretUUID; char *vncListen; char *vncPassword; char *vncSASLdir; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index f1e8806ad2..4235464530 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -9,6 +9,7 @@ module Test_libvirtd_qemu = { "vnc_auto_unix_socket" = "1" } { "vnc_tls" = "1" } { "vnc_tls_x509_cert_dir" = "/etc/pki/libvirt-vnc" } +{ "vnc_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } { "vnc_tls_x509_verify" = "1" } { "vnc_password" = "XYZ12345" } { "vnc_sasl" = "1" } -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
Add an option that lets the user specify the secret that unlocks the server TLS key.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 3 +++ src/qemu/qemu_conf.h | 1 + src/qemu/test_libvirtd_qemu.aug.in | 1 + 5 files changed, 12 insertions(+)
Missing a change to virQEMUDriverConfigDispose in order to VIR_FREE(cfg->vncTLSx509secretUUID); with that, Reviewed-by: John Ferlan <jferlan@redhat.com> John
Use the password stored in the secret driver under the uuid specified by the vnc_tls_x509_secret_uuid option in qemu.conf. https://bugzilla.redhat.com/show_bug.cgi?id=1602418 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 11 +++++- src/qemu/qemu_domain.c | 9 +++++ src/qemu/qemu_domain.h | 1 + ...graphics-vnc-tls-secret.x86_64-latest.args | 36 +++++++++++++++++++ .../graphics-vnc-tls-secret.xml | 30 ++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++ 6 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d130d0463c..e17d7ddec7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8037,11 +8037,20 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (cfg->vncTLS) { qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); if (gfxPriv->tlsAlias) { + const char *secretAlias = NULL; + + if (gfxPriv && gfxPriv->secinfo) { + if (qemuBuildObjectSecretCommandLine(cmd, + gfxPriv->secinfo) < 0) + goto error; + secretAlias = gfxPriv->secinfo->s.aes.alias; + } + if (qemuBuildTLSx509CommandLine(cmd, cfg->vncTLSx509certdir, true, cfg->vncTLSx509verify, - NULL, + secretAlias, gfxPriv->tlsAlias, qemuCaps) < 0) goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6960f0569b..da9c4e566d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1269,6 +1269,7 @@ qemuDomainGraphicsPrivateDispose(void *obj) qemuDomainGraphicsPrivatePtr priv = obj; VIR_FREE(priv->tlsAlias); + qemuDomainSecretInfoFree(&priv->secinfo); } @@ -1750,6 +1751,7 @@ qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics) return; VIR_FREE(gfxPriv->tlsAlias); + qemuDomainSecretInfoFree(&gfxPriv->secinfo); } @@ -1773,6 +1775,13 @@ qemuDomainSecretGraphicsPrepare(virQEMUDriverConfigPtr cfg, if (VIR_STRDUP(gfxPriv->tlsAlias, "vnc-tls-creds0") < 0) return -1; + if (cfg->vncTLSx509secretUUID) { + gfxPriv->secinfo = qemuDomainSecretInfoTLSNew(priv, gfxPriv->tlsAlias, + cfg->vncTLSx509secretUUID); + if (!gfxPriv->secinfo) + return -1; + } + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 01e47996f5..e706ddca31 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -490,6 +490,7 @@ struct _qemuDomainGraphicsPrivate { virObject parent; char *tlsAlias; + qemuDomainSecretInfoPtr secinfo; }; diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args new file mode 100644 index 0000000000..737c4fe8fb --- /dev/null +++ b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args @@ -0,0 +1,36 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +SASL_CONF_PATH=/root/.sasl2 \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-object secret,id=vnc-tls-creds0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-object tls-creds-x509,id=vnc-tls-creds0,dir=/etc/pki/libvirt-vnc,\ +endpoint=server,verify-peer=yes,passwordid=vnc-tls-creds0-secret0 \ +-vnc 127.0.0.1:3,tls-creds=vnc-tls-creds0,sasl \ +-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml new file mode 100644 index 0000000000..079f6241c4 --- /dev/null +++ b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2cb8860d26..ba6fd4db35 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1290,6 +1290,11 @@ mymain(void) DO_TEST("graphics-vnc-tls", QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_CIRRUS_VGA); DO_TEST_CAPS_VER("graphics-vnc-tls", "2.4.0"); DO_TEST_CAPS_LATEST("graphics-vnc-tls"); + if (VIR_STRDUP_QUIET(driver.config->vncTLSx509secretUUID, + "6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea") < 0) + return EXIT_FAILURE; + DO_TEST_CAPS_LATEST("graphics-vnc-tls-secret"); + VIR_FREE(driver.config->vncTLSx509secretUUID); driver.config->vncSASL = driver.config->vncTLSx509verify = driver.config->vncTLS = 0; VIR_FREE(driver.config->vncSASLdir); VIR_FREE(driver.config->vncTLSx509certdir); -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
Use the password stored in the secret driver under the uuid specified by the vnc_tls_x509_secret_uuid option in qemu.conf.
https://bugzilla.redhat.com/show_bug.cgi?id=1602418
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_command.c | 11 +++++- src/qemu/qemu_domain.c | 9 +++++ src/qemu/qemu_domain.h | 1 + ...graphics-vnc-tls-secret.x86_64-latest.args | 36 +++++++++++++++++++ .../graphics-vnc-tls-secret.xml | 30 ++++++++++++++++ tests/qemuxml2argvtest.c | 5 +++ 6 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/graphics-vnc-tls-secret.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d130d0463c..e17d7ddec7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8037,11 +8037,20 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (cfg->vncTLS) { qemuDomainGraphicsPrivatePtr gfxPriv = QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics); if (gfxPriv->tlsAlias) { + const char *secretAlias = NULL; + + if (gfxPriv && gfxPriv->secinfo) {
"gfxPriv" check is unnecessary, we would have already died dereffing tlsAlias.
+ if (qemuBuildObjectSecretCommandLine(cmd, + gfxPriv->secinfo) < 0) + goto error; + secretAlias = gfxPriv->secinfo->s.aes.alias; + } + if (qemuBuildTLSx509CommandLine(cmd, cfg->vncTLSx509certdir, true, cfg->vncTLSx509verify, - NULL, + secretAlias, gfxPriv->tlsAlias, qemuCaps) < 0) goto error; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6960f0569b..da9c4e566d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1269,6 +1269,7 @@ qemuDomainGraphicsPrivateDispose(void *obj) qemuDomainGraphicsPrivatePtr priv = obj;
VIR_FREE(priv->tlsAlias); + qemuDomainSecretInfoFree(&priv->secinfo);> }
@@ -1750,6 +1751,7 @@ qemuDomainSecretGraphicsDestroy(virDomainGraphicsDefPtr graphics) return;
VIR_FREE(gfxPriv->tlsAlias); + qemuDomainSecretInfoFree(&gfxPriv->secinfo);
If you use virObjectUnref as noted in patch4, then the change in the hunk above gives you this for free.
}
@@ -1773,6 +1775,13 @@ qemuDomainSecretGraphicsPrepare(virQEMUDriverConfigPtr cfg, if (VIR_STRDUP(gfxPriv->tlsAlias, "vnc-tls-creds0") < 0) return -1;
+ if (cfg->vncTLSx509secretUUID) { + gfxPriv->secinfo = qemuDomainSecretInfoTLSNew(priv, gfxPriv->tlsAlias, + cfg->vncTLSx509secretUUID); + if (!gfxPriv->secinfo) + return -1; + } + return 0; }
Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]
Add a capability check to qemuDomainDefValidate. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da9c4e566d..851cb6d622 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4127,6 +4127,7 @@ qemuDomainDefValidate(const virDomainDef *def, void *opaque) { virQEMUDriverPtr driver = opaque; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virQEMUCapsPtr qemuCaps = NULL; int ret = -1; @@ -4249,10 +4250,19 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidateMemory(def, qemuCaps) < 0) goto cleanup; + if (cfg->vncTLS && cfg->vncTLSx509secretUUID && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("encrypted VNC TLS keys are not supported with " + "this QEMU binary")); + goto cleanup; + } + ret = 0; cleanup: virObjectUnref(qemuCaps); + virObjectUnref(cfg); return ret; } -- 2.20.1
On 1/16/19 2:41 AM, Ján Tomko wrote:
Add a capability check to qemuDomainDefValidate.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
If it were to be added, this should be merged w/ previous. I think it's pointless due to the check in qemuBuildTLSx509BackendProps which eventually gets called during qemuBuildGraphicsVNCCommandLine by qemuBuildTLSx509CommandLine. All this does is be more specific to VNC... Could have similar checks with/for Chardev, StorageSource, and Migration to be more specific for each and then remove the check in qemuBuildTLSx509BackendProps if the "issue" was that the message there is too generic. But I think the better change is to qemuBuildTLSx509BackendProps in order to print the @tlspath or the @tlsalias in the error message in order to which one failed, e.g. "tls-creds-x509 for %s not supported by this QEMU binary". John
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da9c4e566d..851cb6d622 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4127,6 +4127,7 @@ qemuDomainDefValidate(const virDomainDef *def, void *opaque) { virQEMUDriverPtr driver = opaque; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virQEMUCapsPtr qemuCaps = NULL; int ret = -1;
@@ -4249,10 +4250,19 @@ qemuDomainDefValidate(const virDomainDef *def, if (qemuDomainDefValidateMemory(def, qemuCaps) < 0) goto cleanup;
+ if (cfg->vncTLS && cfg->vncTLSx509secretUUID && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_TLS_CREDS_X509)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("encrypted VNC TLS keys are not supported with " + "this QEMU binary")); + goto cleanup; + } + ret = 0;
cleanup: virObjectUnref(qemuCaps); + virObjectUnref(cfg); return ret; }
On Thu, Jan 17, 2019 at 11:27:06AM -0500, John Ferlan wrote:
On 1/16/19 2:41 AM, Ján Tomko wrote:
Add a capability check to qemuDomainDefValidate.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
If it were to be added, this should be merged w/ previous.
I think it's pointless due to the check in qemuBuildTLSx509BackendProps which eventually gets called during qemuBuildGraphicsVNCCommandLine by qemuBuildTLSx509CommandLine.
The idea was to report an error even at define-time (due to its presence in the *Validate function), but I forgot to add a check that VNC graphics is actually used in the domain. Having the capability checks done in the validation phase lets us report an error as soon as possible and do less capability checking in the *Build functions, at the expense of repeating some logic. Jano
All this does is be more specific to VNC... Could have similar checks with/for Chardev, StorageSource, and Migration to be more specific for each and then remove the check in qemuBuildTLSx509BackendProps if the "issue" was that the message there is too generic.
But I think the better change is to qemuBuildTLSx509BackendProps in order to print the @tlspath or the @tlsalias in the error message in order to which one failed, e.g. "tls-creds-x509 for %s not supported by this QEMU binary".
John
participants (2)
-
John Ferlan -
Ján Tomko