[libvirt] [PATCH 00/12] Add IV Secret Object support
The first 5 patches handle changing the current mechanism of getting the secret while building the command line into a mechanism where the secret is built and stored as part of the disk or hostdev private data and then parsed during command line generation. The end result is to remove the 'conn' parameter from the qemuBuildCommandLine. The next 5 patches deal with some hotplug cleanup that was necessary in order to "more easily" handle having a secret object as a possible option instead of relying on the '-drive' string to hold the secret. The last 2 patches are the resulting initialization vector patches. They haven't been fully tested or vetted, but I figured getting something "out there" for review would give me some more time to figure out a proper test environment. At the very least each of the first two groups of 5 patches could be their own series - it's just that they ended up being part of this code. 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 virtio disk qemu: hotplug: Adjust error path for attach scsi 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 | 28 ++- src/conf/domain_conf.h | 7 +- src/lxc/lxc_native.c | 4 +- src/qemu/qemu_alias.c | 23 ++ src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c | 321 ++++++++++++++++++-------- src/qemu/qemu_command.h | 20 +- src/qemu/qemu_domain.c | 508 +++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 81 ++++++- src/qemu/qemu_driver.c | 13 +- src/qemu/qemu_hotplug.c | 309 ++++++++++++++++--------- 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 +- tests/virhostdevtest.c | 3 +- 19 files changed, 1107 insertions(+), 246 deletions(-) -- 2.5.5
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@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 223154d..e5b7b9d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -720,7 +720,28 @@ qemuDomainMasterKeyCreate(qemuDomainObjPrivatePtr priv) } +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) @@ -728,7 +749,7 @@ qemuDomainDiskPrivateOnceInit(void) qemuDomainDiskPrivateClass = virClassNew(virClassForObject(), "qemuDomainDiskPrivate", sizeof(qemuDomainDiskPrivate), - NULL); + qemuDomainDiskPrivateDispose); if (!qemuDomainDiskPrivateClass) return -1; else @@ -752,6 +773,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 80b6593..3538dca 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -223,6 +223,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) @@ -242,6 +265,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
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@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 bd82682..a804987 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) @@ -9319,8 +9297,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 a3e6a00..97cd7c6 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -113,8 +113,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); @@ -196,7 +195,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 e5b7b9d..cd11bf8 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> @@ -782,6 +783,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, @@ -3735,8 +3876,7 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver, bool -qemuDomainDiskSourceDiffers(virConnectPtr conn, - virDomainDiskDefPtr disk, +qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk, virDomainDiskDefPtr origDisk) { char *diskSrc = NULL, *origDiskSrc = NULL; @@ -3752,8 +3892,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 3538dca..5609b90 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -481,8 +481,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, @@ -599,4 +598,16 @@ int qemuDomainMasterKeyCreate(qemuDomainObjPrivatePtr priv); 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 eaabe58..1008e58 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 bd1a128..cd986f9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -147,7 +147,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 @@ -162,7 +161,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, */ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, - virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, @@ -222,7 +220,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) { @@ -355,7 +354,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))) @@ -410,6 +412,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virDomainDiskInsertPreAlloced(vm->def, disk); cleanup: + qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); VIR_FREE(drivealias); @@ -581,10 +584,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) @@ -615,6 +621,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virDomainDiskInsertPreAlloced(vm->def, disk); cleanup: + qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); virObjectUnref(cfg); @@ -627,8 +634,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, static int -qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn, - virQEMUDriverPtr driver, +qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk) { @@ -652,7 +658,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; @@ -745,7 +751,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; @@ -767,7 +773,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 4140da3..3358e7d 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 6c870f5..aff6852 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5576,6 +5576,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) @@ -5584,6 +5587,8 @@ qemuProcessStart(virConnectPtr conn, } relabel = true; + qemuDomainSecretDestroy(vm); + if (incoming && incoming->deferredURI && qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0) @@ -5645,6 +5650,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
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@redhat.com> --- src/conf/domain_conf.c | 28 +++++++++++++++++++++------ src/conf/domain_conf.h | 7 +++++-- 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, 96 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 48c7bc5..55b16dc 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2115,16 +2115,29 @@ 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_FREE(def); + + if (xmlopt && + xmlopt->privateData.hostdevNew && + !(def->privateData = xmlopt->privateData.hostdevNew())) + goto error; + return def; + + error: + virDomainHostdevDefFree(def); + return NULL; } + static void virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc) { @@ -12271,7 +12284,8 @@ virDomainVideoDefParseXML(xmlNodePtr node, } static virDomainHostdevDefPtr -virDomainHostdevDefParseXML(xmlNodePtr node, +virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, + xmlNodePtr node, xmlXPathContextPtr ctxt, virHashTablePtr bootHash, unsigned int flags) @@ -12283,7 +12297,7 @@ virDomainHostdevDefParseXML(xmlNodePtr node, ctxt->node = node; - if (!(def = virDomainHostdevDefAlloc())) + if (!(def = virDomainHostdevDefAlloc(xmlopt))) goto error; if (mode) { @@ -12933,8 +12947,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: @@ -16454,7 +16469,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 6f93def..d442866 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1,7 +1,7 @@ /* * domain_conf.h: domain XML processing * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2016 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany. * @@ -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; @@ -2486,6 +2488,7 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataAllocFunc alloc; virDomainXMLPrivateDataFreeFunc free; virDomainXMLPrivateDataNewFunc diskNew; + virDomainXMLPrivateDataNewFunc hostdevNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; }; @@ -2563,7 +2566,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 cd11bf8..613bea2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -783,6 +783,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 @@ -1420,6 +1463,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 5609b90..f343590 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -271,6 +271,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 8b294a7..47cf48a 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1161,7 +1161,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; @@ -1211,7 +1211,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 c305eb5..a848285 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1,6 +1,6 @@ /* * Copyright (C) 2014, Taowei Luo (uaedante@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 @@ -3034,7 +3034,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 4dcd484..82ef3a3 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 fdfec2b..7a2d657 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@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
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@redhat.com> --- src/qemu/qemu_command.c | 42 ++++++++++------------------ src/qemu/qemu_command.h | 11 +++----- 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, 122 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a804987..0726abf 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" @@ -4412,29 +4411,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; @@ -4445,14 +4439,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, qemuBuildCommandLineCallbacksPtr callbacks) { @@ -4461,7 +4452,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 { @@ -4760,7 +4751,6 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, static int qemuBuildHostdevCommandLine(virCommandPtr cmd, - virConnectPtr conn, const virDomainDef *def, virQEMUCapsPtr qemuCaps, qemuBuildCommandLineCallbacksPtr callbacks, @@ -4910,8 +4900,8 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, char *drvstr; virCommandAddArg(cmd, "-drive"); - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, - qemuCaps, callbacks))) + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps, + callbacks))) return -1; virCommandAddArg(cmd, drvstr); VIR_FREE(drvstr); @@ -9148,13 +9138,9 @@ qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { /* * 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, @@ -9180,9 +9166,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) @@ -9348,7 +9334,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildRedirdevCommandLine(logManager, cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildHostdevCommandLine(cmd, conn, def, qemuCaps, callbacks, + if (qemuBuildHostdevCommandLine(cmd, def, qemuCaps, callbacks, &bootHostdevNet) < 0) goto error; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 97cd7c6..056bb08 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -58,8 +58,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, @@ -76,8 +75,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, int **nicindexes, const char *domainLibDir, const char *domainChannelTargetDir) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11) - ATTRIBUTE_NONNULL(17) ATTRIBUTE_NONNULL(18); + ATTRIBUTE_NONNULL(10) ATTRIBUTE_NONNULL(16) ATTRIBUTE_NONNULL(17); /* Generate '-device' string for chardev device */ int @@ -179,11 +177,10 @@ char *qemuBuildUSBHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); -char *qemuBuildSCSIHostdevDrvStr(virConnectPtr conn, - virDomainHostdevDefPtr dev, +char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps, qemuBuildCommandLineCallbacksPtr callbacks) - ATTRIBUTE_NONNULL(4); + ATTRIBUTE_NONNULL(3); char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 613bea2..ea5e40f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -923,6 +923,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 * @@ -936,6 +1001,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]); } @@ -962,6 +1030,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 f343590..1963c58 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -617,6 +617,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 1008e58..6b74dac 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 cd986f9..05fa787 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -808,11 +808,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; @@ -852,8 +851,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; } @@ -1919,6 +1921,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, return ret; } + static int qemuDomainAttachHostSCSIDevice(virConnectPtr conn, virQEMUDriverPtr driver, @@ -1974,7 +1977,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, &buildCommandLineCallbacks))) goto cleanup; @@ -2011,6 +2017,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) @@ -2025,10 +2032,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 3358e7d..f3e51c0 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 aff6852..f4cbe97 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5230,7 +5230,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, @@ -5654,8 +5654,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
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@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 0726abf..31e02de 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; @@ -4414,8 +4409,6 @@ static char * qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) { char *source = NULL; - char *secret = NULL; - char *username = NULL; virStorageSource src; qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev); @@ -4424,20 +4417,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
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@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 05fa787..f308738 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -316,7 +316,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; @@ -340,62 +339,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
Shortly a new object could be added making this code even more confusing, so let's just adjust the exit path now to make it clearer. Signed-off-by: John Ferlan <jferlan@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 f308738..c3edd40 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -317,6 +317,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err; char *devstr = NULL; char *drivestr = NULL; char *drivealias = NULL; @@ -367,36 +368,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); @@ -406,6 +395,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
Shortly a new object could be added making this code even more confusing, so let's just adjust the exit path now to make it clearer. Signed-off-by: John Ferlan <jferlan@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 c3edd40..ec30ce7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -592,29 +592,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 failexitmon; - if (ret < 0) - goto error; + virDomainAuditDisk(vm, NULL, disk->src, "attach", true); virDomainDiskInsertPreAlloced(vm->def, disk); + ret = 0; cleanup: qemuDomainSecretDiskDestroy(disk); @@ -623,6 +616,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)); + + failexitmon: + virDomainAuditDisk(vm, NULL, disk->src, "attach", false); + error: ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); goto cleanup; -- 2.5.5
Shortly a new object could be added making this code even more confusing, so let's just adjust the exit path now to make it clearer. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 56 ++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ec30ce7..b6cf196 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -602,7 +602,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, goto failadddevice; if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto failexitmon; + goto failexitmonitor; virDomainAuditDisk(vm, NULL, disk->src, "attach", true); @@ -623,7 +623,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, failadddrive: ignore_value(qemuDomainObjExitMonitor(driver, vm)); - failexitmon: + failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); error: @@ -1929,6 +1929,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err; virDomainControllerDefPtr cont = NULL; char *devstr = NULL; char *drvstr = NULL; @@ -1989,32 +1990,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) { @@ -2029,6 +2022,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
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@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 b6cf196..c82d455 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1685,6 +1685,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, if (qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0) goto failbackend; + props = NULL; if (qemuMonitorAddDevice(priv->mon, devstr) < 0) goto failfrontend; @@ -1702,6 +1703,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); @@ -1715,6 +1717,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
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@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 31e02de..3a69bd5 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 ea5e40f..6510f57 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -731,12 +731,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); } @@ -884,6 +906,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. @@ -892,6 +915,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) */ int qemuDomainSecretDiskPrepare(virConnectPtr conn, + qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk) { virStorageSourcePtr src = disk->src; @@ -943,6 +967,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. @@ -951,6 +976,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev) */ int qemuDomainSecretHostdevPrepare(virConnectPtr conn, + qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED, virDomainHostdevDefPtr hostdev) { virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; @@ -1023,15 +1049,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 1963c58..2062435 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -226,6 +226,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; @@ -237,12 +238,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; }; @@ -614,15 +627,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 c82d455..04e3837 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -353,7 +353,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))) @@ -580,7 +580,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))) @@ -1980,7 +1980,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
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). qemuBuildSecretInfoProps: Generate/return a JSON properties object for the IV secret to be used by both the command building and 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 -drive driver=iscsi,url=iscsi://example.com/target-foo/lun1, user=dan,password-secret=sec0 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 driver=rbd,filename=rbd:pool/image:id=myname: auth_supported=cephx,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. Hotplug Adjustments: Once the qemuDomainSecret{Disk|Hostdev}Prepare completes successfully, a check (qemuDomainSecretInfoGetAlias) to determine whether the IV secret alias is available results in generating the JSON props (or not). (via qemuBuildSecretInfoProps). If the secret alias object exists, then prior to adding the device and drive, the secret object will be added to define the 'password-secret' parameter. 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@redhat.com> --- configure.ac | 1 + src/qemu/qemu_alias.c | 23 ++++++ src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c | 126 ++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 + src/qemu/qemu_domain.c | 199 +++++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_domain.h | 3 + src/qemu/qemu_hotplug.c | 72 +++++++++++++++++- 8 files changed, 418 insertions(+), 12 deletions(-) diff --git a/configure.ac b/configure.ac index 1eb19ee..83a1043 100644 --- a/configure.ac +++ b/configure.ac @@ -1295,6 +1295,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 3a69bd5..9b9e9a0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -639,6 +639,10 @@ qemuBuildGeneralSecinfoURI(virURIPtr uri, break; case VIR_DOMAIN_SECRET_INFO_IV: + if (VIR_STRDUP(uri->user, secinfo->s.iv.username) < 0) + return -1; + break; + case VIR_DOMAIN_SECRET_INFO_LAST: return -1; } @@ -677,6 +681,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; } @@ -809,6 +817,13 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, ret = virURIFormat(uri); + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && + secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_IV) { + virBufferAsprintf(&buf, "%s,user=%s", ret, + secinfo->s.iv.username); + VIR_FREE(ret); + ret = virBufferContentAndReset(&buf); + } break; case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -1069,6 +1084,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. + */ +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, @@ -1343,6 +1449,14 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virBufferAddLit(&opt, ",cache=none"); } + if (diskPriv->secinfo) { + qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + + if (secinfo->type == VIR_DOMAIN_SECRET_INFO_IV) + virBufferAsprintf(&opt, ",password-secret='%s'", + secinfo->s.iv.alias); + } + if (disk->copy_on_read) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_COPY_ON_READ)) { virBufferAsprintf(&opt, ",copy-on-read=%s", @@ -1888,6 +2002,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 +2046,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, break; } + if (qemuBuildSecretIVCommandLine(cmd, secinfo, qemuCaps) < 0) + return -1; + virCommandAddArg(cmd, "-drive"); /* Unfortunately it is not possible to use @@ -4947,8 +5066,15 @@ 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; + virCommandAddArg(cmd, "-drive"); if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps, callbacks))) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 056bb08..188cf5d 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -110,6 +110,10 @@ char *qemuBuildNicDevStr(virDomainDefPtr def, char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps); +int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, + const char **type, + virJSONValuePtr *propsret); + /* Both legacy & current support */ char *qemuBuildDriveStr(virDomainDiskDefPtr disk, bool bootable, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6510f57..1477d47 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -763,6 +763,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); @@ -887,6 +913,143 @@ 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; + char *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, free secret so that it doesn't hang around */ + memset(secret, 0, strlen(secret)); + VIR_FREE(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 * @@ -915,7 +1078,7 @@ qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) */ int qemuDomainSecretDiskPrepare(virConnectPtr conn, - qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED, + qemuDomainObjPrivatePtr priv, virDomainDiskDefPtr disk) { virStorageSourcePtr src = disk->src; @@ -932,9 +1095,18 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, if (VIR_ALLOC(secinfo) < 0) return -1; - if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol, - src->auth) < 0) - goto error; + 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; } @@ -976,7 +1148,7 @@ qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev) */ int qemuDomainSecretHostdevPrepare(virConnectPtr conn, - qemuDomainObjPrivatePtr priv ATTRIBUTE_UNUSED, + qemuDomainObjPrivatePtr priv, virDomainHostdevDefPtr hostdev) { virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; @@ -997,10 +1169,19 @@ 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 (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 2062435..3a5e028 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -624,6 +624,9 @@ int qemuDomainMasterKeyCreate(qemuDomainObjPrivatePtr priv); void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv); +const char *qemuDomainSecretInfoGetAlias(qemuDomainSecretInfoPtr secinfo, + virQEMUCapsPtr qemuCaps); + void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 04e3837..0a1b6f1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -323,6 +323,11 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, char *drivealias = NULL; bool releaseaddr = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *objAlias; + const char *type; + qemuDomainDiskPrivatePtr diskPriv; + qemuDomainSecretInfoPtr secinfo; + virJSONValuePtr props = NULL; const char *src = virDomainDiskGetSource(disk); if (!disk->info.type) { @@ -356,6 +361,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + secinfo = diskPriv->secinfo; + if ((objAlias = qemuDomainSecretInfoGetAlias(secinfo, priv->qemuCaps))) { + if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0) + goto error; + } + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -368,9 +380,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) goto error; - /* Attach the device - 2 step process */ + /* Attach the device - possible 3 step process */ qemuDomainObjEnterMonitor(driver, vm); + if (objAlias && qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0) + goto failaddsecret; + props = NULL; /* qemuMonitorAddObject consumes and frees props */ + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -388,6 +404,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, ret = 0; cleanup: + virJSONValueFree(props); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -409,8 +426,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } failadddrive: + if (objAlias) + ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); + + failaddsecret: if (qemuDomainObjExitMonitor(driver, vm) < 0) releaseaddr = false; + props = NULL; /* qemuMonitorAddObject consumes props on failure too */ failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -565,6 +587,11 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, char *devstr = NULL; int ret = -1; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + const char *objAlias; + const char *type; + qemuDomainDiskPrivatePtr diskPriv; + qemuDomainSecretInfoPtr secinfo; + virJSONValuePtr props = NULL; if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0) goto cleanup; @@ -583,6 +610,13 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + secinfo = diskPriv->secinfo; + if ((objAlias = qemuDomainSecretInfoGetAlias(secinfo, priv->qemuCaps))) { + if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0) + goto error; + } + if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; @@ -592,9 +626,13 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) goto error; - /* Attach the device - 2 step process */ + /* Attach the device - possible 3 step process */ qemuDomainObjEnterMonitor(driver, vm); + if (objAlias && qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0) + goto failaddsecret; + props = NULL; /* qemuMonitorAddObject consumes and frees props */ + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -610,6 +648,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, ret = 0; cleanup: + virJSONValueFree(props); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -621,7 +660,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr); failadddrive: + if (objAlias) + ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); + + failaddsecret: ignore_value(qemuDomainObjExitMonitor(driver, vm)); + props = NULL; /* qemuMonitorAddObject consumes props on failure too */ failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -1938,6 +1982,11 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, char *drvstr = NULL; bool teardowncgroup = false; bool teardownlabel = false; + const char *objAlias; + const char *type; + qemuDomainHostdevPrivatePtr hostdevPriv; + qemuDomainSecretInfoPtr secinfo; + virJSONValuePtr props = NULL; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) || !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { @@ -1983,6 +2032,13 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (qemuDomainSecretHostdevPrepare(conn, priv, hostdev) < 0) goto cleanup; + hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev); + secinfo = hostdevPriv->secinfo; + if ((objAlias = qemuDomainSecretInfoGetAlias(secinfo, priv->qemuCaps))) { + if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0) + goto cleanup; + } + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps, &buildCommandLineCallbacks))) goto cleanup; @@ -1993,9 +2049,13 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) goto cleanup; - /* Attach the device - 2 step process */ + /* Attach the device - possible 3 step process */ qemuDomainObjEnterMonitor(driver, vm); + if (objAlias && qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0) + goto failaddsecret; + props = NULL; /* qemuMonitorAddObject consumes and frees props */ + if (qemuMonitorAddDrive(priv->mon, drvstr) < 0) goto failadddrive; @@ -2012,6 +2072,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, ret = 0; cleanup: + virJSONValueFree(props); qemuDomainSecretHostdevDestroy(hostdev); if (ret < 0) { qemuHostdevReAttachSCSIDevices(driver, vm->def->name, &hostdev, 1); @@ -2038,7 +2099,12 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, } failadddrive: + if (objAlias) + ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); + + failaddsecret: ignore_value(qemuDomainObjExitMonitor(driver, vm)); + props = NULL; /* qemuMonitorAddObject consumes props on failure too */ failexitmonitor: virDomainAuditHostdev(vm, hostdev, "attach", false); -- 2.5.5
[...]
configure.ac | 1 + src/qemu/qemu_alias.c | 23 ++++++ src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c | 126 ++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 + src/qemu/qemu_domain.c | 199 +++++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_domain.h | 3 + src/qemu/qemu_hotplug.c | 72 +++++++++++++++++- 8 files changed, 418 insertions(+), 12 deletions(-)
Digging in deeper to the testing portion proves to me more is needed on this patch - in particular qemuDomainSecretIVSetup has a couple of issues. One being, I needed to do an "if (secret)" in the cleanup section since I VIR_FREE'd it earlier. It was all cleanup, then I thought - probably should clear/free the secret right after using - bad idea ;-)... I also have something not quite right because when the object is passed to qemu and the decrypt is done, the answer isn't correct - even though if I "add" a decrypt call and check the result, I get the right answer. Perhaps something to do with the padding - still not quite sure. Also, I neglected to think about hotunplug and removing the object... John [...]
participants (1)
-
John Ferlan