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); ...