[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/1] Add SCSI pool support.



On Sat, Mar 28, 2009 at 10:40:48AM -0400, Dave Allan wrote:
> Daniel P. Berrange wrote:
> 
> >This unfortauntely does not work on RHEL5, because there are no targetX.X.X
> >links here. Only the LUNs appear in this directory.
> >
> >The location which appears to be present on both old and new kernels is
> >under:
> >
> >   /sys/class/scsi_host/host0/device/targetX.X.X
> >
> >This appears to be present for both SCSI and iSCSI hosts.
> 
> Unfortunately, that directory isn't present for FC hosts on F10.  :( 
> I've put together a scan strategy that I think will for for both RHEL5 
> and F10 for all transport types.  Let me know if it works for you.  The 
> attached patch should be applied on top of the udev fix I sent 
> yesterday.  This patch is another one that doesn't read well as a patch, 
> but should be pretty straightforward once it's applied.
> 
> I have not addressed the question of what device types to allow, because 
> I want to think about that one a bit more.
> 
> I also took out the unused struct you noticed.
> 
> Thanks for all the feedback and testing.

This works now on RHEL-5, F9 and F10.

I still had to disable this code

    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;
    }

because it breaks pools configured with a target dir of /dev/

And add device_type == 5 to allow CDROMs to work

    if (device_type != 0 &&
        device_type != 5) {
        retval = 0;
        goto out;
    }

