10:01 a.m.
On 07/15/2013 10:16 AM, Osier Yang wrote:
On 10/07/13 03:10, John Ferlan wrote:
> From: Osier Yang <jyang(a)redhat.com>
<...snip...>
> diff --git a/src/storage/storage_backend_iscsi.c
> b/src/storage/storage_backend_iscsi.c
> index 0a4cd22..673ca16 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -32,6 +32,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"
> @@ -39,6 +41,7 @@
> #include "virlog.h"
> #include "virfile.h"
> #include "vircommand.h"
> +#include "virobject.h"
> #include "virrandom.h"
> #include "virstring.h"
> @@ -545,9 +548,112 @@ cleanup:
> 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(const char *portal,
> + virConnectPtr conn,
> + virStoragePoolSourcePtr source)
> +{
> + virSecretPtr secret = NULL;
> + unsigned char *secret_value = NULL;
> + const char *passwd = NULL;
> + virStoragePoolAuthChap chap;
> + int ret = -1;
> +
> + if (source->authType == VIR_STORAGE_POOL_AUTH_NONE)
> + return 0;
> +
> + if (source->authType != VIR_STORAGE_POOL_AUTH_CHAP) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("iscsi pool only supports 'chap' auth
type"));
> + return -1;
> + }
> +
> + chap = source->auth.chap;
> + 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,
> + source->devices[0].path,
> + "node.session.auth.authmethod",
> + "CHAP") < 0 ||
> + virStorageBackendISCSINodeUpdate(portal,
> + source->devices[0].path,
> + "node.session.auth.username",
> + source->auth.chap.login) < 0 ||
> + virStorageBackendISCSINodeUpdate(portal,
> + source->devices[0].path,
> + "node.session.auth.password",
> + passwd) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> +cleanup:
> + virObjectUnref(secret);
> + VIR_FREE(secret_value);
> + return ret;
> +}
> static char *
> -virStorageBackendISCSIFindPoolSources(virConnectPtr conn
> ATTRIBUTE_UNUSED,
> +virStorageBackendISCSIFindPoolSources(virConnectPtr conn,
> const char *srcSpec,
> unsigned int flags)
> {
> @@ -590,6 +696,9 @@
> virStorageBackendISCSIFindPoolSources(virConnectPtr conn
> ATTRIBUTE_UNUSED,
> &ntargets, &targets) < 0)
> goto cleanup;
> + if (virStorageBackendISCSISetAuth(portal, conn, source) < 0)
> + goto cleanup;
> +
i think the chap auth iscsi pool won't be able to start with this change,
findPoolSources is an api for discovering the pool sources. however,
we want the chap auth are set up before connecting to the iscsi target
when starting the pool, otherwise the pool starting will fail on
authentication.
note that startPool and findPoolSources are independant apis, they don't
invoke each other.
with this change, if one wants to start the pool successfully, he has to
invoke the findPoolSources first, this dependancy is incorrect. as an
example, in virsh layer, one has to execute find-storage-pool-sources
or find-storage-pool-sources-as before pool-start.
as an alternative way to have the non-null 'conn' for startPool, we can
create a connection in storageDriverAutostart (like qemu driver does),
which is the only place pass an null 'conn' to startPool,
While there is a v3 posted - this code hasn't differed there, so I'll
just use this opportunity to ask you questions...
A 'conn' to what? Should we assume qemu like nwfilter_driver.c does?
if (!driverState->privileged)
return 0;
conn = virConnectOpen("qemu:///system");
Do we further restrict the StartPool code to ensure there is a
privileged qemu connection?
This was the decision point I had to make...
John
regards
osier