[libvirt] [PATCH v2 6/7] storage: Support "chap" authentication for iscsi pool
Osier Yang
jyang at redhat.com
Mon Jul 15 15:11:11 UTC 2013
On 15/07/13 23:01, John Ferlan wrote:
> On 07/15/2013 10:16 AM, Osier Yang wrote:
>> On 10/07/13 03:10, John Ferlan wrote:
>>> From: Osier Yang <jyang at 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;
i think we don't need to restrict it to be a priviledged connection for
storage. otherwise there will be regression.
> conn = virConnectOpen("qemu:///system");
>
> Do we further restrict the StartPool code to ensure there is a
> privileged qemu connection?
yeah, we will want to get a connection object, but as said above, it
should work both priviledged or unpriviledged connection, and no
need to restrict startPool to ensure there is a connection, since
other storage backends may still work without a connection object
osier
More information about the libvir-list
mailing list