[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]