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;
--- 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(). John
virStoragePoolObjPtr pool) { char *portal = NULL; @@ -817,7 +860,8 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, NULL, NULL) < 0) goto cleanup;
- if (virStorageBackendISCSISetAuth(pool->def, + if (virStorageBackendISCSISetAuth(conn, + pool->def, portal, pool->def->source.devices[0].path) < 0) goto cleanup;