Add a helper which will always return the storage source private data even if it was not allocated before. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_domain.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c5b8d91f9a..74392760b8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -570,6 +570,16 @@ qemuDomainStorageSourcePrivateDispose(void *obj) } +qemuDomainStorageSourcePrivatePtr +qemuDomainStorageSourcePrivateFetch(virStorageSourcePtr src) +{ + if (!src->privateData) + src->privateData = qemuDomainStorageSourcePrivateNew(); + + return QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); +} + + static virClassPtr qemuDomainVcpuPrivateClass; static void qemuDomainVcpuPrivateDispose(void *obj); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 15ffd87cb5..ae3c3bf1da 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -320,6 +320,8 @@ struct _qemuDomainStorageSourcePrivate { }; virObjectPtr qemuDomainStorageSourcePrivateNew(void); +qemuDomainStorageSourcePrivatePtr +qemuDomainStorageSourcePrivateFetch(virStorageSourcePtr src); typedef struct _qemuDomainVcpuPrivate qemuDomainVcpuPrivate; typedef qemuDomainVcpuPrivate *qemuDomainVcpuPrivatePtr; -- 2.26.2

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 74392760b8..697ddab727 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1102,7 +1102,7 @@ qemuDomainStorageSourceHasAuth(virStorageSourcePtr src) } -bool +static bool qemuDomainDiskHasEncryptionSecret(virStorageSourcePtr src) { if (!virStorageSourceIsEmpty(src) && src->encryption && diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index ae3c3bf1da..1ddac52092 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -837,9 +837,6 @@ void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) bool qemuDomainStorageSourceHasAuth(virStorageSourcePtr src) ATTRIBUTE_NONNULL(1); -bool qemuDomainDiskHasEncryptionSecret(virStorageSourcePtr src) - ATTRIBUTE_NONNULL(1); - qemuDomainSecretInfoPtr qemuDomainSecretInfoTLSNew(qemuDomainObjPrivatePtr priv, const char *srcAlias, -- 2.26.2

There's no such parameter. Reword the sentence to account for enabling TLS-encrypted migration using API flags. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu.conf | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index f89dbd2c3a..9b04c8534b 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -340,9 +340,10 @@ # In order to override the default TLS certificate location for migration # certificates, supply a valid path to the certificate directory. If the # provided path does not exist, libvirtd will fail to start. If the path is -# not provided, but migrate_tls = 1, then the default_tls_x509_cert_dir path -# will be used. Once/if a default certificate is enabled/defined, migration -# will then be able to use the certificate via migration API flags. +# not provided, but TLS-encrypted migration is requested, then the +# default_tls_x509_cert_dir path will be used. Once/if a default certificate is +# enabled/defined, migration will then be able to use the certificate via +# migration API flags. # #migrate_tls_x509_cert_dir = "/etc/pki/libvirt-migrate" -- 2.26.2

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/libvirtd_qemu.aug | 12 ++++++------ src/qemu/qemu_conf.h | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 404498b611..7a6a33c77c 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -59,6 +59,12 @@ module Libvirtd_qemu = | bool_entry "migrate_tls_x509_verify" | str_entry "migrate_tls_x509_secret_uuid" + let vxhs_entry = bool_entry "vxhs_tls" + | str_entry "vxhs_tls_x509_cert_dir" + + let nbd_entry = bool_entry "nbd_tls" + | str_entry "nbd_tls_x509_cert_dir" + let nogfx_entry = bool_entry "nographics_allow_host_audio" let remote_display_entry = int_entry "remote_display_port_min" @@ -121,12 +127,6 @@ module Libvirtd_qemu = let memory_entry = str_entry "memory_backing_dir" - let vxhs_entry = bool_entry "vxhs_tls" - | str_entry "vxhs_tls_x509_cert_dir" - - let nbd_entry = bool_entry "nbd_tls" - | str_entry "nbd_tls_x509_cert_dir" - let swtpm_entry = str_entry "swtpm_user" | str_entry "swtpm_group" diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index b9ef4551a3..4f54c136db 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -144,6 +144,12 @@ struct _virQEMUDriverConfig { bool migrateTLSx509verifyPresent; char *migrateTLSx509secretUUID; + bool vxhsTLS; + char *vxhsTLSx509certdir; + + bool nbdTLS; + char *nbdTLSx509certdir; + unsigned int remotePortMin; unsigned int remotePortMax; @@ -208,12 +214,6 @@ struct _virQEMUDriverConfig { char *memoryBackingDir; - bool vxhsTLS; - char *vxhsTLSx509certdir; - - bool nbdTLS; - char *nbdTLSx509certdir; - uid_t swtpm_user; gid_t swtpm_group; -- 2.26.2

Move the extraction of the config value so that it makes more sense after upcoming refactors. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 33b3989268..2cbff1348a 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -483,6 +483,8 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, return -1; if (virConfGetValueString(conf, "nbd_tls_x509_cert_dir", &cfg->nbdTLSx509certdir) < 0) return -1; + if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) < 0) + return -1; #define GET_CONFIG_TLS_CERTINFO(val) \ do { \ @@ -500,8 +502,6 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, return -1; \ } while (0) - if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) < 0) - return -1; GET_CONFIG_TLS_CERTINFO(chardev); GET_CONFIG_TLS_CERTINFO(migrate); -- 2.26.2

The '*_tls_x509_verify' options are relevant only when we are going to expose a server socket as client sockets always enable verification. Split up the macro to separate the common bits from the server bits so that when we'll later extend support of 'nbd' and 'vxhs' disks which are client only we can reuse the existing macros. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_conf.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2cbff1348a..b9b90e853f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -486,13 +486,8 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) < 0) return -1; -#define GET_CONFIG_TLS_CERTINFO(val) \ +#define GET_CONFIG_TLS_CERTINFO_COMMON(val) \ do { \ - if ((rv = virConfGetValueBool(conf, #val "_tls_x509_verify", \ - &cfg->val## TLSx509verify)) < 0) \ - return -1; \ - if (rv == 1) \ - cfg->val## TLSx509verifyPresent = true; \ if (virConfGetValueString(conf, #val "_tls_x509_cert_dir", \ &cfg->val## TLSx509certdir) < 0) \ return -1; \ @@ -502,11 +497,23 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, return -1; \ } while (0) - GET_CONFIG_TLS_CERTINFO(chardev); +#define GET_CONFIG_TLS_CERTINFO_SERVER(val) \ + do { \ + if ((rv = virConfGetValueBool(conf, #val "_tls_x509_verify", \ + &cfg->val## TLSx509verify)) < 0) \ + return -1; \ + if (rv == 1) \ + cfg->val## TLSx509verifyPresent = true; \ + } while (0) + + GET_CONFIG_TLS_CERTINFO_COMMON(chardev); + GET_CONFIG_TLS_CERTINFO_SERVER(chardev); - GET_CONFIG_TLS_CERTINFO(migrate); + GET_CONFIG_TLS_CERTINFO_COMMON(migrate); + GET_CONFIG_TLS_CERTINFO_SERVER(migrate); -#undef GET_CONFIG_TLS_CERTINFO +#undef GET_CONFIG_TLS_CERTINFO_COMMON +#undef GET_CONFIG_TLS_CERTINFO_SERVER return 0; } -- 2.26.2

Store the required data in the private data of a storage source and ensure that the 'alias' of the secret is formatted in the status XML. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 10 +++++++++- src/qemu/qemu_domain.h | 3 +++ tests/qemustatusxml2xmldata/modern-in.xml | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 697ddab727..7f0be22f20 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -567,6 +567,7 @@ qemuDomainStorageSourcePrivateDispose(void *obj) g_clear_pointer(&priv->secinfo, qemuDomainSecretInfoFree); g_clear_pointer(&priv->encinfo, qemuDomainSecretInfoFree); g_clear_pointer(&priv->httpcookie, qemuDomainSecretInfoFree); + g_clear_pointer(&priv->tlsKeySecret, qemuDomainSecretInfoFree); } @@ -1083,6 +1084,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) if ((srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(n))) { qemuDomainSecretInfoDestroy(srcPriv->secinfo); qemuDomainSecretInfoDestroy(srcPriv->encinfo); + qemuDomainSecretInfoDestroy(srcPriv->tlsKeySecret); } } } @@ -1750,6 +1752,7 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, g_autofree char *authalias = NULL; g_autofree char *encalias = NULL; g_autofree char *httpcookiealias = NULL; + g_autofree char *tlskeyalias = NULL; src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); @@ -1764,8 +1767,9 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, authalias = virXPathString("string(./objects/secret[@type='auth']/@alias)", ctxt); encalias = virXPathString("string(./objects/secret[@type='encryption']/@alias)", ctxt); httpcookiealias = virXPathString("string(./objects/secret[@type='httpcookie']/@alias)", ctxt); + tlskeyalias = virXPathString("string(./objects/secret[@type='tlskey']/@alias)", ctxt); - if (authalias || encalias || httpcookiealias) { + if (authalias || encalias || httpcookiealias || tlskeyalias) { if (!src->privateData && !(src->privateData = qemuDomainStorageSourcePrivateNew())) return -1; @@ -1780,6 +1784,9 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->httpcookie, &httpcookiealias) < 0) return -1; + + if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->tlsKeySecret, &tlskeyalias) < 0) + return -1; } if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) @@ -1831,6 +1838,7 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, qemuStorageSourcePrivateDataFormatSecinfo(&tmp, srcPriv->secinfo, "auth"); qemuStorageSourcePrivateDataFormatSecinfo(&tmp, srcPriv->encinfo, "encryption"); qemuStorageSourcePrivateDataFormatSecinfo(&tmp, srcPriv->httpcookie, "httpcookie"); + qemuStorageSourcePrivateDataFormatSecinfo(&tmp, srcPriv->tlsKeySecret, "tlskey"); } if (src->tlsAlias) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1ddac52092..e524fd0002 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -317,6 +317,9 @@ struct _qemuDomainStorageSourcePrivate { /* secure passthrough of the http cookie */ qemuDomainSecretInfoPtr httpcookie; + + /* key for decrypting TLS certificate */ + qemuDomainSecretInfoPtr tlsKeySecret; }; virObjectPtr qemuDomainStorageSourcePrivateNew(void); diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index 64d42200e4..2e0e415bc3 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -336,6 +336,7 @@ <secret type='auth' alias='test-auth-alias'/> <secret type='encryption' alias='test-encryption-alias'/> <secret type='httpcookie' alias='http-cookie-alias'/> + <secret type='tlskey' alias='tls certificate key alias'/> <TLSx509 alias='transport-alias'/> </objects> </privateData> -- 2.26.2

