[...]
>>
>> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list