[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 26c19ff..24ed8ed 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -832,7 +832,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
int qemuGetDriveSourceString(virStorageSourcePtr src, - virConnectPtr conn, + qemuDomainSecretInfoPtr secinfo, char **source) { int actualType = virStorageSourceGetActualType(src); @@ -846,31 +846,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src, if (virStorageSourceIsEmpty(src)) return 1;
- if (conn) { - if (actualType == VIR_STORAGE_TYPE_NETWORK && - src->auth && - (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI || - src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { - bool encode = false; - int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; - const char *protocol = virStorageNetProtocolTypeToString(src->protocol); - username = src->auth->username; - - if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { - /* qemu requires the secret to be encoded for RBD */ - encode = true; - secretType = VIR_SECRET_USAGE_TYPE_CEPH; - } - - if (!(secret = virSecretGetSecretString(conn, - protocol, - encode, - src->auth, - secretType))) - goto cleanup; - } - } -
This whole pile is what moves into qemuDomainSecretDiskPrepare. The check for 'conn' was here it seems was a result of commit id '816f0f93' where qemuDomainSnapshotCreateSingleDiskActive calls qemuGetDriveSourceString.
switch ((virStorageType) actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: @@ -881,6 +856,11 @@ qemuGetDriveSourceString(virStorageSourcePtr src, break;
case VIR_STORAGE_TYPE_NETWORK: + if (secinfo) { + username = secinfo->s.plain.username; + secret = secinfo->s.plain.secret; + } +
[...]
+/* qemuDomainSecretDiskPrepare: + * @conn: Pointer to connection + * @disk: Pointer to a disk definition + * + * For the right disk, generate the qemuDomainSecretInfo structure. + * + * Returns 0 on success, -1 on failure + */ +int +qemuDomainSecretDiskPrepare(virConnectPtr conn, + virDomainDiskDefPtr disk) +{ + virStorageSourcePtr src = disk->src; + qemuDomainSecretInfoPtr secinfo = NULL; + + if (conn && !virStorageSourceIsEmpty(src) && + virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK && + src->auth && + (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI || + src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) { +
Is conn ever going to be NULL here?
This code is the former qemuGetDriveSourceString code... So it was mostly a "move" of code. I was being more cautious due to number of callers. I did find one path that explicitly passes NULL: testQemuHotplugAttach -> qemuDomainAttachDeviceDiskLive -> qemuDomainAttachDeviceDiskLive ... which can call either qemuDomainAttachVirtioDiskDevice or qemuDomainAttachSCSIDisk which would call the qemuDomainSecretDiskPrepare And one comment in qemuAutostartDomains that I suppose could be updated to indicate the conn is also possibly needed for secrets.
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (VIR_ALLOC(secinfo) < 0) + return -1; + + if (qemuDomainSecretPlainSetup(conn, secinfo, src->protocol, + src->auth) < 0) + goto error; + + diskPriv->secinfo = secinfo; + } + + return 0; + + error: + qemuDomainSecretInfoFree(&secinfo); + return -1; +} + +
[...]
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ef8696b..692e3e7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -148,7 +148,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, /** * qemuDomainChangeEjectableMedia: * @driver: qemu driver structure - * @conn: connection structure * @vm: domain definition * @disk: disk definition to change the source of * @newsrc: new disk source to change to @@ -163,7 +162,6 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, */ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, - virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, @@ -223,7 +221,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } while (rc < 0);
if (!virStorageSourceIsEmpty(newsrc)) { - if (qemuGetDriveSourceString(newsrc, conn, &sourcestr) < 0) + /* cdrom/floppy won't have secret */ + if (qemuGetDriveSourceString(newsrc, NULL, &sourcestr) < 0) goto error;
Why not? Can't you have an rbd backed cdrom device?
Trying to remember why I wrote that comment... Ahh I see... I must've read the return virStorageSourceIsEmpty incorrectly when I changed that code... good catch! I will insert/squash in a: qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); and change NULL to diskPriv->secinfo John
ACK otherwise
- Cole