Add infrastructure for hot- and cold-plug of the secret object holding decryption key for the TLS key. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_block.c | 12 ++++++++++++ src/qemu/qemu_block.h | 2 ++ src/qemu/qemu_command.c | 11 ++++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b00694c96f..36fc6784de 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1542,7 +1542,9 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) virJSONValueFree(data->httpcookiesecretProps); virJSONValueFree(data->encryptsecretProps); virJSONValueFree(data->tlsProps); + virJSONValueFree(data->tlsKeySecretProps); VIR_FREE(data->tlsAlias); + VIR_FREE(data->tlsKeySecretAlias); VIR_FREE(data->authsecretAlias); VIR_FREE(data->encryptsecretAlias); VIR_FREE(data->httpcookiesecretAlias); @@ -1617,6 +1619,11 @@ qemuBlockStorageSourceAttachApplyStorageDeps(qemuMonitorPtr mon, &data->httpcookiesecretAlias) < 0) return -1; + if (data->tlsKeySecretProps && + qemuMonitorAddObject(mon, &data->tlsKeySecretProps, + &data->tlsKeySecretAlias) < 0) + return -1; + if (data->tlsProps && qemuMonitorAddObject(mon, &data->tlsProps, &data->tlsAlias) < 0) return -1; @@ -1766,6 +1773,8 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, if (data->tlsAlias) ignore_value(qemuMonitorDelObject(mon, data->tlsAlias, false)); + if (data->tlsKeySecretAlias) + ignore_value(qemuMonitorDelObject(mon, data->tlsKeySecretAlias, false)); virErrorRestore(&orig_err); } @@ -1821,6 +1830,9 @@ qemuBlockStorageSourceDetachPrepare(virStorageSourcePtr src, if (srcpriv->httpcookie) data->httpcookiesecretAlias = g_strdup(srcpriv->httpcookie->s.aes.alias); + + if (srcpriv->tlsKeySecret) + data->tlsKeySecretAlias = g_strdup(srcpriv->tlsKeySecret->s.aes.alias); } return g_steal_pointer(&data); diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 24b87e79db..b1bdb39613 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -105,6 +105,8 @@ struct qemuBlockStorageSourceAttachData { virJSONValuePtr tlsProps; char *tlsAlias; + virJSONValuePtr tlsKeySecretProps; + char *tlsKeySecretAlias; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6e7fd59561..0c4c77cf8c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2047,6 +2047,7 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd, qemuBuildObjectCommandline(cmd, data->authsecretProps) < 0 || qemuBuildObjectCommandline(cmd, data->encryptsecretProps) < 0 || qemuBuildObjectCommandline(cmd, data->httpcookiesecretProps) < 0 || + qemuBuildObjectCommandline(cmd, data->tlsKeySecretProps) < 0 || qemuBuildObjectCommandline(cmd, data->tlsProps) < 0) return -1; @@ -10161,6 +10162,7 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, virQEMUCapsPtr qemuCaps) { qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + const char *tlsKeySecretAlias = NULL; if (src->pr && !virStoragePRDefIsManaged(src->pr) && @@ -10180,11 +10182,18 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, if (srcpriv->httpcookie && qemuBuildSecretInfoProps(srcpriv->httpcookie, &data->httpcookiesecretProps) < 0) return -1; + + if (srcpriv->tlsKeySecret) { + if (qemuBuildSecretInfoProps(srcpriv->tlsKeySecret, &data->tlsKeySecretProps) < 0) + return -1; + + tlsKeySecretAlias = srcpriv->tlsKeySecret->s.aes.alias; + } } if (src->haveTLS == VIR_TRISTATE_BOOL_YES && qemuBuildTLSx509BackendProps(src->tlsCertdir, false, true, src->tlsAlias, - NULL, qemuCaps, &data->tlsProps) < 0) + tlsKeySecretAlias, qemuCaps, &data->tlsProps) < 0) return -1; return 0; -- 2.26.2

Until now libvirt didn't allow using encrypted TLS key for disk clients. Add fields for configuring the secret and propagate defaults. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/libvirtd_qemu.aug | 2 ++ src/qemu/qemu.conf | 19 +++++++++++++++++++ src/qemu/qemu_conf.c | 13 +++++++++---- src/qemu/qemu_conf.h | 2 ++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 7a6a33c77c..c19a086c38 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -61,9 +61,11 @@ module Libvirtd_qemu = let vxhs_entry = bool_entry "vxhs_tls" | str_entry "vxhs_tls_x509_cert_dir" + | str_entry "vxhs_tls_x509_secret_uuid" let nbd_entry = bool_entry "nbd_tls" | str_entry "nbd_tls_x509_cert_dir" + | str_entry "nbd_tls_x509_secret_uuid" let nogfx_entry = bool_entry "nographics_allow_host_audio" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 9b04c8534b..ab403c21ac 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -303,6 +303,15 @@ #vxhs_tls_x509_cert_dir = "/etc/pki/libvirt-vxhs" +# Uncomment and use the following option to override the default secret +# UUID provided in the default_tls_x509_secret_uuid parameter. +# +# NB This default all-zeros UUID will not work. Replace it with the +# output from the UUID for the TLS secret from a 'virsh secret-list' +# command and then uncomment the entry +# +#vxhs_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" + # Enable use of TLS encryption for all NBD disk devices that don't # specifically disable it. @@ -337,6 +346,16 @@ #nbd_tls_x509_cert_dir = "/etc/pki/libvirt-nbd" +# Uncomment and use the following option to override the default secret +# UUID provided in the default_tls_x509_secret_uuid parameter. +# +# NB This default all-zeros UUID will not work. Replace it with the +# output from the UUID for the TLS secret from a 'virsh secret-list' +# command and then uncomment the entry +# +#nbd_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" + + # In order to override the default TLS certificate location for migration # certificates, supply a valid path to the certificate directory. If the # provided path does not exist, libvirtd will fail to start. If the path is diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b9b90e853f..6e673e8f62 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -339,7 +339,10 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->chardevTLSx509secretUUID); VIR_FREE(cfg->vxhsTLSx509certdir); + VIR_FREE(cfg->vxhsTLSx509secretUUID); + VIR_FREE(cfg->nbdTLSx509certdir); + VIR_FREE(cfg->nbdTLSx509secretUUID); VIR_FREE(cfg->migrateTLSx509certdir); VIR_FREE(cfg->migrateTLSx509secretUUID); @@ -477,12 +480,8 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0) return -1; - if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir", &cfg->vxhsTLSx509certdir) < 0) - return -1; if (virConfGetValueBool(conf, "nbd_tls", &cfg->nbdTLS) < 0) return -1; - if (virConfGetValueString(conf, "nbd_tls_x509_cert_dir", &cfg->nbdTLSx509certdir) < 0) - return -1; if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) < 0) return -1; @@ -512,6 +511,10 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, GET_CONFIG_TLS_CERTINFO_COMMON(migrate); GET_CONFIG_TLS_CERTINFO_SERVER(migrate); + GET_CONFIG_TLS_CERTINFO_COMMON(vxhs); + + GET_CONFIG_TLS_CERTINFO_COMMON(nbd); + #undef GET_CONFIG_TLS_CERTINFO_COMMON #undef GET_CONFIG_TLS_CERTINFO_SERVER return 0; @@ -1186,6 +1189,8 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) SET_TLS_SECRET_UUID_DEFAULT(vnc); SET_TLS_SECRET_UUID_DEFAULT(chardev); SET_TLS_SECRET_UUID_DEFAULT(migrate); + SET_TLS_SECRET_UUID_DEFAULT(vxhs); + SET_TLS_SECRET_UUID_DEFAULT(nbd); #undef SET_TLS_SECRET_UUID_DEFAULT diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 4f54c136db..6193a7111c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -146,9 +146,11 @@ struct _virQEMUDriverConfig { bool vxhsTLS; char *vxhsTLSx509certdir; + char *vxhsTLSx509secretUUID; bool nbdTLS; char *nbdTLSx509certdir; + char *nbdTLSx509secretUUID; unsigned int remotePortMin; unsigned int remotePortMax; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index e533b9f551..db125bf352 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -28,8 +28,10 @@ module Test_libvirtd_qemu = { "chardev_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } { "vxhs_tls" = "1" } { "vxhs_tls_x509_cert_dir" = "/etc/pki/libvirt-vxhs" } +{ "vxhs_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } { "nbd_tls" = "1" } { "nbd_tls_x509_cert_dir" = "/etc/pki/libvirt-nbd" } +{ "nbd_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } { "migrate_tls_x509_cert_dir" = "/etc/pki/libvirt-migrate" } { "migrate_tls_x509_verify" = "1" } { "migrate_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } -- 2.26.2

