[libvirt] [PATCH 08/19] qemu: Change protocol parameter for secret setup

[Peter Krempa]

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 at 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.