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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list