12:42 p.m.
v1: http://www.redhat.com/archives/libvir-list/2017-February/msg00897.html
v1 cover letter reiterated:
Patches 1, 3 -> 9 are primarily quite a bit of code motion in order to allow
reuse of the "core" of the chardev TLS code.
Theoretically speaking of course, these patches should work - I don't
have a TLS and migration environment to test with, so between following
the qemu command model on Daniel's blog and prior experience with the
chardev TLS would
I added the saving of a flag to the private qemu domain state, although
I'm not 100% sure it was necessary. At one time I created the source TLS
objects during the Begin phase, but later decided to wait until just
before the migration is run. I think the main reason to have the flag
would be a restart of libvirtd to let 'something' know migration using
TLS was configured. I think it may only be "necessary" in order to
repopulate the migSecinfo after libvirtd restart, but it's not entirely
clear. By the time I started thinking more about while writing this cover
letter it was too late to just remove.
Also rather than create the destination host TLS objects on the fly,
I modified the command line generation. That model could change to adding
the TLS objects once the destination is started and before the params are
set for the migration.
This 'model' is also going to be used for the NBD, but I figured I'd get
this posted now since it was already too long of a series.
v2: Changes
Reorder the patches to put the reused 'chardev' code up front. Most of
these patches were "ok" along the way, but only one was officially ACK'd
(and that was pushed).
Patch1 is new - based off code review comment to create a common New
function for secinfo allocation
Patch2 is adjusted to use Patch1
Patch3 is new based on review comment and having ExitMonitor outside
the virSaveLastError ... virSetError
Patch4 mainly follows older logic with adjustments as suggested during
code review
Patch5 -> Patch8 had minor changes as a result of other suggestions
Patch9 just removed the _set logic
Patch10 fixed the order/placement of VIR_MIGRATE_TLS
Patch11 is the old patch1 w/ the fixed #undef
Patch12 is the old patch2 w/o changes
Patch13 Alters the server logic to create the objects on the fly rather
that via command line. It also introduces 3 helpers to perform the
migration TLS manipulation
Patch14 similarly uses those API's
AFAIU - removal of the objects would remove the migration tls-creds,
tls-hostname settings.
NB:
I left the cfg->migrateTLS in for now - it's very simple to remove, but
there would still need to be a key on something to ensure the migrateTLS
environment has been properly configured since that would mean the default
environment would need to be used/configured. Setting up the default
environment is keyed off having the migrateTLS defined. That's all part
of the qemu_conf reading logic.
John Ferlan (14):
qemu: Introduce qemuDomainSecretInfoNew
qemu: Introduce qemuDomainSecretMigratePrepare
qemu: Move exit monitor calls in failure paths
qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
qemu: Refactor qemuDomainGetChardevTLSObjects to converge code
qemu: Move qemuDomainSecretChardevPrepare call
qemu: Move qemuDomainPrepareChardevSourceTLS call
qemu: Introduce qemuDomainGetTLSObjects
qemu: Add TLS params to _qemuMonitorMigrationParams
Add new migration flag VIR_MIGRATE_TLS
qemu: Create #define for TLS configuration setup.
conf: Introduce migrate_tls_x509_cert_dir
qemu: Set up the migrate TLS objects for target
qemu: Set up the migration TLS objects for source
include/libvirt/libvirt-domain.h | 8 +
src/qemu/libvirtd_qemu.aug | 6 +
src/qemu/qemu.conf | 39 +++++
src/qemu/qemu_conf.c | 45 +++--
src/qemu/qemu_conf.h | 5 +
src/qemu/qemu_domain.c | 195 +++++++++++++--------
src/qemu/qemu_domain.h | 89 ++++++----
src/qemu/qemu_hotplug.c | 343 ++++++++++++++++++++-----------------
src/qemu/qemu_hotplug.h | 24 +++
src/qemu/qemu_migration.c | 200 +++++++++++++++++++++
src/qemu/qemu_migration.h | 3 +-
src/qemu/qemu_monitor.c | 11 +-
src/qemu/qemu_monitor.h | 3 +
src/qemu/qemu_monitor_json.c | 10 ++
src/qemu/test_libvirtd_qemu.aug.in | 4 +
tools/virsh-domain.c | 7 +
16 files changed, 705 insertions(+), 287 deletions(-)
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 01/14] qemu: Introduce qemuDomainSecretInfoNew
Create a helper which will create the secinfo used for disks, hostdevs,
and chardevs.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 140 ++++++++++++++++++++++++++-----------------------
1 file changed, 74 insertions(+), 66 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c187214..b7594b3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1112,6 +1112,55 @@ qemuDomainSecretSetup(virConnectPtr conn,
}
+/* qemuDomainSecretInfoNew:
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @srcAlias: Alias base to use for TLS object
+ * @lookupType: Type of secret lookup
+ * @username: username for plain secrets
+ * @looupdef: lookup def describing secret
+ * @isLuks: boolean for luks lookup
+ * @encFmt: string for error message
+ *
+ * Helper function to create a secinfo to be used for secinfo consumers
+ *
+ * Returns @secinfo on success, NULL on failure. Caller is responsible
+ * to eventually free @secinfo.
+ */
+static qemuDomainSecretInfoPtr
+qemuDomainSecretInfoNew(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ const char *srcAlias,
+ virSecretLookupType lookupType,
+ const char *username,
+ virSecretLookupTypeDefPtr lookupDef,
+ bool isLuks,
+ const char *encFmt)
+{
+ qemuDomainSecretInfoPtr secinfo = NULL;
+
+ if (VIR_ALLOC(secinfo) < 0)
+ return NULL;
+
+ if (qemuDomainSecretSetup(conn, priv, secinfo, srcAlias, lookupType,
+ username, lookupDef, isLuks) < 0)
+ goto error;
+
+ if (encFmt && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s requires encrypted secrets to be supported"),
+ encFmt);
+ goto error;
+ }
+
+ return secinfo;
+
+ error:
+ qemuDomainSecretInfoFree(&secinfo);
+ return NULL;
+}
+
+
/* qemuDomainSecretDiskDestroy:
* @disk: Pointer to a disk definition
*
@@ -1171,51 +1220,30 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
{
virStorageSourcePtr src = disk->src;
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
- qemuDomainSecretInfoPtr secinfo = NULL;
if (qemuDomainSecretDiskCapable(src)) {
virSecretUsageType secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI;
- if (VIR_ALLOC(secinfo) < 0)
- return -1;
-
if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)
secretUsageType = VIR_SECRET_USAGE_TYPE_CEPH;
- if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
- secretUsageType, src->auth->username,
- &src->auth->seclookupdef, false) < 0)
- goto error;
-
- diskPriv->secinfo = secinfo;
+ if (!(diskPriv->secinfo =
+ qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
+ secretUsageType, src->auth->username,
+ &src->auth->seclookupdef, false, NULL)))
+ return -1;
}
if (qemuDomainDiskHasEncryptionSecret(src)) {
-
- if (VIR_ALLOC(secinfo) < 0)
- return -1;
-
- if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
- VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
-
&src->encryption->secrets[0]->seclookupdef,
- true) < 0)
- goto error;
-
- if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("luks encryption requires encrypted secrets "
- "to be supported"));
- goto error;
- }
-
- diskPriv->encinfo = secinfo;
+ if (!(diskPriv->encinfo =
+ qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
+ VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
+
&src->encryption->secrets[0]->seclookupdef,
+ true, "luks encryption")))
+ return -1;
}
return 0;
-
- error:
- qemuDomainSecretInfoFree(&secinfo);
- return -1;
}
@@ -1251,8 +1279,6 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
qemuDomainObjPrivatePtr priv,
virDomainHostdevDefPtr hostdev)
{
- qemuDomainSecretInfoPtr secinfo = NULL;
-
if (virHostdevIsSCSIDevice(hostdev)) {
virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
@@ -1263,24 +1289,17 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
qemuDomainHostdevPrivatePtr hostdevPriv =
QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev);
- if (VIR_ALLOC(secinfo) < 0)
+ if (!(hostdevPriv->secinfo =
+ qemuDomainSecretInfoNew(conn, priv, hostdev->info->alias,
+ VIR_SECRET_USAGE_TYPE_ISCSI,
+ iscsisrc->auth->username,
+ &iscsisrc->auth->seclookupdef,
+ false, NULL)))
return -1;
-
- if (qemuDomainSecretSetup(conn, priv, secinfo, hostdev->info->alias,
- VIR_SECRET_USAGE_TYPE_ISCSI,
- iscsisrc->auth->username,
- &iscsisrc->auth->seclookupdef, false)
< 0)
- goto error;
-
- hostdevPriv->secinfo = secinfo;
}
}
return 0;
-
- error:
- qemuDomainSecretInfoFree(&secinfo);
- return -1;
}
@@ -1322,7 +1341,6 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
virDomainChrSourceDefPtr dev)
{
virSecretLookupTypeDef seclookupdef = {0};
- qemuDomainSecretInfoPtr secinfo = NULL;
char *charAlias = NULL;
if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP)
@@ -1337,36 +1355,26 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
seclookupdef.u.uuid) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("malformed chardev TLS secret uuid in
qemu.conf"));
- goto error;
+ return -1;
}
seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
- if (VIR_ALLOC(secinfo) < 0)
- goto error;
-
if (!(charAlias = qemuAliasChardevFromDevAlias(chrAlias)))
- goto error;
-
- if (qemuDomainSecretSetup(conn, priv, secinfo, charAlias,
- VIR_SECRET_USAGE_TYPE_TLS, NULL,
- &seclookupdef, false) < 0)
- goto error;
+ return -1;
- if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("TLS X.509 requires encrypted secrets "
- "to be supported"));
+ if (!(chrSourcePriv->secinfo =
+ qemuDomainSecretInfoNew(conn, priv, charAlias,
+ VIR_SECRET_USAGE_TYPE_TLS, NULL,
+ &seclookupdef, false, "TLS X.509")))
goto error;
- }
- chrSourcePriv->secinfo = secinfo;
+ VIR_FREE(charAlias);
}
- VIR_FREE(charAlias);
return 0;
error:
- qemuDomainSecretInfoFree(&secinfo);
+ VIR_FREE(charAlias);
return -1;
}
--
2.9.3
8 a.m.
New subject: [libvirt] [PATCH v2 01/14] qemu: Introduce qemuDomainSecretInfoNew
On Thu, Feb 23, 2017 at 13:42:03 -0500, John Ferlan wrote:
Create a helper which will create the secinfo used for disks,
hostdevs,
and chardevs.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 140 ++++++++++++++++++++++++++-----------------------
1 file changed, 74 insertions(+), 66 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c187214..b7594b3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1112,6 +1112,55 @@ qemuDomainSecretSetup(virConnectPtr conn,
}
+/* qemuDomainSecretInfoNew:
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @srcAlias: Alias base to use for TLS object
+ * @lookupType: Type of secret lookup
+ * @username: username for plain secrets
+ * @looupdef: lookup def describing secret
+ * @isLuks: boolean for luks lookup
+ * @encFmt: string for error message
+ *
+ * Helper function to create a secinfo to be used for secinfo consumers
+ *
+ * Returns @secinfo on success, NULL on failure. Caller is responsible
+ * to eventually free @secinfo.
+ */
+static qemuDomainSecretInfoPtr
+qemuDomainSecretInfoNew(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ const char *srcAlias,
+ virSecretLookupType lookupType,
This parameter should rather be
virSecretUsageType usageType
+ const char *username,
+ virSecretLookupTypeDefPtr lookupDef,
+ bool isLuks,
+ const char *encFmt)
+{
+ qemuDomainSecretInfoPtr secinfo = NULL;
+
+ if (VIR_ALLOC(secinfo) < 0)
+ return NULL;
+
+ if (qemuDomainSecretSetup(conn, priv, secinfo, srcAlias, lookupType,
+ username, lookupDef, isLuks) < 0)
+ goto error;
+
+ if (encFmt && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s requires encrypted secrets to be supported"),
+ encFmt);
I didn't really get the "encFmt" name, but it's just a minor issue
compared to the way the error message is composed here. This results in
an untranslatable string. I think returning a generic error about
unsupported encrypted secrets would be good enough.
Jirka
3:13 p.m.
New subject: [libvirt] [PATCH v2 01/14] qemu: Introduce qemuDomainSecretInfoNew
On 02/24/2017 09:00 AM, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:03 -0500, John Ferlan wrote:
> Create a helper which will create the secinfo used for disks, hostdevs,
> and chardevs.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_domain.c | 140 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 74 insertions(+), 66 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c187214..b7594b3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1112,6 +1112,55 @@ qemuDomainSecretSetup(virConnectPtr conn,
> }
>
>
> +/* qemuDomainSecretInfoNew:
> + * @conn: Pointer to connection
> + * @priv: pointer to domain private object
> + * @srcAlias: Alias base to use for TLS object
> + * @lookupType: Type of secret lookup
> + * @username: username for plain secrets
> + * @looupdef: lookup def describing secret
> + * @isLuks: boolean for luks lookup
> + * @encFmt: string for error message
> + *
> + * Helper function to create a secinfo to be used for secinfo consumers
> + *
> + * Returns @secinfo on success, NULL on failure. Caller is responsible
> + * to eventually free @secinfo.
> + */
> +static qemuDomainSecretInfoPtr
> +qemuDomainSecretInfoNew(virConnectPtr conn,
> + qemuDomainObjPrivatePtr priv,
> + const char *srcAlias,
> + virSecretLookupType lookupType,
This parameter should rather be
Weird I wonder what I was cut-n-paste'ing.
virSecretUsageType usageType
> + const char *username,
> + virSecretLookupTypeDefPtr lookupDef,
> + bool isLuks,
> + const char *encFmt)
> +{
> + qemuDomainSecretInfoPtr secinfo = NULL;
> +
> + if (VIR_ALLOC(secinfo) < 0)
> + return NULL;
> +
> + if (qemuDomainSecretSetup(conn, priv, secinfo, srcAlias, lookupType,
> + username, lookupDef, isLuks) < 0)
> + goto error;
> +
> + if (encFmt && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("%s requires encrypted secrets to be
supported"),
> + encFmt);
I didn't really get the "encFmt" name, but it's just a minor issue
compared to the way the error message is composed here. This results in
an untranslatable string. I think returning a generic error about
unsupported encrypted secrets would be good enough.
Jirka
I know this kind of thing done elsewhere, but I can remove it. The hope
was to have some sort of message to help indicate which failed, but it's
not that important.
John
12:42 p.m.
New subject: [libvirt] [PATCH v2 02/14] qemu: Introduce qemuDomainSecretMigratePrepare
Introduce API to Prepare a qemuDomainSecretInfoPtr to be
used with a migrate or nbd TLS object
Also alter the error message in ChardevPrepare when UUIDParse fails
to be consistent with the message for MigratePrepare
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 48 ++++++++++++++++++++++++++--
src/qemu/qemu_domain.h | 85 ++++++++++++++++++++++++++++----------------------
2 files changed, 94 insertions(+), 39 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b7594b3..40c9dab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1353,8 +1353,9 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
if (virUUIDParse(cfg->chardevTLSx509secretUUID,
seclookupdef.u.uuid) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("malformed chardev TLS secret uuid in
qemu.conf"));
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("malformed TLS secret uuid '%s' in
qemu.conf"),
+ cfg->chardevTLSx509secretUUID);
return -1;
}
seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
@@ -1379,6 +1380,47 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
}
+/* qemuDomainSecretMigratePrepare
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @srcAlias: Alias to use (either migrate or nbd)
+ * @secretUUID: UUID for the secret from the cfg (migrate or nbd)
+ *
+ * Create and prepare the qemuDomainSecretInfoPtr to be used for either
+ * a migration or nbd. Unlike other domain secret prepare functions, this
+ * is only expected to be called for a single object/instance. Theoretically
+ * the object could be reused, although that results in keeping a secret
+ * stored in memory for perhaps longer than expected or necessary.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretMigratePrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ const char *srcAlias,
+ const char *secretUUID)
+{
+ virSecretLookupTypeDef seclookupdef = {0};
+
+ if (virUUIDParse(secretUUID, seclookupdef.u.uuid) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("malformed TLS secret uuid '%s' in
qemu.conf"),
+ secretUUID);
+ return -1;
+ }
+ seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
+
+ if (!(priv->migSecinfo =
+ qemuDomainSecretInfoNew(conn, priv, srcAlias,
+ VIR_SECRET_USAGE_TYPE_TLS, NULL,
+ &seclookupdef, false, "TLS X.509")))
+ return -1;
+
+ return 0;
+}
+
+
+
/* qemuDomainSecretDestroy:
* @vm: Domain object
*
@@ -1643,6 +1685,8 @@ qemuDomainObjPrivateFree(void *data)
VIR_FREE(priv->libDir);
VIR_FREE(priv->channelTargetDir);
+
+ qemuDomainSecretInfoFree(&priv->migSecinfo);
qemuDomainMasterKeyFree(priv);
VIR_FREE(priv);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 72efa33..85f7eb6 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -175,6 +175,43 @@ VIR_ENUM_DECL(qemuDomainNamespace)
bool qemuDomainNamespaceEnabled(virDomainObjPtr vm,
qemuDomainNamespace ns);
+/* Type of domain secret */
+typedef enum {
+ VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN = 0,
+ VIR_DOMAIN_SECRET_INFO_TYPE_AES, /* utilize GNUTLS_CIPHER_AES_256_CBC */
+
+ VIR_DOMAIN_SECRET_INFO_TYPE_LAST
+} qemuDomainSecretInfoType;
+
+typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain;
+typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr;
+struct _qemuDomainSecretPlain {
+ char *username;
+ uint8_t *secret;
+ size_t secretlen;
+};
+
+# define QEMU_DOMAIN_AES_IV_LEN 16 /* 16 bytes for 128 bit random */
+ /* initialization vector */
+typedef struct _qemuDomainSecretAES qemuDomainSecretAES;
+typedef struct _qemuDomainSecretAES *qemuDomainSecretAESPtr;
+struct _qemuDomainSecretAES {
+ char *username;
+ char *alias; /* generated alias for secret */
+ char *iv; /* base64 encoded initialization vector */
+ char *ciphertext; /* encoded/encrypted secret */
+};
+
+typedef struct _qemuDomainSecretInfo qemuDomainSecretInfo;
+typedef qemuDomainSecretInfo *qemuDomainSecretInfoPtr;
+struct _qemuDomainSecretInfo {
+ qemuDomainSecretInfoType type;
+ union {
+ qemuDomainSecretPlain plain;
+ qemuDomainSecretAES aes;
+ } s;
+};
+
typedef struct _qemuDomainObjPrivate qemuDomainObjPrivate;
typedef qemuDomainObjPrivate *qemuDomainObjPrivatePtr;
struct _qemuDomainObjPrivate {
@@ -246,48 +283,15 @@ struct _qemuDomainObjPrivate {
/* note whether memory device alias does not correspond to slot number */
bool memAliasOrderMismatch;
+
+ /* for migration's using TLS with a secret (not to be saved in our */
+ /* private XML). */
+ qemuDomainSecretInfoPtr migSecinfo;
};
# define QEMU_DOMAIN_PRIVATE(vm) \
((qemuDomainObjPrivatePtr) (vm)->privateData)
-/* Type of domain secret */
-typedef enum {
- VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN = 0,
- VIR_DOMAIN_SECRET_INFO_TYPE_AES, /* utilize GNUTLS_CIPHER_AES_256_CBC */
-
- VIR_DOMAIN_SECRET_INFO_TYPE_LAST
-} qemuDomainSecretInfoType;
-
-typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain;
-typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr;
-struct _qemuDomainSecretPlain {
- char *username;
- uint8_t *secret;
- size_t secretlen;
-};
-
-# define QEMU_DOMAIN_AES_IV_LEN 16 /* 16 bytes for 128 bit random */
- /* initialization vector */
-typedef struct _qemuDomainSecretAES qemuDomainSecretAES;
-typedef struct _qemuDomainSecretAES *qemuDomainSecretAESPtr;
-struct _qemuDomainSecretAES {
- char *username;
- char *alias; /* generated alias for secret */
- char *iv; /* base64 encoded initialization vector */
- char *ciphertext; /* encoded/encrypted secret */
-};
-
-typedef struct _qemuDomainSecretInfo qemuDomainSecretInfo;
-typedef qemuDomainSecretInfo *qemuDomainSecretInfoPtr;
-struct _qemuDomainSecretInfo {
- qemuDomainSecretInfoType type;
- union {
- qemuDomainSecretPlain plain;
- qemuDomainSecretAES aes;
- } s;
-};
-
# define QEMU_DOMAIN_DISK_PRIVATE(disk) \
((qemuDomainDiskPrivatePtr) (disk)->privateData)
@@ -763,6 +767,13 @@ int qemuDomainSecretChardevPrepare(virConnectPtr conn,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
+int qemuDomainSecretMigratePrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ const char *srcAlias,
+ const char *secretUUID)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4);
+
void qemuDomainSecretDestroy(virDomainObjPtr vm)
ATTRIBUTE_NONNULL(1);
--
2.9.3
11:08 a.m.
New subject: [libvirt] [PATCH v2 02/14] qemu: Introduce qemuDomainSecretMigratePrepare
On Thu, Feb 23, 2017 at 13:42:04 -0500, John Ferlan wrote:
Introduce API to Prepare a qemuDomainSecretInfoPtr to be
used with a migrate or nbd TLS object
Also alter the error message in ChardevPrepare when UUIDParse fails
to be consistent with the message for MigratePrepare
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 48 ++++++++++++++++++++++++++--
src/qemu/qemu_domain.h | 85 ++++++++++++++++++++++++++++----------------------
2 files changed, 94 insertions(+), 39 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b7594b3..40c9dab 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1353,8 +1353,9 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
if (virUUIDParse(cfg->chardevTLSx509secretUUID,
seclookupdef.u.uuid) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("malformed chardev TLS secret uuid in
qemu.conf"));
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("malformed TLS secret uuid '%s' in
qemu.conf"),
+ cfg->chardevTLSx509secretUUID);
return -1;
}
seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
@@ -1379,6 +1380,47 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
}
+/* qemuDomainSecretMigratePrepare
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @srcAlias: Alias to use (either migrate or nbd)
+ * @secretUUID: UUID for the secret from the cfg (migrate or nbd)
+ *
+ * Create and prepare the qemuDomainSecretInfoPtr to be used for either
+ * a migration or nbd. Unlike other domain secret prepare functions, this
+ * is only expected to be called for a single object/instance. Theoretically
+ * the object could be reused, although that results in keeping a secret
+ * stored in memory for perhaps longer than expected or necessary.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretMigratePrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ const char *srcAlias,
+ const char *secretUUID)
+{
+ virSecretLookupTypeDef seclookupdef = {0};
+
+ if (virUUIDParse(secretUUID, seclookupdef.u.uuid) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("malformed TLS secret uuid '%s' in
qemu.conf"),
+ secretUUID);
+ return -1;
+ }
+ seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
I hoped this would go inside qemuDomainSecretInfoNew, but you made it
general so that it can be used in places which need different
seclookupdef...
+
+ if (!(priv->migSecinfo =
+ qemuDomainSecretInfoNew(conn, priv, srcAlias,
+ VIR_SECRET_USAGE_TYPE_TLS, NULL,
+ &seclookupdef, false, "TLS X.509")))
This will obviously need to be changed according to the changes in the
previous patch.
Jirka
3:27 p.m.
New subject: [libvirt] [PATCH v2 02/14] qemu: Introduce qemuDomainSecretMigratePrepare
On 02/24/2017 12:08 PM, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:04 -0500, John Ferlan wrote:
> Introduce API to Prepare a qemuDomainSecretInfoPtr to be
> used with a migrate or nbd TLS object
>
> Also alter the error message in ChardevPrepare when UUIDParse fails
> to be consistent with the message for MigratePrepare
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_domain.c | 48 ++++++++++++++++++++++++++--
> src/qemu/qemu_domain.h | 85 ++++++++++++++++++++++++++++----------------------
> 2 files changed, 94 insertions(+), 39 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b7594b3..40c9dab 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1353,8 +1353,9 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
>
> if (virUUIDParse(cfg->chardevTLSx509secretUUID,
> seclookupdef.u.uuid) < 0) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("malformed chardev TLS secret uuid in
qemu.conf"));
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("malformed TLS secret uuid '%s' in
qemu.conf"),
> + cfg->chardevTLSx509secretUUID);
> return -1;
> }
> seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
> @@ -1379,6 +1380,47 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
> }
>
>
> +/* qemuDomainSecretMigratePrepare
> + * @conn: Pointer to connection
> + * @priv: pointer to domain private object
> + * @srcAlias: Alias to use (either migrate or nbd)
> + * @secretUUID: UUID for the secret from the cfg (migrate or nbd)
> + *
> + * Create and prepare the qemuDomainSecretInfoPtr to be used for either
> + * a migration or nbd. Unlike other domain secret prepare functions, this
> + * is only expected to be called for a single object/instance. Theoretically
> + * the object could be reused, although that results in keeping a secret
> + * stored in memory for perhaps longer than expected or necessary.
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +qemuDomainSecretMigratePrepare(virConnectPtr conn,
> + qemuDomainObjPrivatePtr priv,
> + const char *srcAlias,
> + const char *secretUUID)
> +{
> + virSecretLookupTypeDef seclookupdef = {0};
> +
> + if (virUUIDParse(secretUUID, seclookupdef.u.uuid) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("malformed TLS secret uuid '%s' in
qemu.conf"),
> + secretUUID);
> + return -1;
> + }
> + seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
I hoped this would go inside qemuDomainSecretInfoNew, but you made it
general so that it can be used in places which need different
seclookupdef...
Right... and chardev/migration are the only two using a secret UUID from
qemu.conf. The migration one is generic (secretUUID)
I could move the code into the SecretInfoNew, but then someone could say
what does parsing the UUID have to do with creating a SecretInfo - it's
damned if you do and damned if you don't type situation.
I'd rather keep this as is and pass the &seclookupdef
> +
> + if (!(priv->migSecinfo =
> + qemuDomainSecretInfoNew(conn, priv, srcAlias,
> + VIR_SECRET_USAGE_TYPE_TLS, NULL,
> + &seclookupdef, false, "TLS
X.509")))
This will obviously need to be changed according to the changes in the
previous patch.
Yep.
John
Jirka
12:42 p.m.
New subject: [libvirt] [PATCH v2 03/14] qemu: Move exit monitor calls in failure paths
Since qemuDomainObjExitMonitor can also generate error messages,
let's move it inside any error message saving code on error paths
for various hotplug add activities.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 97fb272..9e2f04b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -442,13 +442,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
if (encobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ releaseaddr = false;
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- if (qemuDomainObjExitMonitor(driver, vm) < 0)
- releaseaddr = false;
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
@@ -728,13 +728,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias));
if (encobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias));
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- ignore_value(qemuDomainObjExitMonitor(driver, vm));
-
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
@@ -822,12 +821,12 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
VIR_WARN("Unable to remove drive %s (%s) after failed "
"qemuMonitorAddDevice", drivealias, drivestr);
}
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
error:
@@ -1676,11 +1675,11 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
if (secobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto audit;
}
@@ -1970,12 +1969,12 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
if (secobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto audit;
}
@@ -2156,13 +2155,13 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
if (secobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ releaseaddr = false;
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- if (qemuDomainObjExitMonitor(driver, vm) < 0)
- releaseaddr = false;
goto audit;
}
@@ -2276,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
orig_err = virSaveLastError();
if (objAdded)
ignore_value(qemuMonitorDelObject(priv->mon, objalias));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
mem = NULL;
goto audit;
}
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
removedef:
if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
@@ -2506,12 +2505,12 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
"qemuMonitorAddDevice",
drvstr, devstr);
}
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditHostdev(vm, hostdev, "attach", false);
goto cleanup;
@@ -2798,14 +2797,14 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
ignore_value(qemuMonitorDelObject(priv->mon, memAlias));
}
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ release_address = false;
+
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
- if (qemuDomainObjExitMonitor(driver, vm) < 0)
- release_address = false;
-
goto audit;
}
--
2.9.3
2:48 a.m.
New subject: [libvirt] [PATCH v2 03/14] qemu: Move exit monitor calls in failure paths
On Thu, Feb 23, 2017 at 13:42:05 -0500, John Ferlan wrote:
Since qemuDomainObjExitMonitor can also generate error messages,
let's move it inside any error message saving code on error paths
for various hotplug add activities.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 97fb272..9e2f04b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
...
@@ -2276,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr
driver,
orig_err = virSaveLastError();
if (objAdded)
ignore_value(qemuMonitorDelObject(priv->mon, objalias));
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
mem = NULL;
goto audit;
}
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
removedef:
if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
This hunk adds a memory leak and doesn't prevent
qemuDomainObjExitMonitor from overwriting the error message.
Jirka
7:13 a.m.
New subject: [libvirt] [PATCH v2 03/14] qemu: Move exit monitor calls in failure paths
On 02/27/2017 03:48 AM, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:05 -0500, John Ferlan wrote:
> Since qemuDomainObjExitMonitor can also generate error messages,
> let's move it inside any error message saving code on error paths
> for various hotplug add activities.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_hotplug.c | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 97fb272..9e2f04b 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
...
> @@ -2276,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
> orig_err = virSaveLastError();
> if (objAdded)
> ignore_value(qemuMonitorDelObject(priv->mon, objalias));
> - if (orig_err) {
> - virSetError(orig_err);
> - virFreeError(orig_err);
> - }
> if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> mem = NULL;
> goto audit;
> }
> + if (orig_err) {
> + virSetError(orig_err);
> + virFreeError(orig_err);
> + }
>
> removedef:
> if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
This hunk adds a memory leak and doesn't prevent
qemuDomainObjExitMonitor from overwriting the error message.
Jirka
Always has to be one non-conformist...
I'll merge in the following diff:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9e2f04b..0d629af 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2275,14 +2275,14 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
orig_err = virSaveLastError();
if (objAdded)
ignore_value(qemuMonitorDelObject(priv->mon, objalias));
- if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
mem = NULL;
- goto audit;
- }
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
+ if (!mem)
+ goto audit;
removedef:
if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
12:42 p.m.
New subject: [libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
Refactor the TLS object adding code to make two separate API's that will
handle the add/remove of the "secret" and "tls-creds-x509" objects
including
the Enter/Exit monitor commands.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 165 +++++++++++++++++++++++++++---------------------
src/qemu/qemu_hotplug.h | 13 ++++
2 files changed, 107 insertions(+), 71 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9e2f04b..bb90a34 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1525,6 +1525,85 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
}
+void
+qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *secAlias,
+ const char *tlsAlias)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
+
+ if (!tlsAlias && !secAlias)
+ return;
+
+ orig_err = virSaveLastError();
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ if (tlsAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
+
+ if (secAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
+}
+
+
+int
+qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *secAlias,
+ virJSONValuePtr *secProps,
+ const char *tlsAlias,
+ virJSONValuePtr *tlsProps)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int rc;
+ virErrorPtr orig_err;
+
+ if (!tlsAlias && !secAlias)
+ return 0;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ if (secAlias) {
+ rc = qemuMonitorAddObject(priv->mon, "secret",
+ secAlias, *secProps);
+ *secProps = NULL; /* qemuMonitorAddObject consumes */
+ if (rc < 0)
+ goto exit_monitor;
+ }
+
+ if (tlsAlias) {
+ rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
+ tlsAlias, *tlsProps);
+ *tlsProps = NULL; /* qemuMonitorAddObject consumes */
+ if (rc < 0)
+ goto exit_monitor;
+ }
+
+ return qemuDomainObjExitMonitor(driver, vm);
+
+ exit_monitor:
+ orig_err = virSaveLastError();
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
+ qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
+
+ return -1;
+}
+
+
static int
qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg,
qemuDomainObjPrivatePtr priv,
@@ -1581,8 +1660,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
char *charAlias = NULL;
char *devstr = NULL;
bool chardevAdded = false;
- bool tlsobjAdded = false;
- bool secobjAdded = false;
virJSONValuePtr tlsProps = NULL;
virJSONValuePtr secProps = NULL;
char *tlsAlias = NULL;
@@ -1618,25 +1695,11 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
&secProps, &secAlias) < 0)
goto cleanup;
- qemuDomainObjEnterMonitor(driver, vm);
-
- if (secAlias) {
- rc = qemuMonitorAddObject(priv->mon, "secret",
- secAlias, secProps);
- secProps = NULL;
- if (rc < 0)
- goto exit_monitor;
- secobjAdded = true;
- }
+ if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
+ tlsAlias, &tlsProps) < 0)
+ goto audit;
- if (tlsAlias) {
- rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
- tlsAlias, tlsProps);
- tlsProps = NULL; /* qemuMonitorAddObject consumes */
- if (rc < 0)
- goto exit_monitor;
- tlsobjAdded = true;
- }
+ qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorAttachCharDev(priv->mon,
charAlias,
@@ -1671,15 +1734,12 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
/* detach associated chardev on error */
if (chardevAdded)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
- if (tlsobjAdded)
- ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
- if (secobjAdded)
- ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
ignore_value(qemuDomainObjExitMonitor(driver, vm));
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
+ qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
goto audit;
}
@@ -1857,10 +1917,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
virDomainChrSourceDefPtr dev = chr->source;
char *charAlias = NULL;
bool chardevAttached = false;
- bool tlsobjAdded = false;
bool teardowncgroup = false;
bool teardowndevice = false;
- bool secobjAdded = false;
virJSONValuePtr tlsProps = NULL;
char *tlsAlias = NULL;
virJSONValuePtr secProps = NULL;
@@ -1907,24 +1965,11 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
&secProps, &secAlias) < 0)
goto cleanup;
- qemuDomainObjEnterMonitor(driver, vm);
- if (secAlias) {
- rc = qemuMonitorAddObject(priv->mon, "secret",
- secAlias, secProps);
- secProps = NULL;
- if (rc < 0)
- goto exit_monitor;
- secobjAdded = true;
- }
+ if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
+ tlsAlias, &tlsProps) < 0)
+ goto audit;
- if (tlsAlias) {
- rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
- tlsAlias, tlsProps);
- tlsProps = NULL; /* qemuMonitorAddObject consumes */
- if (rc < 0)
- goto exit_monitor;
- tlsobjAdded = true;
- }
+ qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorAttachCharDev(priv->mon, charAlias, chr->source) < 0)
goto exit_monitor;
@@ -1965,16 +2010,13 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
/* detach associated chardev on error */
if (chardevAttached)
qemuMonitorDetachCharDev(priv->mon, charAlias);
- if (tlsobjAdded)
- ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
- if (secobjAdded)
- ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
ignore_value(qemuDomainObjExitMonitor(driver, vm));
if (orig_err) {
virSetError(orig_err);
virFreeError(orig_err);
}
+ qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
goto audit;
}
@@ -1999,8 +2041,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
bool teardowndevice = false;
bool chardevAdded = false;
bool objAdded = false;
- bool tlsobjAdded = false;
- bool secobjAdded = false;
virJSONValuePtr props = NULL;
virJSONValuePtr tlsProps = NULL;
virJSONValuePtr secProps = NULL;
@@ -2075,27 +2115,13 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
charAlias, &tlsProps, &tlsAlias,
&secProps, &secAlias) < 0)
goto cleanup;
- }
- qemuDomainObjEnterMonitor(driver, vm);
-
- if (secAlias) {
- rv = qemuMonitorAddObject(priv->mon, "secret",
- secAlias, secProps);
- secProps = NULL;
- if (rv < 0)
- goto exit_monitor;
- secobjAdded = true;
+ if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
+ tlsAlias, &tlsProps) < 0)
+ goto audit;
}
- if (tlsAlias) {
- rv = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
- tlsAlias, tlsProps);
- tlsProps = NULL; /* qemuMonitorAddObject consumes */
- if (rv < 0)
- goto exit_monitor;
- tlsobjAdded = true;
- }
+ qemuDomainObjEnterMonitor(driver, vm);
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
qemuMonitorAttachCharDev(priv->mon, charAlias,
@@ -2151,10 +2177,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
ignore_value(qemuMonitorDelObject(priv->mon, objAlias));
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && chardevAdded)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
- if (tlsobjAdded)
- ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
- if (secobjAdded)
- ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
if (qemuDomainObjExitMonitor(driver, vm) < 0)
releaseaddr = false;
if (orig_err) {
@@ -2162,6 +2184,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
virFreeError(orig_err);
}
+ qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
goto audit;
}
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 0b11c1e..24cf033 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -33,6 +33,19 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk,
virStorageSourcePtr newsrc,
bool force);
+
+void qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *secAlias,
+ const char *tlsAlias);
+
+int qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *secAlias,
+ virJSONValuePtr *secProps,
+ const char *tlsAlias,
+ virJSONValuePtr *tlsProps);
+
int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainControllerDefPtr controller);
--
2.9.3
3:41 a.m.
New subject: [libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
On Thu, Feb 23, 2017 at 13:42:06 -0500, John Ferlan wrote:
Refactor the TLS object adding code to make two separate API's
that will
handle the add/remove of the "secret" and "tls-creds-x509" objects
including
the Enter/Exit monitor commands.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 165 +++++++++++++++++++++++++++---------------------
src/qemu/qemu_hotplug.h | 13 ++++
2 files changed, 107 insertions(+), 71 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9e2f04b..bb90a34 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1525,6 +1525,85 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
}
+void
+qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *secAlias,
+ const char *tlsAlias)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
+
+ if (!tlsAlias && !secAlias)
+ return;
+
+ orig_err = virSaveLastError();
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ if (tlsAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
+
+ if (secAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
+}
+
+
+int
+qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *secAlias,
+ virJSONValuePtr *secProps,
+ const char *tlsAlias,
+ virJSONValuePtr *tlsProps)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int rc;
+ virErrorPtr orig_err;
+
+ if (!tlsAlias && !secAlias)
+ return 0;
+
+ qemuDomainObjEnterMonitor(driver, vm);
+
+ if (secAlias) {
+ rc = qemuMonitorAddObject(priv->mon, "secret",
+ secAlias, *secProps);
+ *secProps = NULL; /* qemuMonitorAddObject consumes */
+ if (rc < 0)
+ goto exit_monitor;
+ }
+
+ if (tlsAlias) {
+ rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
+ tlsAlias, *tlsProps);
+ *tlsProps = NULL; /* qemuMonitorAddObject consumes */
+ if (rc < 0)
+ goto exit_monitor;
+ }
+
+ return qemuDomainObjExitMonitor(driver, vm);
+
+ exit_monitor:
I'd prefer "error" label since this is not the only place where
ExitMonitor is called.
+ orig_err = virSaveLastError();
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
+ qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
+
+ return -1;
+}
Jirka
7:20 a.m.
New subject: [libvirt] [PATCH v2 04/14] qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
On 02/27/2017 04:41 AM, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:06 -0500, John Ferlan wrote:
> Refactor the TLS object adding code to make two separate API's that will
> handle the add/remove of the "secret" and "tls-creds-x509"
objects including
> the Enter/Exit monitor commands.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_hotplug.c | 165 +++++++++++++++++++++++++++---------------------
> src/qemu/qemu_hotplug.h | 13 ++++
> 2 files changed, 107 insertions(+), 71 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 9e2f04b..bb90a34 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1525,6 +1525,85 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
> }
>
>
> +void
> +qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + const char *secAlias,
> + const char *tlsAlias)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + virErrorPtr orig_err;
> +
> + if (!tlsAlias && !secAlias)
> + return;
> +
> + orig_err = virSaveLastError();
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> +
> + if (tlsAlias)
> + ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
> +
> + if (secAlias)
> + ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
> +
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +
> + if (orig_err) {
> + virSetError(orig_err);
> + virFreeError(orig_err);
> + }
> +}
> +
> +
> +int
> +qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + const char *secAlias,
> + virJSONValuePtr *secProps,
> + const char *tlsAlias,
> + virJSONValuePtr *tlsProps)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + int rc;
> + virErrorPtr orig_err;
> +
> + if (!tlsAlias && !secAlias)
> + return 0;
> +
> + qemuDomainObjEnterMonitor(driver, vm);
> +
> + if (secAlias) {
> + rc = qemuMonitorAddObject(priv->mon, "secret",
> + secAlias, *secProps);
> + *secProps = NULL; /* qemuMonitorAddObject consumes */
> + if (rc < 0)
> + goto exit_monitor;
> + }
> +
> + if (tlsAlias) {
> + rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
> + tlsAlias, *tlsProps);
> + *tlsProps = NULL; /* qemuMonitorAddObject consumes */
> + if (rc < 0)
> + goto exit_monitor;
> + }
> +
> + return qemuDomainObjExitMonitor(driver, vm);
> +
> + exit_monitor:
I'd prefer "error" label since this is not the only place where
ExitMonitor is called.
I can change to error - doesn't really matter. The 'exit_monitor' label
has been used generically in a number of other places even though an
ExitMonitor is called in each instance on the non failure path. Most of
those though span quite a few lines of scrolling to find the
exit_monitor label.
John
> + orig_err = virSaveLastError();
> + ignore_value(qemuDomainObjExitMonitor(driver, vm));
> + if (orig_err) {
> + virSetError(orig_err);
> + virFreeError(orig_err);
> + }
> + qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
> +
> + return -1;
> +}
Jirka
12:42 p.m.
New subject: [libvirt] [PATCH v2 05/14] qemu: Refactor qemuDomainGetChardevTLSObjects to converge code
Create a qemuDomainAddChardevTLSObjects which will encapsulate the
qemuDomainGetChardevTLSObjects and qemuDomainAddTLSObjects so that
the callers don't need to worry about the props.
Move the dev->type and haveTLS checks in to the Add function to avoid
an unnecessary call to qemuDomainAddTLSObjects
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 80 ++++++++++++++++++++++++++-----------------------
1 file changed, 43 insertions(+), 37 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bb90a34..503fb30 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1617,10 +1617,6 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg,
qemuDomainChrSourcePrivatePtr chrSourcePriv =
QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
- if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP ||
- dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES)
- return 0;
-
/* Add a secret object in order to access the TLS environment.
* The secinfo will only be created for serial TCP device. */
if (chrSourcePriv && chrSourcePriv->secinfo) {
@@ -1647,6 +1643,43 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg,
}
+static int
+qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver,
+ virQEMUDriverConfigPtr cfg,
+ virDomainObjPtr vm,
+ virDomainChrSourceDefPtr dev,
+ char *charAlias,
+ char **tlsAlias,
+ char **secAlias)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virJSONValuePtr tlsProps = NULL;
+ virJSONValuePtr secProps = NULL;
+
+ if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP ||
+ dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES)
+ return 0;
+
+ if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias,
+ &tlsProps, tlsAlias,
+ &secProps, secAlias) < 0)
+ goto cleanup;
+
+ if (qemuDomainAddTLSObjects(driver, vm, *secAlias, &secProps,
+ *tlsAlias, &tlsProps) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ virJSONValueFree(tlsProps);
+ virJSONValueFree(secProps);
+
+ return ret;
+}
+
+
int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -1660,8 +1693,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
char *charAlias = NULL;
char *devstr = NULL;
bool chardevAdded = false;
- virJSONValuePtr tlsProps = NULL;
- virJSONValuePtr secProps = NULL;
char *tlsAlias = NULL;
char *secAlias = NULL;
bool need_release = false;
@@ -1690,13 +1721,8 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
redirdev->source) < 0)
goto cleanup;
- if (qemuDomainGetChardevTLSObjects(cfg, priv, redirdev->source,
- charAlias, &tlsProps, &tlsAlias,
- &secProps, &secAlias) < 0)
- goto cleanup;
-
- if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
- tlsAlias, &tlsProps) < 0)
+ if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, redirdev->source,
+ charAlias, &tlsAlias, &secAlias) < 0)
goto audit;
qemuDomainObjEnterMonitor(driver, vm);
@@ -1721,9 +1747,7 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
if (ret < 0 && need_release)
qemuDomainReleaseDeviceAddress(vm, &redirdev->info, NULL);
VIR_FREE(tlsAlias);
- virJSONValueFree(tlsProps);
VIR_FREE(secAlias);
- virJSONValueFree(secProps);
VIR_FREE(charAlias);
VIR_FREE(devstr);
virObjectUnref(cfg);
@@ -1919,9 +1943,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
bool chardevAttached = false;
bool teardowncgroup = false;
bool teardowndevice = false;
- virJSONValuePtr tlsProps = NULL;
char *tlsAlias = NULL;
- virJSONValuePtr secProps = NULL;
char *secAlias = NULL;
bool need_release = false;
@@ -1960,13 +1982,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
dev) < 0)
goto cleanup;
- if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias,
- &tlsProps, &tlsAlias,
- &secProps, &secAlias) < 0)
- goto cleanup;
-
- if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
- tlsAlias, &tlsProps) < 0)
+ if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, dev, charAlias,
+ &tlsAlias, &secAlias) < 0)
goto audit;
qemuDomainObjEnterMonitor(driver, vm);
@@ -1997,9 +2014,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
VIR_WARN("Unable to remove chr device from /dev");
}
VIR_FREE(tlsAlias);
- virJSONValueFree(tlsProps);
VIR_FREE(secAlias);
- virJSONValueFree(secProps);
VIR_FREE(charAlias);
VIR_FREE(devstr);
virObjectUnref(cfg);
@@ -2042,8 +2057,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
bool chardevAdded = false;
bool objAdded = false;
virJSONValuePtr props = NULL;
- virJSONValuePtr tlsProps = NULL;
- virJSONValuePtr secProps = NULL;
virDomainCCWAddressSetPtr ccwaddrs = NULL;
const char *type;
int ret = -1;
@@ -2111,13 +2124,8 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
rng->source.chardev) < 0)
goto cleanup;
- if (qemuDomainGetChardevTLSObjects(cfg, priv, rng->source.chardev,
- charAlias, &tlsProps, &tlsAlias,
- &secProps, &secAlias) < 0)
- goto cleanup;
-
- if (qemuDomainAddTLSObjects(driver, vm, secAlias, &secProps,
- tlsAlias, &tlsProps) < 0)
+ if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, rng->source.chardev,
+ charAlias, &tlsAlias, &secAlias) <
0)
goto audit;
}
@@ -2150,8 +2158,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
audit:
virDomainAuditRNG(vm, NULL, rng, "attach", ret == 0);
cleanup:
- virJSONValueFree(tlsProps);
- virJSONValueFree(secProps);
virJSONValueFree(props);
if (ret < 0) {
if (releaseaddr)
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 06/14] qemu: Move qemuDomainSecretChardevPrepare call
Move the call to inside the qemuDomainAddChardevTLSObjects in order to
further converge the code.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 503fb30..b59f3ab 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1644,10 +1644,12 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg,
static int
-qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver,
+qemuDomainAddChardevTLSObjects(virConnectPtr conn,
+ virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm,
virDomainChrSourceDefPtr dev,
+ char *devAlias,
char *charAlias,
char **tlsAlias,
char **secAlias)
@@ -1661,6 +1663,9 @@ qemuDomainAddChardevTLSObjects(virQEMUDriverPtr driver,
dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES)
return 0;
+ if (qemuDomainSecretChardevPrepare(conn, cfg, priv, devAlias, dev) < 0)
+ goto cleanup;
+
if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias,
&tlsProps, tlsAlias,
&secProps, secAlias) < 0)
@@ -1717,12 +1722,9 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
if (VIR_REALLOC_N(def->redirdevs, def->nredirdevs+1) < 0)
goto cleanup;
- if (qemuDomainSecretChardevPrepare(conn, cfg, priv, redirdev->info.alias,
- redirdev->source) < 0)
- goto cleanup;
-
- if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, redirdev->source,
- charAlias, &tlsAlias, &secAlias) < 0)
+ if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, redirdev->source,
+ redirdev->info.alias, charAlias,
+ &tlsAlias, &secAlias) < 0)
goto audit;
qemuDomainObjEnterMonitor(driver, vm);
@@ -1978,11 +1980,8 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
if (qemuDomainChrPreInsert(vmdef, chr) < 0)
goto cleanup;
- if (qemuDomainSecretChardevPrepare(conn, cfg, priv, chr->info.alias,
- dev) < 0)
- goto cleanup;
-
- if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, dev, charAlias,
+ if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, dev,
+ chr->info.alias, charAlias,
&tlsAlias, &secAlias) < 0)
goto audit;
@@ -2120,12 +2119,10 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
goto cleanup;
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
- if (qemuDomainSecretChardevPrepare(conn, cfg, priv, rng->info.alias,
- rng->source.chardev) < 0)
- goto cleanup;
-
- if (qemuDomainAddChardevTLSObjects(driver, cfg, vm, rng->source.chardev,
- charAlias, &tlsAlias, &secAlias) <
0)
+ if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm,
+ rng->source.chardev,
+ rng->info.alias, charAlias,
+ &tlsAlias, &secAlias) < 0)
goto audit;
}
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 07/14] qemu: Move qemuDomainPrepareChardevSourceTLS call
Move the call to inside the qemuDomainAddChardevTLSObjects in order to
further converge the code.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 31 ++++++++++++-------------------
1 file changed, 12 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b59f3ab..d7a1f1f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1646,7 +1646,6 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg,
static int
qemuDomainAddChardevTLSObjects(virConnectPtr conn,
virQEMUDriverPtr driver,
- virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm,
virDomainChrSourceDefPtr dev,
char *devAlias,
@@ -1655,13 +1654,19 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn,
char **secAlias)
{
int ret = -1;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
virJSONValuePtr tlsProps = NULL;
virJSONValuePtr secProps = NULL;
+ /* NB: This may alter haveTLS based on cfg */
+ qemuDomainPrepareChardevSourceTLS(dev, cfg);
+
if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP ||
- dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES)
- return 0;
+ dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) {
+ ret = 0;
+ goto cleanup;
+ }
if (qemuDomainSecretChardevPrepare(conn, cfg, priv, devAlias, dev) < 0)
goto cleanup;
@@ -1680,6 +1685,7 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn,
cleanup:
virJSONValueFree(tlsProps);
virJSONValueFree(secProps);
+ virObjectUnref(cfg);
return ret;
}
@@ -1692,7 +1698,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
{
int ret = -1;
int rc;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainDefPtr def = vm->def;
char *charAlias = NULL;
@@ -1703,8 +1708,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
bool need_release = false;
virErrorPtr orig_err;
- qemuDomainPrepareChardevSourceTLS(redirdev->source, cfg);
-
if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0)
goto cleanup;
@@ -1722,7 +1725,7 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
if (VIR_REALLOC_N(def->redirdevs, def->nredirdevs+1) < 0)
goto cleanup;
- if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, redirdev->source,
+ if (qemuDomainAddChardevTLSObjects(conn, driver, vm, redirdev->source,
redirdev->info.alias, charAlias,
&tlsAlias, &secAlias) < 0)
goto audit;
@@ -1752,7 +1755,6 @@ int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
VIR_FREE(secAlias);
VIR_FREE(charAlias);
VIR_FREE(devstr);
- virObjectUnref(cfg);
return ret;
exit_monitor:
@@ -1935,7 +1937,6 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
virDomainChrDefPtr chr)
{
int ret = -1, rc;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
virErrorPtr orig_err;
virDomainDefPtr vmdef = vm->def;
@@ -1953,8 +1954,6 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0)
goto cleanup;
- qemuDomainPrepareChardevSourceTLS(dev, cfg);
-
if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
goto cleanup;
@@ -1980,7 +1979,7 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
if (qemuDomainChrPreInsert(vmdef, chr) < 0)
goto cleanup;
- if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm, dev,
+ if (qemuDomainAddChardevTLSObjects(conn, driver, vm, dev,
chr->info.alias, charAlias,
&tlsAlias, &secAlias) < 0)
goto audit;
@@ -2016,7 +2015,6 @@ int qemuDomainAttachChrDevice(virConnectPtr conn,
VIR_FREE(secAlias);
VIR_FREE(charAlias);
VIR_FREE(devstr);
- virObjectUnref(cfg);
return ret;
exit_monitor:
@@ -2041,7 +2039,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
virDomainObjPtr vm,
virDomainRNGDefPtr rng)
{
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_RNG, { .rng = rng } };
virErrorPtr orig_err;
@@ -2102,9 +2099,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
goto cleanup;
teardowncgroup = true;
- if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
- qemuDomainPrepareChardevSourceTLS(rng->source.chardev, cfg);
-
/* build required metadata */
if (!(devstr = qemuBuildRNGDevStr(vm->def, rng, priv->qemuCaps)))
goto cleanup;
@@ -2119,7 +2113,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
goto cleanup;
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
- if (qemuDomainAddChardevTLSObjects(conn, driver, cfg, vm,
+ if (qemuDomainAddChardevTLSObjects(conn, driver, vm,
rng->source.chardev,
rng->info.alias, charAlias,
&tlsAlias, &secAlias) < 0)
@@ -2171,7 +2165,6 @@ qemuDomainAttachRNGDevice(virConnectPtr conn,
VIR_FREE(objAlias);
VIR_FREE(devstr);
virDomainCCWAddressSetFree(ccwaddrs);
- virObjectUnref(cfg);
return ret;
exit_monitor:
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 08/14] qemu: Introduce qemuDomainGetTLSObjects
Split apart and rename qemuDomainGetChardevTLSObjects in order to make a
more generic API that can create the TLS JSON prop objects (secret and
tls-creds-x509) to be used to create the objects
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 55 ++++++++++++++++++++++++++-----------------------
src/qemu/qemu_hotplug.h | 11 ++++++++++
2 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d7a1f1f..9728b43 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1604,40 +1604,34 @@ qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
}
-static int
-qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg,
- qemuDomainObjPrivatePtr priv,
- virDomainChrSourceDefPtr dev,
- char *charAlias,
- virJSONValuePtr *tlsProps,
- char **tlsAlias,
- virJSONValuePtr *secProps,
- char **secAlias)
+int
+qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps,
+ qemuDomainSecretInfoPtr secinfo,
+ const char *tlsCertdir,
+ bool tlsListen,
+ bool tlsVerify,
+ const char *srcAlias,
+ virJSONValuePtr *tlsProps,
+ char **tlsAlias,
+ virJSONValuePtr *secProps,
+ char **secAlias)
{
- qemuDomainChrSourcePrivatePtr chrSourcePriv =
- QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
-
/* Add a secret object in order to access the TLS environment.
* The secinfo will only be created for serial TCP device. */
- if (chrSourcePriv && chrSourcePriv->secinfo) {
- if (qemuBuildSecretInfoProps(chrSourcePriv->secinfo, secProps) < 0)
+ if (secinfo) {
+ if (qemuBuildSecretInfoProps(secinfo, secProps) < 0)
return -1;
- if (!(*secAlias = qemuDomainGetSecretAESAlias(charAlias, false)))
+ if (!(*secAlias = qemuDomainGetSecretAESAlias(srcAlias, false)))
return -1;
}
- if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
- dev->data.tcp.listen,
- cfg->chardevTLSx509verify,
- *secAlias,
- priv->qemuCaps,
- tlsProps) < 0)
+ if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify,
+ *secAlias, qemuCaps, tlsProps) < 0)
return -1;
- if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(charAlias)))
+ if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias)))
return -1;
- dev->data.tcp.tlscreds = true;
return 0;
}
@@ -1656,6 +1650,8 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn,
int ret = -1;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
+ qemuDomainChrSourcePrivatePtr chrSourcePriv;
+ qemuDomainSecretInfoPtr secinfo = NULL;
virJSONValuePtr tlsProps = NULL;
virJSONValuePtr secProps = NULL;
@@ -1671,10 +1667,17 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn,
if (qemuDomainSecretChardevPrepare(conn, cfg, priv, devAlias, dev) < 0)
goto cleanup;
- if (qemuDomainGetChardevTLSObjects(cfg, priv, dev, charAlias,
- &tlsProps, tlsAlias,
- &secProps, secAlias) < 0)
+ if ((chrSourcePriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev)))
+ secinfo = chrSourcePriv->secinfo;
+
+ if (qemuDomainGetTLSObjects(priv->qemuCaps, secinfo,
+ cfg->chardevTLSx509certdir,
+ dev->data.tcp.listen,
+ cfg->chardevTLSx509verify,
+ charAlias, &tlsProps, tlsAlias,
+ &secProps, secAlias) < 0)
goto cleanup;
+ dev->data.tcp.tlscreds = true;
if (qemuDomainAddTLSObjects(driver, vm, *secAlias, &secProps,
*tlsAlias, &tlsProps) < 0)
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 24cf033..73f2b1f 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -46,6 +46,17 @@ int qemuDomainAddTLSObjects(virQEMUDriverPtr driver,
const char *tlsAlias,
virJSONValuePtr *tlsProps);
+int qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps,
+ qemuDomainSecretInfoPtr secinfo,
+ const char *tlsCertdir,
+ bool tlsListen,
+ bool tlsVerify,
+ const char *srcAlias,
+ virJSONValuePtr *tlsProps,
+ char **tlsAlias,
+ virJSONValuePtr *secProps,
+ char **secAlias);
+
int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainControllerDefPtr controller);
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 09/14] qemu: Add TLS params to _qemuMonitorMigrationParams
Add the fields to support setting tls-creds and tls-hostname during
a migration (either source or target)
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_monitor.c | 11 ++++++++---
src/qemu/qemu_monitor.h | 3 +++
src/qemu/qemu_monitor_json.c | 10 ++++++++++
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index b15207a..898bbe1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2504,12 +2504,15 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon,
{
VIR_DEBUG("compressLevel=%d:%d compressThreads=%d:%d "
"decompressThreads=%d:%d cpuThrottleInitial=%d:%d "
- "cpuThrottleIncrement=%d:%d",
+ "cpuThrottleIncrement=%d:%d tlsAlias=%s "
+ "tlsHostname=%s",
params->compressLevel_set, params->compressLevel,
params->compressThreads_set, params->compressThreads,
params->decompressThreads_set, params->decompressThreads,
params->cpuThrottleInitial_set, params->cpuThrottleInitial,
- params->cpuThrottleIncrement_set, params->cpuThrottleIncrement);
+ params->cpuThrottleIncrement_set, params->cpuThrottleIncrement,
+ NULLSTR(params->migrateTLSAlias),
+ NULLSTR(params->migrateTLSHostname));
QEMU_CHECK_MONITOR_JSON(mon);
@@ -2517,7 +2520,9 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon,
!params->compressThreads_set &&
!params->decompressThreads_set &&
!params->cpuThrottleInitial_set &&
- !params->cpuThrottleIncrement_set)
+ !params->cpuThrottleIncrement_set &&
+ !params->migrateTLSAlias &&
+ !params->migrateTLSHostname)
return 0;
return qemuMonitorJSONSetMigrationParams(mon, params);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8811d85..b292be5 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -570,6 +570,9 @@ struct _qemuMonitorMigrationParams {
bool cpuThrottleIncrement_set;
int cpuThrottleIncrement;
+
+ char *migrateTLSAlias;
+ char *migrateTLSHostname;
};
int qemuMonitorGetMigrationParams(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7aa9e31..1112d10 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2637,6 +2637,16 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon,
#undef APPEND
+ if (params->migrateTLSAlias &&
+ virJSONValueObjectAppendString(args, "tls-creds",
+ params->migrateTLSAlias) < 0)
+ goto cleanup;
+
+ if (params->migrateTLSHostname &&
+ virJSONValueObjectAppendString(args, "tls-hostname",
+ params->migrateTLSHostname) < 0)
+ goto cleanup;
+
if (virJSONValueObjectAppend(cmd, "arguments", args) < 0)
goto cleanup;
args = NULL;
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 10/14] Add new migration flag VIR_MIGRATE_TLS
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
include/libvirt/libvirt-domain.h | 8 ++++++++
src/qemu/qemu_migration.h | 3 ++-
tools/virsh-domain.c | 7 +++++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index c0f715d..bca04d3 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -815,6 +815,14 @@ typedef enum {
* post-copy mode. See virDomainMigrateStartPostCopy for more details.
*/
VIR_MIGRATE_POSTCOPY = (1 << 15),
+
+ /* Setting the VIR_MIGRATE_TLS flag will cause the migration to attempt
+ * to use the TLS environment configured by the hypervisor in order to
+ * perform the migration. If incorrectly configured on either source or
+ * destination, the migration will fail.
+ */
+ VIR_MIGRATE_TLS = (1 << 16),
+
} virDomainMigrateFlags;
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 14c6178..bcebf06 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -45,7 +45,8 @@ typedef qemuMigrationCompression *qemuMigrationCompressionPtr;
VIR_MIGRATE_ABORT_ON_ERROR | \
VIR_MIGRATE_AUTO_CONVERGE | \
VIR_MIGRATE_RDMA_PIN_ALL | \
- VIR_MIGRATE_POSTCOPY)
+ VIR_MIGRATE_POSTCOPY | \
+ VIR_MIGRATE_TLS)
/* All supported migration parameters and their types. */
# define QEMU_MIGRATION_PARAMETERS \
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 09a9f82..ebd4b33 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10222,6 +10222,10 @@ static const vshCmdOptDef opts_migrate[] = {
.type = VSH_OT_STRING,
.help = N_("filename containing updated persistent XML for the target")
},
+ {.name = "tls",
+ .type = VSH_OT_BOOL,
+ .help = N_("use TLS for migration")
+ },
{.name = NULL}
};
@@ -10463,6 +10467,9 @@ doMigrate(void *opaque)
if (vshCommandOptBool(cmd, "postcopy"))
flags |= VIR_MIGRATE_POSTCOPY;
+ if (vshCommandOptBool(cmd, "tls"))
+ flags |= VIR_MIGRATE_TLS;
+
if (flags & VIR_MIGRATE_PEER2PEER || vshCommandOptBool(cmd, "direct"))
{
if (virDomainMigrateToURI3(dom, desturi, params, nparams, flags) == 0)
ret = '0';
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 11/14] qemu: Create #define for TLS configuration setup.
Create GET_CONFIG_TLS_CERT to set up the TLS for 'chardev' TLS setting.
Soon to be reused.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_conf.c | 43 +++++++++++++++++++++++++++----------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b5b0645..b75cd54 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -529,22 +529,33 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
if (virConfGetValueBool(conf, "spice_auto_unix_socket",
&cfg->spiceAutoUnixSocket) < 0)
goto cleanup;
- if (virConfGetValueBool(conf, "chardev_tls", &cfg->chardevTLS) <
0)
- goto cleanup;
- if (virConfGetValueString(conf, "chardev_tls_x509_cert_dir",
&cfg->chardevTLSx509certdir) < 0)
- goto cleanup;
- if ((rv = virConfGetValueBool(conf, "chardev_tls_x509_verify",
&cfg->chardevTLSx509verify)) < 0)
- goto cleanup;
- if (rv == 0)
- cfg->chardevTLSx509verify = cfg->defaultTLSx509verify;
- if (virConfGetValueString(conf, "chardev_tls_x509_secret_uuid",
- &cfg->chardevTLSx509secretUUID) < 0)
- goto cleanup;
- if (!cfg->chardevTLSx509secretUUID && cfg->defaultTLSx509secretUUID) {
- if (VIR_STRDUP(cfg->chardevTLSx509secretUUID,
- cfg->defaultTLSx509secretUUID) < 0)
- goto cleanup;
- }
+#define GET_CONFIG_TLS_CERT(val) \
+ do { \
+ if (virConfGetValueBool(conf, #val "_tls", &cfg->val## TLS) <
0) \
+ goto cleanup; \
+ if ((rv = virConfGetValueBool(conf, #val "_tls_x509_verify", \
+ &cfg->val## TLSx509verify)) < 0) \
+ goto cleanup; \
+ if (rv == 0) \
+ cfg->val## TLSx509verify = cfg->defaultTLSx509verify; \
+ if (virConfGetValueString(conf, #val "_tls_x509_cert_dir", \
+ &cfg->val## TLSx509certdir) < 0) \
+ goto cleanup; \
+ if (virConfGetValueString(conf, \
+ #val "_tls_x509_secret_uuid", \
+ &cfg->val## TLSx509secretUUID) < 0) \
+ goto cleanup; \
+ if (!cfg->val## TLSx509secretUUID && \
+ cfg->defaultTLSx509secretUUID) { \
+ if (VIR_STRDUP(cfg->val## TLSx509secretUUID, \
+ cfg->defaultTLSx509secretUUID) < 0) \
+ goto cleanup; \
+ } \
+ } while (false);
+
+ GET_CONFIG_TLS_CERT(chardev);
+
+#undef GET_CONFIG_TLS_CERT
if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
goto cleanup;
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 12/14] conf: Introduce migrate_tls_x509_cert_dir
Add a new TLS X.509 certificate type - "migrate". This will handle the
creation of a TLS certificate capability (and possibly repository) to
be used for migrations. Similar to chardev's, credentials will be handled
via a libvirt secrets.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 6 ++++++
src/qemu/qemu.conf | 39 ++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_conf.c | 2 ++
src/qemu/qemu_conf.h | 5 +++++
src/qemu/test_libvirtd_qemu.aug.in | 4 ++++
5 files changed, 56 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 82bae9e..18679c1 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -54,6 +54,11 @@ module Libvirtd_qemu =
| bool_entry "chardev_tls_x509_verify"
| str_entry "chardev_tls_x509_secret_uuid"
+ let migrate_entry = bool_entry "migrate_tls"
+ | str_entry "migrate_tls_x509_cert_dir"
+ | bool_entry "migrate_tls_x509_verify"
+ | str_entry "migrate_tls_x509_secret_uuid"
+
let nogfx_entry = bool_entry "nographics_allow_host_audio"
let remote_display_entry = int_entry "remote_display_port_min"
@@ -116,6 +121,7 @@ module Libvirtd_qemu =
| vnc_entry
| spice_entry
| chardev_entry
+ | migrate_entry
| nogfx_entry
| remote_display_entry
| security_entry
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 9f990c2..c4e228b 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -238,6 +238,45 @@
#chardev_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
+# Enable use of TLS encryption for migration
+#
+# It is necessary to setup CA and issue a server certificate
+# before enabling this.
+#
+#migrate_tls = 1
+
+
+# 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 then the default_tls_x509_cert_dir path
+# will be used.
+#
+#migrate_tls_x509_cert_dir = "/etc/pki/libvirt-migrate"
+
+
+# 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 a x509 certificate to every client who needs to connect.
+#
+# Enabling this option will reject any client who does not have a
+# certificate signed by the CA in /etc/pki/libvirt-migrate/ca-cert.pem
+#
+#migrate_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
+#
+#migrate_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 b75cd54..f63d9c2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -555,6 +555,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
GET_CONFIG_TLS_CERT(chardev);
+ GET_CONFIG_TLS_CERT(migrate);
+
#undef GET_CONFIG_TLS_CERT
if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index e585f81..ac7badb 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -137,6 +137,11 @@ struct _virQEMUDriverConfig {
bool chardevTLSx509verify;
char *chardevTLSx509secretUUID;
+ bool migrateTLS;
+ char *migrateTLSx509certdir;
+ bool migrateTLSx509verify;
+ char *migrateTLSx509secretUUID;
+
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 6f03898..71ddf7d 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -25,6 +25,10 @@ module Test_libvirtd_qemu =
{ "chardev_tls_x509_cert_dir" = "/etc/pki/libvirt-chardev" }
{ "chardev_tls_x509_verify" = "1" }
{ "chardev_tls_x509_secret_uuid" =
"00000000-0000-0000-0000-000000000000" }
+{ "migrate_tls" = "1" }
+{ "migrate_tls_x509_cert_dir" = "/etc/pki/libvirt-migrate" }
+{ "migrate_tls_x509_verify" = "1" }
+{ "migrate_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.9.3
7 a.m.
New subject: [libvirt] [PATCH v2 12/14] conf: Introduce migrate_tls_x509_cert_dir
On 02/23/2017 01:42 PM, John Ferlan wrote:
Add a new TLS X.509 certificate type - "migrate". This will
handle the
creation of a TLS certificate capability (and possibly repository) to
be used for migrations. Similar to chardev's, credentials will be handled
via a libvirt secrets.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 6 ++++++
src/qemu/qemu.conf | 39 ++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_conf.c | 2 ++
src/qemu/qemu_conf.h | 5 +++++
src/qemu/test_libvirtd_qemu.aug.in | 4 ++++
5 files changed, 56 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 82bae9e..18679c1 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -54,6 +54,11 @@ module Libvirtd_qemu =
| bool_entry "chardev_tls_x509_verify"
| str_entry "chardev_tls_x509_secret_uuid"
+ let migrate_entry = bool_entry "migrate_tls"
+ | str_entry "migrate_tls_x509_cert_dir"
+ | bool_entry "migrate_tls_x509_verify"
+ | str_entry "migrate_tls_x509_secret_uuid"
+
let nogfx_entry = bool_entry "nographics_allow_host_audio"
let remote_display_entry = int_entry "remote_display_port_min"
@@ -116,6 +121,7 @@ module Libvirtd_qemu =
| vnc_entry
| spice_entry
| chardev_entry
+ | migrate_entry
| nogfx_entry
| remote_display_entry
| security_entry
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 9f990c2..c4e228b 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -238,6 +238,45 @@
#chardev_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
+# Enable use of TLS encryption for migration
+#
+# It is necessary to setup CA and issue a server certificate
+# before enabling this.
+#
+#migrate_tls = 1
+
+
+# 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 then the default_tls_x509_cert_dir path
+# will be used.
+#
+#migrate_tls_x509_cert_dir = "/etc/pki/libvirt-migrate"
+
+
+# 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 a x509 certificate to every client who needs to connect.
+#
+# Enabling this option will reject any client who does not have a
+# certificate signed by the CA in /etc/pki/libvirt-migrate/ca-cert.pem
+#
+#migrate_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
+#
+#migrate_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 b75cd54..f63d9c2 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -555,6 +555,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
GET_CONFIG_TLS_CERT(chardev);
+ GET_CONFIG_TLS_CERT(migrate);
+
#undef GET_CONFIG_TLS_CERT
if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index e585f81..ac7badb 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -137,6 +137,11 @@ struct _virQEMUDriverConfig {
bool chardevTLSx509verify;
char *chardevTLSx509secretUUID;
+ bool migrateTLS;
+ char *migrateTLSx509certdir;
+ bool migrateTLSx509verify;
+ char *migrateTLSx509secretUUID;
+
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 6f03898..71ddf7d 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -25,6 +25,10 @@ module Test_libvirtd_qemu =
{ "chardev_tls_x509_cert_dir" = "/etc/pki/libvirt-chardev" }
{ "chardev_tls_x509_verify" = "1" }
{ "chardev_tls_x509_secret_uuid" =
"00000000-0000-0000-0000-000000000000" }
+{ "migrate_tls" = "1" }
+{ "migrate_tls_x509_cert_dir" = "/etc/pki/libvirt-migrate" }
+{ "migrate_tls_x509_verify" = "1" }
+{ "migrate_tls_x509_secret_uuid" =
"00000000-0000-0000-0000-000000000000" }
{ "nographics_allow_host_audio" = "1" }
{ "remote_display_port_min" = "5900" }
{ "remote_display_port_max" = "65535" }
Consider the following diff to be merged into this one (reminded while
looking at the code going through how the default config is set up...)
John
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index f63d9c2..f1ee4ee 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -279,6 +279,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
privileged)
SET_TLS_X509_CERT_DEFAULT(vnc);
SET_TLS_X509_CERT_DEFAULT(spice);
SET_TLS_X509_CERT_DEFAULT(chardev);
+ SET_TLS_X509_CERT_DEFAULT(migrate);
#undef SET_TLS_X509_CERT_DEFAULT
@@ -394,6 +395,9 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->chardevTLSx509certdir);
VIR_FREE(cfg->chardevTLSx509secretUUID);
+ VIR_FREE(cfg->migrateTLSx509certdir);
+ VIR_FREE(cfg->migrateTLSx509secretUUID);
+
while (cfg->nhugetlbfs) {
cfg->nhugetlbfs--;
VIR_FREE(cfg->hugetlbfs[cfg->nhugetlbfs].mnt_dir);
12:42 p.m.
New subject: [libvirt] [PATCH v2 13/14] qemu: Set up the migrate TLS objects for target
Support TLS for a migration is a multistep process. The target guest must
be started using the "-object tls-creds-x509,endpoint=server,...". If that
TLS object requires a passphrase an addition "-object security..." would
also be created. The alias/id used for the TLS object is "objmigrate_tls0",
while the alias/id used for the security object is "migrate-secret0".
Once the domain is started, the "tls-creds" migration parameter must be
set to the alias/id of the "tls-creds-x509" object.
Once the migration completes, removing the two objects is necessary since
the newly started domain could then become the source of a migration and
thus would not be an endpoint.
Handle the possibility of libvirtd stop/reconnect by saving the fact that
a migration is using TLS was started in a "migrateTLS" boolean.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 7 +-
src/qemu/qemu_domain.h | 4 ++
src/qemu/qemu_migration.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 176 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 40c9dab..af84aac 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -766,7 +766,7 @@ qemuDomainSecretAESClear(qemuDomainSecretAES secret)
}
-static void
+void
qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo)
{
if (!*secinfo)
@@ -1844,6 +1844,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
virBufferEscapeString(buf, "<channelTargetDir
path='%s'/>\n",
priv->channelTargetDir);
+ if (priv->migrateTLS)
+ virBufferAddLit(buf, "<migrateTLS/>\n");
+
return 0;
}
@@ -2112,6 +2115,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
if (qemuDomainSetPrivatePathsOld(driver, vm) < 0)
goto error;
+ priv->migrateTLS = virXPathBoolean("boolean(./migrateTLS)", ctxt) == 1;
+
return 0;
error:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 85f7eb6..9ce8677 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -287,6 +287,7 @@ struct _qemuDomainObjPrivate {
/* for migration's using TLS with a secret (not to be saved in our */
/* private XML). */
qemuDomainSecretInfoPtr migSecinfo;
+ bool migrateTLS;
};
# define QEMU_DOMAIN_PRIVATE(vm) \
@@ -734,6 +735,9 @@ int qemuDomainMasterKeyCreate(virDomainObjPtr vm);
void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv);
+void qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo)
+ ATTRIBUTE_NONNULL(1);
+
void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0db1616..0e95fd9 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1487,6 +1487,145 @@ qemuMigrationEatCookie(virQEMUDriverPtr driver,
return NULL;
}
+
+/* qemuMigrationCheckSetupTLS
+ * @driver: Pointer to driver
+ * @dconn: Connection pointer
+ * @vm: vm object
+ * @flags: migration flags
+ *
+ * Check if flags desired to use TLS and whether it's configured for the
+ * host it's being run on (src or dst depending on caller). If configured
+ * to use a secret for the TLS config, generate and save the migSecinfo.
+ *
+ * Returns 0 on success (or no TLS)
+ */
+static int
+qemuMigrationCheckSetupTLS(virQEMUDriverPtr driver,
+ virConnectPtr dconn,
+ virDomainObjPtr vm,
+ unsigned int flags)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virQEMUDriverConfigPtr cfg = NULL;
+
+ if (flags & VIR_MIGRATE_TLS) {
+ cfg = virQEMUDriverGetConfig(driver);
+
+ if (!cfg->migrateTLS) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("migration TLS not enabled for the host"));
+ goto cleanup;
+ }
+
+ priv->migrateTLS = true;
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir,
+ vm, driver->caps) < 0)
+ VIR_WARN("Failed to save migrateTLS for vm %s",
vm->def->name);
+
+ /* If there's a secret associated with the migrate TLS, then we
+ * need to grab it now while we have the connection. */
+ if (cfg->migrateTLSx509secretUUID &&
+ qemuDomainSecretMigratePrepare(dconn, priv, "migrate",
+ cfg->migrateTLSx509secretUUID) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ virObjectUnref(cfg);
+ return ret;
+}
+
+
+/* qemuMigrationAddTLSObjects
+ * @driver: pointer to qemu driver
+ * @priv: private qemu data
+ * @srcAlias: source alias to be used (migrate or nbd)
+ * @tlsCertDir: where to find certs
+ * @tlsListen: server or client
+ * @tlsVerify: tls verify
+ * @tlsAlias: alias to be generated for TLS object
+ * @secAlias: alias to be generated for a secinfo object
+ * @migParams: migration parameters to set
+ *
+ * Create the TLS objects for the migration and set the migParams value
+ *
+ * Returns 0 on success, -1 on failure
+ */
+static int
+qemuMigrationAddTLSObjects(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ const char *srcAlias,
+ const char *tlsCertDir,
+ bool tlsListen,
+ bool tlsVerify,
+ char **tlsAlias,
+ char **secAlias,
+ qemuMonitorMigrationParamsPtr migParams)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virJSONValuePtr tlsProps = NULL;
+ virJSONValuePtr secProps = NULL;
+
+ if (qemuDomainGetTLSObjects(priv->qemuCaps, priv->migSecinfo,
+ tlsCertDir, tlsListen, tlsVerify,
+ srcAlias, &tlsProps, tlsAlias,
+ &secProps, secAlias) < 0)
+ return -1;
+
+ /* Ensure the domain doesn't already have the TLS objects defined...
+ * This should prevent any issues just in case some cleanup wasn't
+ * properly completed (both src and dst use the same aliases) or
+ * some other error path between now and perform . */
+ qemuDomainDelTLSObjects(driver, vm, *secAlias, *tlsAlias);
+
+ /* Add the migrate TLS objects to the domain */
+ if (qemuDomainAddTLSObjects(driver, vm, *secAlias, &secProps,
+ *tlsAlias, &tlsProps) < 0)
+ return -1;
+
+ migParams->migrateTLSAlias = *tlsAlias;
+
+ return 0;
+}
+
+
+/* qemuMigrationCheckSetupTLS
+ * @driver: Pointer to driver
+ * @cfg: Configuration pointer
+ * @vm: vm object
+ * @tlsAlias: alias to be free'd for TLS object
+ * @secAlias: alias to be free'd for a secinfo object
+ *
+ * Remove the TLS associated objects and memory
+ */
+static void
+qemuMigrationDelTLSObjects(virQEMUDriverPtr driver,
+ virQEMUDriverConfigPtr cfg,
+ virDomainObjPtr vm,
+ char **tlsAlias,
+ char **secAlias)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (!priv->migrateTLS)
+ return;
+
+ qemuDomainDelTLSObjects(driver, vm, *secAlias, *tlsAlias);
+ qemuDomainSecretInfoFree(&priv->migSecinfo);
+ VIR_FREE(*tlsAlias);
+ VIR_FREE(*secAlias);
+
+ priv->migrateTLS = false;
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir,
+ vm, driver->caps) < 0)
+ VIR_WARN("Failed to save migrateTLS on vm %s", vm->def->name);
+}
+
+
static void
qemuMigrationStoreDomainState(virDomainObjPtr vm)
{
@@ -3600,6 +3739,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
{
virDomainObjPtr vm = NULL;
virObjectEventPtr event = NULL;
+ virQEMUDriverConfigPtr cfg = NULL;
int ret = -1;
int dataFD[2] = { -1, -1 };
qemuDomainObjPrivatePtr priv = NULL;
@@ -3613,6 +3753,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
bool stopProcess = false;
bool relabel = false;
int rv;
+ char *tlsAlias = NULL;
+ char *secAlias = NULL;
qemuMonitorMigrationParams migParams = { 0 };
virNWFilterReadLockFilterUpdates();
@@ -3779,6 +3921,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
VIR_QEMU_PROCESS_START_AUTODESTROY) < 0)
goto stopjob;
+ if (qemuMigrationCheckSetupTLS(driver, dconn, vm, flags) < 0)
+ goto stopjob;
+
if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
goto stopjob;
@@ -3806,6 +3951,16 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
compression, &migParams) < 0)
goto stopjob;
+ /* A set only parameter to indicate the "tls-creds-x509" object id */
+ if (priv->migrateTLS) {
+ cfg = virQEMUDriverGetConfig(driver);
+ if (qemuMigrationAddTLSObjects(driver, vm, "migrate",
+ cfg->migrateTLSx509certdir, true,
+ cfg->migrateTLSx509verify,
+ &tlsAlias, &secAlias, &migParams) <
0)
+ goto stopjob;
+ }
+
if (STREQ_NULLABLE(protocol, "rdma") &&
virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10)
< 0) {
goto stopjob;
@@ -3891,6 +4046,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
ret = 0;
cleanup:
+ qemuMigrationDelTLSObjects(driver, cfg, vm, &tlsAlias, &secAlias);
+ virObjectUnref(cfg);
qemuProcessIncomingDefFree(incoming);
VIR_FREE(xmlout);
VIR_FORCE_CLOSE(dataFD[0]);
@@ -6185,6 +6342,15 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
qemuDomainCleanupRemove(vm, qemuMigrationPrepareCleanup);
VIR_FREE(priv->job.completed);
+ /* If for some reason at this point in time something didn't properly
+ * remove the TLS objects, then make one last gasp attempt */
+ if (priv->migrateTLS) {
+ char *tlsAlias = qemuAliasTLSObjFromSrcAlias("migrate");
+ char *secAlias = qemuDomainGetSecretAESAlias("migrate", false);
+
+ qemuMigrationDelTLSObjects(driver, cfg, vm, &tlsAlias, &secAlias);
+ }
+
cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK |
QEMU_MIGRATION_COOKIE_STATS |
QEMU_MIGRATION_COOKIE_NBD;
--
2.9.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 14/14] qemu: Set up the migration TLS objects for source
https://bugzilla.redhat.com/show_bug.cgi?id=1300769
Modify the Begin phase to add the checks to determine whether a migration
wishes to use TLS and whether it's configured including adding the secret
into the priv->migSecinfo for the source domain.
Modify the Perform phase in qemuMigrationRun in order to generate the
TLS objects to be used for the migration and set the migration channel
parameters 'tls-creds' and possibly 'tls-hostname' in order to enable
TLS.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_migration.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0e95fd9..4779d23 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3452,6 +3452,9 @@ qemuMigrationBegin(virConnectPtr conn,
goto endjob;
}
+ if (qemuMigrationCheckSetupTLS(driver, conn, vm, flags) < 0)
+ goto endjob;
+
/* Check if there is any ejected media.
* We don't want to require them on the destination.
*/
@@ -4802,8 +4805,12 @@ qemuMigrationRun(virQEMUDriverPtr driver,
{
int ret = -1;
unsigned int migrate_flags = QEMU_MONITOR_MIGRATE_BACKGROUND;
+ virQEMUDriverConfigPtr cfg = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuMigrationCookiePtr mig = NULL;
+ char *tlsAlias = NULL;
+ char *tlsHostname = NULL;
+ char *secAlias = NULL;
qemuMigrationIOThreadPtr iothread = NULL;
int fd = -1;
unsigned long migrate_speed = resource ? resource : priv->migMaxBandwidth;
@@ -4867,6 +4874,29 @@ qemuMigrationRun(virQEMUDriverPtr driver,
if (qemuDomainMigrateGraphicsRelocate(driver, vm, mig, graphicsuri) < 0)
VIR_WARN("unable to provide data for graphics client relocation");
+ /* If we're using TLS attempt to add the objects */
+ if (priv->migrateTLS) {
+ cfg = virQEMUDriverGetConfig(driver);
+ if (qemuMigrationAddTLSObjects(driver, vm, "migrate",
+ cfg->migrateTLSx509certdir, false,
+ cfg->migrateTLSx509verify,
+ &tlsAlias, &secAlias, migParams) < 0)
+ goto cleanup;
+
+ /* We need to add the tls-hostname only for special circumstances.
+ * When using "fd:" or "exec:", qemu needs to know the
hostname of
+ * the target qemu to correctly validate the x509 certificate
+ * it receives. */
+ if (STREQ(spec->dest.host.protocol, "fd") ||
+ STREQ(spec->dest.host.protocol, "exec")) {
+ if (VIR_STRDUP(tlsHostname, spec->dest.host.name) < 0) {
+ qemuDomainDelTLSObjects(driver, vm, secAlias, tlsAlias);
+ return -1;
+ }
+ migParams->migrateTLSHostname = tlsHostname;
+ }
+ }
+
if (migrate_flags & (QEMU_MONITOR_MIGRATE_NON_SHARED_DISK |
QEMU_MONITOR_MIGRATE_NON_SHARED_INC)) {
if (mig->nbd) {
@@ -5047,6 +5077,10 @@ qemuMigrationRun(virQEMUDriverPtr driver,
ret = -1;
}
+ qemuMigrationDelTLSObjects(driver, cfg, vm, &secAlias, &tlsAlias);
+ VIR_FREE(tlsHostname);
+ virObjectUnref(cfg);
+
if (spec->fwdType != MIGRATION_FWD_DIRECT) {
if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
ret = -1;
--
2.9.3
3:01 a.m.
On Thu, 2017-02-23 at 13:42 -0500, John Ferlan wrote:
v1:
http://www.redhat.com/archives/libvir-list/2017-February/msg00897.html
v1 cover letter reiterated:
Â
Patches 1, 3 -> 9 are primarily quite a bit of code motion in order to allow
reuse of the "core" of the chardev TLS code.
Â
Theoretically speaking of course, these patches should work - I don't
have a TLS and migration environment to test with, so between following
the qemu command model on Daniel's blog and prior experience with the
chardev TLS wouldÂ
Â
I added the saving of a flag to the private qemu domain state, although
I'm not 100% sure it was necessary. At one time I created the source TLS
objects during the Begin phase, but later decided to wait until just
before the migration is run. I think the main reason to have the flag
would be a restart of libvirtd to let 'something' know migration using
TLS was configured. I think it may only be "necessary" in order to
repopulate the migSecinfo after libvirtd restart, but it's not entirely
clear. By the time I started thinking more about while writing this cover
letter it was too late to just remove.
Â
Also rather than create the destination host TLS objects on the fly,
I modified the command line generation. That model could change to adding
the TLS objects once the destination is started and before the params are
set for the migration.
Â
This 'model' is also going to be used for the NBD, but I figured I'd get
this posted now since it was already too long of a series.
These changes are user-visible, and should be documented
in the release notes accordingly.
--Â
Andrea Bolognani / Red Hat / Virtualization
6:46 a.m.
On 02/24/2017 04:01 AM, Andrea Bolognani wrote:
On Thu, 2017-02-23 at 13:42 -0500, John Ferlan wrote:
> v1: http://www.redhat.com/archives/libvir-list/2017-February/msg00897.html
> v1 cover letter reiterated:
>
> Patches 1, 3 -> 9 are primarily quite a bit of code motion in order to allow
> reuse of the "core" of the chardev TLS code.
>
> Theoretically speaking of course, these patches should work - I don't
> have a TLS and migration environment to test with, so between following
> the qemu command model on Daniel's blog and prior experience with the
> chardev TLS would
>
> I added the saving of a flag to the private qemu domain state, although
> I'm not 100% sure it was necessary. At one time I created the source TLS
> objects during the Begin phase, but later decided to wait until just
> before the migration is run. I think the main reason to have the flag
> would be a restart of libvirtd to let 'something' know migration using
> TLS was configured. I think it may only be "necessary" in order to
> repopulate the migSecinfo after libvirtd restart, but it's not entirely
> clear. By the time I started thinking more about while writing this cover
> letter it was too late to just remove.
>
> Also rather than create the destination host TLS objects on the fly,
> I modified the command line generation. That model could change to adding
> the TLS objects once the destination is started and before the params are
> set for the migration.
>
> This 'model' is also going to be used for the NBD, but I figured I'd get
> this posted now since it was already too long of a series.
These changes are user-visible, and should be documented
in the release notes accordingly.
Yes I know - depends on "when" then get reviewed and ACK'd too. There
are parts of the series that are essentially code motion - so I made
conscious decision to wait.
John
--
Andrea Bolognani / Red Hat / Virtualization
7:48 a.m.
On Thu, Feb 23, 2017 at 13:42:02 -0500, John Ferlan wrote:
AFAIU - removal of the objects would remove the migration tls-creds,
tls-hostname settings.
This would be a very strange behavior.
I left the cfg->migrateTLS in for now - it's very simple to
remove, but
there would still need to be a key on something to ensure the migrateTLS
environment has been properly configured since that would mean the default
environment would need to be used/configured. Setting up the default
environment is keyed off having the migrateTLS defined. That's all part
of the qemu_conf reading logic.
I still don't see the value in keeping migrate_tls option in qemu.conf.
If we want to check the environment is setup correctly we should check
the certificates are in place rather than relying on the option. The
error reported when migrate_tls is set, but the environment is not
prepared is:
internal error: unable to execute QEMU command 'object-add': Unable
to access credentials /etc/pki/qemu/ca-cert.pem: No such file or
directory
Not ideal but at least it names the file which is missing.
John Ferlan (14):
qemu: Introduce qemuDomainSecretInfoNew
qemu: Introduce qemuDomainSecretMigratePrepare
qemu: Move exit monitor calls in failure paths
qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
qemu: Refactor qemuDomainGetChardevTLSObjects to converge code
qemu: Move qemuDomainSecretChardevPrepare call
qemu: Move qemuDomainPrepareChardevSourceTLS call
qemu: Introduce qemuDomainGetTLSObjects
The eight patches above are mostly OK except for some small issues.
But the rest of the series needs more work...
qemu: Add TLS params to _qemuMonitorMigrationParams
Add new migration flag VIR_MIGRATE_TLS
qemu: Create #define for TLS configuration setup.
conf: Introduce migrate_tls_x509_cert_dir
qemu: Set up the migrate TLS objects for target
qemu: Set up the migration TLS objects for source
The code should check whether QEMU actually supports TLS migration,
otherwise you will get
internal error: unable to execute QEMU command
'migrate-set-parameters': Invalid parameter 'tls-creds'
As I mentioned in my v1 review, we should always set the parameters if
QEMU supports them to make sure they don't contain any leftovers from a
previous migration. I also said we should properly clean the TLS stuff
somewhere inside qemuProcessRecoverMigration*. And if we do it in
addition to properly cleaning up everything in the Finish and Confirm
phases, we should not need to store migrateTLS in the status XML. We can
just always do the cleanup if QEMU supports TLS migration.
Anyway, TLS migration doesn't work even if it is properly configured:
operation failed: migration job: unexpectedly failed
And the destination QEMU log contains:
qemu-system-x86_64: No TLS credentials with id 'objmigrate_tls0'
The reason is an interesting flow of QMP commands issued by libvirt
during the Prepare phase:
{
"execute": "object-add",
"arguments": {
"qom-type": "tls-creds-x509",
"id": "objmigrate_tls0",
"props": {
"dir": "/etc/pki/libvirt",
"endpoint": "server",
"verify-peer": false
}
},
"id": "libvirt-21"
}
{
"execute": "migrate-set-parameters",
"arguments": {
"tls-creds": "objmigrate_tls0"
},
"id": "libvirt-26"
}
{
"execute": "migrate-incoming",
"arguments": {
"uri": "tcp:[::]:49152"
},
"id": "libvirt-27"
}
{
"execute": "object-del",
"arguments": {
"id": "objmigrate_tls0"
},
"id": "libvirt-28"
}
The error reported after you deleted the objmigrate_tls0 object doesn't
seem to confirm your idea about migration parameters begin unset when
the object is removed.
Please, test your series before you send a new version of it.
Jirka
7:53 a.m.
On Tue, Feb 28, 2017 at 02:48:19PM +0100, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:02 -0500, John Ferlan wrote:
> AFAIU - removal of the objects would remove the migration tls-creds,
> tls-hostname settings.
This would be a very strange behavior.
> I left the cfg->migrateTLS in for now - it's very simple to remove, but
> there would still need to be a key on something to ensure the migrateTLS
> environment has been properly configured since that would mean the default
> environment would need to be used/configured. Setting up the default
> environment is keyed off having the migrateTLS defined. That's all part
> of the qemu_conf reading logic.
I still don't see the value in keeping migrate_tls option in qemu.conf.
If we want to check the environment is setup correctly we should check
the certificates are in place rather than relying on the option. The
error reported when migrate_tls is set, but the environment is not
prepared is:
internal error: unable to execute QEMU command 'object-add': Unable
to access credentials /etc/pki/qemu/ca-cert.pem: No such file or
directory
Not ideal but at least it names the file which is missing.
> John Ferlan (14):
> qemu: Introduce qemuDomainSecretInfoNew
> qemu: Introduce qemuDomainSecretMigratePrepare
> qemu: Move exit monitor calls in failure paths
> qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
> qemu: Refactor qemuDomainGetChardevTLSObjects to converge code
> qemu: Move qemuDomainSecretChardevPrepare call
> qemu: Move qemuDomainPrepareChardevSourceTLS call
> qemu: Introduce qemuDomainGetTLSObjects
The eight patches above are mostly OK except for some small issues.
But the rest of the series needs more work...
> qemu: Add TLS params to _qemuMonitorMigrationParams
> Add new migration flag VIR_MIGRATE_TLS
> qemu: Create #define for TLS configuration setup.
> conf: Introduce migrate_tls_x509_cert_dir
> qemu: Set up the migrate TLS objects for target
> qemu: Set up the migration TLS objects for source
The code should check whether QEMU actually supports TLS migration,
otherwise you will get
internal error: unable to execute QEMU command
'migrate-set-parameters': Invalid parameter 'tls-creds'
As I mentioned in my v1 review, we should always set the parameters if
QEMU supports them to make sure they don't contain any leftovers from a
previous migration. I also said we should properly clean the TLS stuff
somewhere inside qemuProcessRecoverMigration*. And if we do it in
addition to properly cleaning up everything in the Finish and Confirm
phases, we should not need to store migrateTLS in the status XML. We can
just always do the cleanup if QEMU supports TLS migration.
Anyway, TLS migration doesn't work even if it is properly configured:
operation failed: migration job: unexpectedly failed
And the destination QEMU log contains:
qemu-system-x86_64: No TLS credentials with id 'objmigrate_tls0'
The reason is an interesting flow of QMP commands issued by libvirt
during the Prepare phase:
{
"execute": "object-add",
"arguments": {
"qom-type": "tls-creds-x509",
"id": "objmigrate_tls0",
"props": {
"dir": "/etc/pki/libvirt",
"endpoint": "server",
"verify-peer": false
}
},
"id": "libvirt-21"
}
{
"execute": "migrate-set-parameters",
"arguments": {
"tls-creds": "objmigrate_tls0"
},
"id": "libvirt-26"
}
{
"execute": "migrate-incoming",
"arguments": {
"uri": "tcp:[::]:49152"
},
"id": "libvirt-27"
}
{
"execute": "object-del",
"arguments": {
"id": "objmigrate_tls0"
},
"id": "libvirt-28"
}
The error reported after you deleted the objmigrate_tls0 object doesn't
seem to confirm your idea about migration parameters begin unset when
the object is removed.
The problem here is that 'migrate-incoming' merely sets up a TCP
listener, it doesn't actually initiate a connection or TLS handshake.
The object-del thus deletes the TLS creds before the src QEMU has had
a chance to even connect.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
10:07 a.m.
On 02/28/2017 08:48 AM, Jiri Denemark wrote:
On Thu, Feb 23, 2017 at 13:42:02 -0500, John Ferlan wrote:
> AFAIU - removal of the objects would remove the migration tls-creds,
> tls-hostname settings.
This would be a very strange behavior.
> I left the cfg->migrateTLS in for now - it's very simple to remove, but
> there would still need to be a key on something to ensure the migrateTLS
> environment has been properly configured since that would mean the default
> environment would need to be used/configured. Setting up the default
> environment is keyed off having the migrateTLS defined. That's all part
> of the qemu_conf reading logic.
I still don't see the value in keeping migrate_tls option in qemu.conf.
If we want to check the environment is setup correctly we should check
the certificates are in place rather than relying on the option. The
error reported when migrate_tls is set, but the environment is not
prepared is:
internal error: unable to execute QEMU command 'object-add': Unable
to access credentials /etc/pki/qemu/ca-cert.pem: No such file or
directory
Not ideal but at least it names the file which is missing.
In my latest branch I've removed migrate_tls.
> John Ferlan (14):
> qemu: Introduce qemuDomainSecretInfoNew
> qemu: Introduce qemuDomainSecretMigratePrepare
> qemu: Move exit monitor calls in failure paths
> qemu: Refactor hotplug to introduce qemuDomain{Add|Del}TLSObjects
> qemu: Refactor qemuDomainGetChardevTLSObjects to converge code
> qemu: Move qemuDomainSecretChardevPrepare call
> qemu: Move qemuDomainPrepareChardevSourceTLS call
> qemu: Introduce qemuDomainGetTLSObjects
The eight patches above are mostly OK except for some small issues.
OK - I will separate them out with the latest adjustments from review
and repost.
But the rest of the series needs more work...
> qemu: Add TLS params to _qemuMonitorMigrationParams
> Add new migration flag VIR_MIGRATE_TLS
> qemu: Create #define for TLS configuration setup.
> conf: Introduce migrate_tls_x509_cert_dir
> qemu: Set up the migrate TLS objects for target
> qemu: Set up the migration TLS objects for source
The code should check whether QEMU actually supports TLS migration,
otherwise you will get
internal error: unable to execute QEMU command
'migrate-set-parameters': Invalid parameter 'tls-creds'
Hmm... I see tls-creds added to migrate-set-parameters in 2.7 while
they were added to ChardevSocket in 2.6... Ugh. Have to refresh my
recollection of how to get the answer I need from capabilities.
As I mentioned in my v1 review, we should always set the parameters
if
QEMU supports them to make sure they don't contain any leftovers from a
previous migration.
I see from a quick scan of the qemu code though that it appears as if
the code checks for a non null value being passed:
params->has_tls_creds = !!s->parameters.tls_creds;
params->has_tls_hostname = !!s->parameters.tls_hostname;
So in order to "allow" clearing the tls_creds and tls_hostname, what
would one do? The clearing would be necessary since a target of a
migration will become a source for a migration and the tls-creds object
would be different (using endpoint={server|client}).
This would be the I used TLS on one migration, but not on the next type
operation for the same domain. Doesn't seem we can assume that TLS will
be used for every migration since the desire is to have usage controlled
via API option and not host config option
I also said we should properly clean the TLS stuff
somewhere inside qemuProcessRecoverMigration*. And if we do it in
addition to properly cleaning up everything in the Finish and Confirm
phases, we should not need to store migrateTLS in the status XML. We can
just always do the cleanup if QEMU supports TLS migration.
Anyway, TLS migration doesn't work even if it is properly configured:
operation failed: migration job: unexpectedly failed
And the destination QEMU log contains:
qemu-system-x86_64: No TLS credentials with id 'objmigrate_tls0'
The reason is an interesting flow of QMP commands issued by libvirt
during the Prepare phase:
{
"execute": "object-add",
"arguments": {
"qom-type": "tls-creds-x509",
"id": "objmigrate_tls0",
"props": {
"dir": "/etc/pki/libvirt",
"endpoint": "server",
"verify-peer": false
}
},
"id": "libvirt-21"
}
{
"execute": "migrate-set-parameters",
"arguments": {
"tls-creds": "objmigrate_tls0"
},
"id": "libvirt-26"
}
{
"execute": "migrate-incoming",
"arguments": {
"uri": "tcp:[::]:49152"
},
"id": "libvirt-27"
}
{
"execute": "object-del",
"arguments": {
"id": "objmigrate_tls0"
},
"id": "libvirt-28"
}
The error reported after you deleted the objmigrate_tls0 object doesn't
seem to confirm your idea about migration parameters begin unset when
the object is removed.
OK - well obviously there's still quite a bit for me to understand about
the migration model.
Please, test your series before you send a new version of it.
Yep - something I noted in my cover letter - the need for a test
environment... Hopefully I'll find a kind soul that will allow me to
have access to an already configured environment to test with...
John
5:57 a.m.
On Tue, Feb 28, 2017 at 11:07:21 -0500, John Ferlan wrote:
On 02/28/2017 08:48 AM, Jiri Denemark wrote:
> The code should check whether QEMU actually supports TLS migration,
> otherwise you will get
>
> internal error: unable to execute QEMU command
> 'migrate-set-parameters': Invalid parameter 'tls-creds'
>
Hmm... I see tls-creds added to migrate-set-parameters in 2.7 while
they were added to ChardevSocket in 2.6... Ugh. Have to refresh my
recollection of how to get the answer I need from capabilities.
query-migrate-parameters used by qemuMonitorJSONGetMigrationParams is
your friend. And that's the reason why I said you actually might need
the *_set variables.
> As I mentioned in my v1 review, we should always set the
parameters if
> QEMU supports them to make sure they don't contain any leftovers from a
> previous migration.
I see from a quick scan of the qemu code though that it appears as if
the code checks for a non null value being passed:
params->has_tls_creds = !!s->parameters.tls_creds;
params->has_tls_hostname = !!s->parameters.tls_hostname;
So in order to "allow" clearing the tls_creds and tls_hostname, what
would one do?
I was told Daniel should be the right person to ask.
> {
> "execute": "migrate-incoming",
> "arguments": {
> "uri": "tcp:[::]:49152"
> },
> "id": "libvirt-27"
> }
>
> {
> "execute": "object-del",
> "arguments": {
> "id": "objmigrate_tls0"
> },
> "id": "libvirt-28"
> }
>
> The error reported after you deleted the objmigrate_tls0 object doesn't
> seem to confirm your idea about migration parameters begin unset when
> the object is removed.
OK - well obviously there's still quite a bit for me to understand about
the migration model.
I think you had this part correctly in the previous version. The Prepare
phase should only call qemuMigrationDelTLSObjects in case of error. The
Finish phase is where all the cleanup after a migration is done.
> Please, test your series before you send a new version of it.
Yep - something I noted in my cover letter - the need for a test
environment... Hopefully I'll find a kind soul that will allow me to
have access to an already configured environment to test with...
Setting up migration environment is trivial. You just need two hosts or
two VMs. The easiest way is configuring libvirt to listen on TCP with no
authorization and open the port on both firewalls. Then you only need a
domain to migrate. For most cases it doesn't even need a disk or you
can use a live CD image stored in the same path on both hosts. It's not
required to setup hostnames and make them resolvable from both hosts,
but you can avoid an extra argument to virsh migrate if you do so.
Setting up TLS is not hard either, you can follow
https://kashyapc.fedorapeople.org/gnutls-pki-setup.txt or you can use
easy-rsa.
Jirka
6:29 a.m.
On Wed, Mar 01, 2017 at 12:57:13 +0100, Jiri Denemark wrote:
On Tue, Feb 28, 2017 at 11:07:21 -0500, John Ferlan wrote:
> On 02/28/2017 08:48 AM, Jiri Denemark wrote:
> > The code should check whether QEMU actually supports TLS migration,
> > otherwise you will get
> >
> > internal error: unable to execute QEMU command
> > 'migrate-set-parameters': Invalid parameter 'tls-creds'
> >
>
> Hmm... I see tls-creds added to migrate-set-parameters in 2.7 while
> they were added to ChardevSocket in 2.6... Ugh. Have to refresh my
> recollection of how to get the answer I need from capabilities.
query-migrate-parameters used by qemuMonitorJSONGetMigrationParams is
your friend. And that's the reason why I said you actually might need
the *_set variables.
Oh, but you can't use query-migrate-parameters because the TLS
parameters are not reported when they are not set. This looks like
another thing which needs to be fixed in QEMU.
Jirka
9:14 a.m.
On 03/01/2017 06:57 AM, Jiri Denemark wrote:
On Tue, Feb 28, 2017 at 11:07:21 -0500, John Ferlan wrote:
> On 02/28/2017 08:48 AM, Jiri Denemark wrote:
>> The code should check whether QEMU actually supports TLS migration,
>> otherwise you will get
>>
>> internal error: unable to execute QEMU command
>> 'migrate-set-parameters': Invalid parameter 'tls-creds'
>>
>
> Hmm... I see tls-creds added to migrate-set-parameters in 2.7 while
> they were added to ChardevSocket in 2.6... Ugh. Have to refresh my
> recollection of how to get the answer I need from capabilities.
query-migrate-parameters used by qemuMonitorJSONGetMigrationParams is
your friend. And that's the reason why I said you actually might need
the *_set variables.
That jiggled a memory strand... It really wasn't clear from reading
QEMU's qapi-schema.json description that the Get would return anything
for tls-{creds,hostname}.
So for determining whether the option exists or not I'm left to other
options it seems. Even if code is added (in 2.9) to provide something
like an empty string - that'd have to be backported to 2.8 and 2.7 and
we'd have to somehow ensure/hope that was applied so that the right
answer could be returned from Get...
>> As I mentioned in my v1 review, we should always set the
parameters if
>> QEMU supports them to make sure they don't contain any leftovers from a
>> previous migration.
>
> I see from a quick scan of the qemu code though that it appears as if
> the code checks for a non null value being passed:
>
> params->has_tls_creds = !!s->parameters.tls_creds;
> params->has_tls_hostname = !!s->parameters.tls_hostname;
>
> So in order to "allow" clearing the tls_creds and tls_hostname, what
> would one do?
I was told Daniel should be the right person to ask.
>> {
>> "execute": "migrate-incoming",
>> "arguments": {
>> "uri": "tcp:[::]:49152"
>> },
>> "id": "libvirt-27"
>> }
>>
>> {
>> "execute": "object-del",
>> "arguments": {
>> "id": "objmigrate_tls0"
>> },
>> "id": "libvirt-28"
>> }
>>
>> The error reported after you deleted the objmigrate_tls0 object doesn't
>> seem to confirm your idea about migration parameters begin unset when
>> the object is removed.
>
> OK - well obviously there's still quite a bit for me to understand about
> the migration model.
I think you had this part correctly in the previous version. The Prepare
phase should only call qemuMigrationDelTLSObjects in case of error. The
Finish phase is where all the cleanup after a migration is done.
I'll look at what I changed - last week was a blur and a wedding with an
open bar over the weekend while the cure for many things has resulted in
a few less brain cells to recall my frame of reference!
>> Please, test your series before you send a new version of
it.
>
> Yep - something I noted in my cover letter - the need for a test
> environment... Hopefully I'll find a kind soul that will allow me to
> have access to an already configured environment to test with...
Setting up migration environment is trivial. You just need two hosts or
two VMs. The easiest way is configuring libvirt to listen on TCP with no
authorization and open the port on both firewalls. Then you only need a
domain to migrate. For most cases it doesn't even need a disk or you
can use a live CD image stored in the same path on both hosts. It's not
required to setup hostnames and make them resolvable from both hosts,
but you can avoid an extra argument to virsh migrate if you do so.
Setting up TLS is not hard either, you can follow
https://kashyapc.fedorapeople.org/gnutls-pki-setup.txt or you can use
easy-rsa.
Jirka
Sounds easy if you've done it before many times... It's not something I
do every day nor have I done once for libvirt before... ;-) I assumed
one really needed two hosts - I assume configuring two guests means
setting up nested virt, but that's something else I haven't done... and
the whole listen, open ports on firewalls makes my head spin.
John
9:33 a.m.
On Wed, Mar 01, 2017 at 10:14:10 -0500, John Ferlan wrote:
On 03/01/2017 06:57 AM, Jiri Denemark wrote:
That jiggled a memory strand... It really wasn't clear from reading
QEMU's qapi-schema.json description that the Get would return anything
for tls-{creds,hostname}.
So for determining whether the option exists or not I'm left to other
options it seems. Even if code is added (in 2.9) to provide something
like an empty string - that'd have to be backported to 2.8 and 2.7 and
we'd have to somehow ensure/hope that was applied so that the right
answer could be returned from Get...
Well, unless we have a way to reset the parameters we can't really use
TLS migration anyway.
> I think you had this part correctly in the previous version. The
Prepare
> phase should only call qemuMigrationDelTLSObjects in case of error. The
> Finish phase is where all the cleanup after a migration is done.
I'll look at what I changed
The previous version specified the objects on QEMU command line when. I
think calling qemuMigrationDelTLSObjectsin the cleanup part of
qemuMigrationPrepareAny only in case of error should fix the issue (I
haven't tried it though).
Sounds easy if you've done it before many times... It's not
something I
do every day nor have I done once for libvirt before... ;-) I assumed
one really needed two hosts - I assume configuring two guests means
setting up nested virt, but that's something else I haven't done...
Not necessarily. You can start a type="qemu" domain inside the guests.
Anyway, to enable nested virt, just load kvm-intel module with nested=1
(AMD should have nested enabled by default) and then you need to make
sure vmx (or svm for AMD) CPU feature is enabled. For example, with the
following CPU XML added to the guests' definition:
<cpu>
<model>Skylake-Client</model>
<feature policy='require' name='vmx'/>
</cpu>
Jirka
11:47 a.m.
On 03/01/2017 10:33 AM, Jiri Denemark wrote:
On Wed, Mar 01, 2017 at 10:14:10 -0500, John Ferlan wrote:
> On 03/01/2017 06:57 AM, Jiri Denemark wrote:
> That jiggled a memory strand... It really wasn't clear from reading
> QEMU's qapi-schema.json description that the Get would return anything
> for tls-{creds,hostname}.
>
> So for determining whether the option exists or not I'm left to other
> options it seems. Even if code is added (in 2.9) to provide something
> like an empty string - that'd have to be backported to 2.8 and 2.7 and
> we'd have to somehow ensure/hope that was applied so that the right
> answer could be returned from Get...
Well, unless we have a way to reset the parameters we can't really use
TLS migration anyway.
Based on the qemu patch I see - the only way we'll know is by knowing
which version the patch has been applied.
We could set NULL or "", but that would seem to cause errors in versions
between 2.7 and wherever the fix ends up. <sigh>
While I'm thinking about these types of things... I started down the NBD
path too. The server side would seem to be fairly trivial adding the
tls-creds to the command line; however, the client side is a bit more
tricky.
Currently (if I read the code right), NBD would use 'drive-mirror'
passing along the "*file" parameter as "nbd:%s:%d:exportname=%s",
where
the first %s is the hostname, the %d is the port, and the exportname is
the diskAlias (see nbd_dest in qemuMigrationDriveMirror).
If I look at the qemu end of that - it will parse that nbd: prefixed
string, but the parsing code doesn't accept a tls-creds type parameter.
So more research leads me to a qemu-devel conversation :
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg07721.html
which in the long run I think implies libvirt should be using
blockdev-mirror instead. Of course that gets bogged down in a discussion
about blockdev-add, but let's say blockdev-mirror was usable. How then
would the tls-creds be passed? I see them added to the
@BlockdevOptionsNbd in QEMU's block-core.json, but it's not very clear
how they'd be provided. I assume some sort of json buffer magic, but
that's purely a guess. It's certainly not as simple as providing it
using the "-drive driver=nbd,host..." from as described on Daniel's TLS
blog post:
https://www.berrange.com/posts/2016/04/05/improving-qemu-security-part-5-...
Tks for any feedback -
John
11:57 a.m.
On Wed, Mar 01, 2017 at 12:47:18PM -0500, John Ferlan wrote:
On 03/01/2017 10:33 AM, Jiri Denemark wrote:
> On Wed, Mar 01, 2017 at 10:14:10 -0500, John Ferlan wrote:
>> On 03/01/2017 06:57 AM, Jiri Denemark wrote:
>> That jiggled a memory strand... It really wasn't clear from reading
>> QEMU's qapi-schema.json description that the Get would return anything
>> for tls-{creds,hostname}.
>>
>> So for determining whether the option exists or not I'm left to other
>> options it seems. Even if code is added (in 2.9) to provide something
>> like an empty string - that'd have to be backported to 2.8 and 2.7 and
>> we'd have to somehow ensure/hope that was applied so that the right
>> answer could be returned from Get...
>
> Well, unless we have a way to reset the parameters we can't really use
> TLS migration anyway.
>
Based on the qemu patch I see - the only way we'll know is by knowing
which version the patch has been applied.
We could set NULL or "", but that would seem to cause errors in versions
between 2.7 and wherever the fix ends up. <sigh>
While I'm thinking about these types of things... I started down the NBD
path too. The server side would seem to be fairly trivial adding the
tls-creds to the command line; however, the client side is a bit more
tricky.
Currently (if I read the code right), NBD would use 'drive-mirror'
passing along the "*file" parameter as "nbd:%s:%d:exportname=%s",
where
the first %s is the hostname, the %d is the port, and the exportname is
the diskAlias (see nbd_dest in qemuMigrationDriveMirror).
If I look at the qemu end of that - it will parse that nbd: prefixed
string, but the parsing code doesn't accept a tls-creds type parameter.
Yep, we can't use the legacy URI syntax - we need the block dev property
syntax.
So more research leads me to a qemu-devel conversation :
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg07721.html
which in the long run I think implies libvirt should be using
blockdev-mirror instead. Of course that gets bogged down in a discussion
about blockdev-add, but let's say blockdev-mirror was usable. How then
IIRC, the conclusion was that its possible use blockdev-mirror, without
using blockdev-add
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg07748.html
would the tls-creds be passed? I see them added to the
@BlockdevOptionsNbd in QEMU's block-core.json, but it's not very clear
how they'd be provided. I assume some sort of json buffer magic, but
that's purely a guess. It's certainly not as simple as providing it
using the "-drive driver=nbd,host..." from as described on Daniel's TLS
blog post:
The 'tls-creds' parameter in @BlockdevOptionsNbd just refers to the ID
of the TLS credentials object previously created with object-add. In
fact we should use the same TLS credentials for both the migration
server/client and the associated NBD server/client.
IIRC, in JSon, @BlockdevOptionsNbd ends up looking something like
{
'driver': 'nbd',
'data': {
'server': {
'inet': {
'host': 'foobar'
'port': '9000'
}
},
'tls-creds': 'tls0'
}
(Not entirely sure if i need another level of nesting in between the
'server' and 'inet' bit or not ).
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
4:12 a.m.
On Wed, Mar 01, 2017 at 04:33:36PM +0100, Jiri Denemark wrote:
On Wed, Mar 01, 2017 at 10:14:10 -0500, John Ferlan wrote:
[...]
Well, unless we have a way to reset the parameters we can't
really use
TLS migration anyway.
> > I think you had this part correctly in the previous version. The Prepare
> > phase should only call qemuMigrationDelTLSObjects in case of error. The
> > Finish phase is where all the cleanup after a migration is done.
>
> I'll look at what I changed
The previous version specified the objects on QEMU command line when. I
think calling qemuMigrationDelTLSObjectsin the cleanup part of
qemuMigrationPrepareAny only in case of error should fix the issue (I
haven't tried it though).
> Sounds easy if you've done it before many times... It's not something I
> do every day nor have I done once for libvirt before... ;-) I assumed
> one really needed two hosts - I assume configuring two guests means
> setting up nested virt, but that's something else I haven't done...
Not necessarily. You can start a type="qemu" domain inside the guests.
Anyway, to enable nested virt, just load kvm-intel module with nested=1
(AMD should have nested enabled by default) and then you need to make
sure vmx (or svm for AMD) CPU feature is enabled. For example, with the
following CPU XML added to the guests' definition:
<cpu>
<model>Skylake-Client</model>
<feature policy='require' name='vmx'/>
</cpu>
Or if desired pass-through the host CPU model:
$ virt-xml guest-hyp --edit --cpu host-passthrough,clearxml=yes
Reboot, and don't forget to check /dev/kvm char device exists inside the
L1 guest.
Full notes:
https://kashyapc.fedorapeople.org/virt/procedure-to-enable-nested-virt-on...
--
/kashyap
6:18 a.m.
On Tue, Feb 28, 2017 at 11:07:21AM -0500, John Ferlan wrote:
> As I mentioned in my v1 review, we should always set the
parameters if
> QEMU supports them to make sure they don't contain any leftovers from a
> previous migration.
I see from a quick scan of the qemu code though that it appears as if
the code checks for a non null value being passed:
params->has_tls_creds = !!s->parameters.tls_creds;
params->has_tls_hostname = !!s->parameters.tls_hostname;
That code is in the function for querying whether tls parameters are
currently set.
So in order to "allow" clearing the tls_creds and
tls_hostname, what
would one do? The clearing would be necessary since a target of a
migration will become a source for a migration and the tls-creds object
would be different (using endpoint={server|client}).
Hmm, I see this is a limitation of the migrate-set-parameters method. You
can set new parameters for tls_creds / tls_hostname, but you can't fully
delete the old parameters.
The tls_hostname is only set on the source host of the migration and that
VM will be killed off upon successful migration. The problem only arises
if you have a migration that fails, and you then try to migrate that same
VM again later, *and* you don't have tls_hostname set. I don't think that'd
hit libvirt, since libvirt will always need to set tls-hostname as it uses
fd: migrate method. IOW, I don't see any need to be able to clear
tls-hostname when used with libvirt.
For TLS creds it would be a problem if we do a TLS migration and then need
to migrate the target QEMU again later, but don't want TLS used, as that
would require us to fully clear the tls_creds parameter. At best we can
set it to empty string, which is not good enough. So seems we need a QEMU
fix here.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
6:39 a.m.
On Wed, Mar 01, 2017 at 12:18:23 +0000, Daniel P. Berrange wrote:
Hmm, I see this is a limitation of the migrate-set-parameters method.
You
can set new parameters for tls_creds / tls_hostname, but you can't fully
delete the old parameters.
The tls_hostname is only set on the source host of the migration and that
VM will be killed off upon successful migration. The problem only arises
if you have a migration that fails, and you then try to migrate that same
VM again later, *and* you don't have tls_hostname set. I don't think that'd
hit libvirt, since libvirt will always need to set tls-hostname as it uses
fd: migrate method. IOW, I don't see any need to be able to clear
tls-hostname when used with libvirt.
But we will only set it if TLS is turned on. We certainly don't want to
use this parameter when saving a domain to a file, making a snapshot, or
migrating without TLS.
For TLS creds it would be a problem if we do a TLS migration and then
need
to migrate the target QEMU again later, but don't want TLS used, as that
would require us to fully clear the tls_creds parameter. At best we can
set it to empty string, which is not good enough. So seems we need a QEMU
fix here.
Another problem (I mentioned in another email in this series) is
query-migrate-parameters does not show the tls parameters unless they
are set. So it looks like we need to invent a special value which can be
reported when they are unset and we could use the same special value to
reset them. An empty string sounds like an obvious candidate.
Jirka
6:51 a.m.
On Wed, Mar 01, 2017 at 01:39:18PM +0100, Jiri Denemark wrote:
On Wed, Mar 01, 2017 at 12:18:23 +0000, Daniel P. Berrange wrote:
> Hmm, I see this is a limitation of the migrate-set-parameters method. You
> can set new parameters for tls_creds / tls_hostname, but you can't fully
> delete the old parameters.
>
> The tls_hostname is only set on the source host of the migration and that
> VM will be killed off upon successful migration. The problem only arises
> if you have a migration that fails, and you then try to migrate that same
> VM again later, *and* you don't have tls_hostname set. I don't think
that'd
> hit libvirt, since libvirt will always need to set tls-hostname as it uses
> fd: migrate method. IOW, I don't see any need to be able to clear
> tls-hostname when used with libvirt.
But we will only set it if TLS is turned on. We certainly don't want to
use this parameter when saving a domain to a file, making a snapshot, or
migrating without TLS.
If you don't have TLS enabled, then whether tls-hostname is set or not
is irrelevant - it'll never be used. So the fact that tls-hostname may
be still mistakenly set, when you later save to a file is harmless, as
long as tls-creds is unset.
> For TLS creds it would be a problem if we do a TLS migration and
then need
> to migrate the target QEMU again later, but don't want TLS used, as that
> would require us to fully clear the tls_creds parameter. At best we can
> set it to empty string, which is not good enough. So seems we need a QEMU
> fix here.
Another problem (I mentioned in another email in this series) is
query-migrate-parameters does not show the tls parameters unless they
are set. So it looks like we need to invent a special value which can be
reported when they are unset and we could use the same special value to
reset them. An empty string sounds like an obvious candidate.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
7 a.m.
On Wed, Mar 01, 2017 at 12:51:32 +0000, Daniel P. Berrange wrote:
On Wed, Mar 01, 2017 at 01:39:18PM +0100, Jiri Denemark wrote:
> On Wed, Mar 01, 2017 at 12:18:23 +0000, Daniel P. Berrange wrote:
> > Hmm, I see this is a limitation of the migrate-set-parameters method. You
> > can set new parameters for tls_creds / tls_hostname, but you can't fully
> > delete the old parameters.
> >
> > The tls_hostname is only set on the source host of the migration and that
> > VM will be killed off upon successful migration. The problem only arises
> > if you have a migration that fails, and you then try to migrate that same
> > VM again later, *and* you don't have tls_hostname set. I don't think
that'd
> > hit libvirt, since libvirt will always need to set tls-hostname as it uses
> > fd: migrate method. IOW, I don't see any need to be able to clear
> > tls-hostname when used with libvirt.
>
> But we will only set it if TLS is turned on. We certainly don't want to
> use this parameter when saving a domain to a file, making a snapshot, or
> migrating without TLS.
If you don't have TLS enabled, then whether tls-hostname is set or not
is irrelevant - it'll never be used. So the fact that tls-hostname may
be still mistakenly set, when you later save to a file is harmless, as
long as tls-creds is unset.
OK, makes sense. Although it also makes sense for the two parameters to
provide the same behavior.
> > For TLS creds it would be a problem if we do a TLS
migration and then need
> > to migrate the target QEMU again later, but don't want TLS used, as that
> > would require us to fully clear the tls_creds parameter. At best we can
> > set it to empty string, which is not good enough. So seems we need a QEMU
> > fix here.
>
> Another problem (I mentioned in another email in this series) is
> query-migrate-parameters does not show the tls parameters unless they
> are set. So it looks like we need to invent a special value which can be
> reported when they are unset and we could use the same special value to
> reset them. An empty string sounds like an obvious candidate.
As Pavel mentioned offline, JSON supports null value. So it could be an
option too.
Jirka
2822
days inactive
2829
days old
39 comments
5 participants
participants (5)
-
Andrea Bolognani -
Daniel P. Berrange -
Jiri Denemark -
John Ferlan -
Kashyap Chamarthy