Setup the TLS secret when preparing a virStorageSource for use. https://bugzilla.redhat.com/show_bug.cgi?id=1602328 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7f0be22f20..42cc78ac1b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9537,7 +9537,9 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def, static int qemuProcessPrepareStorageSourceTLSVxhs(virStorageSourcePtr src, - virQEMUDriverConfigPtr cfg) + virQEMUDriverConfigPtr cfg, + qemuDomainObjPrivatePtr priv, + const char *parentAlias) { /* VxHS uses only client certificates and thus has no need for * the server-key.pem nor a secret that could be used to decrypt @@ -9550,9 +9552,19 @@ qemuProcessPrepareStorageSourceTLSVxhs(virStorageSourcePtr src, src->tlsFromConfig = true; } - if (src->haveTLS == VIR_TRISTATE_BOOL_YES) + if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { + src->tlsAlias = qemuAliasTLSObjFromSrcAlias(parentAlias); src->tlsCertdir = g_strdup(cfg->vxhsTLSx509certdir); + if (cfg->vxhsTLSx509secretUUID) { + qemuDomainStorageSourcePrivatePtr srcpriv = qemuDomainStorageSourcePrivateFetch(src); + + if (!(srcpriv->tlsKeySecret = qemuDomainSecretInfoTLSNew(priv, src->tlsAlias, + cfg->vxhsTLSx509secretUUID))) + return -1; + } + } + return 0; } @@ -9560,7 +9572,8 @@ qemuProcessPrepareStorageSourceTLSVxhs(virStorageSourcePtr src, static int qemuProcessPrepareStorageSourceTLSNBD(virStorageSourcePtr src, virQEMUDriverConfigPtr cfg, - virQEMUCapsPtr qemuCaps) + qemuDomainObjPrivatePtr priv, + const char *parentAlias) { if (src->haveTLS == VIR_TRISTATE_BOOL_ABSENT) { if (cfg->nbdTLS) @@ -9571,13 +9584,22 @@ qemuProcessPrepareStorageSourceTLSNBD(virStorageSourcePtr src, } if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NBD_TLS)) { + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_TLS)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("this qemu does not support TLS transport for NBD")); return -1; } + src->tlsAlias = qemuAliasTLSObjFromSrcAlias(parentAlias); src->tlsCertdir = g_strdup(cfg->nbdTLSx509certdir); + + if (cfg->nbdTLSx509secretUUID) { + qemuDomainStorageSourcePrivatePtr srcpriv = qemuDomainStorageSourcePrivateFetch(src); + + if (!(srcpriv->tlsKeySecret = qemuDomainSecretInfoTLSNew(priv, src->tlsAlias, + cfg->nbdTLSx509secretUUID))) + return -1; + } } return 0; @@ -9599,19 +9621,19 @@ static int qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, virQEMUDriverConfigPtr cfg, const char *parentAlias, - virQEMUCapsPtr qemuCaps) + qemuDomainObjPrivatePtr priv) { if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) return 0; switch ((virStorageNetProtocol) src->protocol) { case VIR_STORAGE_NET_PROTOCOL_VXHS: - if (qemuProcessPrepareStorageSourceTLSVxhs(src, cfg) < 0) + if (qemuProcessPrepareStorageSourceTLSVxhs(src, cfg, priv, parentAlias) < 0) return -1; break; case VIR_STORAGE_NET_PROTOCOL_NBD: - if (qemuProcessPrepareStorageSourceTLSNBD(src, cfg, qemuCaps) < 0) + if (qemuProcessPrepareStorageSourceTLSNBD(src, cfg, priv, parentAlias) < 0) return -1; break; @@ -9640,10 +9662,6 @@ qemuDomainPrepareStorageSourceTLS(virStorageSourcePtr src, return -1; } - if (src->haveTLS == VIR_TRISTATE_BOOL_YES && - !(src->tlsAlias = qemuAliasTLSObjFromSrcAlias(parentAlias))) - return -1; - return 0; } @@ -12128,7 +12146,7 @@ qemuDomainPrepareDiskSourceLegacy(virDomainDiskDefPtr disk, return -1; if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg, disk->info.alias, - priv->qemuCaps) < 0) + priv) < 0) return -1; return 0; @@ -12164,7 +12182,7 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, return -1; if (qemuDomainPrepareStorageSourceTLS(src, cfg, src->nodestorage, - priv->qemuCaps) < 0) + priv) < 0) return -1; return 0; -- 2.26.2

