2:25 p.m.
On Thu, Jun 23, 2016 at 12:16:06 -0400, John Ferlan wrote:
On 06/23/2016 11:57 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:47 -0400, John Ferlan wrote:
>> Rather than assume/pass the protocol to the qemuDomainSecretPlainSetup
>> and qemuDomainSecretAESSetup, determine and pass the secretUsageType
>> which is then used in the virSecretGetSecretString call
>>
>> For the two callers that convert from virStorageNetProtocol, add
>> a new helper qemuDomainSecretProtocolGetUsageType.
>>
>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>> ---
>> src/qemu/qemu_domain.c | 105 +++++++++++++++++++++++++++++--------------------
>> 1 file changed, 63 insertions(+), 42 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 34e3d95..52cbc72 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>
> [...]
>
>> +/* qemuDomainSecretGetProtocolUsageType:
>> + * @protocol: The virStorageNetProtocol protocol type
>> + *
>> + * Convert the protocl into the expected virSecretUsageType for
>> + * eventual usage to fetch the secret
>> + *
>> + * Returns matched protocol type or VIR_SECRET_USAGE_TYPE_NONE with an
>> + * error message set on failure.
>> + */
>> +static virSecretUsageType
>> +qemuDomainSecretProtocolGetUsageType(virStorageNetProtocol protocol)
>> +{
>> + switch ((virStorageNetProtocol)protocol) {
>> + case VIR_STORAGE_NET_PROTOCOL_RBD:
>> + return VIR_SECRET_USAGE_TYPE_CEPH;
>> +
>> + case VIR_STORAGE_NET_PROTOCOL_ISCSI:
>> + return VIR_SECRET_USAGE_TYPE_ISCSI;
>> +
>> + case VIR_STORAGE_NET_PROTOCOL_NONE:
>> + case VIR_STORAGE_NET_PROTOCOL_NBD:
>> + case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
>> + case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
>> + case VIR_STORAGE_NET_PROTOCOL_HTTP:
>> + case VIR_STORAGE_NET_PROTOCOL_HTTPS:
>> + case VIR_STORAGE_NET_PROTOCOL_FTP:
>> + case VIR_STORAGE_NET_PROTOCOL_FTPS:
>> + case VIR_STORAGE_NET_PROTOCOL_TFTP:
>> + case VIR_STORAGE_NET_PROTOCOL_LAST:
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("protocol '%s' cannot be used for
encrypted secrets"),
>> + virStorageNetProtocolTypeToString(protocol));
>
> You could change this error message so that it actually makes some
> sense. The protocols above don't support any form of authentication at
> least in context of our interaction with qemu, not only specifically
> encrypted secrets.
>
OK - poof this is gone...
>> + }
>> + return VIR_SECRET_USAGE_TYPE_NONE;
>> +}
>> +
>> +
>> /* qemuDomainSecretDiskPrepare:
>> * @conn: Pointer to connection
>> * @priv: pointer to domain private object
>> @@ -1008,13 +1018,19 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>> (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
>> src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
>>
>> + virSecretUsageType secretUsageType;
>> qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>>
>> if (VIR_ALLOC(secinfo) < 0)
>> return -1;
>>
>> + if ((secretUsageType =
>> + qemuDomainSecretProtocolGetUsageType(src->protocol)) ==
>> + VIR_SECRET_USAGE_TYPE_NONE)
>
> Dead code. The condition above guarantees that this doesn't ever return
> _NONE. I think you could set the usage type here rather than having an
> extra helper that doesn't do much else.
Changed to:
if (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI)
secretUsageType = VIR_SECRET_USAGE_TYPE_ISCSI;
else
secretUsageType = VIR_SECRET_USAGE_TYPE_CEPH;
>
>> + goto error;
>> +
>> if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
>> - src->protocol, src->auth) < 0)
>> + secretUsageType, src->auth) < 0)
>> goto error;
>>
>> diskPriv->secinfo = secinfo;
>> @@ -1072,14 +1088,19 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
>> if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI
&&
>> iscsisrc->auth) {
>>
>> + virSecretUsageType secretUsageType;
Changed to:
virSecretUsageType secretUsageType =
VIR_SECRET_USAGE_TYPE_ISCSI;
Tks -
thanks for doing that. ACK to those.