[libvirt] [PATCH 1/1] Add SCSI pool support.
Daniel P. Berrange
berrange at redhat.com
Fri Mar 27 13:08:50 UTC 2009
On Thu, Mar 26, 2009 at 05:55:48PM -0400, David Allan wrote:
> This version of the code scans a particular host for targets and LUs and adds any LUs it finds as storage volumes.
>
> -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;
> - }
The new generic 'NewLUN' method has lost this bit of retry / sleep logic.
This unfortauntely causes us to randomly loose LUNs due to fact that
udev may not yet have created the /dev/ node or the stable path.
In this example, I have a iSCSI pool with a single LUN that is active.
On the iSCSI target I then export a second LUNs, and refresh the pool
on the libvirt client. The first time I refresh, the OS sees the new
LUN (as per lsscsi), but libvirt misses it. I have to refresh libvirt
again to see it
# virsh vol-list lettuceiscsi
Name Path
-----------------------------------------
5.0.0.4 /dev/disk/by-path/ip-10.33.15.20:3260-iscsi-demo-lun-4
[root at dhcp-1-98 ~]#
[root at dhcp-1-98 ~]# lsscsi
[0:0:0:0] disk ATA HDT722516DLA380 V43O /dev/sda
[1:0:0:0] cd/dvd PLEXTOR DVDR PX-755A 1.04 /dev/sr0
[5:0:0:0] storage IET Controller 0001 -
[5:0:0:4] disk IET VIRTUAL-DISK 0001 /dev/sdb
[root at dhcp-1-98 ~]# virsh pool-refresh lettuceiscsi
Pool lettuceiscsi refreshed
[root at dhcp-1-98 ~]# lsscsi
[0:0:0:0] disk ATA HDT722516DLA380 V43O /dev/sda
[1:0:0:0] cd/dvd PLEXTOR DVDR PX-755A 1.04 /dev/sr0
[5:0:0:0] storage IET Controller 0001 -
[5:0:0:2] disk IET VIRTUAL-DISK 0001 /dev/sdc
[5:0:0:4] disk IET VIRTUAL-DISK 0001 /dev/sdb
[root at dhcp-1-98 ~]#
[root at dhcp-1-98 ~]# virsh vol-list lettuceiscsi
Name Path
-----------------------------------------
5.0.0.4 /dev/disk/by-path/ip-10.33.15.20:3260-iscsi-demo-lun-4
[root at dhcp-1-98 ~]# virsh pool-refresh lettuceiscsi
Pool lettuceiscsi refreshed
[root at dhcp-1-98 ~]# virsh vol-list lettuceiscsi
Name Path
-----------------------------------------
5.0.0.2 /dev/disk/by-path/ip-10.33.15.20:3260-iscsi-demo-lun-2
5.0.0.4 /dev/disk/by-path/ip-10.33.15.20:3260-iscsi-demo-lun-4
> -
> - /* 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, "target") < 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
> new file mode 100644
> index 0000000..62c05ae
> --- /dev/null
> +++ b/src/storage_backend_scsi.c
> @@ -0,0 +1,508 @@
> +/*
> + * 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 <unistd.h>
> +#include <stdio.h>
> +#include <dirent.h>
> +#include <fcntl.h>
> +
> +#include "virterror_internal.h"
> +#include "storage_backend_scsi.h"
> +#include "memory.h"
> +#include "logging.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_STORAGE
> +
> +/* 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, type of
> + * the device otherwise.
> + */
> +static int
> +getDeviceType(virConnectPtr conn,
> + uint32_t host,
> + uint32_t bus,
> + uint32_t target,
> + uint32_t lun,
> + int *type)
> +{
> + char *type_path = NULL;
> + char typestr[3];
> + char *gottype, *p;
> + FILE *typefile;
> + int retval = 0;
> +
> + if (virAsprintf(&type_path, "/sys/bus/scsi/devices/%u:%u:%u:%u/type",
> + host, bus, target, lun) < 0) {
> + virReportOOMError(conn);
> + goto out;
> + }
> +
> + typefile = fopen(type_path, "r");
> + if (typefile == NULL) {
> + virReportSystemError(conn, errno,
> + _("Could not find typefile '%s'"),
> + type_path);
> + /* there was no type file; that doesn't seem right */
> + retval = -1;
> + goto out;
> + }
> +
> + gottype = fgets(typestr, 3, typefile);
> + fclose(typefile);
> +
> + if (gottype == NULL) {
> + virReportSystemError(conn, errno,
> + _("Could not read typefile '%s'"),
> + type_path);
> + /* we couldn't read the type file; have to give up */
> + retval = -1;
> + goto out;
> + }
> +
> + /* 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) {
> + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("Device type '%s' is not an integer"),
> + typestr);
> + /* Hm, type wasn't an integer; seems strange */
> + retval = -1;
> + goto out;
> + }
> +
> + VIR_DEBUG(_("Device type is %d"), *type);
> +
> +out:
> + VIR_FREE(type_path);
> + return retval;
> +}
> +
> +
> +static int
> +virStorageBackendSCSINewLun(virConnectPtr conn,
> + virStoragePoolObjPtr pool,
> + uint32_t host,
> + uint32_t bus,
> + uint32_t target,
> + uint32_t lun,
> + const char *dev)
> +{
> + virStorageVolDefPtr vol;
> + char *devpath = NULL;
> + int retval = 0;
> +
> + if (VIR_ALLOC(vol) < 0) {
> + virReportOOMError(conn);
> + retval = -1;
> + goto out;
> + }
> +
> + vol->type = VIR_STORAGE_VOL_BLOCK;
> +
> + if (virAsprintf(&(vol->name), "%u.%u.%u.%u", host, bus, target, lun) < 0) {
> + virReportOOMError(conn);
> + retval = -1;
> + goto free_vol;
> + }
> +
> + if (virAsprintf(&devpath, "/dev/%s", dev) < 0) {
> + virReportOOMError(conn);
> + retval = -1;
> + goto free_vol;
> + }
> +
> + VIR_DEBUG(_("Trying to create volume for '%s'"), devpath);
> +
This is where I think we need to reintroduce the open/try loop to wait
for the nodes to appear in /dev
> + /* 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) {
> + retval = -1;
> + goto free_vol;
> + }
> +
> + if (STREQLEN(devpath, vol->target.path, PATH_MAX)) {
> + VIR_DEBUG(_("No stable path found for '%s' in '%s'"),
> + devpath, pool->def->target.path);
> + retval = -1;
> + goto free_vol;
> + }
Also this bit needs to be removed, because we need to fallback to the
generic /dev path if no stable link is present, or if the user has
explicitly configured /dev/ as the pool target
> +
> + if (virStorageBackendUpdateVolTargetInfo(conn,
> + &vol->target,
> + &vol->allocation,
> + &vol->capacity) < 0) {
> +
> + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("Failed to update volume for '%s'"),
> + devpath);
> + retval = -1;
> + goto free_vol;
> + }
> +
> + /* XXX should use logical unit's UUID instead */
> + vol->key = strdup(vol->target.path);
> + if (vol->key == NULL) {
> + virReportOOMError(conn);
> + retval = -1;
> + goto free_vol;
> + }
> +
> + pool->def->capacity += vol->capacity;
> + pool->def->allocation += vol->allocation;
> +
> + if (VIR_REALLOC_N(pool->volumes.objs,
> + pool->volumes.count + 1) < 0) {
> + virReportOOMError(conn);
> + retval = -1;
> + goto free_vol;
> + }
> + pool->volumes.objs[pool->volumes.count++] = vol;
> +
> + goto out;
> +
> +free_vol:
> + virStorageVolDefFree(vol);
> +out:
> + VIR_FREE(devpath);
> + return retval;
> +}
> +
> +
> diff --git a/src/storage_backend_scsi.h b/src/storage_backend_scsi.h
> new file mode 100644
> index 0000000..678ccd6
> --- /dev/null
> +++ b/src/storage_backend_scsi.h
> @@ -0,0 +1,54 @@
> +
> +#ifndef __VIR_STORAGE_BACKEND_SCSI_H__
> +#define __VIR_STORAGE_BACKEND_SCSI_H__
> +
> +#include "storage_backend.h"
> +#include "hash.h"
> +
> +#define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host"
> +#define LINUX_SYSFS_SCSI_HOST_POSTFIX "device"
> +
> +struct _virDirectoryWalkData {
> + virConnectPtr conn;
> + virStoragePoolObjPtr pool;
> + const char *dir_path;
> + const char *pattern_to_match;
> + size_t expected_matches;
> + virHashTablePtr matches; // will be created by walk function
> + virHashIterator iterator;
> + virHashDeallocator deallocator;
> +};
> +typedef struct _virDirectoryWalkData virDirectoryWalkData;
> +typedef virDirectoryWalkData *virDirectoryWalkDataPtr;
This struct/typedef doesn't appear to be used anywhere in the code. Guessing
this is just a left-over from earlier version.
FYI, my testing was on 2.6.18 RHEL-5 kernel, and 2.6.27.9 F10/9 kernels
Regards,
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 :|
More information about the libvir-list
mailing list