4 a.m.
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