7:06 a.m.
[...]
>
> 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