qemu: Introduce qemuDomainSecretPrepare and Destroy

Rather than needing to pass the conn parameter to various command
line building API's, add qemuDomainSecretPrepare just prior to the
qemuProcessLaunch which calls qemuBuilCommandLine. The function
must be called after qemuProcessPrepareHost since it's expected
to eventually need the domain masterKey generated during the prepare
host call. Additionally, future patches may require device aliases
(assigned during the prepare domain call) in order to associate
the secret objects.

The qemuDomainSecretDestroy is called after the qemuProcessLaunch
finishes in order to clear and free memory used by the secrets
that were recently prepared, so they are not kept around in memory
too long.

Placing the setup here is beneficial for future patches which will
need the domain masterKey in order to generate an encrypted secret
along with an initialization vector to be saved and passed (since
the masterKey shouldn't be passed around).

Finally, since the secret is not added during command line build,
the hotplug code will need to get the secret into the private disk data.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

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)
@@ -9367,8 +9345,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)

src/qemu/qemu_command.h

@@ -101,8 +101,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);
 
@@ -179,7 +178,7 @@ char *qemuBuildRedirdevDevStr(const virDomainDef *def,
 int qemuNetworkPrepareDevices(virDomainDefPtr def);
 
 int qemuGetDriveSourceString(virStorageSourcePtr src,
-                             virConnectPtr conn,
+                             qemuDomainSecretInfoPtr secinfo,
                              char **source);
 
 int qemuCheckDiskConfig(virDomainDiskDefPtr disk);

src/qemu/qemu_domain.c

@@ -46,6 +46,7 @@
 #include "viratomic.h"
 #include "virprocess.h"
 #include "virrandom.h"
+#include "secret_util.h"
 #include "base64.h"
 #ifdef WITH_GNUTLS
 # include <gnutls/gnutls.h>
@@ -791,6 +792,146 @@ qemuDomainDiskPrivateDispose(void *obj)
 }
 
 