Add a dummy secret so that we see what command line is generated. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- .../disk-network-tlsx509.x86_64-2.12.0.args | 15 ++++++++++++--- .../disk-network-tlsx509.x86_64-latest.args | 18 +++++++++++++++--- tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args index 06686f801d..2a30ad02c9 100644 --- a/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args +++ b/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args @@ -28,8 +28,11 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-object secret,id=objvirtio-disk0_tls0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ -object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/libvirt-vxhs/dummy,\ -,path,endpoint=client,verify-peer=yes \ +,path,endpoint=client,verify-peer=yes,passwordid=objvirtio-disk0_tls0-secret0 \ -drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\ file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\ file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\ @@ -37,8 +40,11 @@ id=drive-virtio-disk0,cache=none \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ id=virtio-disk0,bootindex=1,write-cache=on,\ serial=eb90327c-8302-4725-9e1b-4e85ed4dc251 \ +-object secret,id=objvirtio-disk1_tls0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ -object tls-creds-x509,id=objvirtio-disk1_tls0,dir=/etc/pki/libvirt-vxhs/dummy,\ -,path,endpoint=client,verify-peer=yes \ +,path,endpoint=client,verify-peer=yes,passwordid=objvirtio-disk1_tls0-secret0 \ -drive file.driver=vxhs,file.tls-creds=objvirtio-disk1_tls0,\ file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\ file.server.host=192.168.0.2,file.server.port=9999,format=raw,if=none,\ @@ -50,8 +56,11 @@ file.server.host=192.168.0.3,file.server.port=9999,format=raw,if=none,\ id=drive-virtio-disk2,cache=none \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\ id=virtio-disk2,write-cache=on,serial=eb90327c-8302-4725-9e1b-4e85ed4dc252 \ +-object secret,id=objvirtio-disk3_tls0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ -object tls-creds-x509,id=objvirtio-disk3_tls0,dir=/etc/pki/libvirt-nbd/dummy,,\ -path,endpoint=client,verify-peer=yes \ +path,endpoint=client,verify-peer=yes,passwordid=objvirtio-disk3_tls0-secret0 \ -drive file.driver=nbd,file.server.type=inet,file.server.host=example.com,\ file.server.port=1234,file.tls-creds=objvirtio-disk3_tls0,format=raw,if=none,\ id=drive-virtio-disk3,cache=none \ diff --git a/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-latest.args b/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-latest.args index 5195107b7b..ec4c28e161 100644 --- a/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-latest.args @@ -28,8 +28,12 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-object secret,id=objlibvirt-4-storage_tls0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ -object tls-creds-x509,id=objlibvirt-4-storage_tls0,\ -dir=/etc/pki/libvirt-vxhs/dummy,,path,endpoint=client,verify-peer=yes \ +dir=/etc/pki/libvirt-vxhs/dummy,,path,endpoint=client,verify-peer=yes,\ +passwordid=objlibvirt-4-storage_tls0-secret0 \ -blockdev '{"driver":"vxhs","tls-creds":"objlibvirt-4-storage_tls0",\ "vdisk-id":"eb90327c-8302-4725-9e1b-4e85ed4dc251",\ "server":{"host":"192.168.0.1","port":"9999"},"node-name":"libvirt-4-storage",\ @@ -41,8 +45,12 @@ dir=/etc/pki/libvirt-vxhs/dummy,,path,endpoint=client,verify-peer=yes \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=libvirt-4-format,\ id=virtio-disk0,bootindex=1,write-cache=on,\ serial=eb90327c-8302-4725-9e1b-4e85ed4dc251 \ +-object secret,id=objlibvirt-3-storage_tls0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ -object tls-creds-x509,id=objlibvirt-3-storage_tls0,\ -dir=/etc/pki/libvirt-vxhs/dummy,,path,endpoint=client,verify-peer=yes \ +dir=/etc/pki/libvirt-vxhs/dummy,,path,endpoint=client,verify-peer=yes,\ +passwordid=objlibvirt-3-storage_tls0-secret0 \ -blockdev '{"driver":"vxhs","tls-creds":"objlibvirt-3-storage_tls0",\ "vdisk-id":"eb90327c-8302-4725-9e1b-4e85ed4dc252",\ "server":{"host":"192.168.0.2","port":"9999"},"node-name":"libvirt-3-storage",\ @@ -62,8 +70,12 @@ id=virtio-disk1,write-cache=on,serial=eb90327c-8302-4725-9e1b-4e85ed4dc252 \ "file":"libvirt-2-storage"}' \ -device virtio-blk-pci,bus=pci.0,addr=0x6,drive=libvirt-2-format,\ id=virtio-disk2,write-cache=on,serial=eb90327c-8302-4725-9e1b-4e85ed4dc252 \ +-object secret,id=objlibvirt-1-storage_tls0-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ -object tls-creds-x509,id=objlibvirt-1-storage_tls0,\ -dir=/etc/pki/libvirt-nbd/dummy,,path,endpoint=client,verify-peer=yes \ +dir=/etc/pki/libvirt-nbd/dummy,,path,endpoint=client,verify-peer=yes,\ +passwordid=objlibvirt-1-storage_tls0-secret0 \ -blockdev '{"driver":"nbd","server":{"type":"inet","host":"example.com",\ "port":"1234"},"tls-creds":"objlibvirt-1-storage_tls0",\ "node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},\ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2e06140ea1..26333d8f40 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1192,6 +1192,8 @@ mymain(void) driver.config->vxhsTLS = 1; DO_TEST("disk-network-tlsx509", QEMU_CAPS_VXHS, QEMU_CAPS_OBJECT_TLS_CREDS_X509, QEMU_CAPS_NBD_TLS); + driver.config->nbdTLSx509secretUUID = g_strdup("6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea"); + driver.config->vxhsTLSx509secretUUID = g_strdup("6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea"); DO_TEST_CAPS_VER("disk-network-tlsx509", "2.12.0"); DO_TEST_CAPS_LATEST("disk-network-tlsx509"); DO_TEST_CAPS_LATEST("disk-network-http"); -- 2.26.2

Users may want to use this to create a full backup or even incremental if the checkpoints are pre existing. We still will not allow to create a checkpoint on a read-only disk as that makes no sense. https://bugzilla.redhat.com/show_bug.cgi?id=1840053 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/backup_conf.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 92106d8aaa..e9eea5af75 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -411,13 +411,6 @@ virDomainBackupDefAssignStore(virDomainBackupDiskDefPtr disk, _("disk '%s' has no media"), disk->name); return -1; } - } else if (src->readonly) { - if (disk->store) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("backup of readonly disk '%s' makes no sense"), - disk->name); - return -1; - } } else if (!disk->store) { if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_FILE) { if (!(disk->store = virStorageSourceNew())) -- 2.26.2

Switch to the new format for easier extension. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatbackup.html.in | 191 -------------------------------------- docs/formatbackup.rst | 149 +++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 191 deletions(-) delete mode 100644 docs/formatbackup.html.in create mode 100644 docs/formatbackup.rst diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in deleted file mode 100644 index 9e69d8f7d3..0000000000 --- a/docs/formatbackup.html.in +++ /dev/null @@ -1,191 +0,0 @@ -<?xml version="1.0" encoding="UTF-8"?> -<!DOCTYPE html> -<html xmlns="http://www.w3.org/1999/xhtml"> - <body> - <h1>Backup XML format</h1> - - <ul id="toc"></ul> - - <h2><a id="BackupAttributes">Backup XML</a></h2> - - <p> - Creating a backup, whether full or incremental, is done - via <code>virDomainBackupBegin()</code>, which takes an XML - description of the actions to perform, as well as an optional - second XML document <a href="formatcheckpoint.html">describing a - checkpoint</a> to create at the same point in time. See - also <a href="kbase/domainstatecapture.html">a comparison</a> between - the various state capture APIs. - </p> - <p> - There are two general modes for backups: a push mode (where the - hypervisor writes out the data to the destination file, which - may be local or remote), and a pull mode (where the hypervisor - creates an NBD server that a third-party client can then read as - needed, and which requires the use of temporary storage, - typically local, until the backup is complete). - </p> - <p> - The instructions for beginning a backup job are provided as - attributes and elements of the - top-level <code>domainbackup</code> element. This element - includes an optional attribute <code>mode</code> which can be - either "push" or "pull" (default - push). <code>virDomainBackupGetXMLDesc()</code> can be used to - see the actual values selected for elements omitted during - creation (for example, learning which port the NBD server is - using in the pull model or what file names libvirt generated - when none were supplied). The following child elements and attributes - are supported: - </p> - <dl> - <dt><code>incremental</code></dt> - <dd>An optional element giving the name of an existing - checkpoint of the domain, which will be used to make this - backup an incremental one. In the push model, only changes - since the named checkpoint are written to the destination. In - the pull model, the NBD server uses the - NBD_OPT_SET_META_CONTEXT extension to advertise to the client - which portions of the export contain changes since the named - checkpoint. If omitted, a full backup is performed. - </dd> - <dt><code>server</code></dt> - <dd>Present only for a pull mode backup. Contains the same - attributes as - the <a href="formatdomain.html#elementsDisks"><code>protocol</code> - element of a disk</a> attached via NBD in the domain (such as - transport, socket, name, port, or tls), necessary to set up an - NBD server that exposes the content of each disk at the time - the backup is started. - </dd> - <dt><code>disks</code></dt> - <dd>An optional listing of instructions for disks participating - in the backup (if omitted, all disks participate and libvirt - attempts to generate filenames by appending the current - timestamp as a suffix). If the entire element was omitted on - input, then all disks participate in the backup, otherwise, - only the disks explicitly listed which do not also - use <code>backup='no'</code> will participate. On output, this - is the state of each of the domain's disk in relation to the - backup operation. - <dl> - <dt><code>disk</code></dt> - <dd>This sub-element describes the backup properties of a - specific disk, with the following attributes and child - elements: - <dl> - <dt><code>name</code></dt> - <dd>A mandatory attribute which must match - the <code>&lt;target dev='name'/&gt;</code> - of one of - the <a href="formatdomain.html#elementsDisks">disk - devices</a> specified for the domain at the time of - the checkpoint.</dd> - <dt><code>backup</code></dt> - <dd>Setting this attribute to <code>yes</code>(default) specifies - that the disk should take part in the backup and using - <code>no</code> excludes the disk from the backup.</dd> - <dt><code>exportname</code></dt> - <dd>Allows modification of the NBD export name for the given disk. - By default equal to disk target. - Valid only for pull mode backups.</dd> - <dt><code>exportbitmap</code></dt> - <dd>Allows modification of the name of the bitmap describing dirty - blocks for an incremental backup exported via NBD export name - for the given disk. - Valid only for pull mode backups.</dd> - <dt><code>type</code></dt> - <dd>A mandatory attribute to describe the type of the - disk, except when <code>backup='no'</code> is - used. Valid values include <code>file</code>, or - <code>block</code>. - Similar to a disk declaration for a domain, the choice of type - controls what additional sub-elements are needed to describe - the destination.</dd> - <dt><code>target</code></dt> - <dd>Valid only for push mode backups, this is the - primary sub-element that describes the file name of - the backup destination, similar to - the <code>source</code> sub-element of a domain - disk. An optional sub-element <code>driver</code> can - also be used, with an attribute <code>type</code> to - specify a destination format different from - qcow2. See documentation for <code>scratch</code> below for - additional configuration.</dd> - <dt><code>scratch</code></dt> - <dd>Valid only for pull mode backups, this is the - primary sub-element that describes the file name of - the local scratch file to be used in facilitating the - backup, and is similar to the <code>source</code> - sub-element of a domain disk. Currently only <code>file</code> - and <code>block</code> scratch storage is supported. The - <code>file</code> scratch file is created and deleted by - libvirt in the given location. A <code>block</code> scratch - device must exist prior to starting the backup and is formatted. - The block device must have enough space for the corresponding - disk data including format overhead. - - If <code>VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL</code> flag is - used the file for a scratch of <code>file</code> type must - exist with the correct format and size to hold the copy and is - used without modification. The file is not deleted after the - backup but the contents of the file don't make sense outside - of the backup. The same applies for the block device which - must be formatted appropriately. - - Similarly to the domain - <a href="formatdomain.html#elementsDisks"><code>disk</code></a> - definition <code>scratch</code> and <code>target</code> can - contain <code>seclabel</code> and/or <code>encryption</code> - subelements to configure the corresponding properties. - </dd> - </dl> - </dd> - </dl> - </dd> - </dl> - - <h2><a id="example">Examples</a></h2> - - <p>Use <code>virDomainBackupBegin()</code> to perform a full - backup using push mode. The example lets libvirt pick the - destination and format for 'vda', fully specifies that we want a - raw backup of 'vdb', and omits 'vdc' from the operation. - </p> - <pre> -&lt;domainbackup&gt; - &lt;disks&gt; - &lt;disk name='vda' backup='yes'/&gt; - &lt;disk name='vdb' type='file'&gt; - &lt;target file='/path/to/vdb.backup'/&gt; - &lt;driver type='raw'/&gt; - &lt;/disk&gt; - &lt;disk name='vdc' backup='no'/&gt; - &lt;/disks&gt; -&lt;/domainbackup&gt; - </pre> - - <p>If the previous full backup also passed a parameter describing - <a href="formatcheckpoint.html">checkpoint XML</a> that resulted - in a checkpoint named <code>1525889631</code>, we can make - another call to <code>virDomainBackupBegin()</code> to perform - an incremental backup of just the data changed since that - checkpoint, this time using the following XML to start a pull - model export of the 'vda' and 'vdb' disks, where a third-party - NBD client connecting to '/path/to/server' completes the backup - (omitting 'vdc' from the explicit list has the same effect as - the backup='no' from the previous example): - </p> - <pre> -&lt;domainbackup mode="pull"&gt; - &lt;incremental&gt;1525889631&lt;/incremental&gt; - &lt;server transport="unix" socket="/path/to/server"/&gt; - &lt;disks&gt; - &lt;disk name='vda' backup='yes' type='file'&gt; - &lt;scratch file='/path/to/file1.scratch'/&gt; - &lt;/disk&gt; - &lt;/disks&gt; -&lt;/domainbackup&gt; - </pre> - </body> -</html> diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst new file mode 100644 index 0000000000..66583f562b --- /dev/null +++ b/docs/formatbackup.rst @@ -0,0 +1,149 @@ +Backup XML format +================= + +.. contents:: + +Backup XML +---------- + +Creating a backup, whether full or incremental, is done via +``virDomainBackupBegin()``, which takes an XML description of the actions to +perform, as well as an optional second XML document `describing a +checkpoint <formatcheckpoint.html>`__ to create at the same point in time. See +also `a comparison <kbase/domainstatecapture.html>`__ between the various state +capture APIs. + +There are two general modes for backups: a push mode (where the hypervisor +writes out the data to the destination file, which may be local or remote), and +a pull mode (where the hypervisor creates an NBD server that a third-party +client can then read as needed, and which requires the use of temporary storage, +typically local, until the backup is complete). + +The instructions for beginning a backup job are provided as attributes and +elements of the top-level ``domainbackup`` element. This element includes an +optional attribute ``mode`` which can be either "push" or "pull" (default push). +``virDomainBackupGetXMLDesc()`` can be used to see the actual values selected +for elements omitted during creation (for example, learning which port the NBD +server is using in the pull model or what file names libvirt generated when none +were supplied). The following child elements and attributes are supported: + +``incremental`` + An optional element giving the name of an existing checkpoint of the domain, + which will be used to make this backup an incremental one. In the push model, + only changes since the named checkpoint are written to the destination. In + the pull model, the NBD server uses the NBD_OPT_SET_META_CONTEXT extension to + advertise to the client which portions of the export contain changes since + the named checkpoint. If omitted, a full backup is performed. + +``server`` + Present only for a pull mode backup. Contains the same attributes as the + ```protocol`` element of a disk <formatdomain.html#elementsDisks>`__ attached + via NBD in the domain (such as transport, socket, name, port, or tls), + necessary to set up an NBD server that exposes the content of each disk at + the time the backup is started. + +``disks`` + An optional listing of instructions for disks participating in the backup (if + omitted, all disks participate and libvirt attempts to generate filenames by + appending the current timestamp as a suffix). If the entire element was + omitted on input, then all disks participate in the backup, otherwise, only + the disks explicitly listed which do not also use ``backup='no'`` will + participate. On output, this is the state of each of the domain's disk in + relation to the backup operation. + + ``disk`` + This sub-element describes the backup properties of a specific disk, with + the following attributes and child elements: + + ``name`` + A mandatory attribute which must match the ``<target dev='name'/>`` of + one of the `disk devices <formatdomain.html#elementsDisks>`__ specified + for the domain at the time of the checkpoint. + + ``backup`` + Setting this attribute to ``yes``\ (default) specifies that the disk + should take part in the backup and using ``no`` excludes the disk from + the backup. + + ``exportname`` + Allows modification of the NBD export name for the given disk. By + default equal to disk target. Valid only for pull mode backups. + + ``exportbitmap`` + Allows modification of the name of the bitmap describing dirty blocks + for an incremental backup exported via NBD export name for the given + disk. Valid only for pull mode backups. + + ``type`` + A mandatory attribute to describe the type of the disk, except when + ``backup='no'`` is used. Valid values include ``file``, or ``block``. + Similar to a disk declaration for a domain, the choice of type controls + what additional sub-elements are needed to describe the destination. + + ``target`` + Valid only for push mode backups, this is the primary sub-element that + describes the file name of the backup destination, similar to the + ``source`` sub-element of a domain disk. An optional sub-element + ``driver`` can also be used, with an attribute ``type`` to specify a + destination format different from qcow2. See documentation for + ``scratch`` below for additional configuration. + + ``scratch`` + Valid only for pull mode backups, this is the primary sub-element that + describes the file name of the local scratch file to be used in + facilitating the backup, and is similar to the ``source`` sub-element + of a domain disk. Currently only ``file`` and ``block`` scratch storage + is supported. The ``file`` scratch file is created and deleted by + libvirt in the given location. A ``block`` scratch device must exist + prior to starting the backup and is formatted. The block device must + have enough space for the corresponding disk data including format + overhead. If ``VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL`` flag is used + the file for a scratch of ``file`` type must exist with the correct + format and size to hold the copy and is used without modification. The + file is not deleted after the backup but the contents of the file don't + make sense outside of the backup. The same applies for the block device + which must be formatted appropriately. Similarly to the domain + ```disk`` <formatdomain.html#elementsDisks>`__ definition ``scratch`` + and ``target`` can contain ``seclabel`` and/or ``encryption`` + subelements to configure the corresponding properties. + +Examples +-------- + +Use ``virDomainBackupBegin()`` to perform a full backup using push mode. The +example lets libvirt pick the destination and format for 'vda', fully specifies +that we want a raw backup of 'vdb', and omits 'vdc' from the operation. + +:: + + <domainbackup> + <disks> + <disk name='vda' backup='yes'/> + <disk name='vdb' type='file'> + <target file='/path/to/vdb.backup'/> + <driver type='raw'/> + </disk> + <disk name='vdc' backup='no'/> + </disks> + </domainbackup> + +If the previous full backup also passed a parameter describing `checkpoint +XML <formatcheckpoint.html>`__ that resulted in a checkpoint named +``1525889631``, we can make another call to ``virDomainBackupBegin()`` to +perform an incremental backup of just the data changed since that checkpoint, +this time using the following XML to start a pull model export of the 'vda' and +'vdb' disks, where a third-party NBD client connecting to '/path/to/server' +completes the backup (omitting 'vdc' from the explicit list has the same effect +as the backup='no' from the previous example): + +:: + + <domainbackup mode="pull"> + <incremental>1525889631</incremental> + <server transport="unix" socket="/path/to/server"/> + <disks> + <disk name='vda' backup='yes' type='file'> + <scratch file='/path/to/file1.scratch'/> + </disk> + </disks> + </domainbackup> -- 2.26.2

The semantics of the backup operation don't strictly require that all disks being backed up are part of the same incremental part (when a disk was checkpointed/backed up separately or in a different VM), or even they may not have an previous checkpoint at all (e.g. when the disk was freshly hotplugged to the vm). In such cases we can still create a common checkpoint for all of them and backup differences according to configuration. This patch adds a per-disk configuration of the checkpoint to do the incremental backup from via the 'incremental' attribute and allows perform full backups via the 'backupmode' attribute. Note that no changes to the qemu driver are necessary to take advantage of this as we already obey the per-disk 'incremental' field. https://bugzilla.redhat.com/show_bug.cgi?id=1829829 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatbackup.rst | 11 ++++ docs/schemas/domainbackup.rng | 16 ++++++ src/conf/backup_conf.c | 57 +++++++++++++++++++- src/conf/backup_conf.h | 11 ++++ tests/domainbackupxml2xmlin/backup-pull.xml | 12 +++++ tests/domainbackupxml2xmlout/backup-pull.xml | 12 +++++ 6 files changed, 118 insertions(+), 1 deletion(-) diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst index 66583f562b..e5b6fc6eb0 100644 --- a/docs/formatbackup.rst +++ b/docs/formatbackup.rst @@ -65,6 +65,17 @@ were supplied). The following child elements and attributes are supported: should take part in the backup and using ``no`` excludes the disk from the backup. + ``backupmode`` + This attribute overrides the implied backup mode inherited from the + definition of the backup itself. Value ``full`` forces a full backup + even if the backup calls for an incremental backup and ``incremental`` + coupled with the attribute ``incremental='CHECKPOINTNAME`` for the disk + forces an incremental backup from ``CHECKPOINTNAME``. + + ``incremental`` + An optional attribute giving the name of an existing checkpoint of the + domain which overrides the one set by the ``<incremental>`` element. + ``exportname`` Allows modification of the NBD export name for the given disk. By default equal to disk target. Valid only for pull mode backups. diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng index 5165175152..650f5cd4c3 100644 --- a/docs/schemas/domainbackup.rng +++ b/docs/schemas/domainbackup.rng @@ -89,6 +89,20 @@ </element> </define> + <define name='backupDiskMode'> + <optional> + <attribute name='backupmode'> + <choice> + <value>full</value> + <value>incremental</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name='incremental'/> + </optional> + </define> + <define name='backupPushDriver'> <optional> <element name='driver'> @@ -127,6 +141,7 @@ <attribute name='name'> <ref name='diskTarget'/> </attribute> + <ref name='backupDiskMode'/> <choice> <group> <attribute name='backup'> @@ -196,6 +211,7 @@ <attribute name='name'> <ref name='diskTarget'/> </attribute> + <ref name='backupDiskMode'/> <optional> <attribute name='exportname'> <text/> diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index e9eea5af75..4f28073ab2 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -56,6 +56,13 @@ VIR_ENUM_IMPL(virDomainBackupDiskState, "cancelling", "cancelled"); +VIR_ENUM_DECL(virDomainBackupDiskBackupMode); +VIR_ENUM_IMPL(virDomainBackupDiskBackupMode, + VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_LAST, + "", + "full", + "incremental"); + void virDomainBackupDefFree(virDomainBackupDefPtr def) { @@ -96,6 +103,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node, g_autofree char *driver = NULL; g_autofree char *backup = NULL; g_autofree char *state = NULL; + g_autofree char *backupmode = NULL; int tmp; xmlNodePtr srcNode; unsigned int storageSourceParseFlags = 0; @@ -133,6 +141,19 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node, def->exportbitmap = virXMLPropString(node, "exportbitmap"); } + if ((backupmode = virXMLPropString(node, "backupmode"))) { + if ((tmp = virDomainBackupDiskBackupModeTypeFromString(backupmode)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid backupmode '%s' of disk '%s'"), + backupmode, def->name); + return -1; + } + + def->backupmode = tmp; + } + + def->incremental = virXMLPropString(node, "incremental"); + if (internal) { if (!(state = virXMLPropString(node, "state")) || (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) { @@ -342,6 +363,13 @@ virDomainBackupDiskDefFormat(virBufferPtr buf, if (disk->backup == VIR_TRISTATE_BOOL_YES) { virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(disk->store->type)); + if (disk->backupmode != VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_DEFAULT) { + virBufferAsprintf(&attrBuf, " backupmode='%s'", + virDomainBackupDiskBackupModeTypeToString(disk->backupmode)); + } + + virBufferEscapeString(&attrBuf, " incremental='%s'", disk->incremental); + virBufferEscapeString(&attrBuf, " exportname='%s'", disk->exportname); virBufferEscapeString(&attrBuf, " exportbitmap='%s'", disk->exportbitmap); @@ -465,6 +493,24 @@ virDomainBackupAlignDisks(virDomainBackupDefPtr def, return -1; } + if (backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_FULL && + backupdisk->incremental) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("'full' backup mode incompatible with 'incremental' for disk '%s'"), + backupdisk->name); + return -1; + } + + if (backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL && + !backupdisk->incremental && + !def->incremental) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("'incremental' backup mode of disk '%s' requires setting 'incremental' field for disk or backup"), + backupdisk->name); + return -1; + } + + if (backupdisk->backup == VIR_TRISTATE_BOOL_YES && virDomainBackupDefAssignStore(backupdisk, domdisk->src, suffix) < 0) return -1; @@ -502,7 +548,16 @@ virDomainBackupAlignDisks(virDomainBackupDefPtr def, for (i = 0; i < def->ndisks; i++) { virDomainBackupDiskDefPtr backupdisk = &def->disks[i]; - if (def->incremental && !backupdisk->incremental) + if (backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_DEFAULT) { + if (def->incremental || backupdisk->incremental) { + backupdisk->backupmode = VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL; + } else { + backupdisk->backupmode = VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_FULL; + } + } + + if (!backupdisk->incremental && + backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL) backupdisk->incremental = g_strdup(def->incremental); } diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h index 172eb1cf1c..3f8b592b8d 100644 --- a/src/conf/backup_conf.h +++ b/src/conf/backup_conf.h @@ -45,12 +45,23 @@ typedef enum { VIR_DOMAIN_BACKUP_DISK_STATE_LAST } virDomainBackupDiskState; + +typedef enum { + VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_DEFAULT = 0, + VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_FULL, + VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL, + + VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_LAST +} virDomainBackupDiskBackupMode; + + /* Stores disk-backup information */ typedef struct _virDomainBackupDiskDef virDomainBackupDiskDef; typedef virDomainBackupDiskDef *virDomainBackupDiskDefPtr; struct _virDomainBackupDiskDef { char *name; /* name matching the <target dev='...' of the domain */ virTristateBool backup; /* whether backup is requested */ + virDomainBackupDiskBackupMode backupmode; char *incremental; /* name of the starting point checkpoint of an incremental backup */ char *exportname; /* name of the NBD export for pull mode backup */ char *exportbitmap; /* name of the bitmap exposed in NBD for pull mode backup */ diff --git a/tests/domainbackupxml2xmlin/backup-pull.xml b/tests/domainbackupxml2xmlin/backup-pull.xml index c0bea4771d..c51f0995ac 100644 --- a/tests/domainbackupxml2xmlin/backup-pull.xml +++ b/tests/domainbackupxml2xmlin/backup-pull.xml @@ -6,5 +6,17 @@ <scratch file='/path/to/file'/> </disk> <disk name='hda' backup='no'/> + <disk name='vdc' type='file' backupmode='full'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdd' type='file' backupmode='incremental'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vde' type='file' backupmode='incremental' incremental='blah'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdf' type='file' incremental='bleh'> + <scratch file='/path/to/file'/> + </disk> </disks> </domainbackup> diff --git a/tests/domainbackupxml2xmlout/backup-pull.xml b/tests/domainbackupxml2xmlout/backup-pull.xml index 24fce9c0e7..d2f84cda7a 100644 --- a/tests/domainbackupxml2xmlout/backup-pull.xml +++ b/tests/domainbackupxml2xmlout/backup-pull.xml @@ -6,5 +6,17 @@ <scratch file='/path/to/file'/> </disk> <disk name='hda' backup='no'/> + <disk name='vdc' backup='yes' type='file' backupmode='full'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdd' backup='yes' type='file' backupmode='incremental'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vde' backup='yes' type='file' backupmode='incremental' incremental='blah'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdf' backup='yes' type='file' incremental='bleh'> + <scratch file='/path/to/file'/> + </disk> </disks> </domainbackup> -- 2.26.2

On Thu, Jul 02, 2020 at 14:31:19 -0500, Eric Blake wrote: Nice, I'll use this one. My brain stopped working when doing the schema and I couldn't figure this one out. I'll think about this and possbily post a separate patch. The same also applies to the <incremental> element which also doesn't do validation. Luckily it's not officially supported yet so we can still make it more strict. Sure thing! I actually prefer the RNG solution. No, this check validates that if an explicit incremental mode is requested and neither the per-disk nor per-backup 'incremental' setting is provided we reject such a config because we wouldn't be able to infer which is the point where to backup from. It's an error as noted above. Well the code fills it in in 'virDomainBackupAlignDisks', but that function is not called from the test suite. 'virDomainBackupAlignDisks' requires a domain definition to do some matching around. While I agree that it should be tested, it's not really in scope of this patch.

Data is valid only when queried as guest writes may increase the backup size. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatcheckpoint.rst | 4 ++++ src/libvirt-domain-checkpoint.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/formatcheckpoint.rst b/docs/formatcheckpoint.rst index e45745390a..f159f2a7a3 100644 --- a/docs/formatcheckpoint.rst +++ b/docs/formatcheckpoint.rst @@ -86,6 +86,10 @@ The top-level ``domaincheckpoint`` element may contain the following elements: perform a dynamic query of the estimated size in bytes of the changes made since the checkpoint was created. + Note that updating the backup ``size`` may be expensive and + the actual required size may increase if the guest OS is actively + writing to the disk. + ``creationTime`` A readonly representation of the time this checkpoint was created. The time is specified in seconds since the Epoch, UTC (i.e. Unix time). diff --git a/src/libvirt-domain-checkpoint.c b/src/libvirt-domain-checkpoint.c index 50627c486c..8a7b55dcd2 100644 --- a/src/libvirt-domain-checkpoint.c +++ b/src/libvirt-domain-checkpoint.c @@ -191,7 +191,8 @@ virDomainCheckpointCreateXML(virDomainPtr domain, * VIR_DOMAIN_CHECKPOINT_XML_SIZE, each <disk> listing adds an additional * attribute that shows an estimate of the current size in bytes that * have been dirtied between the time the checkpoint was created and the - * current point in time. + * current point in time. Note that updating the size may be expensive and + * data will be inaccurate once guest OS writes to the disk. * * Returns a 0 terminated UTF-8 encoded XML instance or NULL in case * of error. The caller must free() the returned value. -- 2.26.2

There are few internal fields of the backup XML. Propagate the 'internal' flag so that the test can verify the XML infrastructure. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tests/genericxml2xmltest.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index 74e520522b..cf07f9bb79 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -45,16 +45,27 @@ testCompareXMLToXMLHelper(const void *data) } +struct testCompareBackupXMLData { + const char *testname; + bool internal; +}; + + static int -testCompareBackupXML(const void *data) +testCompareBackupXML(const void *opaque) { - const char *testname = data; + const struct testCompareBackupXMLData *data = opaque; + const char *testname = data->testname; g_autofree char *xml_in = NULL; g_autofree char *file_in = NULL; g_autofree char *file_out = NULL; g_autoptr(virDomainBackupDef) backup = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_autofree char *actual = NULL; + unsigned int parseFlags = 0; + + if (data->internal) + parseFlags |= VIR_DOMAIN_BACKUP_PARSE_INTERNAL; file_in = g_strdup_printf("%s/domainbackupxml2xmlin/%s.xml", abs_srcdir, testname); @@ -64,12 +75,12 @@ testCompareBackupXML(const void *data) if (virFileReadAll(file_in, 1024 * 64, &xml_in) < 0) return -1; - if (!(backup = virDomainBackupDefParseString(xml_in, xmlopt, 0))) { + if (!(backup = virDomainBackupDefParseString(xml_in, xmlopt, parseFlags))) { VIR_TEST_VERBOSE("failed to parse backup def '%s'", file_in); return -1; } - if (virDomainBackupDefFormat(&buf, backup, false) < 0) { + if (virDomainBackupDefFormat(&buf, backup, data->internal) < 0) { VIR_TEST_VERBOSE("failed to format backup def '%s'", file_in); return -1; } @@ -185,9 +196,16 @@ mymain(void) DO_TEST_DIFFERENT("cputune"); +#define DO_TEST_BACKUP_FULL(name, intrnl) \ + do { \ + const struct testCompareBackupXMLData data = { .testname = name, \ + .internal = intrnl }; \ + if (virTestRun("QEMU BACKUP XML-2-XML " name, testCompareBackupXML, &data) < 0) \ + ret = -1; \ + } while (false) + #define DO_TEST_BACKUP(name) \ - if (virTestRun("QEMU BACKUP XML-2-XML " name, testCompareBackupXML, name) < 0) \ - ret = -1; + DO_TEST_BACKUP_FULL(name, false) DO_TEST_BACKUP("empty"); DO_TEST_BACKUP("backup-pull"); -- 2.26.2

Add fields for storing the aliases necessary to clean up the TLS env for a backup job after it finishes. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/backup_conf.c | 42 +++++++++++++++++++ src/conf/backup_conf.h | 5 +++ .../backup-pull-internal-invalid.xml | 36 ++++++++++++++++ .../backup-pull-internal-invalid.xml | 1 + tests/genericxml2xmltest.c | 2 + 5 files changed, 86 insertions(+) create mode 100644 tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml create mode 120000 tests/domainbackupxml2xmlout/backup-pull-internal-invalid.xml diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 4f28073ab2..74f6e4b020 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -86,6 +86,10 @@ virDomainBackupDefFree(virDomainBackupDefPtr def) } g_free(def->disks); + + g_free(def->tlsAlias); + g_free(def->tlsSecretAlias); + g_free(def); } @@ -213,6 +217,19 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node, } +static void +virDomainBackupDefParsePrivate(virDomainBackupDefPtr def, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + if (!(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)) + return; + + def->tlsSecretAlias = virXPathString("string(./privateData/objects/secret[@type='tlskey']/@alias)", ctxt); + def->tlsAlias = virXPathString("string(./privateData/objects/TLSx509/@alias)", ctxt); +} + + static virDomainBackupDefPtr virDomainBackupDefParse(xmlXPathContextPtr ctxt, virDomainXMLOptionPtr xmlopt, @@ -282,6 +299,8 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt, return NULL; } + virDomainBackupDefParsePrivate(def, ctxt, flags); + return g_steal_pointer(&def); } @@ -388,6 +407,26 @@ virDomainBackupDiskDefFormat(virBufferPtr buf, } +static void +virDomainBackupDefFormatPrivate(virBufferPtr buf, + virDomainBackupDefPtr def, + bool internal) +{ + g_auto(virBuffer) privChildBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) objectsChildBuf = VIR_BUFFER_INIT_CHILD(&privChildBuf); + + if (!internal) + return; + + virBufferEscapeString(&objectsChildBuf, "<secret type='tlskey' alias='%s'/>\n", + def->tlsSecretAlias); + virBufferEscapeString(&objectsChildBuf, "<TLSx509 alias='%s'/>\n", def->tlsAlias); + + virXMLFormatElement(&privChildBuf, "objects", NULL, &objectsChildBuf); + virXMLFormatElement(buf, "privateData", NULL, &privChildBuf); +} + + int virDomainBackupDefFormat(virBufferPtr buf, virDomainBackupDefPtr def, @@ -422,6 +461,9 @@ virDomainBackupDefFormat(virBufferPtr buf, } virXMLFormatElement(&childBuf, "disks", NULL, &disksChildBuf); + + virDomainBackupDefFormatPrivate(&childBuf, def, internal); + virXMLFormatElement(buf, "domainbackup", &attrBuf, &childBuf); return 0; diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h index 3f8b592b8d..a1d1e453c1 100644 --- a/src/conf/backup_conf.h +++ b/src/conf/backup_conf.h @@ -86,6 +86,11 @@ struct _virDomainBackupDef { virDomainBackupDiskDef *disks; /* internal data */ + + /* NBD TLS internals */ + char *tlsAlias; + char *tlsSecretAlias; + /* statistic totals for completed disks */ unsigned long long push_transferred; unsigned long long push_total; diff --git a/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml b/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml new file mode 100644 index 0000000000..261dec0eea --- /dev/null +++ b/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml @@ -0,0 +1,36 @@ +<domainbackup mode='pull'> + <incremental>1525889631</incremental> + <server transport='tcp' name='localhost' port='10809'/> + <disks> + <disk name='vda' backup='yes' state='running' type='file' exportname='test-vda' exportbitmap='blah'> + <driver type='qcow2'/> + <scratch file='/path/to/file'> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + </scratch> + </disk> + <disk name='vdb' backup='yes' state='complete' type='file' exportname='test-vda' exportbitmap='blah'> + <driver type='qcow2'/> + <scratch file='/path/to/file'> + <encryption format='luks'> + <secret type='passphrase' usage='/storage/backup/vdb'/> + </encryption> + </scratch> + </disk> + <disk name='vdc' backup='yes' state='running' type='block'> + <driver type='qcow2'/> + <scratch dev='/dev/block'> + <encryption format='luks'> + <secret type='passphrase' usage='/storage/backup/vdc'/> + </encryption> + </scratch> + </disk> + </disks> + <privateData> + <objects> + <secret type='tlskey' alias='test-tlskey'/> + <TLSx509 alias='test-tlsobj'/> + </objects> + </privateData> +</domainbackup> diff --git a/tests/domainbackupxml2xmlout/backup-pull-internal-invalid.xml b/tests/domainbackupxml2xmlout/backup-pull-internal-invalid.xml new file mode 120000 index 0000000000..055ca37a0b --- /dev/null +++ b/tests/domainbackupxml2xmlout/backup-pull-internal-invalid.xml @@ -0,0 +1 @@ +../domainbackupxml2xmlin/backup-pull-internal-invalid.xml \ No newline at end of file diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index cf07f9bb79..2c1e8616dd 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -215,6 +215,8 @@ mymain(void) DO_TEST_BACKUP("backup-push-seclabel"); DO_TEST_BACKUP("backup-push-encrypted"); + DO_TEST_BACKUP_FULL("backup-pull-internal-invalid", true); + virObjectUnref(caps); virObjectUnref(xmlopt); -- 2.26.2

TLS is required to transport backed-up data securely when using pull-mode backups. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/formatbackup.rst | 4 ++++ src/qemu/libvirtd_qemu.aug | 5 ++++ src/qemu/qemu.conf | 37 ++++++++++++++++++++++++++++++ src/qemu/qemu_conf.c | 17 ++++++++++++++ src/qemu/qemu_conf.h | 5 ++++ src/qemu/test_libvirtd_qemu.aug.in | 3 +++ 6 files changed, 71 insertions(+) diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst index e5b6fc6eb0..142b8250d2 100644 --- a/docs/formatbackup.rst +++ b/docs/formatbackup.rst @@ -42,6 +42,10 @@ were supplied). The following child elements and attributes are supported: necessary to set up an NBD server that exposes the content of each disk at the time the backup is started. + Note that for the QEMU hypervisor the TLS environment in controlled using + ``backup_tls_x509_cert_dir``, ``backup_tls_x509_verify``, and + ``backup_tls_x509_secret_uuid`` properties in ``/etc/libvirt/qemu.conf``. + ``disks`` An optional listing of instructions for disks participating in the backup (if omitted, all disks participate and libvirt attempts to generate filenames by diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index c19a086c38..abbac549f2 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -59,6 +59,10 @@ module Libvirtd_qemu = | bool_entry "migrate_tls_x509_verify" | str_entry "migrate_tls_x509_secret_uuid" + let backup_entry = str_entry "backup_tls_x509_cert_dir" + | bool_entry "backup_tls_x509_verify" + | str_entry "backup_tls_x509_secret_uuid" + let vxhs_entry = bool_entry "vxhs_tls" | str_entry "vxhs_tls_x509_cert_dir" | str_entry "vxhs_tls_x509_secret_uuid" @@ -146,6 +150,7 @@ module Libvirtd_qemu = | spice_entry | chardev_entry | migrate_entry + | backup_entry | nogfx_entry | remote_display_entry | security_entry diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index ab403c21ac..a96bedb114 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -395,6 +395,43 @@ #migrate_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" +# In order to override the default TLS certificate location for backup NBD +# server certificates, supply a valid path to the certificate directory. If the +# provided path does not exist, libvirtd will fail to start. If the path is +# not provided, but TLS-encrypted backup is requested, then the +# default_tls_x509_cert_dir path will be used. +# +#backup_tls_x509_cert_dir = "/etc/pki/libvirt-backup" + + +# The default TLS configuration only uses certificates for the server +# allowing the client to verify the server's identity and establish +# an encrypted channel. +# +# It is possible to use x509 certificates for authentication too, by +# issuing an x509 certificate to every client who needs to connect. +# +# Enabling this option will reject any client that does not have a +# ca-cert.pem certificate signed by the CA in the backup_tls_x509_cert_dir +# (or default_tls_x509_cert_dir) as well as the corresponding client-*.pem +# files described in default_tls_x509_cert_dir. +# +# If this option is not supplied, it will be set to the value of +# "default_tls_x509_verify". +# +#backup_tls_x509_verify = 1 + + +# Uncomment and use the following option to override the default secret +# UUID provided in the default_tls_x509_secret_uuid parameter. +# +# NB This default all-zeros UUID will not work. Replace it with the +# output from the UUID for the TLS secret from a 'virsh secret-list' +# command and then uncomment the entry +# +#backup_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" + + # By default, if no graphical front end is configured, libvirt will disable # QEMU audio output since directly talking to alsa/pulseaudio may not work # with various security settings. If you know what you're doing, enable diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 6e673e8f62..30d7c61cf9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -347,6 +347,9 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->migrateTLSx509certdir); VIR_FREE(cfg->migrateTLSx509secretUUID); + VIR_FREE(cfg->backupTLSx509certdir); + VIR_FREE(cfg->backupTLSx509secretUUID); + while (cfg->nhugetlbfs) { cfg->nhugetlbfs--; VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir); @@ -511,6 +514,9 @@ virQEMUDriverConfigLoadSpecificTLSEntry(virQEMUDriverConfigPtr cfg, GET_CONFIG_TLS_CERTINFO_COMMON(migrate); GET_CONFIG_TLS_CERTINFO_SERVER(migrate); + GET_CONFIG_TLS_CERTINFO_COMMON(backup); + GET_CONFIG_TLS_CERTINFO_SERVER(backup); + GET_CONFIG_TLS_CERTINFO_COMMON(vxhs); GET_CONFIG_TLS_CERTINFO_COMMON(nbd); @@ -1154,6 +1160,14 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg) return -1; } + if (cfg->backupTLSx509certdir && + !virFileExists(cfg->backupTLSx509certdir)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("backup_tls_x509_cert_dir directory '%s' does not exist"), + cfg->backupTLSx509certdir); + return -1; + } + if (cfg->vxhsTLSx509certdir && !virFileExists(cfg->vxhsTLSx509certdir)) { virReportError(VIR_ERR_CONF_SYNTAX, @@ -1189,6 +1203,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) SET_TLS_SECRET_UUID_DEFAULT(vnc); SET_TLS_SECRET_UUID_DEFAULT(chardev); SET_TLS_SECRET_UUID_DEFAULT(migrate); + SET_TLS_SECRET_UUID_DEFAULT(backup); SET_TLS_SECRET_UUID_DEFAULT(vxhs); SET_TLS_SECRET_UUID_DEFAULT(nbd); @@ -1216,6 +1231,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) SET_TLS_X509_CERT_DEFAULT(spice); SET_TLS_X509_CERT_DEFAULT(chardev); SET_TLS_X509_CERT_DEFAULT(migrate); + SET_TLS_X509_CERT_DEFAULT(backup); SET_TLS_X509_CERT_DEFAULT(vxhs); SET_TLS_X509_CERT_DEFAULT(nbd); @@ -1230,6 +1246,7 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfigPtr cfg) SET_TLS_VERIFY_DEFAULT(vnc); SET_TLS_VERIFY_DEFAULT(chardev); SET_TLS_VERIFY_DEFAULT(migrate); + SET_TLS_VERIFY_DEFAULT(backup); #undef SET_TLS_VERIFY_DEFAULT diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 6193a7111c..687829123c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -144,6 +144,11 @@ struct _virQEMUDriverConfig { bool migrateTLSx509verifyPresent; char *migrateTLSx509secretUUID; + char *backupTLSx509certdir; + bool backupTLSx509verify; + bool backupTLSx509verifyPresent; + char *backupTLSx509secretUUID; + bool vxhsTLS; char *vxhsTLSx509certdir; char *vxhsTLSx509secretUUID; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index db125bf352..6a54e2322a 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -35,6 +35,9 @@ module Test_libvirtd_qemu = { "migrate_tls_x509_cert_dir" = "/etc/pki/libvirt-migrate" } { "migrate_tls_x509_verify" = "1" } { "migrate_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } +{ "backup_tls_x509_cert_dir" = "/etc/pki/libvirt-backup" } +{ "backup_tls_x509_verify" = "1" } +{ "backup_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } { "nographics_allow_host_audio" = "1" } { "remote_display_port_min" = "5900" } { "remote_display_port_max" = "65535" } -- 2.26.2

Allow enabling TLS for the NBD server used to do pull-mode backups. Note that documentation already mentions 'tls', so this just implements the schema and XML bits. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/schemas/domainbackup.rng | 9 ++++++++- src/conf/backup_conf.c | 17 +++++++++++++++++ src/conf/backup_conf.h | 1 + .../backup-pull-encrypted.xml | 2 +- .../backup-pull-internal-invalid.xml | 2 +- .../backup-pull-encrypted.xml | 2 +- 6 files changed, 29 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng index 650f5cd4c3..c0ca3c3038 100644 --- a/docs/schemas/domainbackup.rng +++ b/docs/schemas/domainbackup.rng @@ -51,6 +51,14 @@ </attribute> <interleave> <element name='server'> + <optional> + <attribute name='tls'> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> <choice> <group> <optional> @@ -69,7 +77,6 @@ <ref name='unsignedInt'/> </attribute> </optional> - <!-- add tls? --> </group> <group> <attribute name='transport'> diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 74f6e4b020..59d7e1dfaf 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -260,6 +260,8 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt, def->incremental = virXPathString("string(./incremental)", ctxt); if ((node = virXPathNode("./server", ctxt))) { + g_autofree char *tls = NULL; + if (def->type != VIR_DOMAIN_BACKUP_TYPE_PULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("use of <server> requires pull mode backup")); @@ -284,6 +286,19 @@ virDomainBackupDefParse(xmlXPathContextPtr ctxt, def->server->socket); return NULL; } + + if ((tls = virXMLPropString(node, "tls"))) { + int tmp; + + if ((tmp = virTristateBoolTypeFromString(tls)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown value '%s' of 'tls' attribute"),\ + tls); + return NULL; + } + + def->tls = tmp; + } } if ((n = virXPathNodeSet("./disks/*", ctxt, &nodes)) < 0) @@ -445,6 +460,8 @@ virDomainBackupDefFormat(virBufferPtr buf, if (def->server) { virBufferAsprintf(&serverAttrBuf, " transport='%s'", virStorageNetHostTransportTypeToString(def->server->transport)); + if (def->tls != VIR_TRISTATE_BOOL_ABSENT) + virBufferAsprintf(&serverAttrBuf, " tls='%s'", virTristateBoolTypeToString(def->tls)); virBufferEscapeString(&serverAttrBuf, " name='%s'", def->server->name); if (def->server->port) virBufferAsprintf(&serverAttrBuf, " port='%u'", def->server->port); diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h index a1d1e453c1..bda2bdcfe4 100644 --- a/src/conf/backup_conf.h +++ b/src/conf/backup_conf.h @@ -81,6 +81,7 @@ struct _virDomainBackupDef { int type; /* virDomainBackupType */ char *incremental; virStorageNetHostDefPtr server; /* only when type == PULL */ + virTristateBool tls; /* use TLS for NBD */ size_t ndisks; /* should not exceed dom->ndisks */ virDomainBackupDiskDef *disks; diff --git a/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml index 1469189a37..48232aa0fe 100644 --- a/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml +++ b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml @@ -1,6 +1,6 @@ <domainbackup mode="pull"> <incremental>1525889631</incremental> - <server transport='tcp' name='localhost' port='10809'/> + <server transport='tcp' tls='yes' name='localhost' port='10809'/> <disks> <disk name='vda' type='file' exportname='test-vda' exportbitmap='blah'> <driver type='qcow2'/> diff --git a/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml b/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml index 261dec0eea..ba8f7ca3ab 100644 --- a/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml +++ b/tests/domainbackupxml2xmlin/backup-pull-internal-invalid.xml @@ -1,6 +1,6 @@ <domainbackup mode='pull'> <incremental>1525889631</incremental> - <server transport='tcp' name='localhost' port='10809'/> + <server transport='tcp' tls='yes' name='localhost' port='10809'/> <disks> <disk name='vda' backup='yes' state='running' type='file' exportname='test-vda' exportbitmap='blah'> <driver type='qcow2'/> diff --git a/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml b/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml index 81519bfcb5..ea9dcf72b9 100644 --- a/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml +++ b/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml @@ -1,6 +1,6 @@ <domainbackup mode='pull'> <incremental>1525889631</incremental> - <server transport='tcp' name='localhost' port='10809'/> + <server transport='tcp' tls='yes' name='localhost' port='10809'/> <disks> <disk name='vda' backup='yes' type='file' exportname='test-vda' exportbitmap='blah'> <driver type='qcow2'/> -- 2.26.2

On Thu, Jul 02, 2020 at 14:53:28 -0500, Eric Blake wrote: Note that patch 22 actually adds the following paragraph to formatbackup.rst into the NBD section: + Note that for the QEMU hypervisor the TLS environment in controlled using + ``backup_tls_x509_cert_dir``, ``backup_tls_x509_verify``, and + ``backup_tls_x509_secret_uuid`` properties in ``/etc/libvirt/qemu.conf``.