[libvirt] [PATCH 07/11] storage: Support to use secret object for iscsi chap "auth"

[Osier Yang]

On 07/06/13 00:51, John Ferlan wrote:
> On 05/28/2013 02:39 AM, Osier Yang wrote:
>> Based on the plain password chap "auth" support, this gets
>> the secret value (password) with the secret driver methods,
>> and apply it for the "iscsiadm" update command.
> Ugh.  Of course - it's the "next" patch (gets me every time).
>
> In any case, since 6/11 cannot "assume" the passwd field is filled in,
> you probably need to combine the two or just make the check in 6/11 for
> type != PLAIN_PASSWORD - return 0;

Prefer the later.

>
>
>> ---
>>   src/storage/storage_backend_iscsi.c | 56 +++++++++++++++++++++++++++++++++----
>>   1 file changed, 50 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
>> index 6a17b5a..516025a 100644
>> --- a/src/storage/storage_backend_iscsi.c
>> +++ b/src/storage/storage_backend_iscsi.c
>> @@ -35,6 +35,8 @@
>>   #include <unistd.h>
>>   #include <sys/stat.h>
>>   
>> +#include "datatypes.h"
>> +#include "driver.h"
>>   #include "virerror.h"
>>   #include "storage_backend_scsi.h"
>>   #include "storage_backend_iscsi.h"
>> @@ -42,6 +44,7 @@
>>   #include "virlog.h"
>>   #include "virfile.h"
>>   #include "vircommand.h"
>> +#include "virobject.h"
>>   #include "virrandom.h"
>>   #include "virstring.h"
>>   
>> @@ -746,10 +749,17 @@ cleanup:
>>   }
>>   
>>   static int
>> -virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
>> +virStorageBackendISCSISetAuth(virConnectPtr conn,
>> +                              virStoragePoolDefPtr def,
>>                                 const char *portal,
>>                                 const char *target)
>>   {
>> +    virSecretPtr secret = NULL;
>> +    unsigned char *secret_value = NULL;
>> +    const char *passwd = NULL;
>> +    virStoragePoolAuthChap chap = def->source.auth.chap;
>> +    int ret = -1;
>> +
>>       if (def->source.authType == VIR_STORAGE_POOL_AUTH_NONE)
>>           return 0;
>>   
>> @@ -759,6 +769,35 @@ virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
>>           return -1;
>>       }
>>   
>> +    if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_SECRET) {
>> +        if (chap.u.secret.uuidUsable)
>> +            secret = virSecretLookupByUUID(conn, chap.u.secret.uuid);
>> +        else
>> +            secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI,
>> +                                            chap.u.secret.usage);
>> +
>> +        if (secret) {
>> +            size_t secret_size;
>> +            secret_value = conn->secretDriver->secretGetValue(secret, &secret_size, 0,
>> +                                                              VIR_SECRET_GET_VALUE_INTERNAL_CALL);
>> +            if (!secret_value) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                               _("could not get the value of the secret "
>> +                                 "for username %s"), chap.login);
>> +                goto cleanup;
>> +            }
>> +        } else {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                           _("username '%s' specified but secret not found"),
>> +                           chap.login);
>> +            goto cleanup;
>> +        }
>> +
>> +        passwd = (const char *)secret_value;
>> +    } else if (chap.type == VIR_STORAGE_POOL_AUTH_CHAP_PLAIN_PASSWORD) {
>> +        passwd = chap.u.passwd;
>> +    }
>> +
>>       if (virStorageBackendISCSINodeUpdate(portal,
>>                                            target,
>>                                            "node.session.auth.authmethod",
>> @@ -770,14 +809,18 @@ virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
>>           virStorageBackendISCSINodeUpdate(portal,
>>                                            target,
>>                                            "node.session.auth.password",
>> -                                         def->source.auth.chap.u.passwd) < 0)
>> -        return -1;
>> +                                         passwd) < 0)
>> +        goto cleanup;
>>   
>> -    return 0;
>> +    ret = 0;
>> +cleanup:
>> +    virObjectUnref(secret);
>> +    VIR_FREE(secret_value);
>> +    return ret;
>>   }
>>   
>>   static int
>> -virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>> +virStorageBackendISCSIStartPool(virConnectPtr conn,
> Seeing this change to used now makes me wonder... Do we need to now
> check that conn != NULL anywhere?
>
> Since "virStorageBackendISCSIStartPool" is the "startPool" entry point -
> I used cscope and looked for "startPool", I found one reference where
> a NULL was passed:
>
> src/storage/storage_driver.c
> storageDriverAutostart()
> ...
>              if (backend->startPool &&
>                  backend->startPool(NULL, pool) < 0) {
>                  virErrorPtr err = virGetLastError();
> ...
>
> If I'm reading things right, I think that will cause issues in
> virSecretLookupByUUID() and virSecretLookupByUsage() when called by
> virStorageBackendISCSISetAuth().

Reasonable, will update.

Osier