+/* qemuDomainSecretPlainSetup:
+ * @conn: Pointer to connection
+ * @secinfo: Pointer to secret info
+ * @protocol: Protocol for secret
+ * @authdef: Pointer to auth data
+ *
+ * Taking a secinfo, fill in the plaintext information
+ *
+ * Returns 0 on success, -1 on failure with error message
+ */
+static int
+qemuDomainSecretPlainSetup(virConnectPtr conn,
+                           qemuDomainSecretInfoPtr secinfo,
+                           virStorageNetProtocol protocol,
+                           virStorageAuthDefPtr authdef)
+{
+    bool encode = false;
+    int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
+    const char *protocolstr = virStorageNetProtocolTypeToString(protocol);
+
+    secinfo->type = VIR_DOMAIN_SECRET_INFO_PLAIN;
+    if (VIR_STRDUP(secinfo->s.plain.username, authdef->username) < 0)
+        return -1;
+
+    if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
+        /* qemu requires the secret to be encoded for RBD */
+        encode = true;
+        secretType = VIR_SECRET_USAGE_TYPE_CEPH;
+    }
+
+    if (!(secinfo->s.plain.secret =
+          virSecretGetSecretString(conn, protocolstr, encode,
+                                   authdef, secretType)))
+        return -1;
+
+    return 0;
+}
+
+
+/* qemuDomainSecretDiskDestroy:
+ * @disk: Pointer to a disk definition
+ *
+ * Clear and destroy memory associated with the secret
+ */
+void
+qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
+{
+    qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+    if (!diskPriv->secinfo)
+        return;
+
+    qemuDomainSecretInfoFree(&diskPriv->secinfo);
+}
+
+
+/* qemuDomainSecretDiskPrepare:
+ * @conn: Pointer to connection
+ * @disk: Pointer to a disk definition
+ *
+ * For the right disk, generate the qemuDomainSecretInfo structure.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretDiskPrepare(virConnectPtr conn,
+                            virDomainDiskDefPtr disk)
+{
+    virStorageSourcePtr src = disk->src;
+    qemuDomainSecretInfoPtr secinfo = NULL;
+
+    if (conn && !virStorageSourceIsEmpty(src) &&
+        virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK &&
+        src->auth &&
+        (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
+         src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
+
+        qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+        if (VIR_ALLOC(secinfo) < 0)
+            return -1;
+
+        if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol,
+                                       src->auth) < 0)
+            goto error;
+
+        diskPriv->secinfo = secinfo;
+    }
+
+    return 0;
+
+ error:
+    qemuDomainSecretInfoFree(&secinfo);
+    return -1;
+}
+
+
+/* qemuDomainSecretDestroy:
+ * @vm: Domain object
+ *
+ * Once completed with the generation of the command line it is
+ * expect to remove the secrets
+ */
+void
+qemuDomainSecretDestroy(virDomainObjPtr vm)
+{
+    size_t i;
+
+    for (i = 0; i < vm->def->ndisks; i++)
+        qemuDomainSecretDiskDestroy(vm->def->disks[i]);
+}
+
+
+/* qemuDomainSecretPrepare:
+ * @conn: Pointer to connection
+ * @vm: Domain object
+ *
+ * For any objects that may require an auth/secret setup, create a
+ * qemuDomainSecretInfo and save it in the approriate place within
+ * the private structures. This will be used by command line build
+ * code in order to pass the secret along to qemu in order to provide
+ * the necessary authentication data.
+ *
+ * Returns 0 on success, -1 on failure with error message set
+ */
+int
+qemuDomainSecretPrepare(virConnectPtr conn,
+                        virDomainObjPtr vm)
+{
+    size_t i;
+
+    for (i = 0; i < vm->def->ndisks; i++) {
+        if (qemuDomainSecretDiskPrepare(conn, vm->def->disks[i]) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
 /* This is the old way of setting up per-domain directories */
 static int
 qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
@@ -3799,8 +3940,7 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
 
 
 bool
-qemuDomainDiskSourceDiffers(virConnectPtr conn,
-                            virDomainDiskDefPtr disk,
+qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
                             virDomainDiskDefPtr origDisk)
 {
     char *diskSrc = NULL, *origDiskSrc = NULL;
@@ -3816,8 +3956,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

src/qemu/qemu_domain.h

@@ -497,8 +497,7 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
                                  bool force_probe,
                                  bool report_broken);
 
-bool qemuDomainDiskSourceDiffers(virConnectPtr conn,
-                                 virDomainDiskDefPtr disk,
+bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
                                  virDomainDiskDefPtr origDisk);
 
 bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
@@ -616,4 +615,16 @@ int qemuDomainMasterKeyCreate(virQEMUDriverPtr driver,
 
 void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv);
 
+void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
+    ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
+void qemuDomainSecretDestroy(virDomainObjPtr vm)
+    ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
 #endif /* __QEMU_DOMAIN_H__ */

src/qemu/qemu_driver.c

@@ -7712,14 +7712,16 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
         orig_disk->startupPolicy = dev->data.disk->startupPolicy;
         orig_disk->snapshot = dev->data.disk->snapshot;
 
-        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;
             }
 

src/qemu/qemu_hotplug.c

@@ -148,7 +148,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
 /**
  * qemuDomainChangeEjectableMedia:
  * @driver: qemu driver structure
- * @conn: connection structure
  * @vm: domain definition
  * @disk: disk definition to change the source of
  * @newsrc: new disk source to change to
@@ -163,7 +162,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
  */
 int
 qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
-                               virConnectPtr conn,
                                virDomainObjPtr vm,
                                virDomainDiskDefPtr disk,
                                virStorageSourcePtr newsrc,
@@ -232,7 +230,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
     } while (rc < 0);
 
     if (!virStorageSourceIsEmpty(newsrc)) {
-        if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0)
+        qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+        if (qemuGetDriveSourceString(newsrc, diskPriv->secinfo, &sourcestr) < 0)
             goto error;
 
         if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) {
@@ -369,7 +369,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
     if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
         goto error;
 
-    if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps)))
+    if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+        goto error;
+
+    if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
         goto error;
 
     if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
@@ -411,6 +414,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
     virDomainDiskInsertPreAlloced(vm->def, disk);
 
  cleanup:
+    qemuDomainSecretDiskDestroy(disk);
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
     VIR_FREE(drivealias);
@@ -574,10 +578,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)
@@ -608,6 +615,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
     virDomainDiskInsertPreAlloced(vm->def, disk);
 
  cleanup:
+    qemuDomainSecretDiskDestroy(disk);
     VIR_FREE(devstr);
     VIR_FREE(drivestr);
     virObjectUnref(cfg);
@@ -620,8 +628,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
 
 
 static int
-qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
-                                     virQEMUDriverPtr driver,
+qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
                                      virDomainObjPtr vm,
                                      virDomainDiskDefPtr disk)
 {
@@ -644,7 +651,7 @@ qemuDomainAttachUSBMassStorageDevice(virConnectPtr conn,
 
     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;
@@ -732,7 +739,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;
 
@@ -754,7 +761,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:

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,

src/qemu/qemu_process.c

@@ -5665,6 +5665,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)
@@ -5673,6 +5676,8 @@ qemuProcessStart(virConnectPtr conn,
     }
     relabel = true;
 
+    qemuDomainSecretDestroy(vm);
+
     if (incoming &&
         incoming->deferredURI &&
         qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) < 0)
@@ -5734,6 +5739,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,