> From c749c3cc3d90cf9ae31db0c68fd41b7ce24c2bf3 Mon Sep 17 00:00:00 2001
> From: David Allan <dallan redhat com>
> Date: Fri, 27 Mar 2009 17:48:18 -0400
> Subject: [PATCH 1/1] Changes based on feedback from DanPB
> 
> * Fixed LU scan logic so that it now hopefully works for all transport types on both RHEL5 and F10.  The preceeding version didn't work on RHEL5.
> * Removed the unused struct from storage_backend_scsi.h
> ---
>  src/storage_backend_iscsi.c |   20 ++++++++++----
>  src/storage_backend_scsi.c  |   61 +++++++++++++++++++-----------------------
>  src/storage_backend_scsi.h  |   24 +++++------------
>  3 files changed, 49 insertions(+), 56 deletions(-)
> 
> diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c
> index 9da7cdc..b516add 100644
> --- a/src/storage_backend_iscsi.c
> +++ b/src/storage_backend_iscsi.c
> @@ -172,23 +172,31 @@ virStorageBackendISCSIConnection(virConnectPtr conn,
>  
>  
>  static int
> -virStorageBackendISCSIFindLUNs(virConnectPtr conn,
> -                               virStoragePoolObjPtr pool,
> -                               const char *session)
> +virStorageBackendISCSIFindLUs(virConnectPtr conn,
> +                              virStoragePoolObjPtr pool,
> +                              const char *session)
>  {
>      char sysfs_path[PATH_MAX];
>      int retval = 0;
> +    uint32_t host;
>  
>      snprintf(sysfs_path, PATH_MAX,
>               "/sys/class/iscsi_session/session%s/device", session);
>  
> -    if (virStorageBackendSCSIFindTargets(conn, pool, sysfs_path, "target") < 0) {
> +    if (virStorageBackendSCSIGetHostNumber(conn, sysfs_path, &host) < 0) {
>          virReportSystemError(conn, errno,
> -                             _("Failed to get target list for path '%s'"),
> +                             _("Failed to get host number for iSCSI session "
> +                               "with path '%s'"),
>                               sysfs_path);
>          retval = -1;
>      }
>  
> +    if (virStorageBackendSCSIFindLUs(conn, pool, host) < 0) {
> +        virReportSystemError(conn, errno,
> +                             _("Failed to find LUs on host %u"), host);
> +        retval = -1;
> +    }
> +
>      return retval;
>  }
>  
> @@ -302,7 +310,7 @@ virStorageBackendISCSIRefreshPool(virConnectPtr conn,
>          goto cleanup;
>      if (virStorageBackendISCSIRescanLUNs(conn, pool, session) < 0)
>          goto cleanup;
> -    if (virStorageBackendISCSIFindLUNs(conn, pool, session) < 0)
> +    if (virStorageBackendISCSIFindLUs(conn, pool, session) < 0)
>          goto cleanup;
>      VIR_FREE(session);
>  
> diff --git a/src/storage_backend_scsi.c b/src/storage_backend_scsi.c
> index e0ced8f..2c5168a 100644
> --- a/src/storage_backend_scsi.c
> +++ b/src/storage_backend_scsi.c
> @@ -382,68 +382,67 @@ out:
>  }
>  
>  
> -static int
> +int
>  virStorageBackendSCSIFindLUs(virConnectPtr conn,
>                               virStoragePoolObjPtr pool,
> -                             uint32_t scanhost,
> -                             uint32_t scanbus,
> -                             uint32_t scantarget)
> +                             uint32_t scanhost)
>  {
>      int retval = 0;
> -    uint32_t host, bus, target, lun;
> -    char *target_path = NULL;
> -    DIR *targetdir = NULL;
> +    uint32_t bus, target, lun;
> +    char *device_path = NULL;
> +    DIR *devicedir = NULL;
>      struct dirent *lun_dirent = NULL;
> +    char devicepattern[64];
> +
> +    VIR_DEBUG(_("Discovering LUs on host %u"), scanhost);
>  
> -    VIR_DEBUG(_("Discovering LUs on host %u bus %u target %u"),
> -              scanhost, scanbus, scantarget);
> +    virStorageBackendWaitForDevices(conn);
>  
> -    if (virAsprintf(&target_path, "/sys/bus/scsi/devices/target%u:%u:%u",
> -             scanhost, scanbus, scantarget) < 0) {
> +    if (virAsprintf(&device_path, "/sys/bus/scsi/devices") < 0) {
>          virReportOOMError(conn);
>          goto out;
>      }
>  
> -    targetdir = opendir(target_path);
> +    devicedir = opendir(device_path);
>  
> -    if (targetdir == NULL) {
> +    if (devicedir == NULL) {
>          virReportSystemError(conn, errno,
> -                             _("Failed to opendir path '%s'"), target_path);
> +                             _("Failed to opendir path '%s'"), device_path);
>          retval = -1;
>          goto out;
>      }
>  
> -    while ((lun_dirent = readdir(targetdir))) {
> -        if (sscanf(lun_dirent->d_name, "%u:%u:%u:%u\n",
> -                   &host, &bus, &target, &lun) != 4) {
> +    snprintf(devicepattern, sizeof(devicepattern), "%u:%%u:%%u:%%u\n", scanhost);
> +
> +    while ((lun_dirent = readdir(devicedir))) {
> +        if (sscanf(lun_dirent->d_name, devicepattern,
> +                   &bus, &target, &lun) != 3) {
>              continue;
>          }
>  
>          VIR_DEBUG(_("Found LU '%s'"), lun_dirent->d_name);
>  
> -        processLU(conn, pool, host, bus, target, lun);
> +        processLU(conn, pool, scanhost, bus, target, lun);
>      }
>  
> -    closedir(targetdir);
> +    closedir(devicedir);
>  
>  out:
> -    VIR_FREE(target_path);
> +    VIR_FREE(device_path);
>      return retval;
>  }
>  
>  
>  int
> -virStorageBackendSCSIFindTargets(virConnectPtr conn,
> -                                 virStoragePoolObjPtr pool,
> -                                 const char *sysfs_path,
> -                                 const char *pattern)
> +virStorageBackendSCSIGetHostNumber(virConnectPtr conn,
> +                                   const char *sysfs_path,
> +                                   uint32_t *host)
>  {
>      int retval = 0;
> -    uint32_t host, bus, target;
>      DIR *sysdir = NULL;
>      struct dirent *dirent = NULL;
>  
> -    VIR_DEBUG(_("Discovering targets in '%s'"), sysfs_path);
> +    VIR_DEBUG(_("Finding host number from '%s'"), sysfs_path);
>  
>      virStorageBackendWaitForDevices(conn);
>  
> @@ -457,14 +456,13 @@ virStorageBackendSCSIFindTargets(virConnectPtr conn,
>      }
>  
>      while ((dirent = readdir(sysdir))) {
> -        if (STREQLEN(dirent->d_name, pattern, strlen(pattern))) {
> +        if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
>              if (sscanf(dirent->d_name,
> -                       "target%u:%u:%u", &host, &bus, &target) != 3) {
> +                       "target%u:", host) != 1) {
>                  VIR_DEBUG(_("Failed to parse target '%s'"), dirent->d_name);
>                  retval = -1;
>                  break;
>              }
> -            virStorageBackendSCSIFindLUs(conn, pool, host, bus, target);
>          }
>      }
>  
> @@ -478,7 +476,6 @@ static int
>  virStorageBackendSCSIRefreshPool(virConnectPtr conn,
>                                   virStoragePoolObjPtr pool)
>  {
> -    char targetN[64];
>      int retval = 0;
>      uint32_t host;
>  
> @@ -492,9 +489,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn,
>  
>      VIR_DEBUG(_("Scanning host%u"), host);
>  
> -    snprintf(targetN, sizeof(targetN), "target%u", host);
> -
> -    virStorageBackendSCSIFindTargets(conn, pool, "/sys/bus/scsi/devices", targetN);
> +    virStorageBackendSCSIFindLUs(conn, pool, host);
>  
>  out:
>      return retval;
> diff --git a/src/storage_backend_scsi.h b/src/storage_backend_scsi.h
> index 678ccd6..d63951d 100644
> --- a/src/storage_backend_scsi.h
> +++ b/src/storage_backend_scsi.h
> @@ -30,25 +30,15 @@
>  #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;
> -
>  extern virStorageBackend virStorageBackendSCSI;
>  
>  int
> -virStorageBackendSCSIFindTargets(virConnectPtr conn,
> -                                 virStoragePoolObjPtr pool,
> -                                 const char *sysfs_path,
> -                                 const char *pattern);
> +virStorageBackendSCSIGetHostNumber(virConnectPtr conn,
> +                                   const char *sysfs_path,
> +                                   uint32_t *host);
> +int
> +virStorageBackendSCSIFindLUs(virConnectPtr conn,
> +                             virStoragePoolObjPtr pool,
> +                             uint32_t scanhost);
>  
>  #endif /* __VIR_STORAGE_BACKEND_SCSI_H__ */
> -- 
> 1.6.0.6
> 


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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]