9:17 a.m.
v1: http://www.redhat.com/archives/libvir-list/2016-April/msg00596.html
Differences since v1:
- Add qemuBuildiSCSICommandLine (and BuildDiskiSCSI && BuildHostdeviSCSI)
These will do the magic necessary in order to support IV secret objects
for the impending iSCSI -drive argument. This API doesn't require any
qemu patches in order to work AFAICT. I also determined that the "id="
*isn't* required for an '-iscsi ...' argument, which made using the
complete 'path' string for 'initiator-name' possible. The other option
was to break it up and pass the "iqn.*" string as the initiator-name
and a "modified" remaining string as the "id=" parameter. The
modified
would be to ensure only alphanumeric, '-', '.', and '_'
characters are
in the 'id=' string.
- Fix up some logic found while actually working through the tests. Some
of it related to what was found for the 'iscsi' options. A couple of
other minor nits.
- Add tests and mocks for virRandomBytes and gnutls_rnd (note: the former
could be used to "randomly" (hah!) generate a UUID of all '0xff').
A mock of 'gnutls_encrypt' is not necessary since, it can only be called
if the function gnutls_encrypt exists *and* we have a secret object
capability. Not having a mock function allows us to validate that
gnutls_encrypt actually generates a value we expect based on some
less than stellar and totally non random key's!
- Remove the hotplug IV code (I've saved it off for future expansion).
Although not needing to do hotplug probably means patches 6-9 are not
required, but still I think better than the existing so I kept them
even though they have nothing to do with IV secrets (they'd need to
go in after patches 1-5 anyways).
- Ran the changes through the coverity checker...
John Ferlan (12):
qemu: Introduce qemuDomainSecretInfo
qemu: Introduce qemuDomainSecretPrepare and Destroy
qemu: Introduce qemuDomainHostdevPrivatePtr
qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI
qemu: hotplug: Assume support for -device for attach virtio disk
qemu: hotplug: Adjust error path for attach scsi disk
qemu: hotplug: Adjust error path for attach virtio disk
qemu: hotplug: Adjust error path for attach hostdev scsi disk
qemu: hotplug: Fix possible memory leak of props
qemu: Introduce qemuDomainSecretIV
qemu: Utilize qemu secret objects for SCSI/RBD auth/secret
configure.ac | 1 +
src/conf/domain_conf.c | 33 +-
src/conf/domain_conf.h | 5 +-
src/lxc/lxc_native.c | 4 +-
src/qemu/qemu_alias.c | 23 +
src/qemu/qemu_alias.h | 2 +
src/qemu/qemu_command.c | 445 ++++++++++++++----
src/qemu/qemu_command.h | 13 +-
src/qemu/qemu_domain.c | 516 ++++++++++++++++++++-
src/qemu/qemu_domain.h | 81 +++-
src/qemu/qemu_driver.c | 13 +-
src/qemu/qemu_hotplug.c | 247 +++++-----
src/qemu/qemu_hotplug.h | 4 +-
src/qemu/qemu_parse_command.c | 4 +-
src/qemu/qemu_process.c | 13 +-
src/vbox/vbox_common.c | 4 +-
src/xenconfig/xen_common.c | 4 +-
src/xenconfig/xen_sxpr.c | 4 +-
...uxml2argv-disk-drive-network-iscsi-auth-IV.args | 39 ++
...muxml2argv-disk-drive-network-iscsi-auth-IV.xml | 43 ++
...emuxml2argv-disk-drive-network-rbd-auth-IV.args | 31 ++
...qemuxml2argv-disk-drive-network-rbd-auth-IV.xml | 42 ++
...emuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args | 41 ++
...qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml | 48 ++
...xml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args | 43 ++
...uxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml | 48 ++
tests/qemuxml2argvmock.c | 31 +-
tests/qemuxml2argvtest.c | 19 +
tests/virhostdevtest.c | 3 +-
29 files changed, 1557 insertions(+), 247 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml
--
2.5.5
9:17 a.m.
New subject: [libvirt] [PATCH v2 01/12] qemu: Introduce qemuDomainSecretInfo
Introduce a new private structure to hold qemu domain auth/secret data.
This will be stored in the qemuDomainDiskPrivate as a means to store the
auth and fetched secret data rather than generating during building of
the command line.
The initial changes will handle the current username and secret values
for rbd and iscsi disks (in their various forms). The rbd secret is
stored as a base64 encoded value, while the iscsi secret is stored as
a plain text value. Future changes will store encoded/encrypted secret
data as well as an initialization vector needed to be given to qemu
in order to decrypt the encoded password along with the domain masterKey.
The inital assumption will be that VIR_DOMAIN_SECRET_INFO_PLAIN is
being used.
Although it's expected that the cleanup of the secret data will be
done immediately after command line generation, reintroduce the object
dispose function qemuDomainDiskPrivateDispose to handle removing
memory associated with the structure for "normal" cleanup paths.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 32 +++++++++++++++++++++++++++++++-
src/qemu/qemu_domain.h | 27 +++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed1e0e5..55cb30f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -729,7 +729,28 @@ qemuDomainMasterKeyCreate(virQEMUDriverPtr driver,
}
+static void
+qemuDomainSecretPlainFree(qemuDomainSecretPlain secret)
+{
+ VIR_FREE(secret.username);
+ memset(secret.secret, 0, strlen(secret.secret));
+ VIR_FREE(secret.secret);
+}
+
+
+static void
+qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo)
+{
+ if (!*secinfo)
+ return;
+
+ qemuDomainSecretPlainFree((*secinfo)->s.plain);
+ VIR_FREE(*secinfo);
+}
+
+
static virClassPtr qemuDomainDiskPrivateClass;
+static void qemuDomainDiskPrivateDispose(void *obj);
static int
qemuDomainDiskPrivateOnceInit(void)
@@ -737,7 +758,7 @@ qemuDomainDiskPrivateOnceInit(void)
qemuDomainDiskPrivateClass = virClassNew(virClassForObject(),
"qemuDomainDiskPrivate",
sizeof(qemuDomainDiskPrivate),
- NULL);
+ qemuDomainDiskPrivateDispose);
if (!qemuDomainDiskPrivateClass)
return -1;
else
@@ -761,6 +782,15 @@ qemuDomainDiskPrivateNew(void)
}
+static void
+qemuDomainDiskPrivateDispose(void *obj)
+{
+ qemuDomainDiskPrivatePtr priv = obj;
+
+ qemuDomainSecretInfoFree(&priv->secinfo);
+}
+
+
/* This is the old way of setting up per-domain directories */
static int
qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7d2c4fd..9cfe3e4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -239,6 +239,29 @@ struct _qemuDomainObjPrivate {
size_t masterKeyLen;
};
+/* Type of domain secret */
+typedef enum {
+ VIR_DOMAIN_SECRET_INFO_PLAIN = 0,
+
+ VIR_DOMAIN_SECRET_INFO_LAST
+} qemuDomainSecretInfoType;
+
+typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain;
+typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr;
+struct _qemuDomainSecretPlain {
+ char *username;
+ char *secret;
+};
+
+typedef struct _qemuDomainSecretInfo qemuDomainSecretInfo;
+typedef qemuDomainSecretInfo *qemuDomainSecretInfoPtr;
+struct _qemuDomainSecretInfo {
+ int type; /* qemuDomainSecretInfoType */
+ union {
+ qemuDomainSecretPlain plain;
+ } s;
+};
+
# define QEMU_DOMAIN_DISK_PRIVATE(disk) \
((qemuDomainDiskPrivatePtr) (disk)->privateData)
@@ -258,6 +281,10 @@ struct _qemuDomainDiskPrivate {
bool blockJobSync; /* the block job needs synchronized termination */
bool migrating; /* the disk is being migrated */
+
+ /* for storage devices using auth/secret
+ * NB: *not* to be written to qemu domain object XML */
+ qemuDomainSecretInfoPtr secinfo;
};
typedef enum {
--
2.5.5
5:41 p.m.
New subject: [libvirt] [PATCH v2 01/12] qemu: Introduce qemuDomainSecretInfo
On 04/16/2016 10:17 AM, John Ferlan wrote:
Introduce a new private structure to hold qemu domain auth/secret
data.
This will be stored in the qemuDomainDiskPrivate as a means to store the
auth and fetched secret data rather than generating during building of
the command line.
The initial changes will handle the current username and secret values
for rbd and iscsi disks (in their various forms). The rbd secret is
stored as a base64 encoded value, while the iscsi secret is stored as
a plain text value. Future changes will store encoded/encrypted secret
data as well as an initialization vector needed to be given to qemu
in order to decrypt the encoded password along with the domain masterKey.
The inital assumption will be that VIR_DOMAIN_SECRET_INFO_PLAIN is
being used.
Although it's expected that the cleanup of the secret data will be
done immediately after command line generation, reintroduce the object
dispose function qemuDomainDiskPrivateDispose to handle removing
memory associated with the structure for "normal" cleanup paths.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 32 +++++++++++++++++++++++++++++++-
src/qemu/qemu_domain.h | 27 +++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 1 deletion(-)
ACK
- Cole
6:17 a.m.
New subject: [libvirt] [PATCH v2 01/12] qemu: Introduce qemuDomainSecretInfo
On Sat, Apr 16, 2016 at 10:17:34AM -0400, John Ferlan wrote:
Introduce a new private structure to hold qemu domain auth/secret
data.
This will be stored in the qemuDomainDiskPrivate as a means to store the
auth and fetched secret data rather than generating during building of
the command line.
The initial changes will handle the current username and secret values
for rbd and iscsi disks (in their various forms). The rbd secret is
stored as a base64 encoded value, while the iscsi secret is stored as
a plain text value. Future changes will store encoded/encrypted secret
data as well as an initialization vector needed to be given to qemu
in order to decrypt the encoded password along with the domain masterKey.
The inital assumption will be that VIR_DOMAIN_SECRET_INFO_PLAIN is
being used.
Although it's expected that the cleanup of the secret data will be
done immediately after command line generation, reintroduce the object
dispose function qemuDomainDiskPrivateDispose to handle removing
memory associated with the structure for "normal" cleanup paths.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_domain.c | 32 +++++++++++++++++++++++++++++++-
src/qemu/qemu_domain.h | 27 +++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7d2c4fd..9cfe3e4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -239,6 +239,29 @@ struct _qemuDomainObjPrivate {
size_t masterKeyLen;
};
+/* Type of domain secret */
+typedef enum {
+ VIR_DOMAIN_SECRET_INFO_PLAIN = 0,
+
+ VIR_DOMAIN_SECRET_INFO_LAST
+} qemuDomainSecretInfoType;
Just a nit: the enum name has 'Type' in it, but the enum values do not.
+
+typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain;
+typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr;
+struct _qemuDomainSecretPlain {
+ char *username;
+ char *secret;
+};
+
+typedef struct _qemuDomainSecretInfo qemuDomainSecretInfo;
+typedef qemuDomainSecretInfo *qemuDomainSecretInfoPtr;
+struct _qemuDomainSecretInfo {
+ int type; /* qemuDomainSecretInfoType */
Is there any issue with using:
qemuDomainSecretInfoType type;
As far as I know we cannot use enums in public structures because their
size varies per-platform/compiler, but they should be safe in internal
code.
Jan
+ union {
+ qemuDomainSecretPlain plain;
+ } s;
+};
+
6:38 a.m.
New subject: [libvirt] [PATCH v2 01/12] qemu: Introduce qemuDomainSecretInfo
On 05/02/2016 07:17 AM, Ján Tomko wrote:
On Sat, Apr 16, 2016 at 10:17:34AM -0400, John Ferlan wrote:
> Introduce a new private structure to hold qemu domain auth/secret data.
> This will be stored in the qemuDomainDiskPrivate as a means to store the
> auth and fetched secret data rather than generating during building of
> the command line.
>
> The initial changes will handle the current username and secret values
> for rbd and iscsi disks (in their various forms). The rbd secret is
> stored as a base64 encoded value, while the iscsi secret is stored as
> a plain text value. Future changes will store encoded/encrypted secret
> data as well as an initialization vector needed to be given to qemu
> in order to decrypt the encoded password along with the domain masterKey.
> The inital assumption will be that VIR_DOMAIN_SECRET_INFO_PLAIN is
> being used.
>
> Although it's expected that the cleanup of the secret data will be
> done immediately after command line generation, reintroduce the object
> dispose function qemuDomainDiskPrivateDispose to handle removing
> memory associated with the structure for "normal" cleanup paths.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_domain.c | 32 +++++++++++++++++++++++++++++++-
> src/qemu/qemu_domain.h | 27 +++++++++++++++++++++++++++
> 2 files changed, 58 insertions(+), 1 deletion(-)
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 7d2c4fd..9cfe3e4 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -239,6 +239,29 @@ struct _qemuDomainObjPrivate {
> size_t masterKeyLen;
> };
>
> +/* Type of domain secret */
> +typedef enum {
> + VIR_DOMAIN_SECRET_INFO_PLAIN = 0,
> +
> + VIR_DOMAIN_SECRET_INFO_LAST
> +} qemuDomainSecretInfoType;
Just a nit: the enum name has 'Type' in it, but the enum values do not.
Nor does qemuProcessEventType in qemu_domain.h
Nor does qemuMonitorJSONObjectPropertyType in qemu_monitor_json.h
But I can make the change to VIR_DOMAIN_SECRET_INFO_TYPE_{PLAIN|IV|LAST}
> +
> +typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain;
> +typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr;
> +struct _qemuDomainSecretPlain {
> + char *username;
> + char *secret;
> +};
> +
> +typedef struct _qemuDomainSecretInfo qemuDomainSecretInfo;
> +typedef qemuDomainSecretInfo *qemuDomainSecretInfoPtr;
> +struct _qemuDomainSecretInfo {
> + int type; /* qemuDomainSecretInfoType */
Is there any issue with using:
qemuDomainSecretInfoType type;
As far as I know we cannot use enums in public structures because their
size varies per-platform/compiler, but they should be safe in internal
code.
OK - I'll make these adjustments and send it along as the 1st patch of
the follow-up series.
John
9:17 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
Rather than needing to pass the conn parameter to various command
line building API's, add qemuDomainSecretPrepare just prior to the
qemuProcessLaunch which calls qemuBuilCommandLine. The function
must be called after qemuProcessPrepareHost since it's expected
to eventually need the domain masterKey generated during the prepare
host call. Additionally, future patches may require device aliases
(assigned during the prepare domain call) in order to associate
the secret objects.
The qemuDomainSecretDestroy is called after the qemuProcessLaunch
finishes in order to clear and free memory used by the secrets
that were recently prepared, so they are not kept around in memory
too long.
Placing the setup here is beneficial for future patches which will
need the domain masterKey in order to generate an encrypted secret
along with an initialization vector to be saved and passed (since
the masterKey shouldn't be passed around).
Finally, since the secret is not added during command line build,
the hotplug code will need to get the secret into the private disk data.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 45 ++++-----------
src/qemu/qemu_command.h | 5 +-
src/qemu/qemu_domain.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_domain.h | 15 ++++-
src/qemu/qemu_driver.c | 10 ++--
src/qemu/qemu_hotplug.c | 26 +++++----
src/qemu/qemu_hotplug.h | 1 -
src/qemu/qemu_process.c | 8 +++
8 files changed, 202 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 26c19ff..24ed8ed 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -832,7 +832,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
int
qemuGetDriveSourceString(virStorageSourcePtr src,
- virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
char **source)
{
int actualType = virStorageSourceGetActualType(src);
@@ -846,31 +846,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
if (virStorageSourceIsEmpty(src))
return 1;
- if (conn) {
- if (actualType == VIR_STORAGE_TYPE_NETWORK &&
- src->auth &&
- (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
- src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
- bool encode = false;
- int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
- const char *protocol = virStorageNetProtocolTypeToString(src->protocol);
- username = src->auth->username;
-
- if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
- /* qemu requires the secret to be encoded for RBD */
- encode = true;
- secretType = VIR_SECRET_USAGE_TYPE_CEPH;
- }
-
- if (!(secret = virSecretGetSecretString(conn,
- protocol,
- encode,
- src->auth,
- secretType)))
- goto cleanup;
- }
- }
-
switch ((virStorageType) actualType) {
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_FILE:
@@ -881,6 +856,11 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
break;
case VIR_STORAGE_TYPE_NETWORK:
+ if (secinfo) {
+ username = secinfo->s.plain.username;
+ secret = secinfo->s.plain.secret;
+ }
+
if (!(*source = qemuBuildNetworkDriveURI(src, username, secret)))
goto cleanup;
break;
@@ -894,7 +874,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
ret = 0;
cleanup:
- VIR_FREE(secret);
return ret;
}
@@ -1033,8 +1012,7 @@ qemuCheckFips(void)
char *
-qemuBuildDriveStr(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+qemuBuildDriveStr(virDomainDiskDefPtr disk,
bool bootable,
virQEMUCapsPtr qemuCaps)
{
@@ -1046,6 +1024,7 @@ qemuBuildDriveStr(virConnectPtr conn,
int busid = -1, unitid = -1;
char *source = NULL;
int actualType = virStorageSourceGetActualType(disk->src);
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
if (idx < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1127,7 +1106,7 @@ qemuBuildDriveStr(virConnectPtr conn,
break;
}
- if (qemuGetDriveSourceString(disk->src, conn, &source) < 0)
+ if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, &source) <
0)
goto error;
if (source &&
@@ -1816,7 +1795,6 @@ qemuBuildDriveDevStr(const virDomainDef *def,
static int
qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
- virConnectPtr conn,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
bool emitBootindex)
@@ -1910,7 +1888,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
deviceFlagMasked = true;
}
}
- optstr = qemuBuildDriveStr(conn, disk,
+ optstr = qemuBuildDriveStr(disk,
emitBootindex ? false : !!bootindex,
qemuCaps);
if (deviceFlagMasked)
@@ -9363,8 +9341,7 @@ qemuBuildCommandLine(virConnectPtr conn,
if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildDiskDriveCommandLine(cmd, conn, def, qemuCaps,
- emitBootindex) < 0)
+ if (qemuBuildDiskDriveCommandLine(cmd, def, qemuCaps, emitBootindex) < 0)
goto error;
if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2e2f133..2662d9b 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -99,8 +99,7 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
virQEMUCapsPtr qemuCaps);
/* Both legacy & current support */
-char *qemuBuildDriveStr(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+char *qemuBuildDriveStr(virDomainDiskDefPtr disk,
bool bootable,
virQEMUCapsPtr qemuCaps);
@@ -177,7 +176,7 @@ char *qemuBuildRedirdevDevStr(const virDomainDef *def,
int qemuNetworkPrepareDevices(virDomainDefPtr def);
int qemuGetDriveSourceString(virStorageSourcePtr src,
- virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
char **source);
int qemuCheckDiskConfig(virDomainDiskDefPtr disk);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 55cb30f..93033d9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -46,6 +46,7 @@
#include "viratomic.h"
#include "virprocess.h"
#include "virrandom.h"
+#include "secret_util.h"
#include "base64.h"
#ifdef WITH_GNUTLS
# include <gnutls/gnutls.h>
@@ -791,6 +792,146 @@ qemuDomainDiskPrivateDispose(void *obj)
}
+/* qemuDomainSecretPlainSetup:
+ * @conn: Pointer to connection
+ * @secinfo: Pointer to secret info
+ * @protocol: Protocol for secret
+ * @authdef: Pointer to auth data
+ *
+ * Taking a secinfo, fill in the plaintext information
+ *
+ * Returns 0 on success, -1 on failure with error message
+ */
+static int
+qemuDomainSecretPlainSetup(virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
+ virStorageNetProtocol protocol,
+ virStorageAuthDefPtr authdef)
+{
+ bool encode = false;
+ int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
+ const char *protocolstr = virStorageNetProtocolTypeToString(protocol);
+
+ secinfo->type = VIR_DOMAIN_SECRET_INFO_PLAIN;
+ if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0)
+ return -1;
+
+ if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
+ /* qemu requires the secret to be encoded for RBD */
+ encode = true;
+ secretType = VIR_SECRET_USAGE_TYPE_CEPH;
+ }
+
+ if (!(secinfo->s.plain.secret =
+ virSecretGetSecretString(conn, protocolstr, encode,
+ authdef, secretType)))
+ return -1;
+
+ return 0;
+}
+
+
+/* qemuDomainSecretDiskDestroy:
+ * @disk: Pointer to a disk definition
+ *
+ * Clear and destroy memory associated with the secret
+ */
+void
+qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
+{
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+ if (!diskPriv->secinfo)
+ return;
+
+ qemuDomainSecretInfoFree(&diskPriv->secinfo);
+}
+
+
+/* qemuDomainSecretDiskPrepare:
+ * @conn: Pointer to connection
+ * @disk: Pointer to a disk definition
+ *
+ * For the right disk, generate the qemuDomainSecretInfo structure.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretDiskPrepare(virConnectPtr conn,
+ virDomainDiskDefPtr disk)
+{
+ virStorageSourcePtr src = disk->src;
+ qemuDomainSecretInfoPtr secinfo = NULL;
+
+ if (conn && !virStorageSourceIsEmpty(src) &&
+ virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK &&
+ src->auth &&
+ (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
+ src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
+
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+ if (VIR_ALLOC(secinfo) < 0)
+ return -1;
+
+ if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
+ src->auth) < 0)
+ goto error;
+
+ diskPriv->secinfo = secinfo;
+ }
+
+ return 0;
+
+ error:
+ qemuDomainSecretInfoFree(&secinfo);
+ return -1;
+}
+
+
+/* qemuDomainSecretDestroy:
+ * @vm: Domain object
+ *
+ * Once completed with the generation of the command line it is
+ * expect to remove the secrets
+ */
+void
+qemuDomainSecretDestroy(virDomainObjPtr vm)
+{
+ size_t i;
+
+ for (i = 0; i < vm->def->ndisks; i++)
+ qemuDomainSecretDiskDestroy(vm->def->disks[i]);
+}
+
+
+/* qemuDomainSecretPrepare:
+ * @conn: Pointer to connection
+ * @vm: Domain object
+ *
+ * For any objects that may require an auth/secret setup, create a
+ * qemuDomainSecretInfo and save it in the approriate place within
+ * the private structures. This will be used by command line build
+ * code in order to pass the secret along to qemu in order to provide
+ * the necessary authentication data.
+ *
+ * Returns 0 on success, -1 on failure with error message set
+ */
+int
+qemuDomainSecretPrepare(virConnectPtr conn,
+ virDomainObjPtr vm)
+{
+ size_t i;
+
+ for (i = 0; i < vm->def->ndisks; i++) {
+ if (qemuDomainSecretDiskPrepare(conn, vm->def->disks[i]) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+
/* This is the old way of setting up per-domain directories */
static int
qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
@@ -3782,8 +3923,7 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
bool
-qemuDomainDiskSourceDiffers(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
virDomainDiskDefPtr origDisk)
{
char *diskSrc = NULL, *origDiskSrc = NULL;
@@ -3799,8 +3939,10 @@ qemuDomainDiskSourceDiffers(virConnectPtr conn,
if (diskEmpty ^ origDiskEmpty)
return true;
- if (qemuGetDriveSourceString(disk->src, conn, &diskSrc) < 0 ||
- qemuGetDriveSourceString(origDisk->src, conn, &origDiskSrc) < 0)
+ /* This won't be a network storage, so no need to get the diskPriv
+ * in order to fetch the secret, thus NULL for param2 */
+ if (qemuGetDriveSourceString(disk->src, NULL, &diskSrc) < 0 ||
+ qemuGetDriveSourceString(origDisk->src, NULL, &origDiskSrc) < 0)
goto cleanup;
/* So far in qemu disk sources are considered different
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9cfe3e4..bde71a4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -497,8 +497,7 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
bool force_probe,
bool report_broken);
-bool qemuDomainDiskSourceDiffers(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
virDomainDiskDefPtr origDisk);
bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
@@ -616,4 +615,16 @@ int qemuDomainMasterKeyCreate(virQEMUDriverPtr driver,
void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv);
+void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+void qemuDomainSecretDestroy(virDomainObjPtr vm)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1ba8ab9..bb55b7d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7710,14 +7710,16 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
orig_disk->startupPolicy = dev->data.disk->startupPolicy;
- if (qemuDomainDiskSourceDiffers(conn, disk, orig_disk)) {
+ if (qemuDomainDiskSourceDiffers(disk, orig_disk)) {
/* Add the new disk src into shared disk hash table */
if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
goto cleanup;
- if (qemuDomainChangeEjectableMedia(driver, conn, vm,
- orig_disk, dev->data.disk->src,
force) < 0) {
- ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
vm->def->name));
+ if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
+ dev->data.disk->src,
+ force) < 0) {
+ ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
+ vm->def->name));
goto rollback;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ef8696b..692e3e7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -148,7 +148,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
/**
* qemuDomainChangeEjectableMedia:
* @driver: qemu driver structure
- * @conn: connection structure
* @vm: domain definition
* @disk: disk definition to change the source of
* @newsrc: new disk source to change to
@@ -163,7 +162,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
*/
int
qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
- virConnectPtr conn,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
virStorageSourcePtr newsrc,
@@ -223,7 +221,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
} while (rc < 0);
if (!virStorageSourceIsEmpty(newsrc)) {
- if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0)
+ /* cdrom/floppy won't have secret */
+ if (qemuGetDriveSourceString(newsrc, NULL, &sourcestr) < 0)
goto error;
if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) {
@@ -356,7 +355,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
- if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
+ if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+ goto error;
+
+ if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
goto error;
if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
@@ -411,6 +413,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virDomainDiskInsertPreAlloced(vm->def, disk);
cleanup:
+ qemuDomainSecretDiskDestroy(disk);
VIR_FREE(devstr);
VIR_FREE(drivestr);
VIR_FREE(drivealias);
@@ -582,10 +585,13 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
+ if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+ goto error;
+
if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
goto error;
- if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
goto error;
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
@@ -616,6 +622,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
virDomainDiskInsertPreAlloced(vm->def, disk);
cleanup:
+ qemuDomainSecretDiskDestroy(disk);
VIR_FREE(devstr);
VIR_FREE(drivestr);
virObjectUnref(cfg);
@@ -628,8 +635,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
static int
-qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
- virQEMUDriverPtr driver,
+qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
@@ -653,7 +659,7 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
- if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
+ if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
goto error;
if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
goto error;
@@ -746,7 +752,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
goto cleanup;
}
- if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
+ if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
disk->src, false) < 0)
goto cleanup;
@@ -768,7 +774,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
_("disk device='lun' is not supported for usb
bus"));
break;
}
- ret = qemuDomainAttachUSBMassStorageDevice(conn, driver, vm, disk);
+ ret = qemuDomainAttachUSBMassStorageDevice(driver, vm, disk);
break;
case VIR_DOMAIN_DISK_BUS_VIRTIO:
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 87d146e..bcce9e8 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -29,7 +29,6 @@
# include "domain_conf.h"
int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
- virConnectPtr conn,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
virStorageSourcePtr newsrc,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 81d86c2..c9f43fa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5640,6 +5640,9 @@ qemuProcessStart(virConnectPtr conn,
if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
goto stop;
+ if (qemuDomainSecretPrepare(conn, vm) < 0)
+ goto cleanup;
+
if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming,
snapshot, vmop, flags)) < 0) {
if (rv == -2)
@@ -5648,6 +5651,8 @@ qemuProcessStart(virConnectPtr conn,
}
relabel = true;
+ qemuDomainSecretDestroy(vm);
+
if (incoming &&
incoming->deferredURI &&
qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0)
@@ -5709,6 +5714,9 @@ qemuProcessCreatePretendCmd(virConnectPtr conn,
if (qemuProcessPrepareDomain(conn, driver, vm, flags) < 0)
goto cleanup;
+ if (qemuDomainSecretPrepare(conn, vm) < 0)
+ goto cleanup;
+
VIR_DEBUG("Building emulator command line");
cmd = qemuBuildCommandLine(conn,
driver,
--
2.5.5
5:49 p.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
On 04/16/2016 10:17 AM, John Ferlan wrote:
Rather than needing to pass the conn parameter to various command
line building API's, add qemuDomainSecretPrepare just prior to the
qemuProcessLaunch which calls qemuBuilCommandLine. The function
must be called after qemuProcessPrepareHost since it's expected
to eventually need the domain masterKey generated during the prepare
host call. Additionally, future patches may require device aliases
(assigned during the prepare domain call) in order to associate
the secret objects.
The qemuDomainSecretDestroy is called after the qemuProcessLaunch
finishes in order to clear and free memory used by the secrets
that were recently prepared, so they are not kept around in memory
too long.
Placing the setup here is beneficial for future patches which will
need the domain masterKey in order to generate an encrypted secret
along with an initialization vector to be saved and passed (since
the masterKey shouldn't be passed around).
Finally, since the secret is not added during command line build,
the hotplug code will need to get the secret into the private disk data.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 45 ++++-----------
src/qemu/qemu_command.h | 5 +-
src/qemu/qemu_domain.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_domain.h | 15 ++++-
src/qemu/qemu_driver.c | 10 ++--
src/qemu/qemu_hotplug.c | 26 +++++----
src/qemu/qemu_hotplug.h | 1 -
src/qemu/qemu_process.c | 8 +++
8 files changed, 202 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 26c19ff..24ed8ed 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -832,7 +832,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
int
qemuGetDriveSourceString(virStorageSourcePtr src,
- virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
char **source)
{
int actualType = virStorageSourceGetActualType(src);
@@ -846,31 +846,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
if (virStorageSourceIsEmpty(src))
return 1;
- if (conn) {
- if (actualType == VIR_STORAGE_TYPE_NETWORK &&
- src->auth &&
- (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
- src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
- bool encode = false;
- int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
- const char *protocol = virStorageNetProtocolTypeToString(src->protocol);
- username = src->auth->username;
-
- if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
- /* qemu requires the secret to be encoded for RBD */
- encode = true;
- secretType = VIR_SECRET_USAGE_TYPE_CEPH;
- }
-
- if (!(secret = virSecretGetSecretString(conn,
- protocol,
- encode,
- src->auth,
- secretType)))
- goto cleanup;
- }
- }
-
switch ((virStorageType) actualType) {
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_FILE:
@@ -881,6 +856,11 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
break;
case VIR_STORAGE_TYPE_NETWORK:
+ if (secinfo) {
+ username = secinfo->s.plain.username;
+ secret = secinfo->s.plain.secret;
+ }
+
if (!(*source = qemuBuildNetworkDriveURI(src, username, secret)))
goto cleanup;
break;
@@ -894,7 +874,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
ret = 0;
cleanup:
- VIR_FREE(secret);
return ret;
}
@@ -1033,8 +1012,7 @@ qemuCheckFips(void)
char *
-qemuBuildDriveStr(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+qemuBuildDriveStr(virDomainDiskDefPtr disk,
bool bootable,
virQEMUCapsPtr qemuCaps)
{
@@ -1046,6 +1024,7 @@ qemuBuildDriveStr(virConnectPtr conn,
int busid = -1, unitid = -1;
char *source = NULL;
int actualType = virStorageSourceGetActualType(disk->src);
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
if (idx < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1127,7 +1106,7 @@ qemuBuildDriveStr(virConnectPtr conn,
break;
}
- if (qemuGetDriveSourceString(disk->src, conn, &source) < 0)
+ if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, &source) <
0)
goto error;
if (source &&
@@ -1816,7 +1795,6 @@ qemuBuildDriveDevStr(const virDomainDef *def,
static int
qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
- virConnectPtr conn,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
bool emitBootindex)
@@ -1910,7 +1888,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
deviceFlagMasked = true;
}
}
- optstr = qemuBuildDriveStr(conn, disk,
+ optstr = qemuBuildDriveStr(disk,
emitBootindex ? false : !!bootindex,
qemuCaps);
if (deviceFlagMasked)
@@ -9363,8 +9341,7 @@ qemuBuildCommandLine(virConnectPtr conn,
if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildDiskDriveCommandLine(cmd, conn, def, qemuCaps,
- emitBootindex) < 0)
+ if (qemuBuildDiskDriveCommandLine(cmd, def, qemuCaps, emitBootindex) < 0)
goto error;
if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2e2f133..2662d9b 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -99,8 +99,7 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
virQEMUCapsPtr qemuCaps);
/* Both legacy & current support */
-char *qemuBuildDriveStr(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+char *qemuBuildDriveStr(virDomainDiskDefPtr disk,
bool bootable,
virQEMUCapsPtr qemuCaps);
@@ -177,7 +176,7 @@ char *qemuBuildRedirdevDevStr(const virDomainDef *def,
int qemuNetworkPrepareDevices(virDomainDefPtr def);
int qemuGetDriveSourceString(virStorageSourcePtr src,
- virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
char **source);
int qemuCheckDiskConfig(virDomainDiskDefPtr disk);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 55cb30f..93033d9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -46,6 +46,7 @@
#include "viratomic.h"
#include "virprocess.h"
#include "virrandom.h"
+#include "secret_util.h"
#include "base64.h"
#ifdef WITH_GNUTLS
# include <gnutls/gnutls.h>
@@ -791,6 +792,146 @@ qemuDomainDiskPrivateDispose(void *obj)
}
+/* qemuDomainSecretPlainSetup:
+ * @conn: Pointer to connection
+ * @secinfo: Pointer to secret info
+ * @protocol: Protocol for secret
+ * @authdef: Pointer to auth data
+ *
+ * Taking a secinfo, fill in the plaintext information
+ *
+ * Returns 0 on success, -1 on failure with error message
+ */
+static int
+qemuDomainSecretPlainSetup(virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
+ virStorageNetProtocol protocol,
+ virStorageAuthDefPtr authdef)
+{
+ bool encode = false;
+ int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
+ const char *protocolstr = virStorageNetProtocolTypeToString(protocol);
+
+ secinfo->type = VIR_DOMAIN_SECRET_INFO_PLAIN;
+ if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0)
+ return -1;
+
+ if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
+ /* qemu requires the secret to be encoded for RBD */
+ encode = true;
+ secretType = VIR_SECRET_USAGE_TYPE_CEPH;
+ }
+
+ if (!(secinfo->s.plain.secret =
+ virSecretGetSecretString(conn, protocolstr, encode,
+ authdef, secretType)))
+ return -1;
+
+ return 0;
+}
+
+
+/* qemuDomainSecretDiskDestroy:
+ * @disk: Pointer to a disk definition
+ *
+ * Clear and destroy memory associated with the secret
+ */
+void
+qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
+{
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+ if (!diskPriv->secinfo)
+ return;
+
+ qemuDomainSecretInfoFree(&diskPriv->secinfo);
+}
+
+
+/* qemuDomainSecretDiskPrepare:
+ * @conn: Pointer to connection
+ * @disk: Pointer to a disk definition
+ *
+ * For the right disk, generate the qemuDomainSecretInfo structure.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretDiskPrepare(virConnectPtr conn,
+ virDomainDiskDefPtr disk)
+{
+ virStorageSourcePtr src = disk->src;
+ qemuDomainSecretInfoPtr secinfo = NULL;
+
+ if (conn && !virStorageSourceIsEmpty(src) &&
+ virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK &&
+ src->auth &&
+ (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
+ src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
+
Is conn ever going to be NULL here?
+ qemuDomainDiskPrivatePtr diskPriv =
QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+ if (VIR_ALLOC(secinfo) < 0)
+ return -1;
+
+ if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
+ src->auth) < 0)
+ goto error;
+
+ diskPriv->secinfo = secinfo;
+ }
+
+ return 0;
+
+ error:
+ qemuDomainSecretInfoFree(&secinfo);
+ return -1;
+}
+
+
+/* qemuDomainSecretDestroy:
+ * @vm: Domain object
+ *
+ * Once completed with the generation of the command line it is
+ * expect to remove the secrets
+ */
+void
+qemuDomainSecretDestroy(virDomainObjPtr vm)
+{
+ size_t i;
+
+ for (i = 0; i < vm->def->ndisks; i++)
+ qemuDomainSecretDiskDestroy(vm->def->disks[i]);
+}
+
+
+/* qemuDomainSecretPrepare:
+ * @conn: Pointer to connection
+ * @vm: Domain object
+ *
+ * For any objects that may require an auth/secret setup, create a
+ * qemuDomainSecretInfo and save it in the approriate place within
+ * the private structures. This will be used by command line build
+ * code in order to pass the secret along to qemu in order to provide
+ * the necessary authentication data.
+ *
+ * Returns 0 on success, -1 on failure with error message set
+ */
+int
+qemuDomainSecretPrepare(virConnectPtr conn,
+ virDomainObjPtr vm)
+{
+ size_t i;
+
+ for (i = 0; i < vm->def->ndisks; i++) {
+ if (qemuDomainSecretDiskPrepare(conn, vm->def->disks[i]) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+
/* This is the old way of setting up per-domain directories */
static int
qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
@@ -3782,8 +3923,7 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
bool
-qemuDomainDiskSourceDiffers(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
virDomainDiskDefPtr origDisk)
{
char *diskSrc = NULL, *origDiskSrc = NULL;
@@ -3799,8 +3939,10 @@ qemuDomainDiskSourceDiffers(virConnectPtr conn,
if (diskEmpty ^ origDiskEmpty)
return true;
- if (qemuGetDriveSourceString(disk->src, conn, &diskSrc) < 0 ||
- qemuGetDriveSourceString(origDisk->src, conn, &origDiskSrc) < 0)
+ /* This won't be a network storage, so no need to get the diskPriv
+ * in order to fetch the secret, thus NULL for param2 */
+ if (qemuGetDriveSourceString(disk->src, NULL, &diskSrc) < 0 ||
+ qemuGetDriveSourceString(origDisk->src, NULL, &origDiskSrc) < 0)
goto cleanup;
/* So far in qemu disk sources are considered different
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9cfe3e4..bde71a4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -497,8 +497,7 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
bool force_probe,
bool report_broken);
-bool qemuDomainDiskSourceDiffers(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
virDomainDiskDefPtr origDisk);
bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
@@ -616,4 +615,16 @@ int qemuDomainMasterKeyCreate(virQEMUDriverPtr driver,
void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv);
+void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+void qemuDomainSecretDestroy(virDomainObjPtr vm)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1ba8ab9..bb55b7d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7710,14 +7710,16 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
orig_disk->startupPolicy = dev->data.disk->startupPolicy;
- if (qemuDomainDiskSourceDiffers(conn, disk, orig_disk)) {
+ if (qemuDomainDiskSourceDiffers(disk, orig_disk)) {
/* Add the new disk src into shared disk hash table */
if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
goto cleanup;
- if (qemuDomainChangeEjectableMedia(driver, conn, vm,
- orig_disk, dev->data.disk->src,
force) < 0) {
- ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
vm->def->name));
+ if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
+ dev->data.disk->src,
+ force) < 0) {
+ ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
+ vm->def->name));
goto rollback;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ef8696b..692e3e7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -148,7 +148,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
/**
* qemuDomainChangeEjectableMedia:
* @driver: qemu driver structure
- * @conn: connection structure
* @vm: domain definition
* @disk: disk definition to change the source of
* @newsrc: new disk source to change to
@@ -163,7 +162,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
*/
int
qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
- virConnectPtr conn,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
virStorageSourcePtr newsrc,
@@ -223,7 +221,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
} while (rc < 0);
if (!virStorageSourceIsEmpty(newsrc)) {
- if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0)
+ /* cdrom/floppy won't have secret */
+ if (qemuGetDriveSourceString(newsrc, NULL, &sourcestr) < 0)
goto error;
Why not? Can't you have an rbd backed cdrom device?
ACK otherwise
- Cole
7:06 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
[...]
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 26c19ff..24ed8ed 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -832,7 +832,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
>
> int
> qemuGetDriveSourceString(virStorageSourcePtr src,
> - virConnectPtr conn,
> + qemuDomainSecretInfoPtr secinfo,
> char **source)
> {
> int actualType = virStorageSourceGetActualType(src);
> @@ -846,31 +846,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
> if (virStorageSourceIsEmpty(src))
> return 1;
>
> - if (conn) {
> - if (actualType == VIR_STORAGE_TYPE_NETWORK &&
> - src->auth &&
> - (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
> - src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
> - bool encode = false;
> - int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
> - const char *protocol =
virStorageNetProtocolTypeToString(src->protocol);
> - username = src->auth->username;
> -
> - if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
> - /* qemu requires the secret to be encoded for RBD */
> - encode = true;
> - secretType = VIR_SECRET_USAGE_TYPE_CEPH;
> - }
> -
> - if (!(secret = virSecretGetSecretString(conn,
> - protocol,
> - encode,
> - src->auth,
> - secretType)))
> - goto cleanup;
> - }
> - }
> -
This whole pile is what moves into qemuDomainSecretDiskPrepare. The
check for 'conn' was here it seems was a result of commit id '816f0f93'
where qemuDomainSnapshotCreateSingleDiskActive calls
qemuGetDriveSourceString.
> switch ((virStorageType) actualType) {
> case VIR_STORAGE_TYPE_BLOCK:
> case VIR_STORAGE_TYPE_FILE:
> @@ -881,6 +856,11 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
> break;
>
> case VIR_STORAGE_TYPE_NETWORK:
> + if (secinfo) {
> + username = secinfo->s.plain.username;
> + secret = secinfo->s.plain.secret;
> + }
> +
[...]
> +/* qemuDomainSecretDiskPrepare:
> + * @conn: Pointer to connection
> + * @disk: Pointer to a disk definition
> + *
> + * For the right disk, generate the qemuDomainSecretInfo structure.
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +qemuDomainSecretDiskPrepare(virConnectPtr conn,
> + virDomainDiskDefPtr disk)
> +{
> + virStorageSourcePtr src = disk->src;
> + qemuDomainSecretInfoPtr secinfo = NULL;
> +
> + if (conn && !virStorageSourceIsEmpty(src) &&
> + virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK &&
> + src->auth &&
> + (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
> + src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
> +
Is conn ever going to be NULL here?
This code is the former qemuGetDriveSourceString code... So it was
mostly a "move" of code. I was being more cautious due to number of callers.
I did find one path that explicitly passes NULL:
testQemuHotplugAttach -> qemuDomainAttachDeviceDiskLive ->
qemuDomainAttachDeviceDiskLive ... which can call either
qemuDomainAttachVirtioDiskDevice or qemuDomainAttachSCSIDisk which would
call the qemuDomainSecretDiskPrepare
And one comment in qemuAutostartDomains that I suppose could be updated
to indicate the conn is also possibly needed for secrets.
> + qemuDomainDiskPrivatePtr diskPriv =
QEMU_DOMAIN_DISK_PRIVATE(disk);
> +
> + if (VIR_ALLOC(secinfo) < 0)
> + return -1;
> +
> + if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
> + src->auth) < 0)
> + goto error;
> +
> + diskPriv->secinfo = secinfo;
> + }
> +
> + return 0;
> +
> + error:
> + qemuDomainSecretInfoFree(&secinfo);
> + return -1;
> +}
> +
> +
[...]
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index ef8696b..692e3e7 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -148,7 +148,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
> /**
> * qemuDomainChangeEjectableMedia:
> * @driver: qemu driver structure
> - * @conn: connection structure
> * @vm: domain definition
> * @disk: disk definition to change the source of
> * @newsrc: new disk source to change to
> @@ -163,7 +162,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
> */
> int
> qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
> - virConnectPtr conn,
> virDomainObjPtr vm,
> virDomainDiskDefPtr disk,
> virStorageSourcePtr newsrc,
> @@ -223,7 +221,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
> } while (rc < 0);
>
> if (!virStorageSourceIsEmpty(newsrc)) {
> - if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0)
> + /* cdrom/floppy won't have secret */
> + if (qemuGetDriveSourceString(newsrc, NULL, &sourcestr) < 0)
> goto error;
>
Why not? Can't you have an rbd backed cdrom device?
Trying to remember why I wrote that comment... Ahh I see... I must've
read the return virStorageSourceIsEmpty incorrectly when I changed that
code... good catch!
I will insert/squash in a:
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
and change NULL to diskPriv->secinfo
John
ACK otherwise
- Cole
6:38 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
On Sat, Apr 16, 2016 at 10:17:35AM -0400, John Ferlan wrote:
Rather than needing to pass the conn parameter to various command
line building API's, add qemuDomainSecretPrepare just prior to the
qemuProcessLaunch which calls qemuBuilCommandLine. The function
must be called after qemuProcessPrepareHost since it's expected
to eventually need the domain masterKey generated during the prepare
host call. Additionally, future patches may require device aliases
(assigned during the prepare domain call) in order to associate
the secret objects.
The qemuDomainSecretDestroy is called after the qemuProcessLaunch
finishes in order to clear and free memory used by the secrets
that were recently prepared, so they are not kept around in memory
too long.
Placing the setup here is beneficial for future patches which will
need the domain masterKey in order to generate an encrypted secret
along with an initialization vector to be saved and passed (since
the masterKey shouldn't be passed around).
Finally, since the secret is not added during command line build,
the hotplug code will need to get the secret into the private disk data.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 45 ++++-----------
src/qemu/qemu_command.h | 5 +-
src/qemu/qemu_domain.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_domain.h | 15 ++++-
src/qemu/qemu_driver.c | 10 ++--
src/qemu/qemu_hotplug.c | 26 +++++----
src/qemu/qemu_hotplug.h | 1 -
src/qemu/qemu_process.c | 8 +++
8 files changed, 202 insertions(+), 58 deletions(-)
@@ -1033,8 +1012,7 @@ qemuCheckFips(void)
char *
-qemuBuildDriveStr(virConnectPtr conn,
- virDomainDiskDefPtr disk,
It's really nice to see the 'conn' go.
+qemuBuildDriveStr(virDomainDiskDefPtr disk,
bool bootable,
virQEMUCapsPtr qemuCaps)
{
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 81d86c2..c9f43fa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5640,6 +5640,9 @@ qemuProcessStart(virConnectPtr conn,
if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
goto stop;
+ if (qemuDomainSecretPrepare(conn, vm) < 0)
+ goto cleanup;
+
The call fits better in qemuProcessPrepareDomain,
that way it will be called even for incoming migration.
Jan
7:48 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
On 05/02/2016 07:38 AM, Ján Tomko wrote:
On Sat, Apr 16, 2016 at 10:17:35AM -0400, John Ferlan wrote:
> Rather than needing to pass the conn parameter to various command
> line building API's, add qemuDomainSecretPrepare just prior to the
> qemuProcessLaunch which calls qemuBuilCommandLine. The function
> must be called after qemuProcessPrepareHost since it's expected
> to eventually need the domain masterKey generated during the prepare
> host call. Additionally, future patches may require device aliases
> (assigned during the prepare domain call) in order to associate
> the secret objects.
>
> The qemuDomainSecretDestroy is called after the qemuProcessLaunch
> finishes in order to clear and free memory used by the secrets
> that were recently prepared, so they are not kept around in memory
> too long.
>
> Placing the setup here is beneficial for future patches which will
> need the domain masterKey in order to generate an encrypted secret
> along with an initialization vector to be saved and passed (since
> the masterKey shouldn't be passed around).
>
> Finally, since the secret is not added during command line build,
> the hotplug code will need to get the secret into the private disk data.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 45 ++++-----------
> src/qemu/qemu_command.h | 5 +-
> src/qemu/qemu_domain.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++--
> src/qemu/qemu_domain.h | 15 ++++-
> src/qemu/qemu_driver.c | 10 ++--
> src/qemu/qemu_hotplug.c | 26 +++++----
> src/qemu/qemu_hotplug.h | 1 -
> src/qemu/qemu_process.c | 8 +++
> 8 files changed, 202 insertions(+), 58 deletions(-)
>
> @@ -1033,8 +1012,7 @@ qemuCheckFips(void)
>
>
> char *
> -qemuBuildDriveStr(virConnectPtr conn,
> - virDomainDiskDefPtr disk,
It's really nice to see the 'conn' go.
> +qemuBuildDriveStr(virDomainDiskDefPtr disk,
> bool bootable,
> virQEMUCapsPtr qemuCaps)
> {
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 81d86c2..c9f43fa 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5640,6 +5640,9 @@ qemuProcessStart(virConnectPtr conn,
> if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
> goto stop;
>
> + if (qemuDomainSecretPrepare(conn, vm) < 0)
> + goto cleanup;
> +
The call fits better in qemuProcessPrepareDomain,
that way it will be called even for incoming migration.
Understood; however, PrepareDomain doesn't have everything that will be
needed. The qemuProcessPrepareHost must run first in order to create
"priv->libDir" in order to write the domain master key secret that will
be used in "future patches" (11/12 of this series) in order to generate
an Initialization Vector secret.
I think it's possible to move the call into qemuProcessPrepareHost if
you think that works better/fine. The qemuProcessCreatePretendCmd
already calls qemuDomainSecretPrepare, so that "should" cover the
testing scenario...
John
8:52 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
On Mon, May 02, 2016 at 08:48:02AM -0400, John Ferlan wrote:
On 05/02/2016 07:38 AM, Ján Tomko wrote:
> On Sat, Apr 16, 2016 at 10:17:35AM -0400, John Ferlan wrote:
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 81d86c2..c9f43fa 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -5640,6 +5640,9 @@ qemuProcessStart(virConnectPtr conn,
>> if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
>> goto stop;
>>
>> + if (qemuDomainSecretPrepare(conn, vm) < 0)
>> + goto cleanup;
>> +
>
> The call fits better in qemuProcessPrepareDomain,
> that way it will be called even for incoming migration.
>
Understood; however, PrepareDomain doesn't have everything that will be
needed. The qemuProcessPrepareHost must run first in order to create
"priv->libDir" in order to write the domain master key secret that will
be used in "future patches" (11/12 of this series) in order to generate
an Initialization Vector secret.
I think it's possible to move the call into qemuProcessPrepareHost if
you think that works better/fine. The qemuProcessCreatePretendCmd
already calls qemuDomainSecretPrepare, so that "should" cover the
testing scenario...
Another possibility could be splitting qemuDomainMasterKeyCreate
into key creation (which does prepare the domain) and writing/labeling
(which prepares the host environment).
Either way, this patch removed formatting secrets from the migration
path.
Jan
9:08 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
On 05/02/2016 09:52 AM, Ján Tomko wrote:
On Mon, May 02, 2016 at 08:48:02AM -0400, John Ferlan wrote:
> On 05/02/2016 07:38 AM, Ján Tomko wrote:
>> On Sat, Apr 16, 2016 at 10:17:35AM -0400, John Ferlan wrote:
>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>> index 81d86c2..c9f43fa 100644
>>> --- a/src/qemu/qemu_process.c
>>> +++ b/src/qemu/qemu_process.c
>>> @@ -5640,6 +5640,9 @@ qemuProcessStart(virConnectPtr conn,
>>> if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
>>> goto stop;
>>>
>>> + if (qemuDomainSecretPrepare(conn, vm) < 0)
>>> + goto cleanup;
>>> +
>>
>> The call fits better in qemuProcessPrepareDomain,
>> that way it will be called even for incoming migration.
>>
>
> Understood; however, PrepareDomain doesn't have everything that will be
> needed. The qemuProcessPrepareHost must run first in order to create
> "priv->libDir" in order to write the domain master key secret that will
> be used in "future patches" (11/12 of this series) in order to generate
> an Initialization Vector secret.
>
> I think it's possible to move the call into qemuProcessPrepareHost if
> you think that works better/fine. The qemuProcessCreatePretendCmd
> already calls qemuDomainSecretPrepare, so that "should" cover the
> testing scenario...
Another possibility could be splitting qemuDomainMasterKeyCreate
into key creation (which does prepare the domain) and writing/labeling
(which prepares the host environment).
Seems a bit of overkill to me to keep split that up. If that's something
you feel strongly about I can review whatever you send.
Either way, this patch removed formatting secrets from the migration
path.
Yes, I see - good catch... I just didn't keep up with all the other
changes made around this by also addressing.
For me the question comes down to whether I add the call to
qemuDomainSecretPrepare in qemuProcessPrepareHost or
qemuMigrationPrepareAny. To a degree adding the secrets doesn't fit
with the "Host" type functions, so I'm starting to lean toward the
latter...
John
9:12 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
On Mon, May 02, 2016 at 10:08:10AM -0400, John Ferlan wrote:
On 05/02/2016 09:52 AM, Ján Tomko wrote:
> On Mon, May 02, 2016 at 08:48:02AM -0400, John Ferlan wrote:
>> On 05/02/2016 07:38 AM, Ján Tomko wrote:
>>> On Sat, Apr 16, 2016 at 10:17:35AM -0400, John Ferlan wrote:
>>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>>> index 81d86c2..c9f43fa 100644
>>>> --- a/src/qemu/qemu_process.c
>>>> +++ b/src/qemu/qemu_process.c
>>>> @@ -5640,6 +5640,9 @@ qemuProcessStart(virConnectPtr conn,
>>>> if (qemuProcessPrepareHost(driver, vm, !!incoming) < 0)
>>>> goto stop;
>>>>
>>>> + if (qemuDomainSecretPrepare(conn, vm) < 0)
>>>> + goto cleanup;
>>>> +
>>>
>>> The call fits better in qemuProcessPrepareDomain,
>>> that way it will be called even for incoming migration.
>>>
>>
>> Understood; however, PrepareDomain doesn't have everything that will be
>> needed. The qemuProcessPrepareHost must run first in order to create
>> "priv->libDir" in order to write the domain master key secret that
will
>> be used in "future patches" (11/12 of this series) in order to
generate
>> an Initialization Vector secret.
>>
>> I think it's possible to move the call into qemuProcessPrepareHost if
>> you think that works better/fine. The qemuProcessCreatePretendCmd
>> already calls qemuDomainSecretPrepare, so that "should" cover the
>> testing scenario...
>
> Another possibility could be splitting qemuDomainMasterKeyCreate
> into key creation (which does prepare the domain) and writing/labeling
> (which prepares the host environment).
Seems a bit of overkill to me to keep split that up. If that's something
you feel strongly about I can review whatever you send.
Still less overkill than copying and pasting it into three different
places.
Jan
9:17 a.m.
New subject: [libvirt] [PATCH v2 03/12] qemu: Introduce qemuDomainHostdevPrivatePtr
Modeled after the qemuDomainDiskPrivatePtr logic, create a privateData
pointer in the _virDomainHostdevDef to allow storage of private data
for a hypervisor in order to at least temporarily store auth/secrets
data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the auth/secret data, there's no need to add
code to handle this new structure there.
Updated copyrights for modules touched. Some didn't have updates in a
couple years even though changes have been made.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 33 +++++++++++++++++++++++++-------
src/conf/domain_conf.h | 5 ++++-
src/lxc/lxc_native.c | 4 ++--
src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 13 +++++++++++++
src/qemu/qemu_parse_command.c | 4 ++--
src/vbox/vbox_common.c | 4 ++--
src/xenconfig/xen_common.c | 4 ++--
src/xenconfig/xen_sxpr.c | 4 ++--
tests/virhostdevtest.c | 3 ++-
10 files changed, 99 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28248c8..07f5b26 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2121,16 +2121,32 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
VIR_FREE(def);
}
-virDomainHostdevDefPtr virDomainHostdevDefAlloc(void)
+
+virDomainHostdevDefPtr
+virDomainHostdevDefAlloc(virDomainXMLOptionPtr xmlopt)
{
virDomainHostdevDefPtr def = NULL;
if (VIR_ALLOC(def) < 0 ||
- VIR_ALLOC(def->info) < 0)
+ VIR_ALLOC(def->info) < 0) {
VIR_FREE(def);
+ return NULL;
+ }
+
+ if (xmlopt &&
+ xmlopt->privateData.hostdevNew &&
+ !(def->privateData = xmlopt->privateData.hostdevNew()))
+ goto error;
+
return def;
+
+ error:
+ VIR_FREE(def->info);
+ VIR_FREE(def);
+ return NULL;
}
+
static void
virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc)
{
@@ -12247,7 +12263,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
}
static virDomainHostdevDefPtr
-virDomainHostdevDefParseXML(xmlNodePtr node,
+virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlNodePtr node,
xmlXPathContextPtr ctxt,
virHashTablePtr bootHash,
unsigned int flags)
@@ -12259,7 +12276,7 @@ virDomainHostdevDefParseXML(xmlNodePtr node,
ctxt->node = node;
- if (!(def = virDomainHostdevDefAlloc()))
+ if (!(def = virDomainHostdevDefAlloc(xmlopt)))
goto error;
if (mode) {
@@ -12909,8 +12926,9 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_HOSTDEV:
- if (!(dev->data.hostdev = virDomainHostdevDefParseXML(node, ctxt,
- NULL, flags)))
+ if (!(dev->data.hostdev = virDomainHostdevDefParseXML(xmlopt, node,
+ ctxt, NULL,
+ flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_CONTROLLER:
@@ -16430,7 +16448,8 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i < n; i++) {
virDomainHostdevDefPtr hostdev;
- hostdev = virDomainHostdevDefParseXML(nodes[i], ctxt, bootHash, flags);
+ hostdev = virDomainHostdevDefParseXML(xmlopt, nodes[i], ctxt,
+ bootHash, flags);
if (!hostdev)
goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1986f53..fb6a02b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -543,6 +543,8 @@ struct _virDomainHostdevCaps {
/* basic device for direct passthrough */
struct _virDomainHostdevDef {
virDomainDeviceDef parent; /* higher level Def containing this */
+ virObjectPtr privateData;
+
int mode; /* enum virDomainHostdevMode */
int startupPolicy; /* enum virDomainStartupPolicy */
bool managed;
@@ -2495,6 +2497,7 @@ struct _virDomainXMLPrivateDataCallbacks {
virDomainXMLPrivateDataAllocFunc alloc;
virDomainXMLPrivateDataFreeFunc free;
virDomainXMLPrivateDataNewFunc diskNew;
+ virDomainXMLPrivateDataNewFunc hostdevNew;
virDomainXMLPrivateDataFormatFunc format;
virDomainXMLPrivateDataParseFunc parse;
};
@@ -2572,7 +2575,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def);
void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def);
void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
void virDomainVideoDefFree(virDomainVideoDefPtr def);
-virDomainHostdevDefPtr virDomainHostdevDefAlloc(void);
+virDomainHostdevDefPtr virDomainHostdevDefAlloc(virDomainXMLOptionPtr xmlopt);
void virDomainHostdevDefClear(virDomainHostdevDefPtr def);
void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainHubDefFree(virDomainHubDefPtr def);
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index ef92c7d..31ffce7 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -1,7 +1,7 @@
/*
* lxc_native.c: LXC native configuration import
*
- * Copyright (c) 2014 Red Hat, Inc.
+ * Copyright (c) 2014-2016 Red Hat, Inc.
* Copyright (c) 2013-2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
*
* This library is free software; you can redistribute it and/or
@@ -394,7 +394,7 @@ lxcCreateNetDef(const char *type,
static virDomainHostdevDefPtr
lxcCreateHostdevDef(int mode, int type, const char *data)
{
- virDomainHostdevDefPtr hostdev = virDomainHostdevDefAlloc();
+ virDomainHostdevDefPtr hostdev = virDomainHostdevDefAlloc(NULL);
if (!hostdev)
return NULL;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 93033d9..89b7899 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -792,6 +792,49 @@ qemuDomainDiskPrivateDispose(void *obj)
}
+static virClassPtr qemuDomainHostdevPrivateClass;
+static void qemuDomainHostdevPrivateDispose(void *obj);
+
+static int
+qemuDomainHostdevPrivateOnceInit(void)
+{
+ qemuDomainHostdevPrivateClass =
+ virClassNew(virClassForObject(),
+ "qemuDomainHostdevPrivate",
+ sizeof(qemuDomainHostdevPrivate),
+ qemuDomainHostdevPrivateDispose);
+ if (!qemuDomainHostdevPrivateClass)
+ return -1;
+ else
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(qemuDomainHostdevPrivate)
+
+static virObjectPtr
+qemuDomainHostdevPrivateNew(void)
+{
+ qemuDomainHostdevPrivatePtr priv;
+
+ if (qemuDomainHostdevPrivateInitialize() < 0)
+ return NULL;
+
+ if (!(priv = virObjectNew(qemuDomainHostdevPrivateClass)))
+ return NULL;
+
+ return (virObjectPtr) priv;
+}
+
+
+static void
+qemuDomainHostdevPrivateDispose(void *obj)
+{
+ qemuDomainHostdevPrivatePtr priv = obj;
+
+ qemuDomainSecretInfoFree(&priv->secinfo);
+}
+
+
/* qemuDomainSecretPlainSetup:
* @conn: Pointer to connection
* @secinfo: Pointer to secret info
@@ -1425,6 +1468,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks =
{
.alloc = qemuDomainObjPrivateAlloc,
.free = qemuDomainObjPrivateFree,
.diskNew = qemuDomainDiskPrivateNew,
+ .hostdevNew = qemuDomainHostdevPrivateNew,
.parse = qemuDomainObjPrivateXMLParse,
.format = qemuDomainObjPrivateXMLFormat,
};
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index bde71a4..3cb961b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -287,6 +287,19 @@ struct _qemuDomainDiskPrivate {
qemuDomainSecretInfoPtr secinfo;
};
+# define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev) \
+ ((qemuDomainHostdevPrivatePtr) (hostdev)->privateData)
+
+typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate;
+typedef qemuDomainHostdevPrivate *qemuDomainHostdevPrivatePtr;
+struct _qemuDomainHostdevPrivate {
+ virObject parent;
+
+ /* for hostdev storage devices using auth/secret
+ * NB: *not* to be written to qemu domain object XML */
+ qemuDomainSecretInfoPtr secinfo;
+};
+
typedef enum {
QEMU_PROCESS_EVENT_WATCHDOG = 0,
QEMU_PROCESS_EVENT_GUESTPANIC,
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 79f5b92..577651b 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -1164,7 +1164,7 @@ qemuParseCommandLinePCI(const char *val)
int bus = 0, slot = 0, func = 0;
const char *start;
char *end;
- virDomainHostdevDefPtr def = virDomainHostdevDefAlloc();
+ virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(NULL);
if (!def)
goto error;
@@ -1214,7 +1214,7 @@ qemuParseCommandLinePCI(const char *val)
static virDomainHostdevDefPtr
qemuParseCommandLineUSB(const char *val)
{
- virDomainHostdevDefPtr def = virDomainHostdevDefAlloc();
+ virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(NULL);
virDomainHostdevSubsysUSBPtr usbsrc;
int first = 0, second = 0;
const char *start;
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 0cead10..05377f4 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2014, Taowei Luo (uaedante(a)gmail.com)
- * Copyright (C) 2010-2015 Red Hat, Inc.
+ * Copyright (C) 2010-2016 Red Hat, Inc.
* Copyright (C) 2008-2009 Sun Microsystems, Inc.
*
* This library is free software; you can redistribute it and/or
@@ -3033,7 +3033,7 @@ vboxHostDeviceGetXMLDesc(vboxGlobalData *data, virDomainDefPtr def,
IMachine *ma
goto release_filters;
for (i = 0; i < def->nhostdevs; i++) {
- def->hostdevs[i] = virDomainHostdevDefAlloc();
+ def->hostdevs[i] = virDomainHostdevDefAlloc(NULL);
if (!def->hostdevs[i])
goto release_hostdevs;
}
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index e1d9cf6..c6aee69 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -3,7 +3,7 @@
* between XM and XL
*
* Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
- * Copyright (C) 2006-2007, 2009-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2016 Red Hat, Inc.
* Copyright (C) 2011 Univention GmbH
* Copyright (C) 2006 Daniel P. Berrange
*
@@ -458,7 +458,7 @@ xenParsePCI(virConfPtr conf, virDomainDefPtr def)
goto skippci;
if (virStrToLong_i(func, NULL, 16, &funcID) < 0)
goto skippci;
- if (!(hostdev = virDomainHostdevDefAlloc()))
+ if (!(hostdev = virDomainHostdevDefAlloc(NULL)))
return -1;
hostdev->managed = false;
diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index 2677510..dc47b4d 100644
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -1,7 +1,7 @@
/*
* xen_sxpr.c: Xen SEXPR parsing functions
*
- * Copyright (C) 2010-2014 Red Hat, Inc.
+ * Copyright (C) 2010-2016 Red Hat, Inc.
* Copyright (C) 2011 Univention GmbH
* Copyright (C) 2005 Anthony Liguori <aliguori(a)us.ibm.com>
*
@@ -1110,7 +1110,7 @@ xenParseSxprPCI(virDomainDefPtr def,
goto error;
}
- if (!(dev = virDomainHostdevDefAlloc()))
+ if (!(dev = virDomainHostdevDefAlloc(NULL)))
goto error;
dev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 610b02a..bc2b44a 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2014-2016 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -86,7 +87,7 @@ myInit(void)
for (i = 0; i < nhostdevs; i++) {
virDomainHostdevSubsys subsys;
- hostdevs[i] = virDomainHostdevDefAlloc();
+ hostdevs[i] = virDomainHostdevDefAlloc(NULL);
if (!hostdevs[i])
goto cleanup;
hostdevs[i]->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
--
2.5.5
5:55 p.m.
New subject: [libvirt] [PATCH v2 03/12] qemu: Introduce qemuDomainHostdevPrivatePtr
On 04/16/2016 10:17 AM, John Ferlan wrote:
Modeled after the qemuDomainDiskPrivatePtr logic, create a
privateData
pointer in the _virDomainHostdevDef to allow storage of private data
for a hypervisor in order to at least temporarily store auth/secrets
data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the auth/secret data, there's no need to add
code to handle this new structure there.
Updated copyrights for modules touched. Some didn't have updates in a
couple years even though changes have been made.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
ACK
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
Similar to the qemuDomainSecretDiskPrepare, generate the secret
for the Hostdev's prior to call qemuProcessLaunch which calls
qemuBuildCommandLine. Additionally, since the secret is not longer
added as part of building the command, the hotplug code will need
to make the call to add the secret in the hostdevPriv.
Since this then is the last requirement to pass a virConnectPtr
to qemuBuildCommandLine, we now can remove that as part of these
changes. That removal has cascading effects through various callers.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 41 +++++++++------------------
src/qemu/qemu_command.h | 8 ++----
src/qemu/qemu_domain.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 7 +++++
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_hotplug.c | 31 +++++++++++++--------
src/qemu/qemu_hotplug.h | 3 +-
src/qemu/qemu_process.c | 5 ++--
8 files changed, 120 insertions(+), 51 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 24ed8ed..9ecb7b6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -50,7 +50,6 @@
#include "secret_conf.h"
#include "network/bridge_driver.h"
#include "virnetdevtap.h"
-#include "secret_util.h"
#include "device_conf.h"
#include "virstoragefile.h"
#include "virtpm.h"
@@ -4460,29 +4459,24 @@ qemuBuildSCSIHostHostdevDrvStr(virDomainHostdevDefPtr dev)
}
static char *
-qemuBuildSCSIiSCSIHostdevDrvStr(virConnectPtr conn,
- virDomainHostdevDefPtr dev)
+qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
{
char *source = NULL;
char *secret = NULL;
char *username = NULL;
virStorageSource src;
+ qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev);
memset(&src, 0, sizeof(src));
virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
- if (conn && iscsisrc->auth) {
- const char *protocol =
- virStorageNetProtocolTypeToString(VIR_STORAGE_NET_PROTOCOL_ISCSI);
- bool encode = false;
- int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
+ if (hostdevPriv->secinfo) {
+ qemuDomainSecretInfoPtr secinfo = hostdevPriv->secinfo;
- username = iscsisrc->auth->username;
- if (!(secret = virSecretGetSecretString(conn, protocol, encode,
- iscsisrc->auth, secretType)))
- goto cleanup;
+ username = secinfo->s.plain.username;
+ secret = secinfo->s.plain.secret;
}
src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
@@ -4493,14 +4487,11 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virConnectPtr conn,
/* Rather than pull what we think we want - use the network disk code */
source = qemuBuildNetworkDriveURI(&src, username, secret);
- cleanup:
- VIR_FREE(secret);
return source;
}
char *
-qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
- virDomainHostdevDefPtr dev,
+qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -4508,7 +4499,7 @@ qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
- if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(conn, dev)))
+ if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(dev)))
goto error;
virBufferAsprintf(&buf, "file=%s,if=none,format=raw", source);
} else {
@@ -4806,7 +4797,6 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
static int
qemuBuildHostdevCommandLine(virCommandPtr cmd,
- virConnectPtr conn,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
unsigned int *bootHostdevNet)
@@ -4955,8 +4945,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
char *drvstr;
virCommandAddArg(cmd, "-drive");
- if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev,
- qemuCaps)))
+ if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps)))
return -1;
virCommandAddArg(cmd, drvstr);
VIR_FREE(drvstr);
@@ -9193,13 +9182,9 @@ qemuBuildCommandLineValidate(virQEMUDriverPtr driver,
/*
* Constructs a argv suitable for launching qemu with config defined
* for a given virtual machine.
- *
- * XXX 'conn' is only required to resolve network -> bridge name
- * figure out how to remove this requirement some day
*/
virCommandPtr
-qemuBuildCommandLine(virConnectPtr conn,
- virQEMUDriverPtr driver,
+qemuBuildCommandLine(virQEMUDriverPtr driver,
virLogManagerPtr logManager,
virDomainDefPtr def,
virDomainChrSourceDefPtr monitor_chr,
@@ -9224,9 +9209,9 @@ qemuBuildCommandLine(virConnectPtr conn,
unsigned int bootHostdevNet = 0;
- VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d "
+ VIR_DEBUG("driver=%p def=%p mon=%p json=%d "
"qemuCaps=%p migrateURI=%s snapshot=%p vmop=%d",
- conn, driver, def, monitor_chr, monitor_json,
+ driver, def, monitor_chr, monitor_json,
qemuCaps, migrateURI, snapshot, vmop);
if (qemuBuildCommandLineValidate(driver, def) < 0)
@@ -9392,7 +9377,7 @@ qemuBuildCommandLine(virConnectPtr conn,
if (qemuBuildRedirdevCommandLine(logManager, cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildHostdevCommandLine(cmd, conn, def, qemuCaps, &bootHostdevNet) <
0)
+ if (qemuBuildHostdevCommandLine(cmd, def, qemuCaps, &bootHostdevNet) < 0)
goto error;
if (migrateURI)
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2662d9b..429864c 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -46,8 +46,7 @@ char *qemuBuildObjectCommandlineFromJSON(const char *type,
const char *alias,
virJSONValuePtr props);
-virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
- virQEMUDriverPtr driver,
+virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver,
virLogManagerPtr logManager,
virDomainDefPtr def,
virDomainChrSourceDefPtr monitor_chr,
@@ -63,7 +62,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
int **nicindexes,
const char *domainLibDir,
const char *domainChannelTargetDir)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(16) ATTRIBUTE_NONNULL(17);
+ ATTRIBUTE_NONNULL(15) ATTRIBUTE_NONNULL(16);
/* Generate '-device' string for chardev device */
int
@@ -161,8 +160,7 @@ char *qemuBuildUSBHostdevDevStr(const virDomainDef *def,
virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps);
-char *qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
- virDomainHostdevDefPtr dev,
+char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps);
char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 89b7899..03fdab1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -932,6 +932,71 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
}
+/* qemuDomainSecretHostdevDestroy:
+ * @disk: Pointer to a hostdev definition
+ *
+ * Clear and destroy memory associated with the secret
+ */
+void
+qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev)
+{
+ qemuDomainHostdevPrivatePtr hostdevPriv =
+ QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev);
+
+ if (!hostdevPriv->secinfo)
+ return;
+
+ qemuDomainSecretInfoFree(&hostdevPriv->secinfo);
+}
+
+
+/* qemuDomainSecretHostdevPrepare:
+ * @conn: Pointer to connection
+ * @hostdev: Pointer to a hostdev definition
+ *
+ * For the right host device, generate the qemuDomainSecretInfo structure.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretHostdevPrepare(virConnectPtr conn,
+ virDomainHostdevDefPtr hostdev)
+{
+ virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
+ qemuDomainSecretInfoPtr secinfo = NULL;
+
+ if (conn && hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+ subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
+
+ virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+ virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
+
+ if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI
&&
+ iscsisrc->auth) {
+
+ qemuDomainHostdevPrivatePtr hostdevPriv =
+ QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev);
+
+ if (VIR_ALLOC(secinfo) < 0)
+ return -1;
+
+ if (qemuDomainSecretPlainSetup(conn, secinfo,
+ VIR_STORAGE_NET_PROTOCOL_ISCSI,
+ iscsisrc->auth) < 0)
+ goto error;
+
+ hostdevPriv->secinfo = secinfo;
+ }
+ }
+
+ return 0;
+
+ error:
+ qemuDomainSecretInfoFree(&secinfo);
+ return -1;
+}
+
+
/* qemuDomainSecretDestroy:
* @vm: Domain object
*
@@ -945,6 +1010,9 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
for (i = 0; i < vm->def->ndisks; i++)
qemuDomainSecretDiskDestroy(vm->def->disks[i]);
+
+ for (i = 0; i < vm->def->nhostdevs; i++)
+ qemuDomainSecretHostdevDestroy(vm->def->hostdevs[i]);
}
@@ -971,6 +1039,11 @@ qemuDomainSecretPrepare(virConnectPtr conn,
return -1;
}
+ for (i = 0; i < vm->def->nhostdevs; i++) {
+ if (qemuDomainSecretHostdevPrepare(conn, vm->def->hostdevs[i]) < 0)
+ return -1;
+ }
+
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 3cb961b..63f98ba 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -634,6 +634,13 @@ void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
int qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr disk)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretHostdevPrepare(virConnectPtr conn,
+ virDomainHostdevDefPtr hostdev)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
void qemuDomainSecretDestroy(virDomainObjPtr vm)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bb55b7d..32ee7dd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7502,8 +7502,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_NET:
qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, NULL);
- ret = qemuDomainAttachNetDevice(dom->conn, driver, vm,
- dev->data.net);
+ ret = qemuDomainAttachNetDevice(driver, vm, dev->data.net);
if (!ret) {
alias = dev->data.net->info.alias;
dev->data.net = NULL;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 692e3e7..94c0222 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -809,11 +809,10 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
}
-/* XXX conn required for network -> bridge resolution */
-int qemuDomainAttachNetDevice(virConnectPtr conn,
- virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virDomainNetDefPtr net)
+int
+qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainNetDefPtr net)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
char **tapfdName = NULL;
@@ -853,8 +852,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
* as a hostdev (the hostdev code will reach over into the
* netdev-specific code as appropriate), then also added to
* the nets list (see cleanup:) if successful.
+ *
+ * qemuDomainAttachHostDevice uses a connection to resolve
+ * a SCSI hostdev secret, which is not this case, so pass NULL.
*/
- ret = qemuDomainAttachHostDevice(conn, driver, vm,
+ ret = qemuDomainAttachHostDevice(NULL, driver, vm,
virDomainNetGetActualHostdev(net));
goto cleanup;
}
@@ -1915,6 +1917,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
return ret;
}
+
static int
qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
virQEMUDriverPtr driver,
@@ -1970,7 +1973,10 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
goto cleanup;
- if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, priv->qemuCaps)))
+ if (qemuDomainSecretHostdevPrepare(conn, hostdev) < 0)
+ goto cleanup;
+
+ if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps)))
goto cleanup;
if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
@@ -2006,6 +2012,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
ret = 0;
cleanup:
+ qemuDomainSecretHostdevDestroy(hostdev);
if (ret < 0) {
qemuHostdevReAttachSCSIDevices(driver, vm->def->name, &hostdev, 1);
if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
@@ -2020,10 +2027,12 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
return ret;
}
-int qemuDomainAttachHostDevice(virConnectPtr conn,
- virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virDomainHostdevDefPtr hostdev)
+
+int
+qemuDomainAttachHostDevice(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainHostdevDefPtr hostdev)
{
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index bcce9e8..868b4cf 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -43,8 +43,7 @@ int qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev);
-int qemuDomainAttachNetDevice(virConnectPtr conn,
- virQEMUDriverPtr driver,
+int qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainNetDefPtr net);
int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c9f43fa..9fb879f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5294,7 +5294,7 @@ qemuProcessLaunch(virConnectPtr conn,
logfile = qemuDomainLogContextGetWriteFD(logCtxt);
VIR_DEBUG("Building emulator command line");
- if (!(cmd = qemuBuildCommandLine(conn, driver,
+ if (!(cmd = qemuBuildCommandLine(driver,
qemuDomainLogContextGetManager(logCtxt),
vm->def, priv->monConfig,
priv->monJSON, priv->qemuCaps,
@@ -5718,8 +5718,7 @@ qemuProcessCreatePretendCmd(virConnectPtr conn,
goto cleanup;
VIR_DEBUG("Building emulator command line");
- cmd = qemuBuildCommandLine(conn,
- driver,
+ cmd = qemuBuildCommandLine(driver,
NULL,
vm->def,
priv->monConfig,
--
2.5.5
6:05 p.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
On 04/16/2016 10:17 AM, John Ferlan wrote:
Similar to the qemuDomainSecretDiskPrepare, generate the secret
for the Hostdev's prior to call qemuProcessLaunch which calls
qemuBuildCommandLine. Additionally, since the secret is not longer
added as part of building the command, the hotplug code will need
to make the call to add the secret in the hostdevPriv.
Since this then is the last requirement to pass a virConnectPtr
to qemuBuildCommandLine, we now can remove that as part of these
changes. That removal has cascading effects through various callers.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
ACK
The mixed in virConnectPtr droppage is a pain :) In the future I'd suggest
annotating it with ATTRIBUTE_UNUSED in the functional patches, then dropping
it entirely in a separate patch. If this series goes v3 please don't worry
about it though
- Cole
7:12 a.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
On 04/27/2016 07:05 PM, Cole Robinson wrote:
On 04/16/2016 10:17 AM, John Ferlan wrote:
> Similar to the qemuDomainSecretDiskPrepare, generate the secret
> for the Hostdev's prior to call qemuProcessLaunch which calls
> qemuBuildCommandLine. Additionally, since the secret is not longer
> added as part of building the command, the hotplug code will need
> to make the call to add the secret in the hostdevPriv.
>
> Since this then is the last requirement to pass a virConnectPtr
> to qemuBuildCommandLine, we now can remove that as part of these
> changes. That removal has cascading effects through various callers.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
ACK
The mixed in virConnectPtr droppage is a pain :) In the future I'd suggest
annotating it with ATTRIBUTE_UNUSED in the functional patches, then dropping
it entirely in a separate patch. If this series goes v3 please don't worry
about it though
Sorry - I was trying to be "patch count efficient"... I've done the
ATTRIBUTE_UNUSED game before too... It's almost always a toss-up.
John
9:17 a.m.
New subject: [libvirt] [PATCH v2 05/12] qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI
Rather than take username and password as parameters, now take
a qemuDomainSecretInfoPtr and decode within the function.
NB: Having secinfo implies having the username for a plain type
from a successful virSecretGetSecretString
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9ecb7b6..8629c15 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -610,8 +610,7 @@ qemuNetworkDriveGetPort(int protocol,
static char *
qemuBuildNetworkDriveURI(virStorageSourcePtr src,
- const char *username,
- const char *secret)
+ qemuDomainSecretInfoPtr secinfo)
{
char *ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -722,12 +721,14 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
virAsprintf(&uri->query, "socket=%s",
src->hosts->socket) < 0)
goto cleanup;
- if (username) {
- if (secret) {
- if (virAsprintf(&uri->user, "%s:%s", username,
secret) < 0)
+ if (secinfo) {
+ if (secinfo->s.plain.secret) {
+ if (virAsprintf(&uri->user, "%s:%s",
+ secinfo->s.plain.username,
+ secinfo->s.plain.secret) < 0)
goto cleanup;
} else {
- if (VIR_STRDUP(uri->user, username) < 0)
+ if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
goto cleanup;
}
}
@@ -776,11 +777,12 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
if (src->snapshot)
virBufferEscape(&buf, '\\', ":", "@%s",
src->snapshot);
- if (username) {
- virBufferEscape(&buf, '\\', ":",
":id=%s", username);
+ if (secinfo) {
+ virBufferEscape(&buf, '\\', ":",
":id=%s",
+ secinfo->s.plain.username);
virBufferEscape(&buf, '\\', ":",
":key=%s:auth_supported=cephx\\;none",
- secret);
+ secinfo->s.plain.secret);
} else {
virBufferAddLit(&buf, ":auth_supported=none");
}
@@ -835,8 +837,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
char **source)
{
int actualType = virStorageSourceGetActualType(src);
- char *secret = NULL;
- char *username = NULL;
int ret = -1;
*source = NULL;
@@ -855,12 +855,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
break;
case VIR_STORAGE_TYPE_NETWORK:
- if (secinfo) {
- username = secinfo->s.plain.username;
- secret = secinfo->s.plain.secret;
- }
-
- if (!(*source = qemuBuildNetworkDriveURI(src, username, secret)))
+ if (!(*source = qemuBuildNetworkDriveURI(src, secinfo)))
goto cleanup;
break;
@@ -4462,8 +4457,6 @@ static char *
qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
{
char *source = NULL;
- char *secret = NULL;
- char *username = NULL;
virStorageSource src;
qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev);
@@ -4472,20 +4465,13 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
- if (hostdevPriv->secinfo) {
- qemuDomainSecretInfoPtr secinfo = hostdevPriv->secinfo;
-
- username = secinfo->s.plain.username;
- secret = secinfo->s.plain.secret;
- }
-
src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
src.path = iscsisrc->path;
src.hosts = iscsisrc->hosts;
src.nhosts = iscsisrc->nhosts;
/* Rather than pull what we think we want - use the network disk code */
- source = qemuBuildNetworkDriveURI(&src, username, secret);
+ source = qemuBuildNetworkDriveURI(&src, hostdevPriv->secinfo);
return source;
}
--
2.5.5
6:06 p.m.
New subject: [libvirt] [PATCH v2 05/12] qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI
On 04/16/2016 10:17 AM, John Ferlan wrote:
Rather than take username and password as parameters, now take
a qemuDomainSecretInfoPtr and decode within the function.
NB: Having secinfo implies having the username for a plain type
from a successful virSecretGetSecretString
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 40 +++++++++++++---------------------------
1 file changed, 13 insertions(+), 27 deletions(-)
ACK
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH v2 06/12] qemu: hotplug: Assume support for -device for attach virtio disk
Since support for QEMU_CAPS_DEVICE is not assumed, let's drop the legacy
code to make life easier going forward.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 79 +++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 46 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 94c0222..11efd7b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -317,7 +317,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virDomainDiskDefPtr disk)
{
int ret = -1;
- const char* type = virDomainDiskBusTypeToString(disk->bus);
qemuDomainObjPrivatePtr priv = vm->privateData;
char *devstr = NULL;
char *drivestr = NULL;
@@ -341,62 +340,50 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
- if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs,
- !disk->info.addr.ccw.assigned) < 0)
- goto error;
- } else if (!disk->info.type ||
- disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) <
0)
- goto error;
- }
- releaseaddr = true;
- if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
+ if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+ if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs,
+ !disk->info.addr.ccw.assigned) < 0)
goto error;
-
- if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+ } else if (!disk->info.type ||
+ disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0)
goto error;
+ }
+ releaseaddr = true;
+ if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
+ goto error;
- if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
- goto error;
+ if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+ goto error;
- if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
- goto error;
+ if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+ goto error;
- if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
- goto error;
- }
+ if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
+ goto error;
+
+ if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
+ goto error;
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
qemuDomainObjEnterMonitor(driver, vm);
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- ret = qemuMonitorAddDrive(priv->mon, drivestr);
- if (ret == 0) {
- ret = qemuMonitorAddDevice(priv->mon, devstr);
- if (ret < 0) {
- virErrorPtr orig_err = virSaveLastError();
- if (!drivealias ||
- qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
- VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- NULLSTR(drivealias), drivestr);
- }
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
+ ret = qemuMonitorAddDrive(priv->mon, drivestr);
+ if (ret == 0) {
+ ret = qemuMonitorAddDevice(priv->mon, devstr);
+ if (ret < 0) {
+ virErrorPtr orig_err = virSaveLastError();
+ if (!drivealias ||
+ qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+ VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ NULLSTR(drivealias), drivestr);
+ }
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
}
- }
- } else if (!disk->info.type ||
- disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- virDevicePCIAddress guestAddr = disk->info.addr.pci;
- ret = qemuMonitorAddPCIDisk(priv->mon, src, type, &guestAddr);
- if (ret == 0) {
- disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
- memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr));
}
}
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
--
2.5.5
6:07 p.m.
New subject: [libvirt] [PATCH v2 06/12] qemu: hotplug: Assume support for -device for attach virtio disk
On 04/16/2016 10:17 AM, John Ferlan wrote:
Since support for QEMU_CAPS_DEVICE is not assumed, let's drop the
legacy
code to make life easier going forward.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 79 +++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 46 deletions(-)
ACK, still applies since pkrempa hasn't pushed his patches but obviously you
guys should coordinate
- Cole
2:31 a.m.
New subject: [libvirt] [PATCH v2 06/12] qemu: hotplug: Assume support for -device for attach virtio disk
On Wed, Apr 27, 2016 at 19:07:57 -0400, Cole Robinson wrote:
On 04/16/2016 10:17 AM, John Ferlan wrote:
> Since support for QEMU_CAPS_DEVICE is not assumed, let's drop the legacy
> code to make life easier going forward.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_hotplug.c | 79 +++++++++++++++++++++----------------------------
> 1 file changed, 33 insertions(+), 46 deletions(-)
>
ACK, still applies since pkrempa hasn't pushed his patches but obviously you
guys should coordinate
I didn't push it since it can't be by any means justified to be pushed
after the freeze. I'll gladly drop it when I'll be about to push the
series.
Peter
9:17 a.m.
New subject: [libvirt] [PATCH v2 07/12] qemu: hotplug: Adjust error path for attach scsi disk
Adjust error path logic to make it clearer how to undo the failed add.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 11efd7b..ae314be 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -584,29 +584,22 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
+ /* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorAddDrive(priv->mon, drivestr);
- if (ret == 0) {
- ret = qemuMonitorAddDevice(priv->mon, devstr);
- if (ret < 0) {
- VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
- drivestr, devstr);
- /* XXX should call 'drive_del' on error but this does not exist yet
*/
- }
- }
+ if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+ goto failadddrive;
- if (qemuDomainObjExitMonitor(driver, vm) < 0) {
- ret = -1;
- goto error;
- }
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+ goto failadddevice;
- virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto failexitmonitor;
- if (ret < 0)
- goto error;
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
virDomainDiskInsertPreAlloced(vm->def, disk);
+ ret = 0;
cleanup:
qemuDomainSecretDiskDestroy(disk);
@@ -615,6 +608,16 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
virObjectUnref(cfg);
return ret;
+ failadddevice:
+ /* XXX should call 'drive_del' on error but this does not exist yet */
+ VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr);
+
+ failadddrive:
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ failexitmonitor:
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
+
error:
ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
goto cleanup;
--
2.5.5
6:17 p.m.
New subject: [libvirt] [PATCH v2 07/12] qemu: hotplug: Adjust error path for attach scsi disk
On 04/16/2016 10:17 AM, John Ferlan wrote:
Adjust error path logic to make it clearer how to undo the failed
add.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
ACK (one misc bit below)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 11efd7b..ae314be 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -584,29 +584,22 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
+ /* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorAddDrive(priv->mon, drivestr);
- if (ret == 0) {
- ret = qemuMonitorAddDevice(priv->mon, devstr);
- if (ret < 0) {
- VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
- drivestr, devstr);
- /* XXX should call 'drive_del' on error but this does not exist yet
*/
Old comment... there is a drive_del, and we even have support for it via
qemuMonitorDriveDel :) Not for this patch series though
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH v2 08/12] qemu: hotplug: Adjust error path for attach virtio disk
Adjust error path logic to make it clearer how to undo the failed add.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 55 ++++++++++++++++++++++++++++---------------------
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ae314be..17a70a3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -318,6 +318,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
char *devstr = NULL;
char *drivestr = NULL;
char *drivealias = NULL;
@@ -368,36 +369,24 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
+ /* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorAddDrive(priv->mon, drivestr);
- if (ret == 0) {
- ret = qemuMonitorAddDevice(priv->mon, devstr);
- if (ret < 0) {
- virErrorPtr orig_err = virSaveLastError();
- if (!drivealias ||
- qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
- VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- NULLSTR(drivealias), drivestr);
- }
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
- }
- }
+
+ if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+ goto failadddrive;
+
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+ goto failadddevice;
+
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
releaseaddr = false;
- ret = -1;
- goto error;
+ goto failexitmonitor;
}
- virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
-
- if (ret < 0)
- goto error;
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
virDomainDiskInsertPreAlloced(vm->def, disk);
+ ret = 0;
cleanup:
qemuDomainSecretDiskDestroy(disk);
@@ -407,6 +396,26 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virObjectUnref(cfg);
return ret;
+ failadddevice:
+ orig_err = virSaveLastError();
+ if (!drivealias ||
+ qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+ VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ NULLSTR(drivealias), drivestr);
+ }
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
+
+ failadddrive:
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ releaseaddr = false;
+
+ failexitmonitor:
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
+
error:
if (releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
--
2.5.5
6:20 p.m.
New subject: [libvirt] [PATCH v2 08/12] qemu: hotplug: Adjust error path for attach virtio disk
On 04/16/2016 10:17 AM, John Ferlan wrote:
Adjust error path logic to make it clearer how to undo the failed
add.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 55 ++++++++++++++++++++++++++++---------------------
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ae314be..17a70a3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -318,6 +318,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
char *devstr = NULL;
char *drivestr = NULL;
char *drivealias = NULL;
@@ -368,36 +369,24 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
+ /* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorAddDrive(priv->mon, drivestr);
- if (ret == 0) {
- ret = qemuMonitorAddDevice(priv->mon, devstr);
- if (ret < 0) {
- virErrorPtr orig_err = virSaveLastError();
- if (!drivealias ||
- qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
- VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- NULLSTR(drivealias), drivestr);
- }
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
- }
- }
+
+ if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+ goto failadddrive;
+
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+ goto failadddevice;
+
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
releaseaddr = false;
- ret = -1;
- goto error;
+ goto failexitmonitor;
}
- virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
-
- if (ret < 0)
- goto error;
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
virDomainDiskInsertPreAlloced(vm->def, disk);
+ ret = 0;
cleanup:
qemuDomainSecretDiskDestroy(disk);
@@ -407,6 +396,26 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virObjectUnref(cfg);
return ret;
+ failadddevice:
+ orig_err = virSaveLastError();
+ if (!drivealias ||
+ qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+ VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ NULLSTR(drivealias), drivestr);
Hey there it is ;)
ACK
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH v2 09/12] qemu: hotplug: Adjust error path for attach hostdev scsi disk
Adjust error path logic to make it clearer how to undo the failed add.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 52 ++++++++++++++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 17a70a3..c206369 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1925,6 +1925,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
virDomainControllerDefPtr cont = NULL;
char *devstr = NULL;
char *drvstr = NULL;
@@ -1984,32 +1985,24 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
goto cleanup;
+ /* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
- if ((ret = qemuMonitorAddDrive(priv->mon, drvstr)) == 0) {
- if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0) {
- virErrorPtr orig_err = virSaveLastError();
- if (qemuMonitorDriveDel(priv->mon, drvstr) < 0)
- VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- drvstr, devstr);
- if (orig_err) {
- virSetError(orig_err);
- virFreeError(orig_err);
- }
- }
- }
- if (qemuDomainObjExitMonitor(driver, vm) < 0) {
- ret = -1;
- goto cleanup;
- }
- virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
- if (ret < 0)
- goto cleanup;
+ if (qemuMonitorAddDrive(priv->mon, drvstr) < 0)
+ goto failadddrive;
+
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+ goto failadddevice;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto failexitmonitor;
+
+ virDomainAuditHostdev(vm, hostdev, "attach", true);
vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
ret = 0;
+
cleanup:
qemuDomainSecretHostdevDestroy(hostdev);
if (ret < 0) {
@@ -2024,6 +2017,25 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
VIR_FREE(drvstr);
VIR_FREE(devstr);
return ret;
+
+ failadddevice:
+ orig_err = virSaveLastError();
+ if (qemuMonitorDriveDel(priv->mon, drvstr) < 0)
+ VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ drvstr, devstr);
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
+
+ failadddrive:
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ failexitmonitor:
+ virDomainAuditHostdev(vm, hostdev, "attach", false);
+
+ goto cleanup;
}
--
2.5.5
6:21 p.m.
New subject: [libvirt] [PATCH v2 09/12] qemu: hotplug: Adjust error path for attach hostdev scsi disk
On 04/16/2016 10:17 AM, John Ferlan wrote:
Adjust error path logic to make it clearer how to undo the failed
add.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 52 ++++++++++++++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 20 deletions(-)
ACK
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH v2 10/12] qemu: hotplug: Fix possible memory leak of props
If we failed to build the aliases or attach the chardev, then the props
would be leaked - fix that.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c206369..c05f88c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1686,6 +1686,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
if (qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0)
goto failbackend;
+ props = NULL;
if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
goto failfrontend;
@@ -1703,6 +1704,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
audit:
virDomainAuditRNG(vm, NULL, rng, "attach", ret == 0);
cleanup:
+ virJSONValueFree(props);
if (ret < 0 && vm)
qemuDomainReleaseDeviceAddress(vm, &rng->info, NULL);
VIR_FREE(charAlias);
@@ -1716,6 +1718,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
failbackend:
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
+ props = NULL; /* qemuMonitorAddObject consumes on failure */
failchardev:
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
vm = NULL;
--
2.5.5
6:23 p.m.
New subject: [libvirt] [PATCH v2 10/12] qemu: hotplug: Fix possible memory leak of props
On 04/16/2016 10:17 AM, John Ferlan wrote:
If we failed to build the aliases or attach the chardev, then the
props
would be leaked - fix that.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 3 +++
1 file changed, 3 insertions(+)
ACK
- Cole
9:17 a.m.
New subject: [libvirt] [PATCH v2 11/12] qemu: Introduce qemuDomainSecretIV
Add the data structure and infrastructure to support an initialization
vector (IV) secret. The IV secret generation will need to have access
to the domain private master key, so let's make sure the prepare disk
and hostdev functions can accept that now.
Anywhere that needs to make a decision over which secret type to use
in order to fill in or use the IV secret has a switch added. In particular,
when building the command line for the network URI, create a couple of
helper functions which make the decision to add the secret info.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 106 ++++++++++++++++++++++++++++++++++++++----------
src/qemu/qemu_domain.c | 34 ++++++++++++++--
src/qemu/qemu_domain.h | 22 ++++++++--
src/qemu/qemu_hotplug.c | 6 +--
4 files changed, 138 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8629c15..a8cacb3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -606,6 +606,85 @@ qemuNetworkDriveGetPort(int protocol,
return -1;
}
+
+/* qemuBuildGeneralSecinfoURI:
+ * @uri: Pointer to the URI structure to add to
+ * @secinfo: Pointer to the secret info data (if present)
+ *
+ * If we have a secinfo, then build the command line options for
+ * the secret info for the "general" case (somewhat a misnomer since
+ * an iscsi disk is the only one with a secinfo).
+ *
+ * Returns 0 on success or if no secinfo,
+ * -1 and error message if fail to add secret information
+ */
+static int
+qemuBuildGeneralSecinfoURI(virURIPtr uri,
+ qemuDomainSecretInfoPtr secinfo)
+{
+ if (!secinfo)
+ return 0;
+
+ switch ((qemuDomainSecretInfoType) secinfo->type) {
+ case VIR_DOMAIN_SECRET_INFO_PLAIN:
+ if (secinfo->s.plain.secret) {
+ if (virAsprintf(&uri->user, "%s:%s",
+ secinfo->s.plain.username,
+ secinfo->s.plain.secret) < 0)
+ return -1;
+ } else {
+ if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
+ return -1;
+ }
+ break;
+
+ case VIR_DOMAIN_SECRET_INFO_IV:
+ case VIR_DOMAIN_SECRET_INFO_LAST:
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/* qemuBuildRBDSecinfoURI:
+ * @uri: Pointer to the URI structure to add to
+ * @secinfo: Pointer to the secret info data (if present)
+ *
+ * If we have a secinfo, then build the command line options for
+ * the secret info for the RBD network storage. Assumption for this
+ * is both username and secret exist for plaintext
+ *
+ * Returns 0 on success or if no secinfo,
+ * -1 and error message if fail to add secret information
+ */
+static int
+qemuBuildRBDSecinfoURI(virBufferPtr buf,
+ qemuDomainSecretInfoPtr secinfo)
+{
+ if (!secinfo) {
+ virBufferAddLit(buf, ":auth_supported=none");
+ return 0;
+ }
+
+ switch ((qemuDomainSecretInfoType) secinfo->type) {
+ case VIR_DOMAIN_SECRET_INFO_PLAIN:
+ virBufferEscape(buf, '\\', ":", ":id=%s",
+ secinfo->s.plain.username);
+ virBufferEscape(buf, '\\', ":",
+ ":key=%s:auth_supported=cephx\\;none",
+ secinfo->s.plain.secret);
+ break;
+
+ case VIR_DOMAIN_SECRET_INFO_IV:
+ case VIR_DOMAIN_SECRET_INFO_LAST:
+ return -1;
+ }
+
+ return 0;
+}
+
+
#define QEMU_DEFAULT_NBD_PORT "10809"
static char *
@@ -701,7 +780,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
goto cleanup;
}
- if ((uri->port = qemuNetworkDriveGetPort(src->protocol,
src->hosts->port)) < 0)
+ if ((uri->port = qemuNetworkDriveGetPort(src->protocol,
+ src->hosts->port)) < 0)
goto cleanup;
if (src->path) {
@@ -721,17 +801,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
virAsprintf(&uri->query, "socket=%s",
src->hosts->socket) < 0)
goto cleanup;
- if (secinfo) {
- if (secinfo->s.plain.secret) {
- if (virAsprintf(&uri->user, "%s:%s",
- secinfo->s.plain.username,
- secinfo->s.plain.secret) < 0)
- goto cleanup;
- } else {
- if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
- goto cleanup;
- }
- }
+ if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0)
+ goto cleanup;
if (VIR_STRDUP(uri->server, src->hosts->name) < 0)
goto cleanup;
@@ -777,15 +848,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
if (src->snapshot)
virBufferEscape(&buf, '\\', ":", "@%s",
src->snapshot);
- if (secinfo) {
- virBufferEscape(&buf, '\\', ":",
":id=%s",
- secinfo->s.plain.username);
- virBufferEscape(&buf, '\\', ":",
- ":key=%s:auth_supported=cephx\\;none",
- secinfo->s.plain.secret);
- } else {
- virBufferAddLit(&buf, ":auth_supported=none");
- }
+ if (qemuBuildRBDSecinfoURI(&buf, secinfo) < 0)
+ goto cleanup;
if (src->nhosts > 0) {
virBufferAddLit(&buf, ":mon_host=");
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 03fdab1..17577ef 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -740,12 +740,34 @@ qemuDomainSecretPlainFree(qemuDomainSecretPlain secret)
static void
+qemuDomainSecretIVFree(qemuDomainSecretIV secret)
+{
+ VIR_FREE(secret.username);
+ VIR_FREE(secret.alias);
+ VIR_FREE(secret.iv);
+ VIR_FREE(secret.ciphertext);
+}
+
+
+static void
qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo)
{
if (!*secinfo)
return;
- qemuDomainSecretPlainFree((*secinfo)->s.plain);
+ switch ((qemuDomainSecretInfoType) (*secinfo)->type) {
+ case VIR_DOMAIN_SECRET_INFO_PLAIN:
+ qemuDomainSecretPlainFree((*secinfo)->s.plain);
+ break;
+
+ case VIR_DOMAIN_SECRET_INFO_IV:
+ qemuDomainSecretIVFree((*secinfo)->s.iv);
+ break;
+
+ case VIR_DOMAIN_SECRET_INFO_LAST:
+ break;
+ }
+
VIR_FREE(*secinfo);
}
@@ -893,6 +915,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
/* qemuDomainSecretDiskPrepare:
* @conn: Pointer to connection
+ * @priv: pointer to domain private object
* @disk: Pointer to a disk definition
*
* For the right disk, generate the qemuDomainSecretInfo structure.
@@ -901,6 +924,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
*/
int
qemuDomainSecretDiskPrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED,
virDomainDiskDefPtr disk)
{
virStorageSourcePtr src = disk->src;
@@ -952,6 +976,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev)
/* qemuDomainSecretHostdevPrepare:
* @conn: Pointer to connection
+ * @priv: pointer to domain private object
* @hostdev: Pointer to a hostdev definition
*
* For the right host device, generate the qemuDomainSecretInfo structure.
@@ -960,6 +985,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev)
*/
int
qemuDomainSecretHostdevPrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED,
virDomainHostdevDefPtr hostdev)
{
virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
@@ -1032,15 +1058,17 @@ int
qemuDomainSecretPrepare(virConnectPtr conn,
virDomainObjPtr vm)
{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
size_t i;
for (i = 0; i < vm->def->ndisks; i++) {
- if (qemuDomainSecretDiskPrepare(conn, vm->def->disks[i]) < 0)
+ if (qemuDomainSecretDiskPrepare(conn, priv, vm->def->disks[i]) < 0)
return -1;
}
for (i = 0; i < vm->def->nhostdevs; i++) {
- if (qemuDomainSecretHostdevPrepare(conn, vm->def->hostdevs[i]) < 0)
+ if (qemuDomainSecretHostdevPrepare(conn, priv,
+ vm->def->hostdevs[i]) < 0)
return -1;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 63f98ba..9b5f108 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -242,6 +242,7 @@ struct _qemuDomainObjPrivate {
/* Type of domain secret */
typedef enum {
VIR_DOMAIN_SECRET_INFO_PLAIN = 0,
+ VIR_DOMAIN_SECRET_INFO_IV = 1,
VIR_DOMAIN_SECRET_INFO_LAST
} qemuDomainSecretInfoType;
@@ -253,12 +254,24 @@ struct _qemuDomainSecretPlain {
char *secret;
};
+# define QEMU_DOMAIN_IV_KEY_LEN 16 /* 16 bytes for 128 bit random */
+ /* initialization vector key */
+typedef struct _qemuDomainSecretIV qemuDomainSecretIV;
+typedef struct _qemuDomainSecretIV *qemuDomainSecretIVPtr;
+struct _qemuDomainSecretIV {
+ 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 {
int type; /* qemuDomainSecretInfoType */
union {
qemuDomainSecretPlain plain;
+ qemuDomainSecretIV iv;
} s;
};
@@ -631,15 +644,18 @@ void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv);
void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
ATTRIBUTE_NONNULL(1);
-int qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuDomainSecretDiskPrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ virDomainDiskDefPtr disk)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
void qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr disk)
ATTRIBUTE_NONNULL(1);
int qemuDomainSecretHostdevPrepare(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
virDomainHostdevDefPtr hostdev)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
void qemuDomainSecretDestroy(virDomainObjPtr vm)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c05f88c..a3989e9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -354,7 +354,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
- if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+ if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0)
goto error;
if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
@@ -581,7 +581,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
- if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+ if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0)
goto error;
if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
@@ -1976,7 +1976,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1)
< 0)
goto cleanup;
- if (qemuDomainSecretHostdevPrepare(conn, hostdev) < 0)
+ if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0)
goto cleanup;
if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps)))
--
2.5.5
9:17 a.m.
New subject: [libvirt] [PATCH v2 12/12] qemu: Utilize qemu secret objects for SCSI/RBD auth/secret
https://bugzilla.redhat.com/show_bug.cgi?id=1182074
If they're available and we need to pass secrets to qemu, then use the
qemu domain secret object in order to pass the secrets for iSCSI and
RBD volumes instead of passing plaintext or base64 encoded secrets on
the command line.
New APIs:
qemuDomainGetIVKeyAlias:
Generate/return the secret object alias for an initialization
vector (IV) secret info type. This will be saved in the secret
info block.
qemuDomainSecretInfoGetAlias:
Return a pointer to the alias to the specific secret info as
long as the secret object is supported (future patches may add
a new secret info type with a different pointer to return).
qemuDomainSecretHaveEncrypt:
Boolean function to determine whether the underly encryption
API is available. This function will utilize a similar mechanism
as the 'gnutls_rnd' did in configure.ac. This function creates
the encrypted secret based upon the domain master key, an
initialization vector (16 byte random value), and the stored secret.
qemuDomainSecretIVSetup: (private)
This API handles the details of the generation of the IV secret
and saves the pieces that need to be passed to qemu in order for
the secret to be decrypted. The requirement from qemu is the IV
and encrypted secret are to be base64 encoded. They can be passed
either directly or within a file. This implementation chooses
to pass directly rather than a file (file passing is shown below).
qemuBuildiSCSICommandLine: (private)
qemuBuildDiskiSCSICommandLine
qemuBuildHostdeviSCSICommandLine
These API's will handle adding the IV secret onto an '-iscsi'
command line option.
qemuBuildSecretInfoProps: (private)
Generate/return a JSON properties object for the IV secret to
be used by both the command building and eventually the hotplug
code in order to add the secret object.
Changes for qemuDomainSecret{Disk|Hostdev}Prepare:
If both the encryption API exists and the secret object exist, then
setup the IV secret (qemuDomainSecretIVSetup) as the default means
for the disk/hostdev to provide the secret to qemu. Prior to command
line generation and during hotplug, these prepare API's are called
allowing for code after the API to perform the right steps.
Command Building:
Adjust the qemuBuild{General|RBD}SecinfoURI API's in order to generate
the specific command options for an IV secret, such as:
For iSCSI:
-object secret,id=sec0,keyid=$masterKey,filename=/path/to/example.pw
or
-object secret,id=sec0,keyid=$masterKey,data=$base64encodedencrypted,
format=base64
-iscsi -initiator-name=$iqn,user=user,password-secret=sec0
-drive file=iscsi://example.com/$iqn/1,...
For RBD:
-object secret,id=secret0,keyid=$masterKey,file=/path/to/poolkey.b64,
format=base64
or
-object secret,id=secret0,keyid=$masterKey,data=$base64encodedencrypted,
format=base64
-drive file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\
mon_host=mon1.example.org\:6321,password-secret=secret0,...
where for both 'id=' value is the secret object alias, the 'keyid=
$masterKey' is the master key shared with qemu, and the -drive
syntax will reference that alias as the 'password-secret'. For
the iSCSI object 'user=' replaces the URI generated 'user:secret@'
prepended to the iSCSI 'host' name (example.com). For the RBD -drive
syntax, the 'id=myname' is kept to define the username, while the
'key=$base64 encoded secret' is removed.
While according to the syntax described for qemu commits 'b189346eb'
(iSCSI) and '60390a21' (RBD) or as seen in the email archive:
https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04083.html
it is possible to pass a plaintext password via a file, the qemu
commit 'ac1d8878' describes the more feature rich 'keyid=' option
based upon the shared masterKey.
Tests:
Add mock's for virRandomBytes and gnutls_rnd in order to return a
constant stream of '0xff' in the bytes for a non random key in order
to generate "constant" values for the secrets so that the tests can
use those results to compare results.
Hotplug:
Since the hotplug code doesn't add command line arguments, passing
the encoded/plaintext secrets directly to the monitor will suffice.
Besides, it's passing the IV secret via '-iscsi' won't be possible.
Perhaps when the -drive command is modified to accept not only the
initiator-name, but -user and -password-secret arguments, then the
IV code can be utilized for hotplug secrets.
The goal is to make this the default and have no user interaction
required in order to allow using the IV mechanism. If the mechanism
is not available, then fall back to the current mechanism.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
configure.ac | 1 +
src/qemu/qemu_alias.c | 23 ++
src/qemu/qemu_alias.h | 2 +
src/qemu/qemu_command.c | 257 ++++++++++++++++++++-
src/qemu/qemu_domain.c | 207 ++++++++++++++++-
src/qemu/qemu_domain.h | 3 +
...uxml2argv-disk-drive-network-iscsi-auth-IV.args | 39 ++++
...muxml2argv-disk-drive-network-iscsi-auth-IV.xml | 43 ++++
...emuxml2argv-disk-drive-network-rbd-auth-IV.args | 31 +++
...qemuxml2argv-disk-drive-network-rbd-auth-IV.xml | 42 ++++
...emuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args | 41 ++++
...qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml | 48 ++++
...xml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args | 43 ++++
...uxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml | 48 ++++
tests/qemuxml2argvmock.c | 31 ++-
tests/qemuxml2argvtest.c | 19 ++
16 files changed, 865 insertions(+), 13 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml
diff --git a/configure.ac b/configure.ac
index 450f02c..f9c66e6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1305,6 +1305,7 @@ if test "x$with_gnutls" != "xno"; then
]])
AC_CHECK_FUNCS([gnutls_rnd])
+ AC_CHECK_FUNCS([gnutls_cipher_encrypt])
CFLAGS="$old_CFLAGS"
LIBS="$old_LIBS"
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index ade2033..de9a74f 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -556,3 +556,26 @@ qemuDomainGetMasterKeyAlias(void)
return alias;
}
+
+
+/* qemuDomainGetIVKeyAlias:
+ *
+ * Generate and return an initialization vector alias
+ *
+ * Returns NULL or a string containing the IV key alias
+ */
+char *
+qemuDomainGetIVKeyAlias(const char *srcalias)
+{
+ char *alias;
+
+ if (!srcalias) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("secret iv alias requires valid source alias"));
+ return NULL;
+ }
+
+ ignore_value(virAsprintf(&alias, "%s-ivKey0", srcalias));
+
+ return alias;
+}
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 2d7bc9b..435747e 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -69,4 +69,6 @@ char *qemuAliasFromDisk(const virDomainDiskDef *disk);
char *qemuDomainGetMasterKeyAlias(void);
+char *qemuDomainGetIVKeyAlias(const char *srcalias);
+
#endif /* __QEMU_ALIAS_H__*/
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a8cacb3..db851ed 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -607,9 +607,131 @@ qemuNetworkDriveGetPort(int protocol,
}
+/* qemuBuildiSCSICommandLine:
+ * @cmd: Pointer to the command string
+ * @src: Pointer to a possible disk source
+ * @secinfo: Pointer to a possible secinfo
+ *
+ * Add an "-iscsi" argument for the command line. This provides the
+ * capability to add just an "iqn." string to the command line in order
+ * to override the default IQN (iqn.2008-11.org.linux-kvm) for qemu.
+ * Additionally, this will be the mechanism used to pass the IV secret
+ * to qemu since (currently - as of 2.6), the "user=" and
"password-secret="
+ * arguments are only handled with the "-iscsi" argument.
+ *
+ * RFC 3720 indicates that the IQN is made up of at least one part - a
+ * string starting with "iqn." which describes the naming authority. The
+ * standard references an optional field after a colon (:) is naming
+ * authority specific. For libvirt XML, an iSCSI name string typically
+ * has both the "iqn." and the colon along with a '/#' field denoting
the
+ * iSCSI LUN to use, e.g.:
+ *
+ * iqn.2016-04.com.example:storage/1
+ *
+ * We can pass this entire iqn string in the "initiator-name=" option
+ * of the "-iscsi" switch. As it turns out the "id=" option is not
+ * required, so no need to break our string. Although it would be
+ * possible to split at the colon, passing the "iqn." as just the
+ * "initiator-name=" option and adjusting the "storage/1" to
"storage_1"
+ * and passing in the "id=" option. This second option has limitations
+ * since the "id=" value must be letters, digits, '-', '.', and
"_"
+ * starting with a letter.
+ *
+ * Returns 0 on success or if not necessary, -1 on some sort of failure
+ */
+static int
+qemuBuildiSCSICommandLine(virCommandPtr cmd,
+ const char *path,
+ qemuDomainSecretInfoPtr secinfo)
+{
+ virBuffer opt = VIR_BUFFER_INITIALIZER;
+ char *bufstr;
+
+ virBufferAsprintf(&opt, "initiator-name=%s", path);
+
+ /* If we have a secinfo, then append on the user/password-secret as
+ * this will be something for an iSCSI disk using IV secrets */
+ if (secinfo) {
+ virBufferAsprintf(&opt, ",user=%s,password-secret=%s",
+ secinfo->s.iv.username,
+ secinfo->s.iv.alias);
+ }
+
+ if (virBufferCheckError(&opt) < 0) {
+ virBufferFreeAndReset(&opt);
+ return -1;
+ }
+
+ bufstr = virBufferContentAndReset(&opt);
+ virCommandAddArg(cmd, "-iscsi");
+ virCommandAddArg(cmd, bufstr);
+ VIR_FREE(bufstr);
+
+ return 0;
+}
+
+
+/* qemuBuildDiskiSCSICommandLine:
+ * @cmd: Pointer to the command string
+ * @src: Pointer to a possible disk source
+ * @secinfo: Pointer to a possible secinfo
+ *
+ * Adding an IV secret to the command line for an iSCSI disk is a multistep
+ * and complicated endeavor. It is not possible to simply append "user=name,
+ * password-secret=alias-id" to a "-drive file=iscsi://..." command.
Instead,
+ * one must first add an -iscsi option with the IQN data in order to utilize
+ * the "user" and "password-secret" options.
+ *
+ * This function will check the storage source to see if it requires
+ * an '-iscsi' argument and make the call to do.
+ *
+ * Returns 0 on success, -1 w/ error on some sort of failure.
+ */
+static int
+qemuBuildDiskiSCSICommandLine(virCommandPtr cmd,
+ virStorageSourcePtr src,
+ qemuDomainSecretInfoPtr secinfo)
+{
+ /* Only necessary for networked, iSCSI disks with an IV secret to
+ * hide as qemu parsing logic parses the user and password-secret
+ * using the -iscsi command argument.
+ */
+ if (virStorageSourceIsEmpty(src) ||
+ src->protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI ||
+ !secinfo || secinfo->type != VIR_DOMAIN_SECRET_INFO_IV)
+ return 0;
+
+ return qemuBuildiSCSICommandLine(cmd, src->path, secinfo);
+}
+
+
+/* qemuBuildHostdeviSCSICommandLine:
+ * @cmd: Pointer to the command string
+ * @hostdev: Pointer to a host device
+ * @secinfo: Pointer to a possible secinfo
+ *
+ * See qemuBuildDiskiSCSICommandLine for details.
+ */
+static int
+qemuBuildHostdeviSCSICommandLine(virCommandPtr cmd,
+ virDomainHostdevDefPtr hostdev,
+ qemuDomainSecretInfoPtr secinfo)
+{
+ virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+ virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
+
+ /* Only necessary for hostdev's using the IV secret */
+ if (!secinfo || secinfo->type != VIR_DOMAIN_SECRET_INFO_IV)
+ return 0;
+
+ return qemuBuildiSCSICommandLine(cmd, iscsisrc->path, secinfo);
+}
+
+
/* qemuBuildGeneralSecinfoURI:
* @uri: Pointer to the URI structure to add to
* @secinfo: Pointer to the secret info data (if present)
+ * @protocol: Protocol for the source device
*
* If we have a secinfo, then build the command line options for
* the secret info for the "general" case (somewhat a misnomer since
@@ -620,7 +742,8 @@ qemuNetworkDriveGetPort(int protocol,
*/
static int
qemuBuildGeneralSecinfoURI(virURIPtr uri,
- qemuDomainSecretInfoPtr secinfo)
+ qemuDomainSecretInfoPtr secinfo,
+ int protocol)
{
if (!secinfo)
return 0;
@@ -639,6 +762,13 @@ qemuBuildGeneralSecinfoURI(virURIPtr uri,
break;
case VIR_DOMAIN_SECRET_INFO_IV:
+ /* iSCSI has a completely different way to handle this
+ * see qemuBuildiSCSICommandLine */
+ if (protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI &&
+ VIR_STRDUP(uri->user, secinfo->s.iv.username) < 0)
+ return -1;
+ break;
+
case VIR_DOMAIN_SECRET_INFO_LAST:
return -1;
}
@@ -677,6 +807,10 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
break;
case VIR_DOMAIN_SECRET_INFO_IV:
+ virBufferEscape(buf, '\\', ":",
":id=%s:auth_supported=cephx\\;none",
+ secinfo->s.iv.username);
+ break;
+
case VIR_DOMAIN_SECRET_INFO_LAST:
return -1;
}
@@ -801,7 +935,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
virAsprintf(&uri->query, "socket=%s",
src->hosts->socket) < 0)
goto cleanup;
- if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0)
+ if (qemuBuildGeneralSecinfoURI(uri, secinfo, src->protocol) < 0)
goto cleanup;
if (VIR_STRDUP(uri->server, src->hosts->name) < 0)
@@ -1069,6 +1203,97 @@ qemuCheckFips(void)
}
+/**
+ * qemuBuildSecretInfoProps:
+ * @secinfo: pointer to the secret info object
+ * @type: returns a pointer to a character string for object name
+ * @props: json properties to return
+ *
+ * Build the JSON properties for the secret info type.
+ *
+ * Returns 0 on success with the filled in JSON property; otherwise,
+ * returns -1 on failure error message set.
+ */
+static int
+qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
+ const char **type,
+ virJSONValuePtr *propsret)
+{
+ int ret = -1;
+ char *keyid = NULL;
+ virJSONValuePtr props = NULL;
+
+ *type = "secret";
+
+ if (!(keyid = qemuDomainGetMasterKeyAlias()))
+ return -1;
+
+ if (!(props = virJSONValueNewObject()))
+ goto cleanup;
+
+ if (virJSONValueObjectAdd(props,
+ "s:data", secinfo->s.iv.ciphertext,
+ "s:keyid", keyid,
+ "s:iv", secinfo->s.iv.iv,
+ "s:format", "base64", NULL) < 0)
+ goto cleanup;
+
+ *propsret = props;
+ props = NULL;
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(keyid);
+ virJSONValueFree(props);
+
+ return ret;
+}
+
+
+/**
+ * qemuBuildSecretIVCommandLine:
+ * @cmd: the command to modify
+ * @secinfo: pointer to the secret info object
+ * @qemuCaps: pointer to the emulator capabilities
+ *
+ * If the secinfo is available and associated with an IV secret,
+ * then format the command line for the secret object. This object
+ * will be referenced by the device that needs/uses it, so it needs
+ * to be in place first.
+ *
+ * Returns 0 on success, -1 w/ error message on failure
+ */
+static int
+qemuBuildSecretIVCommandLine(virCommandPtr cmd,
+ qemuDomainSecretInfoPtr secinfo,
+ virQEMUCapsPtr qemuCaps)
+{
+ int ret = -1;
+ virJSONValuePtr props = NULL;
+ const char *objAlias;
+ const char *type;
+ char *tmp = NULL;
+
+ if (!(objAlias = qemuDomainSecretInfoGetAlias(secinfo, qemuCaps)))
+ return 0;
+
+ if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0)
+ return -1;
+
+ if (!(tmp = qemuBuildObjectCommandlineFromJSON(type, objAlias, props)))
+ goto cleanup;
+
+ virCommandAddArgList(cmd, "-object", tmp, NULL);
+ ret = 0;
+
+ cleanup:
+ virJSONValueFree(props);
+ VIR_FREE(tmp);
+
+ return ret;
+}
+
+
char *
qemuBuildDriveStr(virDomainDiskDefPtr disk,
bool bootable,
@@ -1083,6 +1308,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
char *source = NULL;
int actualType = virStorageSourceGetActualType(disk->src);
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+ qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
if (idx < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1164,7 +1390,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
break;
}
- if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, &source) <
0)
+ if (qemuGetDriveSourceString(disk->src, secinfo, &source) < 0)
goto error;
if (source &&
@@ -1215,6 +1441,11 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
virBufferEscape(&opt, ',', ",", "%s,", source);
+ if (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD &&
+ secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_IV) {
+ virBufferAsprintf(&opt, "password-secret=%s,",
secinfo->s.iv.alias);
+ }
+
if (disk->src->format > 0 &&
disk->src->type != VIR_STORAGE_TYPE_DIR)
virBufferAsprintf(&opt, "format=%s,",
@@ -1888,6 +2119,8 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
virDomainDiskDefPtr disk = def->disks[i];
bool withDeviceArg = false;
bool deviceFlagMasked = false;
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+ qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
/* Unless we have -device, then USB disks need special
handling */
@@ -1930,6 +2163,12 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
break;
}
+ if (qemuBuildSecretIVCommandLine(cmd, secinfo, qemuCaps) < 0)
+ return -1;
+
+ if (qemuBuildDiskiSCSICommandLine(cmd, disk->src, secinfo) < 0)
+ return -1;
+
virCommandAddArg(cmd, "-drive");
/* Unfortunately it is not possible to use
@@ -4551,7 +4790,9 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(dev)))
goto error;
+
virBufferAsprintf(&buf, "file=%s,if=none,format=raw", source);
+
} else {
if (!(source = qemuBuildSCSIHostHostdevDrvStr(dev)))
goto error;
@@ -4992,8 +5233,18 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
+
+ qemuDomainHostdevPrivatePtr hostdevPriv =
+ QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev);
+ qemuDomainSecretInfoPtr secinfo = hostdevPriv->secinfo;
char *drvstr;
+ if (qemuBuildSecretIVCommandLine(cmd, secinfo, qemuCaps) < 0)
+ return -1;
+
+ if (qemuBuildHostdeviSCSICommandLine(cmd, hostdev, secinfo) < 0)
+ return -1;
+
virCommandAddArg(cmd, "-drive");
if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps)))
return -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 17577ef..3f4f7f9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -772,6 +772,32 @@ qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo)
}
+/* qemuDomainSecretInfoGetAlias:
+ * @secinfo: pointer to the secret info object
+ * @qemuCaps: pointer to the emulator capabilities
+ *
+ * If the emulator supports it, secinfo is available and associated with
+ * an IV secret, then return the alias created during the disk or hostdev
+ * prepare step.
+ *
+ * Returns pointer to the object alias string or NULL if not found/supported
+ */
+const char *
+qemuDomainSecretInfoGetAlias(qemuDomainSecretInfoPtr secinfo,
+ virQEMUCapsPtr qemuCaps)
+{
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET)) {
+ VIR_INFO("secret object is not supported by this QEMU binary");
+ return NULL;
+ }
+
+ if (!secinfo || secinfo->type != VIR_DOMAIN_SECRET_INFO_IV)
+ return NULL;
+
+ return secinfo->s.iv.alias;
+}
+
+
static virClassPtr qemuDomainDiskPrivateClass;
static void qemuDomainDiskPrivateDispose(void *obj);
@@ -896,6 +922,142 @@ qemuDomainSecretPlainSetup(virConnectPtr conn,
}
+/* qemuDomainSecretHaveEncypt:
+ *
+ * Returns true if we can support the encryption code, false otherwise
+ */
+static bool
+qemuDomainSecretHaveEncrypt(void)
+{
+#ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
+ return true;
+#else
+ return false;
+#endif
+}
+
+
+/* qemuDomainSecretIVSetup:
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @secinfo: Pointer to secret info
+ * @protocol: Protocol for secret
+ * @authdef: Pointer to auth data
+ *
+ * Taking a secinfo, fill in the initialization vector information
+ *
+ * Returns 0 on success, -1 on failure with error message
+ */
+static int
+qemuDomainSecretIVSetup(virConnectPtr conn,
+ qemuDomainObjPrivatePtr priv,
+ qemuDomainSecretInfoPtr secinfo,
+ const char *srcalias,
+ virStorageNetProtocol protocol,
+ virStorageAuthDefPtr authdef)
+{
+ int ret = -1;
+ int rc;
+ uint8_t *raw_iv = NULL;
+ char *secret = NULL;
+ uint8_t *ciphertext = NULL;
+ size_t secretlen = 0, ciphertextlen = 0, paddinglen;
+ int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
+ const char *protocolstr = virStorageNetProtocolTypeToString(protocol);
+ gnutls_cipher_hd_t handle = NULL;
+ gnutls_datum_t enc_key;
+ gnutls_datum_t iv_key;
+
+ secinfo->type = VIR_DOMAIN_SECRET_INFO_IV;
+ if (VIR_STRDUP(secinfo->s.iv.username, authdef->username) < 0)
+ return -1;
+
+ if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD)
+ secretType = VIR_SECRET_USAGE_TYPE_CEPH;
+
+ if (!(secinfo->s.iv.alias = qemuDomainGetIVKeyAlias(srcalias)))
+ return -1;
+
+ /* Create a random initialization vector */
+ if (!(raw_iv = qemuDomainGenerateRandomKey(QEMU_DOMAIN_IV_KEY_LEN)))
+ return -1;
+
+ /* Encode the IV and save that since qemu will need it */
+ base64_encode_alloc((const char *)raw_iv, QEMU_DOMAIN_IV_KEY_LEN,
+ &secinfo->s.iv.iv);
+ if (!secinfo->s.iv.iv) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to encode initialization vector"));
+ goto cleanup;
+ }
+
+ /* Grab the unencoded secret */
+ if (!(secret = virSecretGetSecretString(conn, protocolstr, false,
+ authdef, secretType)))
+ goto cleanup;
+
+ /* Allocate a padded buffer */
+ secretlen = strlen(secret);
+ paddinglen = 16 - (secretlen % 16);
+ ciphertextlen = secretlen + paddinglen;
+ if (VIR_ALLOC_N(ciphertext, ciphertextlen) < 0)
+ goto cleanup;
+ memcpy(ciphertext, secret, secretlen);
+ memset(ciphertext + secretlen, paddinglen, paddinglen);
+
+ /* clear secret so that it doesn't hang around */
+ memset(secret, 0, strlen(secret));
+
+ /* Initialize the gnutls cipher */
+ enc_key.size = QEMU_DOMAIN_MASTER_KEY_LEN;
+ enc_key.data = priv->masterKey;
+ iv_key.size = QEMU_DOMAIN_IV_KEY_LEN;
+ iv_key.data = raw_iv;
+ if ((rc = gnutls_cipher_init(&handle, GNUTLS_CIPHER_AES_256_CBC,
+ &enc_key, &iv_key)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to initialize cipher: '%s'"),
+ gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ /* Encrypt the secret and free the memory for cipher operations */
+ rc = gnutls_cipher_encrypt(handle, ciphertext, ciphertextlen);
+ gnutls_cipher_deinit(handle);
+ if (rc < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to encrypt the secret: '%s'"),
+ gnutls_strerror(rc));
+ goto cleanup;
+ }
+
+ /* Now base64 encode the ciphertext and store to be passed to qemu */
+ base64_encode_alloc((const char *)ciphertext, ciphertextlen,
+ &secinfo->s.iv.ciphertext);
+ if (!secinfo->s.iv.ciphertext) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to encode ciphertext"));
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+
+ /* Clear and free the raw_iv, plaintext secret, and ciphertext */
+ memset(raw_iv, 0, QEMU_DOMAIN_IV_KEY_LEN);
+ VIR_FREE(raw_iv);
+
+ memset(secret, 0, secretlen);
+ VIR_FREE(secret);
+
+ memset(ciphertext, 0, ciphertextlen);
+ VIR_FREE(ciphertext);
+
+ return ret;
+}
+
+
/* qemuDomainSecretDiskDestroy:
* @disk: Pointer to a disk definition
*
@@ -924,7 +1086,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
*/
int
qemuDomainSecretDiskPrepare(virConnectPtr conn,
- qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED,
+ qemuDomainObjPrivatePtr priv,
virDomainDiskDefPtr disk)
{
virStorageSourcePtr src = disk->src;
@@ -941,9 +1103,23 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
if (VIR_ALLOC(secinfo) < 0)
return -1;
- if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
- src->auth) < 0)
- goto error;
+ /* If we have the encryption API present and can support a
+ * secret object, then build the IV secret - this is the magic
+ * decision point for utilizing the IV secrets for a disk
+ * whether it's an iSCSI or an RBD disk.
+ */
+ if (qemuDomainSecretHaveEncrypt() &&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET)) {
+ if (qemuDomainSecretIVSetup(conn, priv, secinfo,
+ disk->info.alias,
+ src->protocol,
+ src->auth) < 0)
+ goto error;
+ } else {
+ if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
+ src->auth) < 0)
+ goto error;
+ }
diskPriv->secinfo = secinfo;
}
@@ -985,7 +1161,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev)
*/
int
qemuDomainSecretHostdevPrepare(virConnectPtr conn,
- qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED,
+ qemuDomainObjPrivatePtr priv,
virDomainHostdevDefPtr hostdev)
{
virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys;
@@ -1006,10 +1182,23 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
if (VIR_ALLOC(secinfo) < 0)
return -1;
- if (qemuDomainSecretPlainSetup(conn, secinfo,
- VIR_STORAGE_NET_PROTOCOL_ISCSI,
- iscsisrc->auth) < 0)
- goto error;
+ /* If we have the encryption API present and can support a
+ * secret object, then build the IV secret. This is the magic
+ * decision point for utilizing IV secrets for an iSCSI hostdev
+ */
+ if (qemuDomainSecretHaveEncrypt() &&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET)) {
+ if (qemuDomainSecretIVSetup(conn, priv, secinfo,
+ hostdev->info->alias,
+ VIR_STORAGE_NET_PROTOCOL_ISCSI,
+ iscsisrc->auth) < 0)
+ goto error;
+ } else {
+ if (qemuDomainSecretPlainSetup(conn, secinfo,
+ VIR_STORAGE_NET_PROTOCOL_ISCSI,
+ iscsisrc->auth) < 0)
+ goto error;
+ }
hostdevPriv->secinfo = secinfo;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9b5f108..c52679a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -641,6 +641,9 @@ int qemuDomainMasterKeyCreate(virQEMUDriverPtr driver,
void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv);
+const char *qemuDomainSecretInfoGetAlias(qemuDomainSecretInfoPtr secinfo,
+ virQEMUCapsPtr qemuCaps);
+
void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
ATTRIBUTE_NONNULL(1);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args
new file mode 100644
index 0000000..bce8d2f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args
@@ -0,0 +1,39 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-object secret,id=virtio-disk0-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-iscsi initiator-name=iqn.1992-01.com.example:storage/1,user=myname,\
+password-secret=virtio-disk0-ivKey0 \
+-drive file=iscsi://example.org:6000/iqn.1992-01.com.example%3Astorage/1,\
+format=raw,if=none,id=drive-virtio-disk0 \
+-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-object secret,id=virtio-disk1-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-iscsi initiator-name=iqn.1992-01.com.example:storage/2,user=myname,\
+password-secret=virtio-disk1-ivKey0 \
+-drive file=iscsi://example.org:6000/iqn.1992-01.com.example%3Astorage/2,\
+format=raw,if=none,id=drive-virtio-disk1 \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\
+id=virtio-disk1
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml
new file mode 100644
index 0000000..1f80d3b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml
@@ -0,0 +1,43 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='network' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <auth username='myname'>
+ <secret type='iscsi' usage='mycluster_myname'/>
+ </auth>
+ <source protocol='iscsi'
name='iqn.1992-01.com.example:storage/1'>
+ <host name='example.org' port='6000'/>
+ </source>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <disk type='network' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <auth username='myname'>
+ <secret type='iscsi' usage='mycluster_myname'/>
+ </auth>
+ <source protocol='iscsi'
name='iqn.1992-01.com.example:storage/2'>
+ <host name='example.org' port='6000'/>
+ </source>
+ <target dev='vdb' bus='virtio'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
new file mode 100644
index 0000000..f6c0906
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-object secret,id=virtio-disk0-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-drive 'file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\
+mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\
+password-secret=virtio-disk0-ivKey0,format=raw,if=none,id=drive-virtio-disk0' \
+-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
new file mode 100644
index 0000000..ac2e942
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
@@ -0,0 +1,42 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <disk type='network' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <auth username='myname'>
+ <secret type='ceph' usage='mycluster_myname'/>
+ </auth>
+ <source protocol='rbd' name='pool/image'>
+ <host name='mon1.example.org' port='6321'/>
+ <host name='mon2.example.org' port='6322'/>
+ <host name='mon3.example.org' port='6322'/>
+ </source>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args
new file mode 100644
index 0000000..6b86aef
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args
@@ -0,0 +1,41 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest2 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device lsi,id=scsi0,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-object secret,id=hostdev0-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-iscsi initiator-name=iqn.1992-01.com.example:storage/1,user=myname,\
+password-secret=hostdev0-ivKey0 \
+-drive file=iscsi://example.org:3260/iqn.1992-01.com.example%3Astorage/1,if=none,\
+format=raw,id=drive-hostdev0 \
+-device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \
+-object secret,id=hostdev1-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-iscsi initiator-name=iqn.1992-01.com.example:storage/2,user=myname,\
+password-secret=hostdev1-ivKey0 \
+-drive file=iscsi://example.org:3260/iqn.1992-01.com.example%3Astorage/2,if=none,\
+format=raw,id=drive-hostdev1 \
+-device scsi-generic,bus=scsi0.0,scsi-id=5,drive=drive-hostdev1,id=hostdev1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml
new file mode 100644
index 0000000..f988165
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml
@@ -0,0 +1,48 @@
+<domain type='qemu'>
+ <name>QEMUGuest2</name>
+ <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='scsi' index='0'/>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <hostdev mode='subsystem' type='scsi' managed='yes'>
+ <source protocol='iscsi'
name='iqn.1992-01.com.example:storage/1'>
+ <host name='example.org' port='3260'/>
+ <auth username='myname'>
+ <secret type='iscsi' usage='mycluster_myname'/>
+ </auth>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='0' unit='4'/>
+ </hostdev>
+ <hostdev mode='subsystem' type='scsi' managed='yes'>
+ <source protocol='iscsi'
name='iqn.1992-01.com.example:storage/2'>
+ <host name='example.org' port='3260'/>
+ <auth username='myname'>
+ <secret type='iscsi' usage='mycluster_myname'/>
+ </auth>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='0' unit='5'/>
+ </hostdev>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args
new file mode 100644
index 0000000..4ecbe53
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args
@@ -0,0 +1,43 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest2 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1 \
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-object secret,id=hostdev0-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-iscsi initiator-name=iqn.1992-01.com.example:storage/1,user=myname,\
+password-secret=hostdev0-ivKey0 \
+-drive file=iscsi://example.org:3260/iqn.1992-01.com.example%3Astorage/1,if=none,\
+format=raw,id=drive-hostdev0 \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=4,drive=drive-hostdev0,\
+id=hostdev0 \
+-object secret,id=hostdev1-ivKey0,\
+data=/i1QO1S81K4UELoLXmtCNQzxOaE1Tc4DvecFBfyvFKKWUAawbjGD+yZaz9oyEcnW,\
+keyid=masterKey0,iv=/////////////////////w==,format=base64 \
+-iscsi initiator-name=iqn.1992-01.com.example:storage/2,user=myname,\
+password-secret=hostdev1-ivKey0 \
+-drive file=iscsi://example.org:3260/iqn.1992-01.com.example%3Astorage/2,if=none,\
+format=raw,id=drive-hostdev1 \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=5,drive=drive-hostdev1,\
+id=hostdev1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml
new file mode 100644
index 0000000..b70b84e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml
@@ -0,0 +1,48 @@
+<domain type='qemu'>
+ <name>QEMUGuest2</name>
+ <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='scsi' index='0'
model='virtio-scsi'/>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <hostdev mode='subsystem' type='scsi' managed='yes'>
+ <source protocol='iscsi'
name='iqn.1992-01.com.example:storage/1'>
+ <host name='example.org' port='3260'/>
+ <auth username='myname'>
+ <secret type='iscsi' usage='mycluster_myname'/>
+ </auth>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='2' unit='4'/>
+ </hostdev>
+ <hostdev mode='subsystem' type='scsi' managed='yes'>
+ <source protocol='iscsi'
name='iqn.1992-01.com.example:storage/2'>
+ <host name='example.org' port='3260'/>
+ <auth username='myname'>
+ <secret type='iscsi' usage='mycluster_myname'/>
+ </auth>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='2' unit='5'/>
+ </hostdev>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 1616eed..2bfbf6b 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014 Red Hat, Inc.
+ * Copyright (C) 2014-2016 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -30,6 +30,13 @@
#include "virstring.h"
#include "virtpm.h"
#include "virutil.h"
+#include "virrandom.h"
+#ifdef WITH_GNUTLS
+# include <gnutls/gnutls.h>
+# if HAVE_GNUTLS_CRYPTO_H
+# include <gnutls/crypto.h>
+# endif
+#endif
#include <time.h>
#include <unistd.h>
@@ -145,3 +152,25 @@ virCommandPassFD(virCommandPtr cmd ATTRIBUTE_UNUSED,
{
/* nada */
}
+
+int
+virRandomBytes(unsigned char *buf,
+ size_t buflen)
+{
+ size_t i;
+
+ for (i = 0; i < buflen; i++)
+ buf[i] = 0xff;
+
+ return 0;
+}
+
+#ifdef WITH_GNUTLS
+int
+gnutls_rnd(gnutls_rnd_level_t level ATTRIBUTE_UNUSED,
+ void *data,
+ size_t len)
+{
+ return virRandomBytes(data, len);
+#endif
+}
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8842b2f..c444410 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -26,6 +26,7 @@
# include "virstring.h"
# include "storage/storage_driver.h"
# include "virmock.h"
+# include "virrandom.h"
# include "testutilsqemu.h"
@@ -330,6 +331,14 @@ static int testCompareXMLToArgvFiles(const char *xml,
}
}
+ /* Create a fake master key */
+ if (VIR_ALLOC_N(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN) < 0)
+ goto out;
+
+ if (virRandomBytes(priv->masterKey, QEMU_DOMAIN_MASTER_KEY_LEN) < 0)
+ goto out;
+ priv->masterKeyLen = QEMU_DOMAIN_MASTER_KEY_LEN;
+
if (!(cmd = qemuProcessCreatePretendCmd(conn, &driver, vm, migrateURI,
(flags & FLAG_FIPS), false,
VIR_QEMU_PROCESS_START_COLD))) {
@@ -769,6 +778,8 @@ mymain(void)
DO_TEST("disk-drive-network-nbd-unix", NONE);
DO_TEST("disk-drive-network-iscsi", NONE);
DO_TEST("disk-drive-network-iscsi-auth", NONE);
+ DO_TEST("disk-drive-network-iscsi-auth-IV",
+ QEMU_CAPS_OBJECT_SECRET);
DO_TEST("disk-drive-network-iscsi-lun",
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI,
QEMU_CAPS_VIRTIO_BLK_SG_IO, QEMU_CAPS_SCSI_BLOCK);
@@ -776,6 +787,8 @@ mymain(void)
DO_TEST("disk-drive-network-rbd", NONE);
DO_TEST("disk-drive-network-sheepdog", NONE);
DO_TEST("disk-drive-network-rbd-auth", NONE);
+ DO_TEST("disk-drive-network-rbd-auth-IV",
+ QEMU_CAPS_OBJECT_SECRET);
DO_TEST("disk-drive-network-rbd-ipv6", NONE);
DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE);
DO_TEST("disk-drive-no-boot",
@@ -1606,12 +1619,18 @@ mymain(void)
DO_TEST("hostdev-scsi-lsi-iscsi-auth",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,
QEMU_CAPS_DEVICE_SCSI_GENERIC);
+ DO_TEST("hostdev-scsi-lsi-iscsi-auth-IV",
+ QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,
+ QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_OBJECT_SECRET);
DO_TEST("hostdev-scsi-virtio-iscsi",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI,
QEMU_CAPS_DEVICE_SCSI_GENERIC);
DO_TEST("hostdev-scsi-virtio-iscsi-auth",
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI,
QEMU_CAPS_DEVICE_SCSI_GENERIC);
+ DO_TEST("hostdev-scsi-virtio-iscsi-auth-IV",
+ QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_OBJECT_SECRET);
DO_TEST("mlock-on", QEMU_CAPS_MLOCK);
DO_TEST_FAILURE("mlock-on", NONE);
--
2.5.5
8:47 a.m.
New subject: [libvirt] [PATCH v2 12/12] qemu: Utilize qemu secret objects for SCSI/RBD auth/secret
On Sat, Apr 16, 2016 at 10:17:45AM -0400, John Ferlan wrote:
+/* qemuDomainSecretInfoGetAlias:
+ * @secinfo: pointer to the secret info object
+ * @qemuCaps: pointer to the emulator capabilities
+ *
+ * If the emulator supports it, secinfo is available and associated with
+ * an IV secret, then return the alias created during the disk or hostdev
+ * prepare step.
+ *
+ * Returns pointer to the object alias string or NULL if not found/supported
+ */
+const char *
+qemuDomainSecretInfoGetAlias(qemuDomainSecretInfoPtr secinfo,
+ virQEMUCapsPtr qemuCaps)
+{
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET)) {
+ VIR_INFO("secret object is not supported by this QEMU binary");
+ return NULL;
+ }
This check is not necessary - if QEMU does not support OBJECT_SECRET,
we did not generate SECRET_INFO_IV in the first place.
@@ -941,9 +1103,23 @@ qemuDomainSecretDiskPrepare(virConnectPtr
conn,
if (VIR_ALLOC(secinfo) < 0)
return -1;
- if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
- src->auth) < 0)
- goto error;
+ /* If we have the encryption API present and can support a
+ * secret object, then build the IV secret - this is the magic
+ * decision point for utilizing the IV secrets for a disk
+ * whether it's an iSCSI or an RBD disk.
+ */
+ if (qemuDomainSecretHaveEncrypt() &&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET)) {
This code is shared with HostdevPrepare and could be separated to
another function.
Jan
9:12 a.m.
New subject: [libvirt] [PATCH v2 12/12] qemu: Utilize qemu secret objects for SCSI/RBD auth/secret
On 05/02/2016 09:47 AM, Ján Tomko wrote:
On Sat, Apr 16, 2016 at 10:17:45AM -0400, John Ferlan wrote:
> +/* qemuDomainSecretInfoGetAlias:
> + * @secinfo: pointer to the secret info object
> + * @qemuCaps: pointer to the emulator capabilities
> + *
> + * If the emulator supports it, secinfo is available and associated with
> + * an IV secret, then return the alias created during the disk or hostdev
> + * prepare step.
> + *
> + * Returns pointer to the object alias string or NULL if not found/supported
> + */
> +const char *
> +qemuDomainSecretInfoGetAlias(qemuDomainSecretInfoPtr secinfo,
> + virQEMUCapsPtr qemuCaps)
> +{
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET)) {
> + VIR_INFO("secret object is not supported by this QEMU binary");
> + return NULL;
> + }
This check is not necessary - if QEMU does not support OBJECT_SECRET,
we did not generate SECRET_INFO_IV in the first place.
OK probably a remnant of over checking things.
> @@ -941,9 +1103,23 @@ qemuDomainSecretDiskPrepare(virConnectPtr
conn,
> if (VIR_ALLOC(secinfo) < 0)
> return -1;
>
> - if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
> - src->auth) < 0)
> - goto error;
> + /* If we have the encryption API present and can support a
> + * secret object, then build the IV secret - this is the magic
> + * decision point for utilizing the IV secrets for a disk
> + * whether it's an iSCSI or an RBD disk.
> + */
> + if (qemuDomainSecretHaveEncrypt() &&
> + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET)) {
This code is shared with HostdevPrepare and could be separated to
another function.
Well more or less, but I'll make it work.
Tks -
John
8:47 a.m.
On 04/16/2016 10:17 AM, John Ferlan wrote:
v1:
http://www.redhat.com/archives/libvir-list/2016-April/msg00596.html
Differences since v1:
- Add qemuBuildiSCSICommandLine (and BuildDiskiSCSI && BuildHostdeviSCSI)
These will do the magic necessary in order to support IV secret objects
for the impending iSCSI -drive argument. This API doesn't require any
qemu patches in order to work AFAICT. I also determined that the "id="
*isn't* required for an '-iscsi ...' argument, which made using the
complete 'path' string for 'initiator-name' possible. The other
option
was to break it up and pass the "iqn.*" string as the initiator-name
and a "modified" remaining string as the "id=" parameter. The
modified
would be to ensure only alphanumeric, '-', '.', and '_'
characters are
in the 'id=' string.
- Fix up some logic found while actually working through the tests. Some
of it related to what was found for the 'iscsi' options. A couple of
other minor nits.
- Add tests and mocks for virRandomBytes and gnutls_rnd (note: the former
could be used to "randomly" (hah!) generate a UUID of all '0xff').
A mock of 'gnutls_encrypt' is not necessary since, it can only be called
if the function gnutls_encrypt exists *and* we have a secret object
capability. Not having a mock function allows us to validate that
gnutls_encrypt actually generates a value we expect based on some
less than stellar and totally non random key's!
- Remove the hotplug IV code (I've saved it off for future expansion).
Although not needing to do hotplug probably means patches 6-9 are not
required, but still I think better than the existing so I kept them
even though they have nothing to do with IV secrets (they'd need to
go in after patches 1-5 anyways).
- Ran the changes through the coverity checker...
John Ferlan (12):
qemu: Introduce qemuDomainSecretInfo
qemu: Introduce qemuDomainSecretPrepare and Destroy
qemu: Introduce qemuDomainHostdevPrivatePtr
qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI
qemu: hotplug: Assume support for -device for attach virtio disk
qemu: hotplug: Adjust error path for attach scsi disk
qemu: hotplug: Adjust error path for attach virtio disk
qemu: hotplug: Adjust error path for attach hostdev scsi disk
qemu: hotplug: Fix possible memory leak of props
qemu: Introduce qemuDomainSecretIV
qemu: Utilize qemu secret objects for SCSI/RBD auth/secret
configure.ac | 1 +
src/conf/domain_conf.c | 33 +-
src/conf/domain_conf.h | 5 +-
src/lxc/lxc_native.c | 4 +-
src/qemu/qemu_alias.c | 23 +
src/qemu/qemu_alias.h | 2 +
src/qemu/qemu_command.c | 445 ++++++++++++++----
src/qemu/qemu_command.h | 13 +-
src/qemu/qemu_domain.c | 516 ++++++++++++++++++++-
src/qemu/qemu_domain.h | 81 +++-
src/qemu/qemu_driver.c | 13 +-
src/qemu/qemu_hotplug.c | 247 +++++-----
src/qemu/qemu_hotplug.h | 4 +-
src/qemu/qemu_parse_command.c | 4 +-
src/qemu/qemu_process.c | 13 +-
src/vbox/vbox_common.c | 4 +-
src/xenconfig/xen_common.c | 4 +-
src/xenconfig/xen_sxpr.c | 4 +-
...uxml2argv-disk-drive-network-iscsi-auth-IV.args | 39 ++
...muxml2argv-disk-drive-network-iscsi-auth-IV.xml | 43 ++
...emuxml2argv-disk-drive-network-rbd-auth-IV.args | 31 ++
...qemuxml2argv-disk-drive-network-rbd-auth-IV.xml | 42 ++
...emuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args | 41 ++
...qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml | 48 ++
...xml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args | 43 ++
...uxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml | 48 ++
tests/qemuxml2argvmock.c | 31 +-
tests/qemuxml2argvtest.c | 19 +
tests/virhostdevtest.c | 3 +-
29 files changed, 1557 insertions(+), 247 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml
ping?
Note there are parts of this that will repeat Peter's just posted series
to remove QEMU_CAPS_DEVICE from qemu_hotplug.c.
Even if the IV specific portions of this change (last two patches) don't
make the release, it still would be "nice" to get the rest in...
Tks -
John
5:34 a.m.
On 04/16/2016 10:17 AM, John Ferlan wrote:
v1:
http://www.redhat.com/archives/libvir-list/2016-April/msg00596.html
Differences since v1:
- Add qemuBuildiSCSICommandLine (and BuildDiskiSCSI && BuildHostdeviSCSI)
These will do the magic necessary in order to support IV secret objects
for the impending iSCSI -drive argument. This API doesn't require any
qemu patches in order to work AFAICT. I also determined that the "id="
*isn't* required for an '-iscsi ...' argument, which made using the
complete 'path' string for 'initiator-name' possible. The other
option
was to break it up and pass the "iqn.*" string as the initiator-name
and a "modified" remaining string as the "id=" parameter. The
modified
would be to ensure only alphanumeric, '-', '.', and '_'
characters are
in the 'id=' string.
- Fix up some logic found while actually working through the tests. Some
of it related to what was found for the 'iscsi' options. A couple of
other minor nits.
- Add tests and mocks for virRandomBytes and gnutls_rnd (note: the former
could be used to "randomly" (hah!) generate a UUID of all '0xff').
A mock of 'gnutls_encrypt' is not necessary since, it can only be called
if the function gnutls_encrypt exists *and* we have a secret object
capability. Not having a mock function allows us to validate that
gnutls_encrypt actually generates a value we expect based on some
less than stellar and totally non random key's!
- Remove the hotplug IV code (I've saved it off for future expansion).
Although not needing to do hotplug probably means patches 6-9 are not
required, but still I think better than the existing so I kept them
even though they have nothing to do with IV secrets (they'd need to
go in after patches 1-5 anyways).
- Ran the changes through the coverity checker...
John Ferlan (12):
qemu: Introduce qemuDomainSecretInfo
qemu: Introduce qemuDomainSecretPrepare and Destroy
qemu: Introduce qemuDomainHostdevPrivatePtr
qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI
qemu: hotplug: Assume support for -device for attach virtio disk
qemu: hotplug: Adjust error path for attach scsi disk
qemu: hotplug: Adjust error path for attach virtio disk
qemu: hotplug: Adjust error path for attach hostdev scsi disk
qemu: hotplug: Fix possible memory leak of props
qemu: Introduce qemuDomainSecretIV
qemu: Utilize qemu secret objects for SCSI/RBD auth/secret
configure.ac | 1 +
src/conf/domain_conf.c | 33 +-
src/conf/domain_conf.h | 5 +-
src/lxc/lxc_native.c | 4 +-
src/qemu/qemu_alias.c | 23 +
src/qemu/qemu_alias.h | 2 +
src/qemu/qemu_command.c | 445 ++++++++++++++----
src/qemu/qemu_command.h | 13 +-
src/qemu/qemu_domain.c | 516 ++++++++++++++++++++-
src/qemu/qemu_domain.h | 81 +++-
src/qemu/qemu_driver.c | 13 +-
src/qemu/qemu_hotplug.c | 247 +++++-----
src/qemu/qemu_hotplug.h | 4 +-
src/qemu/qemu_parse_command.c | 4 +-
src/qemu/qemu_process.c | 13 +-
src/vbox/vbox_common.c | 4 +-
src/xenconfig/xen_common.c | 4 +-
src/xenconfig/xen_sxpr.c | 4 +-
...uxml2argv-disk-drive-network-iscsi-auth-IV.args | 39 ++
...muxml2argv-disk-drive-network-iscsi-auth-IV.xml | 43 ++
...emuxml2argv-disk-drive-network-rbd-auth-IV.args | 31 ++
...qemuxml2argv-disk-drive-network-rbd-auth-IV.xml | 42 ++
...emuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args | 41 ++
...qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml | 48 ++
...xml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args | 43 ++
...uxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml | 48 ++
tests/qemuxml2argvmock.c | 31 +-
tests/qemuxml2argvtest.c | 19 +
tests/virhostdevtest.c | 3 +-
29 files changed, 1557 insertions(+), 247 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml
After merging with Peter's pushed remove QEMU_CAPS_DEVICE... Patch 6
vanishes and Patch 8 is modified to adjust for the removed !drivealias
check in qemuDomainAttachVirtioDiskDevice error path code.
I pushed the reviewed/ACK'd patches - all but last 2. I'll split up the
RBD and iSCSI code and repost those later today.
Tks -
John
3126
days inactive
3142
days old
36 comments
4 participants
participants (4)
-
Cole Robinson -
John Ferlan -
Ján Tomko -
Peter Krempa