[libvirt] [PATCH v2 3/3] storage: Implement iscsi_direct pool backend
Pavel Hrdina
phrdina at redhat.com
Tue Jul 24 19:02:42 UTC 2018
On Mon, Jul 23, 2018 at 08:43:01PM +0200, clem at lse.epita.fr wrote:
> From: Clementine Hayat <clem at lse.epita.fr>
>
> We need here libiscsi for the storgae pool backend.
> For the iscsi-direct storage pool, only checkPool and refreshPool should
> be necessary.
> The pool is state-less and just need the informations within the volume
> to work.
>
> Signed-off-by: Clementine Hayat <clem at lse.epita.fr>
> ---
> m4/virt-storage-iscsi-direct.m4 | 3 +
> src/storage/Makefile.inc.am | 2 +
> src/storage/storage_backend_iscsi_direct.c | 408 ++++++++++++++++++++-
> 3 files changed, 410 insertions(+), 3 deletions(-)
>
> diff --git a/m4/virt-storage-iscsi-direct.m4 b/m4/virt-storage-iscsi-direct.m4
> index cc2d490352..dab4414169 100644
> --- a/m4/virt-storage-iscsi-direct.m4
> +++ b/m4/virt-storage-iscsi-direct.m4
> @@ -29,6 +29,9 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_ISCSI_DIRECT], [
> with_storage_iscsi_direct=$with_libiscsi
> fi
> if test "$with_storage_iscsi_direct" = "yes"; then
> + if test "$with_libiscsi" = "no"; then
> + AC_MSG_ERROR([Need libiscsi for iscsi-direct storage driver])
> + fi
> AC_DEFINE_UNQUOTED([WITH_STORAGE_ISCSI_DIRECT], [1],
> [whether iSCSI backend for storage driver is enabled])
> fi
> diff --git a/src/storage/Makefile.inc.am b/src/storage/Makefile.inc.am
> index b2714fd960..bd5ea06f8b 100644
> --- a/src/storage/Makefile.inc.am
> +++ b/src/storage/Makefile.inc.am
> @@ -203,6 +203,7 @@ if WITH_STORAGE_ISCSI_DIRECT
> libvirt_storage_backend_iscsi_direct_la_SOURCES = $(STORAGE_DRIVER_ISCSI_DIRECT_SOURCES)
> libvirt_storage_backend_iscsi_direct_la_CFLAGS = \
> -I$(srcdir)/conf \
> + -I$(srcdir)/secret \
> $(LIBISCSI_CFLAGS) \
> $(AM_CFLAGS) \
> $(NULL)
> @@ -211,6 +212,7 @@ storagebackend_LTLIBRARIES += libvirt_storage_backend_iscsi-direct.la
> libvirt_storage_backend_iscsi_direct_la_LDFLAGS = $(AM_LDFLAGS_MOD)
> libvirt_storage_backend_iscsi_direct_la_LIBADD = \
> libvirt.la \
> + $(LIBISCSI_LIBS) \
> ../gnulib/lib/libgnu.la \
> $(NULL)
> endif WITH_STORAGE_ISCSI_DIRECT
> diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c
> index 94c4c989ff..b9a9bb3eb0 100644
> --- a/src/storage/storage_backend_iscsi_direct.c
> +++ b/src/storage/storage_backend_iscsi_direct.c
> @@ -22,28 +22,430 @@
>
> #include <config.h>
>
> +#include <iscsi/iscsi.h>
> +#include <iscsi/scsi-lowlevel.h>
> +
> +#include "datatypes.h"
> +#include "secret_util.h"
> #include "storage_backend_iscsi_direct.h"
> #include "storage_util.h"
> +#include "viralloc.h"
> +#include "virerror.h"
> #include "virlog.h"
> +#include "virobject.h"
> +#include "virstring.h"
> +#include "virtime.h"
> +#include "viruuid.h"
>
> #define VIR_FROM_THIS VIR_FROM_STORAGE
>
> +#define ISCSI_DEFAULT_TARGET_PORT 3260
> +#define VIR_ISCSI_TEST_UNIT_TIMEOUT 30 * 1000
> +
> VIR_LOG_INIT("storage.storage_backend_iscsi_direct");
>
> +static struct iscsi_context *
> +virISCSIDirectCreateContext(const char* initiator_iqn)
> +{
> + struct iscsi_context *iscsi = NULL;
> +
> + iscsi = iscsi_create_context(initiator_iqn);
> + if (!iscsi)
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to create iscsi context for %s"),
> + initiator_iqn);
> + return iscsi;
> +}
> +
> +static char *
> +virStorageBackendISCSIDirectPortal(virStoragePoolSourcePtr source)
> +{
> + char *portal = NULL;
> +
> + if (source->nhost != 1) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Expected exactly 1 host for the storage pool"));
> + return NULL;
> + }
> + if (source->hosts[0].port == 0) {
> + ignore_value(virAsprintf(&portal, "%s:%d",
> + source->hosts[0].name,
> + ISCSI_DEFAULT_TARGET_PORT));
> + } else if (strchr(source->hosts[0].name, ':')) {
> + ignore_value(virAsprintf(&portal, "[%s]:%d",
> + source->hosts[0].name,
> + source->hosts[0].port));
> + } else {
> + ignore_value(virAsprintf(&portal, "%s:%d",
> + source->hosts[0].name,
> + source->hosts[0].port));
> + }
> + return portal;
> +}
> +
> +static int
> +virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi,
> + virStoragePoolSourcePtr source)
> +{
> + unsigned char *secret_value = NULL;
> + size_t secret_size;
> + virStorageAuthDefPtr authdef = source->auth;
> + int ret = -1;
> + virConnectPtr conn = NULL;
> +
> + if (!authdef || authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE)
> + return 0;
> +
> + VIR_DEBUG("username='%s' authType=%d seclookupdef.type=%d",
> + authdef->username, authdef->authType, authdef->seclookupdef.type);
> +
> + if (authdef->authType != VIR_STORAGE_AUTH_TYPE_CHAP) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("iscsi-direct pool only supports 'chap' auth type"));
> + return ret;
> + }
> +
> + if (!(conn = virGetConnectSecret()))
> + return ret;
> +
> + if (virSecretGetSecretString(conn, &authdef->seclookupdef,
> + VIR_SECRET_USAGE_TYPE_ISCSI,
> + &secret_value, &secret_size) < 0)
> + goto cleanup;
> +
> + if (iscsi_set_initiator_username_pwd(iscsi,
> + authdef->username,
> + (const char *)secret_value) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to set credential: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + VIR_DISPOSE_N(secret_value, secret_size);
> + virObjectUnref(conn);
> + return ret;
> +}
> +
> +static int
> +virISCSIDirectSetContext(struct iscsi_context *iscsi,
> + const char *target_name)
> +{
> + if (iscsi_init_transport(iscsi, TCP_TRANSPORT) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to init transport: %s"),
> + iscsi_get_error(iscsi));
> + return -1;
> + }
> + if (iscsi_set_targetname(iscsi, target_name) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to set target name: %s"),
> + iscsi_get_error(iscsi));
> + return -1;
> + }
> + if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to set session type: %s"),
> + iscsi_get_error(iscsi));
> + return -1;
> + }
> + return 0;
> +}
> +
> +static int
> +virISCSIDirectConnect(struct iscsi_context *iscsi,
> + const char *portal)
> +{
> + if (iscsi_connect_sync(iscsi, portal) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to connect: %s"),
> + iscsi_get_error(iscsi));
> + return -1;
> + }
> + if (iscsi_login_sync(iscsi) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to login: %s"),
> + iscsi_get_error(iscsi));
> + return -1;
> + }
> + return 0;
> +}
> +
> +static struct scsi_reportluns_list *
> +virISCSIDirectReportLuns(struct iscsi_context *iscsi)
> +{
> + struct scsi_task *task = NULL;
> + struct scsi_reportluns_list *list = NULL;
> + int full_size;
> +
> + if (!(task = iscsi_reportluns_sync(iscsi, 0, 16))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to reportluns: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
Here we create new task.
> +
> + full_size = scsi_datain_getfullsize(task);
> +
> + if (full_size > task->datain.size) {
> + scsi_free_scsi_task(task);
> + if (!(task = iscsi_reportluns_sync(iscsi, 0, full_size))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to reportluns: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> + }
Here we get all the available LUNs.
> +
> + if (!(list = scsi_datain_unmarshall(task))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to unmarshall reportluns: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
And here we get the list of the available LUNs, so far so good. But
there is a hidden issue, the memory returned here and stored into @list
is not newly allocated memory but it's part of the @task data structure.
> +
> + cleanup:
> + scsi_free_scsi_task(task);
Here we free all the memory allocated by the @task together with the
@list and then we return pointer to freed memory.
> + return list;
> +}
> +
> +static int
> +virISCSIDirectTestUnitReady(struct iscsi_context *iscsi,
> + int lun)
> +{
> + struct scsi_task *task = NULL;
> + int ret = -1;
> + virTimeBackOffVar timebackoff;
> +
> + if (virTimeBackOffStart(&timebackoff, 1,
> + VIR_ISCSI_TEST_UNIT_TIMEOUT) < 0)
> + goto cleanup;
> +
> + do {
> + if (!(task = iscsi_testunitready_sync(iscsi, lun))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed testunitready: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> +
> + if (task->status != SCSI_STATUS_CHECK_CONDITION ||
> + task->sense.key != SCSI_SENSE_UNIT_ATTENTION ||
> + task->sense.ascq != SCSI_SENSE_ASCQ_BUS_RESET)
> + break;
> +
> + scsi_free_scsi_task(task);
> + } while (virTimeBackOffWait(&timebackoff));
> +
> + if (task->status != SCSI_STATUS_GOOD) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed testunitready: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + scsi_free_scsi_task(task);
> + return ret;
> +}
> +
> +static int
> +virISCSIDirectSetVolumeAttributes(virStoragePoolObjPtr pool,
> + virStorageVolDefPtr vol,
> + int lun,
> + char *portal)
> +{
> + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> +
> + if (virAsprintf(&vol->name, "%u", lun) < 0)
> + return -1;
> + if (virAsprintf(&vol->key, "ip-%s-iscsi-%s-lun-%u", portal,
> + def->source.devices[0].path, lun) < 0)
> + return -1;
> + if (virAsprintf(&vol->target.path, "ip-%s-iscsi-%s-lun-%u", portal,
> + def->source.devices[0].path, lun) < 0)
> + return -1;
Now that I'm testing the code I'm not sure about these values. For
example the name cannot be only the number of the LUN, that will not
work in domain XML if we use virDomainDiskAddISCSIPoolSourceHost() in
order to translate storage volume into domain disk definition.
I thing we should use these values:
name unit:0:0:$LUN
path $poolName/$volName
key can remain as it is.
> + return 0;
> +}
> +
> +static int
> +virISCSIDirectSetVolumeCapacity(struct iscsi_context *iscsi,
> + virStorageVolDefPtr vol,
> + int lun)
> +{
> + struct scsi_task *task = NULL;
> + struct scsi_inquiry_standard *inq = NULL;
> + long long size = 0;
> + int ret = -1;
> +
> + if (!(task = iscsi_inquiry_sync(iscsi, lun, 0, 0, 64)) ||
> + task->status != SCSI_STATUS_GOOD) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to send inquiry command: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> +
> + if (!(inq = scsi_datain_unmarshall(task))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to unmarshall reply: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> +
> + if (inq->device_type == SCSI_INQUIRY_PERIPHERAL_DEVICE_TYPE_DIRECT_ACCESS) {
> + struct scsi_readcapacity10 *rc10 = NULL;
> +
> + scsi_free_scsi_task(task);
> + task = NULL;
> +
> + if (!(task = iscsi_readcapacity10_sync(iscsi, lun, 0, 0)) ||
> + task->status != SCSI_STATUS_GOOD) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to get capacity of lun: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> +
> + if (!(rc10 = scsi_datain_unmarshall(task))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to unmarshall reply: %s"),
> + iscsi_get_error(iscsi));
> + goto cleanup;
> + }
> +
> + size = rc10->block_size;
> + size *= rc10->lba;
> + vol->target.capacity = size;
> + vol->target.allocation = size;
> +
> + }
> +
> + ret = 0;
> + cleanup:
> + scsi_free_scsi_task(task);
> + return ret;
> +}
> +
> +static int
> +virISCSIDirectRefreshVol(virStoragePoolObjPtr pool,
> + struct iscsi_context *iscsi,
> + int lun,
> + char *portal)
> +{
> + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> + virStorageVolDefPtr vol = NULL;
> + int ret = -1;
> +
> + virStoragePoolObjClearVols(pool);
> + if (virISCSIDirectTestUnitReady(iscsi, lun) < 0)
> + goto cleanup;
> +
> + if (VIR_ALLOC(vol) < 0)
> + goto cleanup;
> +
> + vol->type = VIR_STORAGE_VOL_NETWORK;
> +
> + if (virISCSIDirectSetVolumeCapacity(iscsi, vol, lun) < 0)
> + goto cleanup;
> +
> + def->capacity += vol->target.capacity;
> + def->allocation += vol->target.allocation;
> +
> + if (virISCSIDirectSetVolumeAttributes(pool, vol, lun, portal) < 0)
> + goto cleanup;
> +
> + if (virStoragePoolObjAddVol(pool, vol) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to create volume: %d"),
> + lun);
> + goto cleanup;
> + }
> + vol = NULL;
> +
> + ret = 0;
> + cleanup:
> + virStorageVolDefFree(vol);
> + return ret;
> +}
>
> static int
> -virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
> - bool *isActive ATTRIBUTE_UNUSED)
> +virStorageBackendISCSIDirectRefreshVols(virStoragePoolObjPtr pool,
> + struct iscsi_context *iscsi,
> + char *portal)
> {
> + struct scsi_reportluns_list *list = NULL;
> + size_t i;
> +
> + if (!(list = virISCSIDirectReportLuns(iscsi)))
> + return -1;
Unfortunately this will not work :/. Documentation of libiscsi really
sucks, sigh.
> + for (i = 0; i < list->num; i++) {
> + if (virISCSIDirectRefreshVol(pool, iscsi, list->luns[i], portal) < 0)
> + return -1;
> + }
This for loop needs to be moved into virISCSIDirectReportLuns(), witch
means that we can remove expand content of that function here and remove
it completely. The result would be only having
virStorageBackendISCSIDirectRefreshVols() with code from
virISCSIDirectReportLuns() as well.
Pavel
> +
> return 0;
> }
>
> static int
> -virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED)
> +virISCSIDirectDisconnect(struct iscsi_context *iscsi)
> {
> + if (iscsi_logout_sync(iscsi) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to logout: %s"),
> + iscsi_get_error(iscsi));
> + return -1;
> + }
> + if (iscsi_disconnect(iscsi) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to disconnect: %s"),
> + iscsi_get_error(iscsi));
> + return -1;
> + }
> return 0;
> }
>
> +static int
> +virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool,
> + bool *isActive)
> +{
> + *isActive = virStoragePoolObjIsActive(pool);
> + return 0;
> +}
> +
> +static int
> +virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
> +{
> + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
> + struct iscsi_context *iscsi = NULL;
> + char *portal = NULL;
> + int ret = -1;
> +
> + if (!(iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
> + goto cleanup;
> + if (!(portal = virStorageBackendISCSIDirectPortal(&def->source)))
> + goto cleanup;
> + if (virStorageBackendISCSIDirectSetAuth(iscsi, &def->source) < 0)
> + goto cleanup;
> + if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path) < 0)
> + goto cleanup;
> + if (virISCSIDirectConnect(iscsi, portal) < 0)
> + goto cleanup;
> + if (virStorageBackendISCSIDirectRefreshVols(pool, iscsi, portal) < 0)
> + goto disconect;
> +
> + ret = 0;
> + disconect:
> + virISCSIDirectDisconnect(iscsi);
> + cleanup:
> + VIR_FREE(portal);
> + iscsi_destroy_context(iscsi);
> + return ret;
> +}
> +
> virStorageBackend virStorageBackendISCSIDirect = {
> .type = VIR_STORAGE_POOL_ISCSI_DIRECT,
>
> --
> 2.18.0
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180724/e035c8a2/attachment-0001.sig>
More information about the libvir-list
mailing list