[libvirt] [PATCH 0/1] Merge DanPB's SCSI HBA pool code
This patch contains the implementation Daniel Berrange did of storage pools using SCSI HBAs. I have updated it for the current tree in preparation for implementing NPIV support. Let me know what you think. I would also like feedback on the integration into the build system, particularly where POLKIT_CFLAGS should go. Dave
Last March, DanPB posted code to create pools from SCSI HBAs. This patch is simply bringing that code into the current tree. * src/storage_backend_scsi.[ch] contain the pool implementataion * There are small changes to several other source files to integrate the new pool type. --- configure.in | 13 ++- po/POTFILES.in | 1 + src/Makefile.am | 9 + src/storage_backend.c | 6 + src/storage_backend_scsi.c | 471 ++++++++++++++++++++++++++++++++++++++++++++ src/storage_backend_scsi.h | 45 +++++ src/storage_conf.c | 18 ++ 7 files changed, 562 insertions(+), 1 deletions(-) create mode 100644 src/storage_backend_scsi.c create mode 100644 src/storage_backend_scsi.h diff --git a/configure.in b/configure.in index d2e8252..b88a7d5 100644 --- a/configure.in +++ b/configure.in @@ -739,6 +739,8 @@ AC_ARG_WITH([storage-lvm], [ --with-storage-lvm with LVM backend for the storage driver (on)],[],[with_storage_lvm=check]) AC_ARG_WITH([storage-iscsi], [ --with-storage-iscsi with iSCSI backend for the storage driver (on)],[],[with_storage_iscsi=check]) +AC_ARG_WITH([storage-scsi], +[ --with-storage-scsi with SCSI backend for the storage driver (on)],[],[with_storage_scsi=check]) AC_ARG_WITH([storage-disk], [ --with-storage-disk with GPartd Disk backend for the storage driver (on)],[],[with_storage_disk=check]) @@ -748,6 +750,7 @@ if test "$with_libvirtd" = "no"; then with_storage_fs=no with_storage_lvm=no with_storage_iscsi=no + with_storage_scsi=no with_storage_disk=no fi if test "$with_storage_dir" = "yes" ; then @@ -876,6 +879,13 @@ if test "$with_storage_iscsi" = "yes" -o "$with_storage_iscsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_ISCSI], [test "$with_storage_iscsi" = "yes"]) +if test "$with_storage_scsi" = "check"; then + with_storage_scsi=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_SCSI], 1, + [whether SCSI backend for storage driver is enabled]) + AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) +fi LIBPARTED_CFLAGS= @@ -1158,7 +1168,7 @@ LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.} AC_SUBST([LV_LIBTOOL_OBJDIR]) dnl HAL or DeviceKit library for host device enumeration -HAL_REQUIRED=0.0 +HAL_REQUIRED=0.5.0 HAL_CFLAGS= HAL_LIBS= AC_ARG_WITH([hal], @@ -1312,6 +1322,7 @@ AC_MSG_NOTICE([ FS: $with_storage_fs]) AC_MSG_NOTICE([ NetFS: $with_storage_fs]) AC_MSG_NOTICE([ LVM: $with_storage_lvm]) AC_MSG_NOTICE([ iSCSI: $with_storage_iscsi]) +AC_MSG_NOTICE([ SCSI: $with_storage_scsi]) AC_MSG_NOTICE([ Disk: $with_storage_disk]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Driver Loadable Modules]) diff --git a/po/POTFILES.in b/po/POTFILES.in index 7461f93..4433183 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -27,6 +27,7 @@ src/storage_backend_disk.c src/storage_backend_fs.c src/storage_backend_iscsi.c src/storage_backend_logical.c +src/storage_backend_scsi.c src/storage_conf.c src/storage_driver.c src/test.c diff --git a/src/Makefile.am b/src/Makefile.am index 3a798d2..f1dd789 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,6 +9,7 @@ INCLUDES = \ $(XEN_CFLAGS) \ $(SELINUX_CFLAGS) \ $(DRIVER_MODULE_CFLAGS) \ + $(POLKIT_CFLAGS) \ -DLIBDIR=\""$(libdir)"\" \ -DBINDIR=\""$(libexecdir)"\" \ -DSBINDIR=\""$(sbindir)"\" \ @@ -158,6 +159,9 @@ STORAGE_DRIVER_LVM_SOURCES = \ STORAGE_DRIVER_ISCSI_SOURCES = \ storage_backend_iscsi.h storage_backend_iscsi.c +STORAGE_DRIVER_SCSI_SOURCES = \ + storage_backend_scsi.h storage_backend_scsi.c + STORAGE_DRIVER_DISK_SOURCES = \ storage_backend_disk.h storage_backend_disk.c @@ -344,6 +348,10 @@ if WITH_STORAGE_ISCSI libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_ISCSI_SOURCES) endif +if WITH_STORAGE_SCSI +libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_SCSI_SOURCES) +endif + if WITH_STORAGE_DISK libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES) endif @@ -392,6 +400,7 @@ EXTRA_DIST += \ $(STORAGE_DRIVER_FS_SOURCES) \ $(STORAGE_DRIVER_LVM_SOURCES) \ $(STORAGE_DRIVER_ISCSI_SOURCES) \ + $(STORAGE_DRIVER_SCSI_SOURCES) \ $(STORAGE_DRIVER_DISK_SOURCES) \ $(NODE_DEVICE_DRIVER_SOURCES) \ $(NODE_DEVICE_DRIVER_HAL_SOURCES) \ diff --git a/src/storage_backend.c b/src/storage_backend.c index 8408f34..95173fa 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -54,6 +54,9 @@ #if WITH_STORAGE_ISCSI #include "storage_backend_iscsi.h" #endif +#if WITH_STORAGE_SCSI +#include "storage_backend_scsi.h" +#endif #if WITH_STORAGE_DISK #include "storage_backend_disk.h" #endif @@ -78,6 +81,9 @@ static virStorageBackendPtr backends[] = { #if WITH_STORAGE_ISCSI &virStorageBackendISCSI, #endif +#if WITH_STORAGE_SCSI + &virStorageBackendSCSI, +#endif #if WITH_STORAGE_DISK &virStorageBackendDisk, #endif diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c new file mode 100644 index 0000000..236d894 --- /dev/null +++ b/src/storage_backend_scsi.c @@ -0,0 +1,471 @@ +/* + * storage_backend_scsi.c: storage backend for SCSI handling + * + * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange redhat com> + */ + +#include <config.h> + +#include <c-ctype.h> +#include <unistd.h> +#include <stdio.h> +#include <hal/libhal.h> + +#include "virterror_internal.h" +#include "storage_backend_scsi.h" +#include "memory.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +#define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host/" +#define LINUX_SYSFS_SCSI_HOST_POSTFIX "/device" + + +/** + * virStorageBackendSCSIMakeHostDevice: + * @conn: report errors agains + * @pool: pool to resolve + * @buf: pre-allocated buffer to fill with path name + * @buflen: size of buffer + * + * Resolve a HBA name to HBA device path + * + * Given a pool object configured for a SCSI HBA, resolve the HBA name + * into the canonical sysfs path for the HBA device. This can then be + * used to lookup the device in HAL. + * + * Returns 0 on success, -1 on failure + */ +static int virStorageBackendSCSIMakeHostDevice(virConnectPtr conn, + virStoragePoolObjPtr pool, + char *buf, + size_t buflen) +{ + char *dev = NULL; + char *tmp = pool->def->source.adapter; + char relLink[PATH_MAX], absLink[PATH_MAX]; + ssize_t len; + + if (!tmp) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Missing adapter name for pool")); + return -1; + } + + /* Sanity check host adapter name - should only be 0-9, a-z. + * Anything else is bogus & so we reject it, to prevent them + * making us read arbitrary paths on host + */ + while (*tmp) { + if (!c_isalnum(*tmp)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Invalid character in pool adapter name '%s'"), + pool->def->source.adapter); + return -1; + } + tmp++; + } + + /* + * First get the class based path eg + * + * /sys/class/scsi_host/host1/device + */ + if ((dev = malloc(sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + + strlen(pool->def->source.adapter) + + sizeof(LINUX_SYSFS_SCSI_HOST_POSTFIX) + + 1)) == NULL) { + virReportOOMError(conn); + return -1; + } + + strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); + strcat(dev, pool->def->source.adapter); + strcat(dev, LINUX_SYSFS_SCSI_HOST_POSTFIX); + + /* + * Now resolve the class based path symlink to the real + * device path, which is likely under PCI bus hierarchy + * and is the path tracked by HAL + */ + /* Readlink does not null terminate, so we reserve one byte */ + if ((len = readlink(dev, relLink, sizeof(relLink)-1)) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot find SCSI host adapter '%s' at '%s'"), + pool->def->source.adapter, dev); + free(dev); + return -1; + } + relLink[len] = '\0'; + + + /* + * The symlink is relative, so now we have to turn it + * into a absolute path + */ + if ((tmp = realloc(dev, + sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + + strlen(pool->def->source.adapter) + + 1 + + strlen(relLink) + + 1)) == NULL) { + virReportOOMError(conn); + free(dev); + return -1; + } + dev = tmp; + + strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); + strcat(dev, pool->def->source.adapter); + strcat(dev, "/"); + strcat(dev, relLink); + + /* + * And finally canonicalize the absolute path to remove the + * copious .. components + */ + if (!realpath(dev, absLink)) { + char ebuf[1024]; + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot canonicalize link: %s"), + virStrerror(errno, ebuf, sizeof ebuf)); + free(dev); + return -1; + } + free(dev); + + if (strlen(absLink) >= buflen) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("link too long for buffer")); + return -1; + } + strcpy(buf, absLink); + + return 0; +} + + +/** + * virStorageBackendSCSICreateVol + * + * Allocate a virStorageVolDef object for the specified + * metadata and hook it into the pool + * + * Returns 0 on success, -1 on failure + */ +static int +virStorageBackendSCSICreateVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + const char *name, + const char *path, + const char *key, + unsigned long long size) +{ + virStorageVolDefPtr vol; + char *tmppath; + + if (VIR_ALLOC(vol) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + vol->type = VIR_STORAGE_VOL_BLOCK; + + tmppath = strdup(path); + vol->name = strdup(name); + vol->key = strdup(key); + vol->allocation = vol->capacity = size; + + if ((vol->name == NULL) || + (vol->key == NULL) || + (tmppath == NULL)) { + virReportOOMError(conn); + goto cleanup; + } + + /* Now figure out the stable path + * + * XXX this method is O(N) because it scans the pool target + * dir every time its run. Should figure out a more efficient + * way of doing this... + */ + if ((vol->target.path = virStorageBackendStablePath(conn, + pool, + tmppath)) == NULL) + goto cleanup; + + if (tmppath != vol->target.path) + free(tmppath); + tmppath = NULL; + + if (virStorageBackendUpdateVolInfo(conn, vol, 1) < 0) + goto cleanup; + + pool->def->capacity += vol->capacity; + pool->def->allocation += vol->allocation; + + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count+1) < 0) { + virReportOOMError(conn); + virStorageVolDefFree(vol); + goto cleanup; + } + pool->volumes.objs[pool->volumes.count++] = vol; + + return 0; + +cleanup: + free(tmppath); + virStorageVolDefFree(vol); + return -1; +} + + +/** + * virStorageBackendSCSIAddLUN: + * + * @conn: connection to report errors against + * @pool: pool to register volume with + * @ctx: HAL context + * @devname: HAL device name for LUN + * + * Given a HAL device supported 'block' and 'storage' capabilities + * register it as a volume in the pool + * + * Returns 0 on success, -1 on error + */ +static int +virStorageBackendSCSIAddLUN(virConnectPtr conn, + virStoragePoolObjPtr pool, + LibHalContext *ctx, + const char *devname) +{ + char **strdevs = NULL; + int numstrdevs = 0; + char *dev = NULL, *key = NULL; + unsigned long long size; + int host, bus, target, lun; + int n = -1, i; + int ret = -1; + char name[100]; + DBusError derr = DBUS_ERROR_INIT; + + if ((strdevs = libhal_manager_find_device_string_match(ctx, + "info.parent", + devname, + &numstrdevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("HAL failed to lookup LUNs for '%s': %s"), + devname, derr.message); + goto cleanup; + } + for (i = 0 ; i < numstrdevs && n == -1 ; i++) { + char *cat = libhal_device_get_property_string(ctx, + strdevs[0], + "info.category", + NULL); + /* XXX derr */ + if (cat != NULL) { + if (STREQ(cat, "storage")) + n = i; + free(cat); + } + } + /* No storage vol for device - probably a removable vol so ignore */ + if (n == -1) { + ret = 0; + goto cleanup; + } + +#define HAL_GET_PROPERTY(path, name, err, type, value) \ + (value) = libhal_device_get_property_ ## type(ctx, (path), (name), (err)); \ + if (dbus_error_is_set((err))) { \ + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, \ + "unable to lookup '%s' property on %s: %s", \ + (name), (path), derr.message); \ + goto cleanup; \ + } + + /* These are props of the physical device */ + HAL_GET_PROPERTY(devname, "scsi.host", &derr, int, host); + HAL_GET_PROPERTY(devname, "scsi.bus", &derr, int, bus); + HAL_GET_PROPERTY(devname, "scsi.target", &derr, int, target); + HAL_GET_PROPERTY(devname, "scsi.lun", &derr, int, lun); + + /* These are props of the logic device */ + HAL_GET_PROPERTY(strdevs[0], "block.device", &derr, string, dev); + /* + * XXX storage.serial is not actually unique if they have + * multipath on the fibre channel adapter + */ + HAL_GET_PROPERTY(strdevs[0], "storage.serial", &derr, string, key); + HAL_GET_PROPERTY(strdevs[0], "storage.size", &derr, uint64, size); + +#undef HAL_GET_PROPERTY + + snprintf(name, sizeof(name), "%d:%d:%d:%d", host, bus, target, lun); + name[sizeof(name)-1] = '\0'; + + if (virStorageBackendSCSICreateVol(conn, pool, name, dev, key, size) < 0) + goto cleanup; + ret = 0; + +cleanup: + for (i = 0 ; strdevs && i < numstrdevs ; i++) + free(strdevs[i]); + free(strdevs); + free(dev); + free(key); + if (dbus_error_is_set(&derr)) + dbus_error_free(&derr); + return ret; +} + + +/** + * virStorageBackendSCSIRefreshPool: + * + * Query HAL for all storage devices associated with a specific + * SCSI HBA. + * + * XXX, currently we only support regular devices in /dev/, + * or /dev/disk/by-XXX. In future we also need to be able to + * map to multipath devices setup under /dev/mpath/XXXX. + */ +static int +virStorageBackendSCSIRefreshPool(virConnectPtr conn, + virStoragePoolObjPtr pool) +{ + char hostdevice[PATH_MAX]; + LibHalContext *ctx = NULL; + DBusConnection *sysbus = NULL; + DBusError derr = DBUS_ERROR_INIT; + char **hbadevs = NULL, **blkdevs = NULL; + int numhbadevs = 0, numblkdevs = 0; + int i, ret = -1; + + /* Get the canonical sysfs path for the HBA device */ + if (virStorageBackendSCSIMakeHostDevice(conn, pool, + hostdevice, sizeof(hostdevice)) < 0) + goto cleanup; + + if ((ctx = libhal_ctx_new()) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to allocate HAL context")); + goto cleanup; + } + + sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr); + if (sysbus == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to get dbus system bus: '%s'"), + derr.message); + goto cleanup; + } + + if (!libhal_ctx_set_dbus_connection(ctx, sysbus)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to set HAL dbus connection")); + goto cleanup; + } + + + if (!libhal_ctx_init(ctx, &derr)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to initialize HAL context: '%s'"), + derr.message); + goto cleanup; + } + + if ((hbadevs = libhal_manager_find_device_string_match(ctx, + "linux.sysfs_path", + hostdevice, + &numhbadevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("HAL failed to lookup device '%s': %s"), + hostdevice, derr.message); + goto cleanup; + } + + if (numhbadevs != 1) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected number of HBA devices from HAL " + "(expected 1, got %d) when looking up device " + "'%s'"), numhbadevs, hostdevice); + goto cleanup; + } + + if ((blkdevs = libhal_manager_find_device_string_match(ctx, + "info.parent", + hbadevs[0], + &numblkdevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to lookup LUNs for %s with HAL: %s"), + hbadevs[0], derr.message); + goto cleanup; + } + + for (i = 0 ; i < numblkdevs ; i++) + virStorageBackendSCSIAddLUN(conn, + pool, + ctx, + blkdevs[i]); + + ret = 0; + +cleanup: + for (i = 0 ; hbadevs && i < numhbadevs ; i++) + free(hbadevs[i]); + free(hbadevs); + for (i = 0 ; blkdevs && i < numblkdevs ; i++) + free(blkdevs[i]); + free(blkdevs); + libhal_ctx_shutdown(ctx, NULL); + libhal_ctx_free(ctx); + if (sysbus) + dbus_connection_unref(sysbus); + if (dbus_error_is_set(&derr)) + dbus_error_free(&derr); + return ret; +} + + + +virStorageBackend virStorageBackendSCSI = { + .type = VIR_STORAGE_POOL_SCSI, + .refreshPool = virStorageBackendSCSIRefreshPool, +}; + +/* + * vim: set tabstop=4: + * vim: set shiftwidth=4: + * vim: set expandtab: + */ +/* + * Local variables: + * indent-tabs-mode: nil + * c-indent-level: 4 + * c-basic-offset: 4 + * tab-width: 4 + * End: + */ diff --git a/src/storage_backend_scsi.h b/src/storage_backend_scsi.h new file mode 100644 index 0000000..c912269 --- /dev/null +++ b/src/storage_backend_scsi.h @@ -0,0 +1,45 @@ +/* + * storage_backend_scsi.h: storage backend for SCSI handling + * + * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange redhat com> + */ + +#ifndef __VIR_STORAGE_BACKEND_SCSI_H__ +#define __VIR_STORAGE_BACKEND_SCSI_H__ + +#include "storage_backend.h" + +extern virStorageBackend virStorageBackendSCSI; + +#endif /* __VIR_STORAGE_BACKEND_SCSI_H__ */ + +/* + * vim: set tabstop=4: + * vim: set shiftwidth=4: + * vim: set expandtab: + */ +/* + * Local variables: + * indent-tabs-mode: nil + * c-indent-level: 4 + * c-basic-offset: 4 + * tab-width: 4 + * End: + */ diff --git a/src/storage_conf.c b/src/storage_conf.c index 70107a2..2ea6f8e 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -187,6 +187,14 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatDiskTypeToString, } }, + { .poolType = VIR_STORAGE_POOL_SCSI, + .poolOptions = { + .flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER), + }, + .volOptions = { + .formatToString = virStoragePoolFormatDiskTypeToString, + } + }, { .poolType = VIR_STORAGE_POOL_DISK, .poolOptions = { .flags = (VIR_STORAGE_POOL_SOURCE_DEVICE), @@ -269,6 +277,7 @@ virStoragePoolSourceFree(virStoragePoolSourcePtr source) { VIR_FREE(source->devices); VIR_FREE(source->dir); VIR_FREE(source->name); + VIR_FREE(source->adapter); if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) { VIR_FREE(source->auth.chap.login); @@ -550,6 +559,15 @@ virStoragePoolDefParseDoc(virConnectPtr conn, } } + if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) { + if ((ret->source.adapter = virXPathString(conn, + "string(/pool/source/adapter/@name)", + ctxt)) == NULL) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("missing storage pool source adapter name")); + goto cleanup; + } + } authType = virXPathString(conn, "string(/pool/source/auth/@type)", ctxt); if (authType == NULL) { -- 1.6.0.6
David Allan wrote:
Last March, DanPB posted code to create pools from SCSI HBAs. This patch is simply bringing that code into the current tree.
* src/storage_backend_scsi.[ch] contain the pool implementataion * There are small changes to several other source files to integrate the new pool type.
[hmm... I wrote this a couple days ago and nearly forgot to send it. ] This looks fine, overall. a few suggestions below. ...
diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c ... + /* + * First get the class based path eg + * + * /sys/class/scsi_host/host1/device + */ + if ((dev = malloc(sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + + strlen(pool->def->source.adapter) + + sizeof(LINUX_SYSFS_SCSI_HOST_POSTFIX) + + 1)) == NULL) { + virReportOOMError(conn); + return -1; + } + + strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); + strcat(dev, pool->def->source.adapter); + strcat(dev, LINUX_SYSFS_SCSI_HOST_POSTFIX);
use virAsprintf in place of the above malloc/strcpy/strcat calls
+ /* + * Now resolve the class based path symlink to the real + * device path, which is likely under PCI bus hierarchy + * and is the path tracked by HAL + */ + /* Readlink does not null terminate, so we reserve one byte */ + if ((len = readlink(dev, relLink, sizeof(relLink)-1)) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot find SCSI host adapter '%s' at '%s'"), + pool->def->source.adapter, dev); + free(dev); + return -1; + } + relLink[len] = '\0'; + + + /* + * The symlink is relative, so now we have to turn it + * into a absolute path + */ + if ((tmp = realloc(dev, + sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + + strlen(pool->def->source.adapter) + + 1 + + strlen(relLink) + + 1)) == NULL) { + virReportOOMError(conn); + free(dev); + return -1; + } + dev = tmp; + + strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); + strcat(dev, pool->def->source.adapter); + strcat(dev, "/"); + strcat(dev, relLink);
use virAsprintf here, too.
+ /* + * And finally canonicalize the absolute path to remove the + * copious .. components + */ + if (!realpath(dev, absLink)) {
Don't use realpath. e.g. mingw lacks it. Instead, use canonicalize_file_name, which should be portable thanks to gnulib's canonicalize-lgpl module -- which you can include by adding that to the list of module names in bootstrap.
+ char ebuf[1024]; + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot canonicalize link: %s"), + virStrerror(errno, ebuf, sizeof ebuf)); + free(dev); + return -1; + } + free(dev); + + if (strlen(absLink) >= buflen) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("link too long for buffer")); + return -1; + } + strcpy(buf, absLink); + + return 0; +} ... +/** + * virStorageBackendSCSIAddLUN: + * + * @conn: connection to report errors against + * @pool: pool to register volume with + * @ctx: HAL context + * @devname: HAL device name for LUN + * + * Given a HAL device supported 'block' and 'storage' capabilities + * register it as a volume in the pool + * + * Returns 0 on success, -1 on error + */ +static int +virStorageBackendSCSIAddLUN(virConnectPtr conn, + virStoragePoolObjPtr pool, + LibHalContext *ctx, + const char *devname) +{ + char **strdevs = NULL; + int numstrdevs = 0; + char *dev = NULL, *key = NULL; + unsigned long long size;
How about uint64_t instead, since it's the value returned by libhal_device_get_property_uint64?
+ int host, bus, target, lun; + int n = -1, i; + int ret = -1; + char name[100]; ... + HAL_GET_PROPERTY(strdevs[0], "storage.serial", &derr, string, key); + HAL_GET_PROPERTY(strdevs[0], "storage.size", &derr, uint64, size); ...
On Fri, Feb 20, 2009 at 05:14:55PM -0500, David Allan wrote:
Last March, DanPB posted code to create pools from SCSI HBAs. This patch is simply bringing that code into the current tree.
* src/storage_backend_scsi.[ch] contain the pool implementataion * There are small changes to several other source files to integrate the new pool type.
@@ -876,6 +879,13 @@ if test "$with_storage_iscsi" = "yes" -o "$with_storage_iscsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_ISCSI], [test "$with_storage_iscsi" = "yes"])
+if test "$with_storage_scsi" = "check"; then + with_storage_scsi=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_SCSI], 1, + [whether SCSI backend for storage driver is enabled]) + AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) +fi
This isn't quite right, because the code depends on HAL but we're not checking for it here. There are other checks in configure.ac that look for HAL, but they are not neccessarily enabled based on the same flags. But see my comments later about whether we should just avoid HAL....
diff --git a/src/Makefile.am b/src/Makefile.am index 3a798d2..f1dd789 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,6 +9,7 @@ INCLUDES = \ $(XEN_CFLAGS) \ $(SELINUX_CFLAGS) \ $(DRIVER_MODULE_CFLAGS) \ + $(POLKIT_CFLAGS) \
This isn't related to HAL or SCSI drivers.
+/** + * virStorageBackendSCSIRefreshPool: + * + * Query HAL for all storage devices associated with a specific + * SCSI HBA. + * + * XXX, currently we only support regular devices in /dev/, + * or /dev/disk/by-XXX. In future we also need to be able to + * map to multipath devices setup under /dev/mpath/XXXX. + */ +static int +virStorageBackendSCSIRefreshPool(virConnectPtr conn, + virStoragePoolObjPtr pool) +{
In this method we basically have a HBA number, and ask HAL for all devices which are children of this. We still have a whole bunch of code in the virStorageBackendSCSIAddLUN() which is called per device we find, that pokes around in sysfs to get out more metadata. We also have similar, but different code in the iSCSI code that pokes around in sysfs. Further more HAL is deprecated for dealing with storage devices in Fedora 10 and later, with DeviceKit targetted to take over its role. It is a bit of a mess really. I'm inclined to say thta we should not use HAL for this SCSI stuff at all, and instead we should slightly generalize our existing iSCSI method virStorageBackendISCSIFindLUNs() so that it works for both the SCSI and iSCSI storage pools, letting us share the code.
+ char hostdevice[PATH_MAX]; + LibHalContext *ctx = NULL; + DBusConnection *sysbus = NULL; + DBusError derr = DBUS_ERROR_INIT; + char **hbadevs = NULL, **blkdevs = NULL; + int numhbadevs = 0, numblkdevs = 0; + int i, ret = -1; + + /* Get the canonical sysfs path for the HBA device */ + if (virStorageBackendSCSIMakeHostDevice(conn, pool, + hostdevice, sizeof(hostdevice)) < 0) + goto cleanup; + + if ((ctx = libhal_ctx_new()) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to allocate HAL context")); + goto cleanup; + } + + sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr); + if (sysbus == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to get dbus system bus: '%s'"), + derr.message); + goto cleanup; + } + + if (!libhal_ctx_set_dbus_connection(ctx, sysbus)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to set HAL dbus connection")); + goto cleanup; + } + + + if (!libhal_ctx_init(ctx, &derr)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to initialize HAL context: '%s'"), + derr.message); + goto cleanup; + } + + if ((hbadevs = libhal_manager_find_device_string_match(ctx, + "linux.sysfs_path", + hostdevice, + &numhbadevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("HAL failed to lookup device '%s': %s"), + hostdevice, derr.message); + goto cleanup; + } + + if (numhbadevs != 1) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected number of HBA devices from HAL " + "(expected 1, got %d) when looking up device " + "'%s'"), numhbadevs, hostdevice); + goto cleanup; + } + + if ((blkdevs = libhal_manager_find_device_string_match(ctx, + "info.parent", + hbadevs[0], + &numblkdevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to lookup LUNs for %s with HAL: %s"), + hbadevs[0], derr.message); + goto cleanup; + } + + for (i = 0 ; i < numblkdevs ; i++) + virStorageBackendSCSIAddLUN(conn, + pool, + ctx, + blkdevs[i]); + + ret = 0; + +cleanup: + for (i = 0 ; hbadevs && i < numhbadevs ; i++) + free(hbadevs[i]); + free(hbadevs); + for (i = 0 ; blkdevs && i < numblkdevs ; i++) + free(blkdevs[i]); + free(blkdevs); + libhal_ctx_shutdown(ctx, NULL); + libhal_ctx_free(ctx); + if (sysbus) + dbus_connection_unref(sysbus); + if (dbus_error_is_set(&derr)) + dbus_error_free(&derr); + return ret; +} + + + +virStorageBackend virStorageBackendSCSI = { + .type = VIR_STORAGE_POOL_SCSI, + .refreshPool = virStorageBackendSCSIRefreshPool, +}; + +/* + * vim: set tabstop=4: + * vim: set shiftwidth=4: + * vim: set expandtab: + */ +/* + * Local variables: + * indent-tabs-mode: nil + * c-indent-level: 4 + * c-basic-offset: 4 + * tab-width: 4 + * End: + */
We've since decided against adding these editor footers. The virStorageBackendISCSIFindLUNs() method currently takes an iSCSI session name and uses that to find the host:bus:target prefix for the virtual iSCSI HBA, then scans for LUNS with that prefix. If we split this into 2 methods, the first virStorageBackendISCSIFindLUNsForSession() Just does the session -> host:bus:target lookup, then calling virStorageBackendSCSIFindLUNs(int host, int bus, int target) which does the scan matching /sys/bus/scsi/host:bus:target:lun to find the LUNs. Then, this new SCSI pool would merely need a short piece of code to call virStorageBackendSCSIFindLUNs() and avoid all this HAL stuff. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Fri, Feb 20, 2009 at 05:14:55PM -0500, David Allan wrote:
Last March, DanPB posted code to create pools from SCSI HBAs. This patch is simply bringing that code into the current tree.
* src/storage_backend_scsi.[ch] contain the pool implementataion * There are small changes to several other source files to integrate the new pool type.
@@ -876,6 +879,13 @@ if test "$with_storage_iscsi" = "yes" -o "$with_storage_iscsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_ISCSI], [test "$with_storage_iscsi" = "yes"])
+if test "$with_storage_scsi" = "check"; then + with_storage_scsi=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_SCSI], 1, + [whether SCSI backend for storage driver is enabled]) + AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) +fi
This isn't quite right, because the code depends on HAL but we're not checking for it here.
There are other checks in configure.ac that look for HAL, but they are not neccessarily enabled based on the same flags. But see my comments later about whether we should just avoid HAL....
Per your later comments, let's just avoid HAL.
diff --git a/src/Makefile.am b/src/Makefile.am index 3a798d2..f1dd789 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,6 +9,7 @@ INCLUDES = \ $(XEN_CFLAGS) \ $(SELINUX_CFLAGS) \ $(DRIVER_MODULE_CFLAGS) \ + $(POLKIT_CFLAGS) \
This isn't related to HAL or SCSI drivers.
+/** + * virStorageBackendSCSIRefreshPool: + * + * Query HAL for all storage devices associated with a specific + * SCSI HBA. + * + * XXX, currently we only support regular devices in /dev/, + * or /dev/disk/by-XXX. In future we also need to be able to + * map to multipath devices setup under /dev/mpath/XXXX. + */ +static int +virStorageBackendSCSIRefreshPool(virConnectPtr conn, + virStoragePoolObjPtr pool) +{
In this method we basically have a HBA number, and ask HAL for all devices which are children of this. We still have a whole bunch of code in the virStorageBackendSCSIAddLUN() which is called per device we find, that pokes around in sysfs to get out more metadata. We also have similar, but different code in the iSCSI code that pokes around in sysfs.
Further more HAL is deprecated for dealing with storage devices in Fedora 10 and later, with DeviceKit targetted to take over its role. It is a bit of a mess really. I'm inclined to say thta we should not use HAL for this SCSI stuff at all, and instead we should slightly generalize our existing iSCSI method virStorageBackendISCSIFindLUNs() so that it works for both the SCSI and iSCSI storage pools, letting us share the code.
That sounds good to me. The important thing to me is that we consolidate, and if HAL is not the way to go, then I'm fine with generalizing the existing iSCSI method. I'll break it out so that a) we can change it easily in the future and b) non-Linux platforms can implement something to make it work without having to modify any other code. [snipped a bunch of patch]
+/* + * vim: set tabstop=4: + * vim: set shiftwidth=4: + * vim: set expandtab: + */ +/* + * Local variables: + * indent-tabs-mode: nil + * c-indent-level: 4 + * c-basic-offset: 4 + * tab-width: 4 + * End: + */
We've since decided against adding these editor footers.
I'll remove them.
The virStorageBackendISCSIFindLUNs() method currently takes an iSCSI session name and uses that to find the host:bus:target prefix for the virtual iSCSI HBA, then scans for LUNS with that prefix.
If we split this into 2 methods, the first
virStorageBackendISCSIFindLUNsForSession()
Just does the session -> host:bus:target lookup, then calling
virStorageBackendSCSIFindLUNs(int host, int bus, int target)
which does the scan matching /sys/bus/scsi/host:bus:target:lun to find the LUNs. Then, this new SCSI pool would merely need a short piece of code to call virStorageBackendSCSIFindLUNs() and avoid all this HAL stuff.
I'll put that together and send a revised patch that incorporates your comments as well as Jim's. Dave
These three patches contain a rework of the SCSI host storage pool patch. The rework extracts the LUN discovery code from the iSCSI backend and generalizes it be be usable by both the SCSI and iSCSI backends. The first patch is the one I submitted last week that contains a port of Dan's HAL code into the current tree. The second is the rework, and the third is the removal of the HAL dependencies from the build system. In addition to whatever feedback people have, I'd like help regression testing iSCSI pools. I have tested on my machine, but it should have a little additional run time before commit. One change that results from the new way of discovering logical units is that LUs on iSCSI hosts will not be discovered by the SCSI host pool code--i.e., they have to be discovered as iSCSI sessions. I'm not terribly happy about that--it's a result of the way the entries in sysfs appear, so I'm going to think about that some more, but I wanted to get the first cut out to people for feedback today. Specific changes are as follows: Moved the following functions from storage_backend_iscsi.c to storage_backend_scsi.c and modified them to work for both iSCSI and SCSI host adapter backends: virStorageBackendISCSINewLun() -> virStorageBackendSCSINewLun() virStorageBackendISCSIFindLUNs -> virStorageBackendSCSIFindLUNs() The static functions notdotdir() and directAccessDevice() were also moved unmodified to storage_backend_scsi.c. Created the following function in storage_backend_scsi.c: virStorageBackendSCSIFindTargets() Removed the following functions from storage_backend_scsi.c: virStorageBackendSCSIMakeHostDevice() virStorageBackendSCSIAddLUN() virStorageBackendSCSICreateVol() Reworked the following functions in storage_backend_scsi.c and storage_backend_iscsi.c to use the common code: virStorageBackendISCSIFindLUNs() virStorageBackendSCSIRefreshPool()
On Mon, Mar 02, 2009 at 02:41:10PM -0500, David Allan wrote:
These three patches contain a rework of the SCSI host storage pool patch. The rework extracts the LUN discovery code from the iSCSI backend and generalizes it be be usable by both the SCSI and iSCSI backends. The first patch is the one I submitted last week that contains a port of Dan's HAL code into the current tree. The second is the rework, and the third is the removal of the HAL dependencies from the build system.
In addition to whatever feedback people have, I'd like help regression testing iSCSI pools. I have tested on my machine, but it should have a little additional run time before commit. One change that results from the new way of discovering logical units is that LUs on iSCSI hosts will not be discovered by the SCSI host pool code--i.e., they have to be discovered as iSCSI sessions. I'm not terribly happy about that--it's a result of the way the entries in sysfs appear, so I'm going to think about that some more, but I wanted to get the first cut out to people for feedback today.
I don't think its a huge problem that you can't scan iSCSI LUs via the SCSI pool. The only case that'd be needed is if you had setup the iSCSI connections outside the context of libvirt, but even then our iSCSI pool impl is happy to enumerate an existing logged in iSCSI target.
From a testing POV the most important thing is to try it out on a variety of kernel versions - eg RHEL-5 2.6.18, and Fedora 9/10/11 since sysfs has been known to change over time.
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
From a testing POV the most important thing is to try it out on a variety of kernel versions - eg RHEL-5 2.6.18, and Fedora 9/10/11 since sysfs has been known to change over time.
Yes, this is a very good point. There was definitely a sysfs breakage somewhere in the 2.6.21 or .22 era that caused problems for iscsid and for libvirtd (at the time), so it's a good idea to test both before and after that breakage. -- Chris Lalancette
Last March, DanPB posted code to create pools from SCSI HBAs. This patch is simply bringing that code into the current tree. * src/storage_backend_scsi.[ch] contain the pool implementataion * There are small changes to several other source files to integrate the new pool type. --- configure.in | 13 ++- po/POTFILES.in | 1 + src/Makefile.am | 9 + src/storage_backend.c | 6 + src/storage_backend_scsi.c | 471 ++++++++++++++++++++++++++++++++++++++++++++ src/storage_backend_scsi.h | 45 +++++ src/storage_conf.c | 18 ++ 7 files changed, 562 insertions(+), 1 deletions(-) create mode 100644 src/storage_backend_scsi.c create mode 100644 src/storage_backend_scsi.h diff --git a/configure.in b/configure.in index d2e8252..b88a7d5 100644 --- a/configure.in +++ b/configure.in @@ -739,6 +739,8 @@ AC_ARG_WITH([storage-lvm], [ --with-storage-lvm with LVM backend for the storage driver (on)],[],[with_storage_lvm=check]) AC_ARG_WITH([storage-iscsi], [ --with-storage-iscsi with iSCSI backend for the storage driver (on)],[],[with_storage_iscsi=check]) +AC_ARG_WITH([storage-scsi], +[ --with-storage-scsi with SCSI backend for the storage driver (on)],[],[with_storage_scsi=check]) AC_ARG_WITH([storage-disk], [ --with-storage-disk with GPartd Disk backend for the storage driver (on)],[],[with_storage_disk=check]) @@ -748,6 +750,7 @@ if test "$with_libvirtd" = "no"; then with_storage_fs=no with_storage_lvm=no with_storage_iscsi=no + with_storage_scsi=no with_storage_disk=no fi if test "$with_storage_dir" = "yes" ; then @@ -876,6 +879,13 @@ if test "$with_storage_iscsi" = "yes" -o "$with_storage_iscsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_ISCSI], [test "$with_storage_iscsi" = "yes"]) +if test "$with_storage_scsi" = "check"; then + with_storage_scsi=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_SCSI], 1, + [whether SCSI backend for storage driver is enabled]) + AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) +fi LIBPARTED_CFLAGS= @@ -1158,7 +1168,7 @@ LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.} AC_SUBST([LV_LIBTOOL_OBJDIR]) dnl HAL or DeviceKit library for host device enumeration -HAL_REQUIRED=0.0 +HAL_REQUIRED=0.5.0 HAL_CFLAGS= HAL_LIBS= AC_ARG_WITH([hal], @@ -1312,6 +1322,7 @@ AC_MSG_NOTICE([ FS: $with_storage_fs]) AC_MSG_NOTICE([ NetFS: $with_storage_fs]) AC_MSG_NOTICE([ LVM: $with_storage_lvm]) AC_MSG_NOTICE([ iSCSI: $with_storage_iscsi]) +AC_MSG_NOTICE([ SCSI: $with_storage_scsi]) AC_MSG_NOTICE([ Disk: $with_storage_disk]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Driver Loadable Modules]) diff --git a/po/POTFILES.in b/po/POTFILES.in index 7461f93..4433183 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -27,6 +27,7 @@ src/storage_backend_disk.c src/storage_backend_fs.c src/storage_backend_iscsi.c src/storage_backend_logical.c +src/storage_backend_scsi.c src/storage_conf.c src/storage_driver.c src/test.c diff --git a/src/Makefile.am b/src/Makefile.am index 3a798d2..f1dd789 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,6 +9,7 @@ INCLUDES = \ $(XEN_CFLAGS) \ $(SELINUX_CFLAGS) \ $(DRIVER_MODULE_CFLAGS) \ + $(POLKIT_CFLAGS) \ -DLIBDIR=\""$(libdir)"\" \ -DBINDIR=\""$(libexecdir)"\" \ -DSBINDIR=\""$(sbindir)"\" \ @@ -158,6 +159,9 @@ STORAGE_DRIVER_LVM_SOURCES = \ STORAGE_DRIVER_ISCSI_SOURCES = \ storage_backend_iscsi.h storage_backend_iscsi.c +STORAGE_DRIVER_SCSI_SOURCES = \ + storage_backend_scsi.h storage_backend_scsi.c + STORAGE_DRIVER_DISK_SOURCES = \ storage_backend_disk.h storage_backend_disk.c @@ -344,6 +348,10 @@ if WITH_STORAGE_ISCSI libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_ISCSI_SOURCES) endif +if WITH_STORAGE_SCSI +libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_SCSI_SOURCES) +endif + if WITH_STORAGE_DISK libvirt_driver_storage_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES) endif @@ -392,6 +400,7 @@ EXTRA_DIST += \ $(STORAGE_DRIVER_FS_SOURCES) \ $(STORAGE_DRIVER_LVM_SOURCES) \ $(STORAGE_DRIVER_ISCSI_SOURCES) \ + $(STORAGE_DRIVER_SCSI_SOURCES) \ $(STORAGE_DRIVER_DISK_SOURCES) \ $(NODE_DEVICE_DRIVER_SOURCES) \ $(NODE_DEVICE_DRIVER_HAL_SOURCES) \ diff --git a/src/storage_backend.c b/src/storage_backend.c index 8408f34..95173fa 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -54,6 +54,9 @@ #if WITH_STORAGE_ISCSI #include "storage_backend_iscsi.h" #endif +#if WITH_STORAGE_SCSI +#include "storage_backend_scsi.h" +#endif #if WITH_STORAGE_DISK #include "storage_backend_disk.h" #endif @@ -78,6 +81,9 @@ static virStorageBackendPtr backends[] = { #if WITH_STORAGE_ISCSI &virStorageBackendISCSI, #endif +#if WITH_STORAGE_SCSI + &virStorageBackendSCSI, +#endif #if WITH_STORAGE_DISK &virStorageBackendDisk, #endif diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c new file mode 100644 index 0000000..236d894 --- /dev/null +++ b/src/storage_backend_scsi.c @@ -0,0 +1,471 @@ +/* + * storage_backend_scsi.c: storage backend for SCSI handling + * + * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange redhat com> + */ + +#include <config.h> + +#include <c-ctype.h> +#include <unistd.h> +#include <stdio.h> +#include <hal/libhal.h> + +#include "virterror_internal.h" +#include "storage_backend_scsi.h" +#include "memory.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +#define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host/" +#define LINUX_SYSFS_SCSI_HOST_POSTFIX "/device" + + +/** + * virStorageBackendSCSIMakeHostDevice: + * @conn: report errors agains + * @pool: pool to resolve + * @buf: pre-allocated buffer to fill with path name + * @buflen: size of buffer + * + * Resolve a HBA name to HBA device path + * + * Given a pool object configured for a SCSI HBA, resolve the HBA name + * into the canonical sysfs path for the HBA device. This can then be + * used to lookup the device in HAL. + * + * Returns 0 on success, -1 on failure + */ +static int virStorageBackendSCSIMakeHostDevice(virConnectPtr conn, + virStoragePoolObjPtr pool, + char *buf, + size_t buflen) +{ + char *dev = NULL; + char *tmp = pool->def->source.adapter; + char relLink[PATH_MAX], absLink[PATH_MAX]; + ssize_t len; + + if (!tmp) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Missing adapter name for pool")); + return -1; + } + + /* Sanity check host adapter name - should only be 0-9, a-z. + * Anything else is bogus & so we reject it, to prevent them + * making us read arbitrary paths on host + */ + while (*tmp) { + if (!c_isalnum(*tmp)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Invalid character in pool adapter name '%s'"), + pool->def->source.adapter); + return -1; + } + tmp++; + } + + /* + * First get the class based path eg + * + * /sys/class/scsi_host/host1/device + */ + if ((dev = malloc(sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + + strlen(pool->def->source.adapter) + + sizeof(LINUX_SYSFS_SCSI_HOST_POSTFIX) + + 1)) == NULL) { + virReportOOMError(conn); + return -1; + } + + strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); + strcat(dev, pool->def->source.adapter); + strcat(dev, LINUX_SYSFS_SCSI_HOST_POSTFIX); + + /* + * Now resolve the class based path symlink to the real + * device path, which is likely under PCI bus hierarchy + * and is the path tracked by HAL + */ + /* Readlink does not null terminate, so we reserve one byte */ + if ((len = readlink(dev, relLink, sizeof(relLink)-1)) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot find SCSI host adapter '%s' at '%s'"), + pool->def->source.adapter, dev); + free(dev); + return -1; + } + relLink[len] = '\0'; + + + /* + * The symlink is relative, so now we have to turn it + * into a absolute path + */ + if ((tmp = realloc(dev, + sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + + strlen(pool->def->source.adapter) + + 1 + + strlen(relLink) + + 1)) == NULL) { + virReportOOMError(conn); + free(dev); + return -1; + } + dev = tmp; + + strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); + strcat(dev, pool->def->source.adapter); + strcat(dev, "/"); + strcat(dev, relLink); + + /* + * And finally canonicalize the absolute path to remove the + * copious .. components + */ + if (!realpath(dev, absLink)) { + char ebuf[1024]; + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("cannot canonicalize link: %s"), + virStrerror(errno, ebuf, sizeof ebuf)); + free(dev); + return -1; + } + free(dev); + + if (strlen(absLink) >= buflen) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("link too long for buffer")); + return -1; + } + strcpy(buf, absLink); + + return 0; +} + + +/** + * virStorageBackendSCSICreateVol + * + * Allocate a virStorageVolDef object for the specified + * metadata and hook it into the pool + * + * Returns 0 on success, -1 on failure + */ +static int +virStorageBackendSCSICreateVol(virConnectPtr conn, + virStoragePoolObjPtr pool, + const char *name, + const char *path, + const char *key, + unsigned long long size) +{ + virStorageVolDefPtr vol; + char *tmppath; + + if (VIR_ALLOC(vol) < 0) { + virReportOOMError(conn); + goto cleanup; + } + + vol->type = VIR_STORAGE_VOL_BLOCK; + + tmppath = strdup(path); + vol->name = strdup(name); + vol->key = strdup(key); + vol->allocation = vol->capacity = size; + + if ((vol->name == NULL) || + (vol->key == NULL) || + (tmppath == NULL)) { + virReportOOMError(conn); + goto cleanup; + } + + /* Now figure out the stable path + * + * XXX this method is O(N) because it scans the pool target + * dir every time its run. Should figure out a more efficient + * way of doing this... + */ + if ((vol->target.path = virStorageBackendStablePath(conn, + pool, + tmppath)) == NULL) + goto cleanup; + + if (tmppath != vol->target.path) + free(tmppath); + tmppath = NULL; + + if (virStorageBackendUpdateVolInfo(conn, vol, 1) < 0) + goto cleanup; + + pool->def->capacity += vol->capacity; + pool->def->allocation += vol->allocation; + + if (VIR_REALLOC_N(pool->volumes.objs, + pool->volumes.count+1) < 0) { + virReportOOMError(conn); + virStorageVolDefFree(vol); + goto cleanup; + } + pool->volumes.objs[pool->volumes.count++] = vol; + + return 0; + +cleanup: + free(tmppath); + virStorageVolDefFree(vol); + return -1; +} + + +/** + * virStorageBackendSCSIAddLUN: + * + * @conn: connection to report errors against + * @pool: pool to register volume with + * @ctx: HAL context + * @devname: HAL device name for LUN + * + * Given a HAL device supported 'block' and 'storage' capabilities + * register it as a volume in the pool + * + * Returns 0 on success, -1 on error + */ +static int +virStorageBackendSCSIAddLUN(virConnectPtr conn, + virStoragePoolObjPtr pool, + LibHalContext *ctx, + const char *devname) +{ + char **strdevs = NULL; + int numstrdevs = 0; + char *dev = NULL, *key = NULL; + unsigned long long size; + int host, bus, target, lun; + int n = -1, i; + int ret = -1; + char name[100]; + DBusError derr = DBUS_ERROR_INIT; + + if ((strdevs = libhal_manager_find_device_string_match(ctx, + "info.parent", + devname, + &numstrdevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("HAL failed to lookup LUNs for '%s': %s"), + devname, derr.message); + goto cleanup; + } + for (i = 0 ; i < numstrdevs && n == -1 ; i++) { + char *cat = libhal_device_get_property_string(ctx, + strdevs[0], + "info.category", + NULL); + /* XXX derr */ + if (cat != NULL) { + if (STREQ(cat, "storage")) + n = i; + free(cat); + } + } + /* No storage vol for device - probably a removable vol so ignore */ + if (n == -1) { + ret = 0; + goto cleanup; + } + +#define HAL_GET_PROPERTY(path, name, err, type, value) \ + (value) = libhal_device_get_property_ ## type(ctx, (path), (name), (err)); \ + if (dbus_error_is_set((err))) { \ + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, \ + "unable to lookup '%s' property on %s: %s", \ + (name), (path), derr.message); \ + goto cleanup; \ + } + + /* These are props of the physical device */ + HAL_GET_PROPERTY(devname, "scsi.host", &derr, int, host); + HAL_GET_PROPERTY(devname, "scsi.bus", &derr, int, bus); + HAL_GET_PROPERTY(devname, "scsi.target", &derr, int, target); + HAL_GET_PROPERTY(devname, "scsi.lun", &derr, int, lun); + + /* These are props of the logic device */ + HAL_GET_PROPERTY(strdevs[0], "block.device", &derr, string, dev); + /* + * XXX storage.serial is not actually unique if they have + * multipath on the fibre channel adapter + */ + HAL_GET_PROPERTY(strdevs[0], "storage.serial", &derr, string, key); + HAL_GET_PROPERTY(strdevs[0], "storage.size", &derr, uint64, size); + +#undef HAL_GET_PROPERTY + + snprintf(name, sizeof(name), "%d:%d:%d:%d", host, bus, target, lun); + name[sizeof(name)-1] = '\0'; + + if (virStorageBackendSCSICreateVol(conn, pool, name, dev, key, size) < 0) + goto cleanup; + ret = 0; + +cleanup: + for (i = 0 ; strdevs && i < numstrdevs ; i++) + free(strdevs[i]); + free(strdevs); + free(dev); + free(key); + if (dbus_error_is_set(&derr)) + dbus_error_free(&derr); + return ret; +} + + +/** + * virStorageBackendSCSIRefreshPool: + * + * Query HAL for all storage devices associated with a specific + * SCSI HBA. + * + * XXX, currently we only support regular devices in /dev/, + * or /dev/disk/by-XXX. In future we also need to be able to + * map to multipath devices setup under /dev/mpath/XXXX. + */ +static int +virStorageBackendSCSIRefreshPool(virConnectPtr conn, + virStoragePoolObjPtr pool) +{ + char hostdevice[PATH_MAX]; + LibHalContext *ctx = NULL; + DBusConnection *sysbus = NULL; + DBusError derr = DBUS_ERROR_INIT; + char **hbadevs = NULL, **blkdevs = NULL; + int numhbadevs = 0, numblkdevs = 0; + int i, ret = -1; + + /* Get the canonical sysfs path for the HBA device */ + if (virStorageBackendSCSIMakeHostDevice(conn, pool, + hostdevice, sizeof(hostdevice)) < 0) + goto cleanup; + + if ((ctx = libhal_ctx_new()) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to allocate HAL context")); + goto cleanup; + } + + sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr); + if (sysbus == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to get dbus system bus: '%s'"), + derr.message); + goto cleanup; + } + + if (!libhal_ctx_set_dbus_connection(ctx, sysbus)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to set HAL dbus connection")); + goto cleanup; + } + + + if (!libhal_ctx_init(ctx, &derr)) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to initialize HAL context: '%s'"), + derr.message); + goto cleanup; + } + + if ((hbadevs = libhal_manager_find_device_string_match(ctx, + "linux.sysfs_path", + hostdevice, + &numhbadevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("HAL failed to lookup device '%s': %s"), + hostdevice, derr.message); + goto cleanup; + } + + if (numhbadevs != 1) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected number of HBA devices from HAL " + "(expected 1, got %d) when looking up device " + "'%s'"), numhbadevs, hostdevice); + goto cleanup; + } + + if ((blkdevs = libhal_manager_find_device_string_match(ctx, + "info.parent", + hbadevs[0], + &numblkdevs, + &derr)) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("failed to lookup LUNs for %s with HAL: %s"), + hbadevs[0], derr.message); + goto cleanup; + } + + for (i = 0 ; i < numblkdevs ; i++) + virStorageBackendSCSIAddLUN(conn, + pool, + ctx, + blkdevs[i]); + + ret = 0; + +cleanup: + for (i = 0 ; hbadevs && i < numhbadevs ; i++) + free(hbadevs[i]); + free(hbadevs); + for (i = 0 ; blkdevs && i < numblkdevs ; i++) + free(blkdevs[i]); + free(blkdevs); + libhal_ctx_shutdown(ctx, NULL); + libhal_ctx_free(ctx); + if (sysbus) + dbus_connection_unref(sysbus); + if (dbus_error_is_set(&derr)) + dbus_error_free(&derr); + return ret; +} + + + +virStorageBackend virStorageBackendSCSI = { + .type = VIR_STORAGE_POOL_SCSI, + .refreshPool = virStorageBackendSCSIRefreshPool, +}; + +/* + * vim: set tabstop=4: + * vim: set shiftwidth=4: + * vim: set expandtab: + */ +/* + * Local variables: + * indent-tabs-mode: nil + * c-indent-level: 4 + * c-basic-offset: 4 + * tab-width: 4 + * End: + */ diff --git a/src/storage_backend_scsi.h b/src/storage_backend_scsi.h new file mode 100644 index 0000000..c912269 --- /dev/null +++ b/src/storage_backend_scsi.h @@ -0,0 +1,45 @@ +/* + * storage_backend_scsi.h: storage backend for SCSI handling + * + * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange redhat com> + */ + +#ifndef __VIR_STORAGE_BACKEND_SCSI_H__ +#define __VIR_STORAGE_BACKEND_SCSI_H__ + +#include "storage_backend.h" + +extern virStorageBackend virStorageBackendSCSI; + +#endif /* __VIR_STORAGE_BACKEND_SCSI_H__ */ + +/* + * vim: set tabstop=4: + * vim: set shiftwidth=4: + * vim: set expandtab: + */ +/* + * Local variables: + * indent-tabs-mode: nil + * c-indent-level: 4 + * c-basic-offset: 4 + * tab-width: 4 + * End: + */ diff --git a/src/storage_conf.c b/src/storage_conf.c index 70107a2..2ea6f8e 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -187,6 +187,14 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatDiskTypeToString, } }, + { .poolType = VIR_STORAGE_POOL_SCSI, + .poolOptions = { + .flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER), + }, + .volOptions = { + .formatToString = virStoragePoolFormatDiskTypeToString, + } + }, { .poolType = VIR_STORAGE_POOL_DISK, .poolOptions = { .flags = (VIR_STORAGE_POOL_SOURCE_DEVICE), @@ -269,6 +277,7 @@ virStoragePoolSourceFree(virStoragePoolSourcePtr source) { VIR_FREE(source->devices); VIR_FREE(source->dir); VIR_FREE(source->name); + VIR_FREE(source->adapter); if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) { VIR_FREE(source->auth.chap.login); @@ -550,6 +559,15 @@ virStoragePoolDefParseDoc(virConnectPtr conn, } } + if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) { + if ((ret->source.adapter = virXPathString(conn, + "string(/pool/source/adapter/@name)", + ctxt)) == NULL) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + "%s", _("missing storage pool source adapter name")); + goto cleanup; + } + } authType = virXPathString(conn, "string(/pool/source/auth/@type)", ctxt); if (authType == NULL) { -- 1.6.0.6
--- src/storage_backend_iscsi.c | 327 +--------------------- src/storage_backend_scsi.c | 649 ++++++++++++++++++++----------------------- src/storage_backend_scsi.h | 22 +- 3 files changed, 319 insertions(+), 679 deletions(-) diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c index d5b10e5..1b2dfd0 100644 --- a/src/storage_backend_iscsi.c +++ b/src/storage_backend_iscsi.c @@ -35,6 +35,7 @@ #include <dirent.h> #include "virterror_internal.h" +#include "storage_backend_scsi.h" #include "storage_backend_iscsi.h" #include "util.h" #include "memory.h" @@ -169,145 +170,6 @@ virStorageBackendISCSIConnection(virConnectPtr conn, return 0; } -static int -virStorageBackendISCSINewLun(virConnectPtr conn, virStoragePoolObjPtr pool, - unsigned int lun, const char *dev) -{ - virStorageVolDefPtr vol; - int fd = -1; - char *devpath = NULL; - int opentries = 0; - - if (VIR_ALLOC(vol) < 0) { - virReportOOMError(conn); - goto cleanup; - } - - vol->type = VIR_STORAGE_VOL_BLOCK; - - if (virAsprintf(&(vol->name), "lun-%d", lun) < 0) { - virReportOOMError(conn); - goto cleanup; - } - - if (virAsprintf(&devpath, "/dev/%s", dev) < 0) { - virReportOOMError(conn); - goto cleanup; - } - - /* It can take a little while between logging into the ISCSI - * server and udev creating the /dev nodes, so if we get ENOENT - * we must retry a few times - they should eventually appear. - * We currently wait for upto 5 seconds. Is this good enough ? - * Perhaps not on a very heavily loaded system Any other - * options... ? - */ - reopen: - if ((fd = open(devpath, O_RDONLY)) < 0) { - opentries++; - if (errno == ENOENT && opentries < 50) { - usleep(100 * 1000); - goto reopen; - } - virReportSystemError(conn, errno, - _("cannot open '%s'"), - devpath); - goto cleanup; - } - - /* Now figure out the stable path - * - * XXX this method is O(N) because it scans the pool target - * dir every time its run. Should figure out a more efficient - * way of doing this... - */ - if ((vol->target.path = virStorageBackendStablePath(conn, - pool, - devpath)) == NULL) - goto cleanup; - - VIR_FREE(devpath); - - if (virStorageBackendUpdateVolTargetInfoFD(conn, - &vol->target, - fd, - &vol->allocation, - &vol->capacity) < 0) - goto cleanup; - - /* XXX use unique iSCSI id instead */ - vol->key = strdup(vol->target.path); - if (vol->key == NULL) { - virReportOOMError(conn); - goto cleanup; - } - - - pool->def->capacity += vol->capacity; - pool->def->allocation += vol->allocation; - - if (VIR_REALLOC_N(pool->volumes.objs, - pool->volumes.count+1) < 0) { - virReportOOMError(conn); - goto cleanup; - } - pool->volumes.objs[pool->volumes.count++] = vol; - - close(fd); - - return 0; - - cleanup: - if (fd != -1) close(fd); - VIR_FREE(devpath); - virStorageVolDefFree(vol); - return -1; -} - -static int notdotdir(const struct dirent *dir) -{ - return !(STREQLEN(dir->d_name, ".", 1) || STREQLEN(dir->d_name, "..", 2)); -} - -/* Function to check if the type file in the given sysfs_path is a - * Direct-Access device (i.e. type 0). Return -1 on failure, 0 if not - * a Direct-Access device, and 1 if a Direct-Access device - */ -static int directAccessDevice(const char *sysfs_path) -{ - char typestr[3]; - char *gottype, *p; - FILE *typefile; - int type; - - typefile = fopen(sysfs_path, "r"); - if (typefile == NULL) { - /* there was no type file; that doesn't seem right */ - return -1; - } - gottype = fgets(typestr, 3, typefile); - fclose(typefile); - - if (gottype == NULL) { - /* we couldn't read the type file; have to give up */ - return -1; - } - - /* we don't actually care about p, but if you pass NULL and the last - * character is not \0, virStrToLong_i complains - */ - if (virStrToLong_i(typestr, &p, 10, &type) < 0) { - /* Hm, type wasn't an integer; seems strange */ - return -1; - } - - if (type != 0) { - /* saw a device other than Direct-Access */ - return 0; - } - - return 1; -} static int virStorageBackendISCSIFindLUNs(virConnectPtr conn, @@ -315,196 +177,17 @@ virStorageBackendISCSIFindLUNs(virConnectPtr conn, const char *session) { char sysfs_path[PATH_MAX]; - uint32_t host, bus, target, lun; - DIR *sysdir; - struct dirent *sys_dirent; - struct dirent **namelist; - int i, n, tries, retval, directaccess; - char *block, *scsidev, *block2; - - retval = 0; - block = NULL; - scsidev = NULL; + int retval = 0; snprintf(sysfs_path, PATH_MAX, "/sys/class/iscsi_session/session%s/device", session); - sysdir = opendir(sysfs_path); - if (sysdir == NULL) { + if (virStorageBackendSCSIFindTargets(conn, pool, sysfs_path) < 0) { virReportSystemError(conn, errno, - _("Failed to opendir sysfs path '%s'"), + _("Failed to get target list for path '%s'"), sysfs_path); - return -1; + retval = -1; } - while ((sys_dirent = readdir(sysdir))) { - /* double-negative, so we can use the same function for scandir below */ - if (!notdotdir(sys_dirent)) - continue; - - if (STREQLEN(sys_dirent->d_name, "target", 6)) { - if (sscanf(sys_dirent->d_name, "target%u:%u:%u", - &host, &bus, &target) != 3) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to parse target from sysfs path %s/%s"), - sysfs_path, sys_dirent->d_name); - closedir(sysdir); - return -1; - } - break; - } - } - closedir(sysdir); - - /* we now have the host, bus, and target; let's scan for LUNs */ - snprintf(sysfs_path, PATH_MAX, - "/sys/class/iscsi_session/session%s/device/target%u:%u:%u", - session, host, bus, target); - - n = scandir(sysfs_path, &namelist, notdotdir, versionsort); - if (n <= 0) { - /* we didn't find any reasonable entries; return failure */ - virReportSystemError(conn, errno, - _("Failed to find any LUNs for session '%s'"), - session); - return -1; - } - - for (i=0; i<n; i++) { - block = NULL; - scsidev = NULL; - - if (sscanf(namelist[i]->d_name, "%u:%u:%u:%u\n", - &host, &bus, &target, &lun) != 4) - continue; - - /* we found a LUN */ - /* Note, however, that just finding a LUN doesn't mean it is - * actually useful to us. There are a few different types of - * LUNs, enumerated in the linux kernel in - * drivers/scsi/scsi.c:scsi_device_types[]. Luckily, these device - * types form part of the ABI between the kernel and userland, so - * are unlikely to change. For now, we ignore everything that isn't - * type 0; that is, a Direct-Access device - */ - snprintf(sysfs_path, PATH_MAX, - "/sys/bus/scsi/devices/%u:%u:%u:%u/type", - host, bus, target, lun); - - directaccess = directAccessDevice(sysfs_path); - if (directaccess < 0) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to determine if %u:%u:%u:%u is a Direct-Access LUN"), - host, bus, target, lun); - retval = -1; - goto namelist_cleanup; - } - else if (directaccess == 0) { - /* not a direct-access device; skip */ - continue; - } - /* implicit else if (access == 1); Direct-Access device */ - - /* It might take some time for the - * /sys/bus/scsi/devices/host:bus:target:lun/block{:sda,/sda} - * link to show up; wait up to 5 seconds for it, then give up - */ - tries = 0; - while (block == NULL && tries < 50) { - snprintf(sysfs_path, PATH_MAX, "/sys/bus/scsi/devices/%u:%u:%u:%u", - host, bus, target, lun); - - sysdir = opendir(sysfs_path); - if (sysdir == NULL) { - virReportSystemError(conn, errno, - _("Failed to opendir sysfs path '%s'"), - sysfs_path); - retval = -1; - goto namelist_cleanup; - } - while ((sys_dirent = readdir(sysdir))) { - if (!notdotdir(sys_dirent)) - continue; - if (STREQLEN(sys_dirent->d_name, "block", 5)) { - block = strdup(sys_dirent->d_name); - break; - } - } - closedir(sysdir); - tries++; - if (block == NULL) - usleep(100 * 1000); - } - - if (block == NULL) { - /* we couldn't find the device link for this device; fail */ - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to find device link for lun %d"), - lun); - retval = -1; - goto namelist_cleanup; - } - - if (strlen(block) == 5) { - /* OK, this is exactly "block"; must be new-style */ - snprintf(sysfs_path, PATH_MAX, - "/sys/bus/scsi/devices/%u:%u:%u:%u/block", - host, bus, target, lun); - sysdir = opendir(sysfs_path); - if (sysdir == NULL) { - virReportSystemError(conn, errno, - _("Failed to opendir sysfs path '%s'"), - sysfs_path); - retval = -1; - goto namelist_cleanup; - } - while ((sys_dirent = readdir(sysdir))) { - if (!notdotdir(sys_dirent)) - continue; - - scsidev = strdup(sys_dirent->d_name); - break; - } - closedir(sysdir); - } - else { - /* old-style; just parse out the sd */ - block2 = strrchr(block, ':'); - if (block2 == NULL) { - /* Hm, wasn't what we were expecting; have to give up */ - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Failed to parse block path %s"), - block); - retval = -1; - goto namelist_cleanup; - } - block2++; - scsidev = strdup(block2); - } - if (scsidev == NULL) { - virReportOOMError(conn); - retval = -1; - goto namelist_cleanup; - } - - retval = virStorageBackendISCSINewLun(conn, pool, lun, scsidev); - if (retval < 0) - break; - VIR_FREE(scsidev); - VIR_FREE(block); - } - -namelist_cleanup: - /* we call these VIR_FREE here to make sure we don't leak memory on - * error cases; in the success case, these are already freed but NULL, - * which should be fine - */ - VIR_FREE(scsidev); - VIR_FREE(block); - - for (i=0; i<n; i++) - VIR_FREE(namelist[i]); - - VIR_FREE(namelist); return retval; } diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c index 236d894..3752bf2 100644 --- a/src/storage_backend_scsi.c +++ b/src/storage_backend_scsi.c @@ -23,10 +23,10 @@ #include <config.h> -#include <c-ctype.h> #include <unistd.h> #include <stdio.h> -#include <hal/libhal.h> +#include <dirent.h> +#include <fcntl.h> #include "virterror_internal.h" #include "storage_backend_scsi.h" @@ -34,152 +34,91 @@ #define VIR_FROM_THIS VIR_FROM_STORAGE -#define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host/" -#define LINUX_SYSFS_SCSI_HOST_POSTFIX "/device" - -/** - * virStorageBackendSCSIMakeHostDevice: - * @conn: report errors agains - * @pool: pool to resolve - * @buf: pre-allocated buffer to fill with path name - * @buflen: size of buffer - * - * Resolve a HBA name to HBA device path - * - * Given a pool object configured for a SCSI HBA, resolve the HBA name - * into the canonical sysfs path for the HBA device. This can then be - * used to lookup the device in HAL. - * - * Returns 0 on success, -1 on failure - */ -static int virStorageBackendSCSIMakeHostDevice(virConnectPtr conn, - virStoragePoolObjPtr pool, - char *buf, - size_t buflen) +static int +virStorageBackendSCSIRefreshPool(virConnectPtr conn, + virStoragePoolObjPtr pool) { - char *dev = NULL; - char *tmp = pool->def->source.adapter; - char relLink[PATH_MAX], absLink[PATH_MAX]; - ssize_t len; - - if (!tmp) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Missing adapter name for pool")); - return -1; - } + char sysfs_path[PATH_MAX]; + int ret = 0; - /* Sanity check host adapter name - should only be 0-9, a-z. - * Anything else is bogus & so we reject it, to prevent them - * making us read arbitrary paths on host - */ - while (*tmp) { - if (!c_isalnum(*tmp)) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid character in pool adapter name '%s'"), - pool->def->source.adapter); - return -1; - } - tmp++; - } + pool->def->allocation = pool->def->capacity = pool->def->available = 0; - /* - * First get the class based path eg - * - * /sys/class/scsi_host/host1/device - */ - if ((dev = malloc(sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + - strlen(pool->def->source.adapter) + - sizeof(LINUX_SYSFS_SCSI_HOST_POSTFIX) + - 1)) == NULL) { - virReportOOMError(conn); - return -1; - } + virStorageBackendWaitForDevices(conn); - strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); - strcat(dev, pool->def->source.adapter); - strcat(dev, LINUX_SYSFS_SCSI_HOST_POSTFIX); + snprintf(sysfs_path, PATH_MAX, "%s/%s/%s", + LINUX_SYSFS_SCSI_HOST_PREFIX, + pool->def->source.adapter, + LINUX_SYSFS_SCSI_HOST_POSTFIX); - /* - * Now resolve the class based path symlink to the real - * device path, which is likely under PCI bus hierarchy - * and is the path tracked by HAL - */ - /* Readlink does not null terminate, so we reserve one byte */ - if ((len = readlink(dev, relLink, sizeof(relLink)-1)) < 0) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot find SCSI host adapter '%s' at '%s'"), - pool->def->source.adapter, dev); - free(dev); - return -1; + if (virStorageBackendSCSIFindTargets(conn, pool, sysfs_path) < 0) { + virReportSystemError(conn, errno, + _("Failed to get target list for path '%s'"), + sysfs_path); + ret = -1; } - relLink[len] = '\0'; + + return ret; +} - /* - * The symlink is relative, so now we have to turn it - * into a absolute path - */ - if ((tmp = realloc(dev, - sizeof(LINUX_SYSFS_SCSI_HOST_PREFIX) + - strlen(pool->def->source.adapter) + - 1 + - strlen(relLink) + - 1)) == NULL) { - virReportOOMError(conn); - free(dev); +static int +notdotdir(const struct dirent *dir) +{ + return !(STREQLEN(dir->d_name, ".", 1) || STREQLEN(dir->d_name, "..", 2)); +} + + +/* Function to check if the type file in the given sysfs_path is a + * Direct-Access device (i.e. type 0). Return -1 on failure, 0 if not + * a Direct-Access device, and 1 if a Direct-Access device + */ +static int +directAccessDevice(const char *sysfs_path) +{ + char typestr[3]; + char *gottype, *p; + FILE *typefile; + int type; + + typefile = fopen(sysfs_path, "r"); + if (typefile == NULL) { + /* there was no type file; that doesn't seem right */ return -1; } - dev = tmp; + gottype = fgets(typestr, 3, typefile); + fclose(typefile); - strcpy(dev, LINUX_SYSFS_SCSI_HOST_PREFIX); - strcat(dev, pool->def->source.adapter); - strcat(dev, "/"); - strcat(dev, relLink); + if (gottype == NULL) { + /* we couldn't read the type file; have to give up */ + return -1; + } - /* - * And finally canonicalize the absolute path to remove the - * copious .. components + /* we don't actually care about p, but if you pass NULL and the last + * character is not \0, virStrToLong_i complains */ - if (!realpath(dev, absLink)) { - char ebuf[1024]; - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot canonicalize link: %s"), - virStrerror(errno, ebuf, sizeof ebuf)); - free(dev); + if (virStrToLong_i(typestr, &p, 10, &type) < 0) { + /* Hm, type wasn't an integer; seems strange */ return -1; } - free(dev); - if (strlen(absLink) >= buflen) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("link too long for buffer")); - return -1; + if (type != 0) { + /* saw a device other than Direct-Access */ + return 0; } - strcpy(buf, absLink); - return 0; + return 1; } -/** - * virStorageBackendSCSICreateVol - * - * Allocate a virStorageVolDef object for the specified - * metadata and hook it into the pool - * - * Returns 0 on success, -1 on failure - */ static int -virStorageBackendSCSICreateVol(virConnectPtr conn, - virStoragePoolObjPtr pool, - const char *name, - const char *path, - const char *key, - unsigned long long size) +virStorageBackendSCSINewLun(virConnectPtr conn, virStoragePoolObjPtr pool, + unsigned int lun, const char *dev) { virStorageVolDefPtr vol; - char *tmppath; + int fd = -1; + char *devpath = NULL; + int opentries = 0; if (VIR_ALLOC(vol) < 0) { virReportOOMError(conn); @@ -188,18 +127,36 @@ virStorageBackendSCSICreateVol(virConnectPtr conn, vol->type = VIR_STORAGE_VOL_BLOCK; - tmppath = strdup(path); - vol->name = strdup(name); - vol->key = strdup(key); - vol->allocation = vol->capacity = size; + if (virAsprintf(&(vol->name), "lun-%d", lun) < 0) { + virReportOOMError(conn); + goto cleanup; + } - if ((vol->name == NULL) || - (vol->key == NULL) || - (tmppath == NULL)) { + if (virAsprintf(&devpath, "/dev/%s", dev) < 0) { virReportOOMError(conn); goto cleanup; } + /* For SAN storage it can take a little while between getting + * access to the array and udev creating the /dev nodes, so if we + * get ENOENT we must retry a few times - they should eventually + * appear. We currently wait for upto 5 seconds. Is this good + * enough ? Perhaps not on a very heavily loaded system Any other + * options... ? + */ + reopen: + if ((fd = open(devpath, O_RDONLY)) < 0) { + opentries++; + if (errno == ENOENT && opentries < 50) { + usleep(100 * 1000); + goto reopen; + } + virReportSystemError(conn, errno, + _("cannot open '%s'"), + devpath); + goto cleanup; + } + /* Now figure out the stable path * * XXX this method is O(N) because it scans the pool target @@ -208,15 +165,25 @@ virStorageBackendSCSICreateVol(virConnectPtr conn, */ if ((vol->target.path = virStorageBackendStablePath(conn, pool, - tmppath)) == NULL) + devpath)) == NULL) goto cleanup; - if (tmppath != vol->target.path) - free(tmppath); - tmppath = NULL; + VIR_FREE(devpath); + + if (virStorageBackendUpdateVolTargetInfoFD(conn, + &vol->target, + fd, + &vol->allocation, + &vol->capacity) < 0) + goto cleanup; - if (virStorageBackendUpdateVolInfo(conn, vol, 1) < 0) + /* XXX use unique iSCSI id instead */ + vol->key = strdup(vol->target.path); + if (vol->key == NULL) { + virReportOOMError(conn); goto cleanup; + } + pool->def->capacity += vol->capacity; pool->def->allocation += vol->allocation; @@ -224,248 +191,244 @@ virStorageBackendSCSICreateVol(virConnectPtr conn, if (VIR_REALLOC_N(pool->volumes.objs, pool->volumes.count+1) < 0) { virReportOOMError(conn); - virStorageVolDefFree(vol); goto cleanup; } pool->volumes.objs[pool->volumes.count++] = vol; + close(fd); + return 0; -cleanup: - free(tmppath); + cleanup: + if (fd != -1) close(fd); + VIR_FREE(devpath); virStorageVolDefFree(vol); return -1; } -/** - * virStorageBackendSCSIAddLUN: - * - * @conn: connection to report errors against - * @pool: pool to register volume with - * @ctx: HAL context - * @devname: HAL device name for LUN - * - * Given a HAL device supported 'block' and 'storage' capabilities - * register it as a volume in the pool - * - * Returns 0 on success, -1 on error - */ static int -virStorageBackendSCSIAddLUN(virConnectPtr conn, - virStoragePoolObjPtr pool, - LibHalContext *ctx, - const char *devname) +virStorageBackendSCSIFindLUNs(virConnectPtr conn, + virStoragePoolObjPtr pool, + uint32_t host, + uint32_t bus, + uint32_t target) { - char **strdevs = NULL; - int numstrdevs = 0; - char *dev = NULL, *key = NULL; - unsigned long long size; - int host, bus, target, lun; - int n = -1, i; - int ret = -1; - char name[100]; - DBusError derr = DBUS_ERROR_INIT; - - if ((strdevs = libhal_manager_find_device_string_match(ctx, - "info.parent", - devname, - &numstrdevs, - &derr)) == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("HAL failed to lookup LUNs for '%s': %s"), - devname, derr.message); - goto cleanup; - } - for (i = 0 ; i < numstrdevs && n == -1 ; i++) { - char *cat = libhal_device_get_property_string(ctx, - strdevs[0], - "info.category", - NULL); - /* XXX derr */ - if (cat != NULL) { - if (STREQ(cat, "storage")) - n = i; - free(cat); - } - } - /* No storage vol for device - probably a removable vol so ignore */ - if (n == -1) { - ret = 0; - goto cleanup; - } - -#define HAL_GET_PROPERTY(path, name, err, type, value) \ - (value) = libhal_device_get_property_ ## type(ctx, (path), (name), (err)); \ - if (dbus_error_is_set((err))) { \ - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, \ - "unable to lookup '%s' property on %s: %s", \ - (name), (path), derr.message); \ - goto cleanup; \ + char sysfs_path[PATH_MAX]; + int i, n, tries, retval, directaccess; + unsigned int tmphost, tmpbus, tmptarget, tmplun; + DIR *sysdir; + char *block, *block2; + struct dirent *sys_dirent; + struct dirent **namelist; + char *scsidev; + + retval = 0; + + /* we now have the host, bus, and target; let's scan for LUNs */ + snprintf(sysfs_path, PATH_MAX, + "/sys/bus/scsi/devices/target%u:%u:%u", + host, bus, target); + + n = scandir(sysfs_path, &namelist, notdotdir, versionsort); + if (n <= 0) { + /* we didn't find any reasonable entries; return failure */ + virReportSystemError(conn, errno, + _("Failed to find any LUNs for host %d bus %d target %d"), + host, bus, target); + return -1; } - /* These are props of the physical device */ - HAL_GET_PROPERTY(devname, "scsi.host", &derr, int, host); - HAL_GET_PROPERTY(devname, "scsi.bus", &derr, int, bus); - HAL_GET_PROPERTY(devname, "scsi.target", &derr, int, target); - HAL_GET_PROPERTY(devname, "scsi.lun", &derr, int, lun); - - /* These are props of the logic device */ - HAL_GET_PROPERTY(strdevs[0], "block.device", &derr, string, dev); - /* - * XXX storage.serial is not actually unique if they have - * multipath on the fibre channel adapter - */ - HAL_GET_PROPERTY(strdevs[0], "storage.serial", &derr, string, key); - HAL_GET_PROPERTY(strdevs[0], "storage.size", &derr, uint64, size); - -#undef HAL_GET_PROPERTY - - snprintf(name, sizeof(name), "%d:%d:%d:%d", host, bus, target, lun); - name[sizeof(name)-1] = '\0'; - - if (virStorageBackendSCSICreateVol(conn, pool, name, dev, key, size) < 0) - goto cleanup; - ret = 0; - -cleanup: - for (i = 0 ; strdevs && i < numstrdevs ; i++) - free(strdevs[i]); - free(strdevs); - free(dev); - free(key); - if (dbus_error_is_set(&derr)) - dbus_error_free(&derr); - return ret; -} + for (i = 0 ; i < n ; i++) { + block = NULL; + scsidev = NULL; + + if (sscanf(namelist[i]->d_name, "%u:%u:%u:%u\n", + &tmphost, &tmpbus, &tmptarget, &tmplun) != 4) + continue; + + /* we found a LUN */ + /* Note, however, that just finding a LUN doesn't mean it is + * actually useful to us. There are a few different types of + * LUNs, enumerated in the linux kernel in + * drivers/scsi/scsi.c:scsi_device_types[]. Luckily, these device + * types form part of the ABI between the kernel and userland, so + * are unlikely to change. For now, we ignore everything that isn't + * type 0; that is, a Direct-Access device + */ + snprintf(sysfs_path, PATH_MAX, + "/sys/bus/scsi/devices/%u:%u:%u:%u/type", + tmphost, tmpbus, tmptarget, tmplun); + + directaccess = directAccessDevice(sysfs_path); + if (directaccess < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to determine if %u:%u:%u:%u is a Direct-Access LUN"), + tmphost, tmpbus, tmptarget, tmplun); + retval = -1; + goto namelist_cleanup; + } + else if (directaccess == 0) { + /* not a direct-access device; skip */ + continue; + } + /* implicit else if (access == 1); Direct-Access device */ + + /* It might take some time for the + * /sys/bus/scsi/devices/host:bus:target:lun/block{:sda,/sda} + * link to show up; wait up to 5 seconds for it, then give up + */ + tries = 0; + while (block == NULL && tries < 50) { + snprintf(sysfs_path, PATH_MAX, "/sys/bus/scsi/devices/%u:%u:%u:%u", + tmphost, tmpbus, tmptarget, tmplun); + + sysdir = opendir(sysfs_path); + if (sysdir == NULL) { + virReportSystemError(conn, errno, + _("Failed to opendir sysfs path '%s'"), + sysfs_path); + retval = -1; + goto namelist_cleanup; + } + while ((sys_dirent = readdir(sysdir))) { + if (!notdotdir(sys_dirent)) + continue; + if (STREQLEN(sys_dirent->d_name, "block", 5)) { + block = strdup(sys_dirent->d_name); + break; + } + } + closedir(sysdir); + tries++; + if (block == NULL) + usleep(100 * 1000); + } + if (block == NULL) { + /* we couldn't find the device link for this device; fail */ + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to find device link for lun %d"), + tmplun); + retval = -1; + goto namelist_cleanup; + } -/** - * virStorageBackendSCSIRefreshPool: - * - * Query HAL for all storage devices associated with a specific - * SCSI HBA. - * - * XXX, currently we only support regular devices in /dev/, - * or /dev/disk/by-XXX. In future we also need to be able to - * map to multipath devices setup under /dev/mpath/XXXX. - */ -static int -virStorageBackendSCSIRefreshPool(virConnectPtr conn, - virStoragePoolObjPtr pool) -{ - char hostdevice[PATH_MAX]; - LibHalContext *ctx = NULL; - DBusConnection *sysbus = NULL; - DBusError derr = DBUS_ERROR_INIT; - char **hbadevs = NULL, **blkdevs = NULL; - int numhbadevs = 0, numblkdevs = 0; - int i, ret = -1; - - /* Get the canonical sysfs path for the HBA device */ - if (virStorageBackendSCSIMakeHostDevice(conn, pool, - hostdevice, sizeof(hostdevice)) < 0) - goto cleanup; + if (strlen(block) == 5) { + /* OK, this is exactly "block"; must be new-style */ + snprintf(sysfs_path, PATH_MAX, + "/sys/bus/scsi/devices/%u:%u:%u:%u/block", + tmphost, tmpbus, tmptarget, tmplun); + sysdir = opendir(sysfs_path); + if (sysdir == NULL) { + virReportSystemError(conn, errno, + _("Failed to opendir sysfs path '%s'"), + sysfs_path); + retval = -1; + goto namelist_cleanup; + } + while ((sys_dirent = readdir(sysdir))) { + if (!notdotdir(sys_dirent)) + continue; + + scsidev = strdup(sys_dirent->d_name); + break; + } + closedir(sysdir); + } + else { + /* old-style; just parse out the sd */ + block2 = strrchr(block, ':'); + if (block2 == NULL) { + /* Hm, wasn't what we were expecting; have to give up */ + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to parse block path %s"), + block); + retval = -1; + goto namelist_cleanup; + } + block2++; + scsidev = strdup(block2); + } + if (scsidev == NULL) { + virReportOOMError(conn); + retval = -1; + goto namelist_cleanup; + } - if ((ctx = libhal_ctx_new()) == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("failed to allocate HAL context")); - goto cleanup; + retval = virStorageBackendSCSINewLun(conn, pool, tmplun, scsidev); + if (retval < 0) + break; + VIR_FREE(scsidev); + VIR_FREE(block); } - sysbus = dbus_bus_get(DBUS_BUS_SYSTEM, &derr); - if (sysbus == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("failed to get dbus system bus: '%s'"), - derr.message); - goto cleanup; - } +namelist_cleanup: + /* we call these VIR_FREE here to make sure we don't leak memory on + * error cases; in the success case, these are already freed but NULL, + * which should be fine + */ + VIR_FREE(scsidev); + VIR_FREE(block); - if (!libhal_ctx_set_dbus_connection(ctx, sysbus)) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("failed to set HAL dbus connection")); - goto cleanup; + for (i = 0 ; i < n ; i++) { + VIR_FREE(namelist[i]); } + VIR_FREE(namelist); - if (!libhal_ctx_init(ctx, &derr)) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("failed to initialize HAL context: '%s'"), - derr.message); - goto cleanup; - } + return retval; +} - if ((hbadevs = libhal_manager_find_device_string_match(ctx, - "linux.sysfs_path", - hostdevice, - &numhbadevs, - &derr)) == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("HAL failed to lookup device '%s': %s"), - hostdevice, derr.message); - goto cleanup; - } - if (numhbadevs != 1) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("unexpected number of HBA devices from HAL " - "(expected 1, got %d) when looking up device " - "'%s'"), numhbadevs, hostdevice); - goto cleanup; +int +virStorageBackendSCSIFindTargets(virConnectPtr conn, + virStoragePoolObjPtr pool, + const char *sysfs_path) +{ + uint32_t host, bus, target; + DIR *sysdir; + struct dirent *sys_dirent; + int retval = 0; + + sysdir = opendir(sysfs_path); + if (sysdir == NULL) { + virReportSystemError(conn, errno, + _("Failed to opendir sysfs path '%s'"), + sysfs_path); + retval = -1; + goto out; } - if ((blkdevs = libhal_manager_find_device_string_match(ctx, - "info.parent", - hbadevs[0], - &numblkdevs, - &derr)) == NULL) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("failed to lookup LUNs for %s with HAL: %s"), - hbadevs[0], derr.message); - goto cleanup; + while ((sys_dirent = readdir(sysdir))) { + /* double-negative, so we can use the same function for scandir below */ + if (!notdotdir(sys_dirent)) + continue; + + if (STREQLEN(sys_dirent->d_name, "target", 6)) { + if (sscanf(sys_dirent->d_name, "target%u:%u:%u", + &host, &bus, &target) != 3) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to parse target from sysfs path %s/%s"), + sysfs_path, sys_dirent->d_name); + closedir(sysdir); + retval = -1; + goto out; + } + /* There may be more than one target on a host, so we + * continue to check after finding one. */ + virStorageBackendSCSIFindLUNs(conn, pool, host, bus, target); + } } + closedir(sysdir); - for (i = 0 ; i < numblkdevs ; i++) - virStorageBackendSCSIAddLUN(conn, - pool, - ctx, - blkdevs[i]); - - ret = 0; - -cleanup: - for (i = 0 ; hbadevs && i < numhbadevs ; i++) - free(hbadevs[i]); - free(hbadevs); - for (i = 0 ; blkdevs && i < numblkdevs ; i++) - free(blkdevs[i]); - free(blkdevs); - libhal_ctx_shutdown(ctx, NULL); - libhal_ctx_free(ctx); - if (sysbus) - dbus_connection_unref(sysbus); - if (dbus_error_is_set(&derr)) - dbus_error_free(&derr); - return ret; +out: + return retval; } - virStorageBackend virStorageBackendSCSI = { .type = VIR_STORAGE_POOL_SCSI, + .refreshPool = virStorageBackendSCSIRefreshPool, }; - -/* - * vim: set tabstop=4: - * vim: set shiftwidth=4: - * vim: set expandtab: - */ -/* - * Local variables: - * indent-tabs-mode: nil - * c-indent-level: 4 - * c-basic-offset: 4 - * tab-width: 4 - * End: - */ diff --git a/src/storage_backend_scsi.h b/src/storage_backend_scsi.h index c912269..022efef 100644 --- a/src/storage_backend_scsi.h +++ b/src/storage_backend_scsi.h @@ -26,20 +26,14 @@ #include "storage_backend.h" +#define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host" +#define LINUX_SYSFS_SCSI_HOST_POSTFIX "device" + extern virStorageBackend virStorageBackendSCSI; -#endif /* __VIR_STORAGE_BACKEND_SCSI_H__ */ +int +virStorageBackendSCSIFindTargets(virConnectPtr conn, + virStoragePoolObjPtr pool, + const char *sysfs_path); -/* - * vim: set tabstop=4: - * vim: set shiftwidth=4: - * vim: set expandtab: - */ -/* - * Local variables: - * indent-tabs-mode: nil - * c-indent-level: 4 - * c-basic-offset: 4 - * tab-width: 4 - * End: - */ +#endif /* __VIR_STORAGE_BACKEND_SCSI_H__ */ -- 1.6.0.6
On Mon, Mar 02, 2009 at 02:41:12PM -0500, David Allan wrote:
--- src/storage_backend_iscsi.c | 327 +--------------------- src/storage_backend_scsi.c | 649 ++++++++++++++++++++----------------------- src/storage_backend_scsi.h | 22 +- 3 files changed, 319 insertions(+), 679 deletions(-)
ACK, this turned out quite nicely really. The patch diff context is hard to follow, but the resulting code after applying the patch looks good to me Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--- configure.in | 2 +- src/Makefile.am | 1 - 2 files changed, 1 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index b88a7d5..995fac5 100644 --- a/configure.in +++ b/configure.in @@ -1168,7 +1168,7 @@ LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.} AC_SUBST([LV_LIBTOOL_OBJDIR]) dnl HAL or DeviceKit library for host device enumeration -HAL_REQUIRED=0.5.0 +HAL_REQUIRED=0.0 HAL_CFLAGS= HAL_LIBS= AC_ARG_WITH([hal], diff --git a/src/Makefile.am b/src/Makefile.am index f1dd789..88f82db 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,7 +9,6 @@ INCLUDES = \ $(XEN_CFLAGS) \ $(SELINUX_CFLAGS) \ $(DRIVER_MODULE_CFLAGS) \ - $(POLKIT_CFLAGS) \ -DLIBDIR=\""$(libdir)"\" \ -DBINDIR=\""$(libexecdir)"\" \ -DSBINDIR=\""$(sbindir)"\" \ -- 1.6.0.6
On Mon, Mar 02, 2009 at 02:41:13PM -0500, David Allan wrote:
--- configure.in | 2 +- src/Makefile.am | 1 - 2 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/configure.in b/configure.in index b88a7d5..995fac5 100644 --- a/configure.in +++ b/configure.in @@ -1168,7 +1168,7 @@ LV_LIBTOOL_OBJDIR=${lt_cv_objdir-.} AC_SUBST([LV_LIBTOOL_OBJDIR])
dnl HAL or DeviceKit library for host device enumeration -HAL_REQUIRED=0.5.0 +HAL_REQUIRED=0.0 HAL_CFLAGS= HAL_LIBS= AC_ARG_WITH([hal],
I think you can leave out this chunk - we should have used 0.5.0 in the first place, so no need to remove it now.
diff --git a/src/Makefile.am b/src/Makefile.am index f1dd789..88f82db 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,7 +9,6 @@ INCLUDES = \ $(XEN_CFLAGS) \ $(SELINUX_CFLAGS) \ $(DRIVER_MODULE_CFLAGS) \ - $(POLKIT_CFLAGS) \ -DLIBDIR=\""$(libdir)"\" \ -DBINDIR=\""$(libexecdir)"\" \ -DSBINDIR=\""$(sbindir)"\" \
ACK to this, since its redundant change added by the first patch Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Feb 20, 2009 at 05:14:54PM -0500, David Allan wrote:
This patch contains the implementation Daniel Berrange did of storage pools using SCSI HBAs. I have updated it for the current tree in preparation for implementing NPIV support. Let me know what you think. This looks like a great addition but I wonder if it is of any real use without supporting multipath? On SANs issuing I/O to some paths might cause severe performance penalties or the LUN might be visible but I/O is rejected until an explicit switch over command is sent. This would be handled transparently if multipath would be used on top. Any plans to add this? (Otherwise I might give this a try once I'll find some more free time to spend on libvirt again). Cheers, -- Guido
I would also like feedback on the integration into the build system, particularly where POLKIT_CFLAGS should go.
Guido Günther wrote:
On Fri, Feb 20, 2009 at 05:14:54PM -0500, David Allan wrote:
This patch contains the implementation Daniel Berrange did of storage pools using SCSI HBAs. I have updated it for the current tree in preparation for implementing NPIV support. Let me know what you think. This looks like a great addition but I wonder if it is of any real use without supporting multipath? On SANs issuing I/O to some paths might cause severe performance penalties or the LUN might be visible but I/O is rejected until an explicit switch over command is sent. This would be handled transparently if multipath would be used on top. Any plans to add this? (Otherwise I might give this a try once I'll find some more free time to spend on libvirt again).
Funny you should mention multipath, that's what I'm working on at the moment. :) The host code will also provide a basis for NPIV support. Dave
On Mon, Feb 23, 2009 at 06:03:07PM +0100, Guido G?nther wrote:
On Fri, Feb 20, 2009 at 05:14:54PM -0500, David Allan wrote:
This patch contains the implementation Daniel Berrange did of storage pools using SCSI HBAs. I have updated it for the current tree in preparation for implementing NPIV support. Let me know what you think. This looks like a great addition but I wonder if it is of any real use without supporting multipath? On SANs issuing I/O to some paths might cause severe performance penalties or the LUN might be visible but I/O is rejected until an explicit switch over command is sent. This would be handled transparently if multipath would be used on top.
This is *not* just for FibreChannel based HBAs. Even single host SATA controllers show up as SCSI HBAs, so its perfectly usable even in that case. So there's no need to block on multipath support Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Feb 23, 2009 at 06:03:07PM +0100, Guido G?nther wrote:
On Fri, Feb 20, 2009 at 05:14:54PM -0500, David Allan wrote:
This patch contains the implementation Daniel Berrange did of storage pools using SCSI HBAs. I have updated it for the current tree in preparation for implementing NPIV support. Let me know what you think. This looks like a great addition but I wonder if it is of any real use without supporting multipath? On SANs issuing I/O to some paths might cause severe performance penalties or the LUN might be visible but I/O is rejected until an explicit switch over command is sent. This would be handled transparently if multipath would be used on top.
This is *not* just for FibreChannel based HBAs. Even single host SATA Sure. controllers show up as SCSI HBAs, so its perfectly usable even in that case. So there's no need to block on multipath support It's just that using it on fibre based HBAs could cause trouble so
On Mon, Feb 23, 2009 at 07:53:07PM +0000, Daniel P. Berrange wrote: thought I'd ask. -- Guido
Guido Günther wrote:
On Mon, Feb 23, 2009 at 06:03:07PM +0100, Guido G?nther wrote:
On Fri, Feb 20, 2009 at 05:14:54PM -0500, David Allan wrote:
This patch contains the implementation Daniel Berrange did of storage pools using SCSI HBAs. I have updated it for the current tree in preparation for implementing NPIV support. Let me know what you think. This looks like a great addition but I wonder if it is of any real use without supporting multipath? On SANs issuing I/O to some paths might cause severe performance penalties or the LUN might be visible but I/O is rejected until an explicit switch over command is sent. This would be handled transparently if multipath would be used on top. This is *not* just for FibreChannel based HBAs. Even single host SATA Sure. controllers show up as SCSI HBAs, so its perfectly usable even in that case. So there's no need to block on multipath support It's just that using it on fibre based HBAs could cause trouble so
On Mon, Feb 23, 2009 at 07:53:07PM +0000, Daniel P. Berrange wrote: thought I'd ask.
Hi Guido, People can already use fibre channel block devices like any other block devices with libvirt, so it's already possible for people to try to use a passive path on an active/passive array, or use two active paths to the same LU thinking they're different LUs, etc. That's being the case, I don't think there's anything in this patch that makes it more likely that people will make those kinds of mistakes. This patch just makes it easy to list the LUs accessible by a particular host and provides a foundation on which NPIV operations can be implemented. Dave
On Mon, Feb 23, 2009 at 09:41:20PM -0500, Dave Allan wrote:
Guido Günther wrote:
On Mon, Feb 23, 2009 at 06:03:07PM +0100, Guido G?nther wrote:
On Fri, Feb 20, 2009 at 05:14:54PM -0500, David Allan wrote:
This patch contains the implementation Daniel Berrange did of storage pools using SCSI HBAs. I have updated it for the current tree in preparation for implementing NPIV support. Let me know what you think. This looks like a great addition but I wonder if it is of any real use without supporting multipath? On SANs issuing I/O to some paths might cause severe performance penalties or the LUN might be visible but I/O is rejected until an explicit switch over command is sent. This would be handled transparently if multipath would be used on top. This is *not* just for FibreChannel based HBAs. Even single host SATA Sure. controllers show up as SCSI HBAs, so its perfectly usable even in that case. So there's no need to block on multipath support It's just that using it on fibre based HBAs could cause trouble so
On Mon, Feb 23, 2009 at 07:53:07PM +0000, Daniel P. Berrange wrote: thought I'd ask.
Hi Guido,
People can already use fibre channel block devices like any other block devices with libvirt, so it's already possible for people to try to use a passive path on an active/passive array, or use two active paths to the same LU thinking they're different LUs, etc. That's being the case, I don't think there's anything in this patch that makes it more likely that people will make those kinds of mistakes. This patch just makes it easy to list the LUs accessible by a particular host and provides a foundation on which NPIV operations can be implemented. You are of course right. My mail probably sounded overly "pessimistic". Dan's SCSI HBA pool code is a great addition! I just can't wait until we support active/passive arrays ;) -- Guido
participants (6)
-
Chris Lalancette -
Daniel P. Berrange -
Dave Allan -
David Allan
-
Guido Günther -
Jim Meyering