Jim Meyering wrote:
> 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.
Oops! Cut and pasted that from elsewhere, and didn't fix up the error messages.
I'll fix that up as you advise.
> + closedir(sysdir);
> + return -1;
> +
The above line has just three TABs.
Best to delete it.
Yeah, probably leftover from my whitespace fixup. I'll fix that as well.
The rest, including patches 1-4, looks fine,
so, pending whatever tests you deem adequate,
ACK.
Thanks for the review!
Chris Lalancette