Chris Lalancette <clalance@xxxxxxxxxx> wrote:
> This rather large patch rewrites the virStorageBackendISCSIFindLUNs() function
> to only rely on sysfs for finding LUNs, given a session number. Along the way,
> it also fixes the bug where we wouldn't find LUNs for older kernels (with the
> block:sda format), and also (possibly) fixes a race condition where we could try
> to find the LUN before udev has finished connecting it. I say it "possibly"
> fixes it because I haven't been able to hit it so far, but I definitely need
> more testing to try and confirm.
...
Hi Chris,
ACK to the first 4 parts.
Here there's one nit and one problem:
> +virStorageBackendISCSINewLun(virConnectPtr conn, virStoragePoolObjPtr pool,
> + unsigned int lun, char *dev)
"dev" const, and doesn't need to go past column 80.
Dan already mentioned TABs.
> + 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) {
> + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("Failed to opendir sysfs path %s: %s"),
> + sysfs_path, strerror(errno));
> + 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);
> + }
This needs a check for scsidev == NULL, since
virStorageBackendISCSINewLun would segfault on a NULL;
it dereferences the pointer (via its "dev" parameter) with this:
> + if (asprintf(&devpath, "/dev/%s", dev) < 0) {
> + retval = virStorageBackendISCSINewLun(conn, pool, lun, scsidev);
> + if (retval < 0)
> + break;
...
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list