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.
>
> Changes since last time:
> 1) Don't blindly ignore the 0'th LUN (thanks Stefan). Instead, look if the
> LUNs are Direct-Access devices (as defined by the kernel); if so, they are LUNs
> we can use.
> 2) Fix up whitespace damage.
> 3) Check the return value of the scsidev strdup as pointed out by Jim.
> 4) Change all ISCSIADM commands to be const char *const as pointed out by Jim.
>
> diff -urp libvirt.sendtarget/src/storage_backend_iscsi.c libvirt.findLun/src/storage_backend_iscsi.c
> --- libvirt.sendtarget/src/storage_backend_iscsi.c 2008-06-16 14:35:34.000000000 +0200
> +++ libvirt.findLun/src/storage_backend_iscsi.c 2008-06-16 14:52:31.000000000 +0200
> @@ -119,7 +119,7 @@ virStorageBackendISCSISession(virConnect
...
> + 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 in sysfs path %s: %s"),
> + sysfs_path, strerror(errno));
You can remove the ": %s" suffix and the strerror(errno) argument,
since errno isn't relevant here. Or maybe better: leave the suffix
and use sys_dirent->d_name as the argument, so the diagnostic shows
what couldn't be parsed.
> + closedir(sysdir);
> + return -1;
> +
The above line has just three TABs.
Best to delete it.
The rest, including patches 1-4, looks fine,
so, pending whatever tests you deem adequate,
ACK.
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list