3:57 a.m.
On 07/06/13 00:31, John Ferlan wrote:
On 05/28/2013 02:39 AM, Osier Yang wrote:
> Though the XML for "chap" authentication with plain "password"
> was introduced long ago, the function was never implemented.
> This patch completes it.
>
> There are two types of CHAP configurations supported for iSCSI
> authentication:
> * Initiator Authentication
> Forward, one-way; The initiator is authenticated by the target.
>
> * Target Authentication
> Reverse, Bi-directional, mutual, two-way; The target is authenticated
> by the initiator; This method also requires Initiator Authentication
>
> This only supports the "Initiator Authentication". (I don't have any
> enterprise iSCSI env for testing, only have a iSCSI target setup with
> tgtd, which doesn't support "Target Authentication").
>
> "Discovery authentication" is not supported by tgt yet too. So this only
> setup the session authentication by executing 3 iscsiadm commands, E.g:
>
> % iscsiadm -m node --target "iqn.2013-05.test:iscsi.foo" --name \
> "node.session.auth.authmethod" -v "CHAP" --op update
>
> % iscsiadm -m node --target "iqn.2013-05.test:iscsi.foo" --name \
> "node.session.auth.username" -v "Jim" --op update
>
> % iscsiadm -m node --target "iqn.2013-05.test:iscsi.foo" --name \
> "node.session.auth.password" -v "Jimsecret" --op update
> ---
> src/storage/storage_backend_iscsi.c | 68 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
> index ad38ab2..6a17b5a 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -713,6 +713,68 @@ virStorageBackendISCSICheckPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
> return ret;
> }
>
> +static int
> +virStorageBackendISCSINodeUpdate(const char *portal,
> + const char *target,
> + const char *name,
> + const char *value)
> +{
> + virCommandPtr cmd = NULL;
> + int status;
> + int ret = -1;
> +
> + cmd = virCommandNewArgList(ISCSIADM,
> + "--mode", "node",
> + "--portal", portal,
> + "--target", target,
> + "--op", "update",
> + "--name", name,
> + "--value", value,
> + NULL);
> +
> + if (virCommandRun(cmd, &status) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to update '%s' of node mode for target
'%s'"),
> + name, target);
> + goto cleanup;
> + }
> +
> + ret = 0;
> +cleanup:
> + virCommandFree(cmd);
> + return ret;
> +}
> +
> +static int
> +virStorageBackendISCSISetAuth(virStoragePoolDefPtr def,
> + const char *portal,
[1] Any reason to not pass portal first like other API's? Not that matters..
No reason, will change. :-)
> + const char *target)
> +{
> + if (def->source.authType == VIR_STORAGE_POOL_AUTH_NONE)
> + return 0;
> +
> + if (def->source.authType != VIR_STORAGE_POOL_AUTH_CHAP) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("iscsi pool only supports 'chap' auth
type"));
> + return -1;
> + }
> +
> + if (virStorageBackendISCSINodeUpdate(portal,
> + target,
> + "node.session.auth.authmethod",
> + "CHAP") < 0 ||
> + virStorageBackendISCSINodeUpdate(portal,
> + target,
> + "node.session.auth.username",
> + def->source.auth.chap.login) < 0 ||
> + virStorageBackendISCSINodeUpdate(portal,
> + target,
> + "node.session.auth.password",
> + def->source.auth.chap.u.passwd) < 0)
In 04/11, you added def->source.auth.type setting/checking which I don't
see here... Or is there some magic happening where auth.u.secret can
take the place of passwd...
It's handled in next patch. Commit log should be enough to clarify this
patch only handles plain password:
<...>
Though the XML for "chap" authentication with plain "password" was
introduced long ago, the function was never implemented. This patch
completes it.
</...>
> + return -1;
> +
> + return 0;
> +}
>
> static int
> virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> @@ -745,6 +807,7 @@ virStorageBackendISCSIStartPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
> if ((session = virStorageBackendISCSISession(pool, 1)) == NULL) {
> if ((portal = virStorageBackendISCSIPortal(&pool->def->source))
== NULL)
> goto cleanup;
> +
> /*
> * iscsiadm doesn't let you login to a target, unless you've
> * first issued a 'sendtargets' command to the portal :-(
> @@ -754,6 +817,11 @@ virStorageBackendISCSIStartPool(virConnectPtr conn
ATTRIBUTE_UNUSED,
> NULL, NULL) < 0)
> goto cleanup;
>
> + if (virStorageBackendISCSISetAuth(pool->def,
> + portal,
[1] Why not follow other code which passes portal first...
Will change.
Also why pass
pool->def and pool->def->source.devices[0].path - it's not like you
couldn't grab "source.devices[0].path" from the passed pool->def
Sounds reasonable, I will see if there is requirement for it, if not
will change too.
Osier