Re: [libvirt] [PATCH 1/3] iscsi: Check for presence of error before creating new one

2015-03-31 Thread Ján Tomko 
On Mon, Mar 30, 2015 at 07:16:32PM -0400, John Ferlan wrote:
 If virStorageBackendSCSIFindLUs fails, but the failure has an error
 message - the iscsi code didn't honor that creating it's own wonderful
 message such as error: Failed to find LUs on host 60: ... - not overly
 helpful. Since a few of the called paths generate a message, check for
 that before using that generic one.

Both of the paths returning -1 generate an error:
* return -1 after opendir failed
* retval = virDirRead

 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_backend_iscsi.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/src/storage/storage_backend_iscsi.c 
 b/src/storage/storage_backend_iscsi.c
 index 079c767..1dac238 100644
 --- a/src/storage/storage_backend_iscsi.c
 +++ b/src/storage/storage_backend_iscsi.c
 @@ -147,8 +147,9 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
  }
  
  if (virStorageBackendSCSIFindLUs(pool, host)  0) {
 -virReportSystemError(errno,
 - _(Failed to find LUs on host %u), host);

 +if (virGetLastError() == NULL)

This condition is never true. And it should only be used if
we can't fix the function to consistently set an error in all
cases.

Jan

 +virReportSystemError(errno,
 + _(Failed to find LUs on host %u), host);
  retval = -1;
  }


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] iscsi: Check for presence of error before creating new one

2015-03-31 Thread John Ferlan 


On 03/31/2015 07:03 AM, Ján Tomko wrote:
 On Mon, Mar 30, 2015 at 07:16:32PM -0400, John Ferlan wrote:
 If virStorageBackendSCSIFindLUs fails, but the failure has an error
 message - the iscsi code didn't honor that creating it's own wonderful
 message such as error: Failed to find LUs on host 60: ... - not overly
 helpful. Since a few of the called paths generate a message, check for
 that before using that generic one.
 
 Both of the paths returning -1 generate an error:
 * return -1 after opendir failed
 * retval = virDirRead
 

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_backend_iscsi.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/src/storage/storage_backend_iscsi.c 
 b/src/storage/storage_backend_iscsi.c
 index 079c767..1dac238 100644
 --- a/src/storage/storage_backend_iscsi.c
 +++ b/src/storage/storage_backend_iscsi.c
 @@ -147,8 +147,9 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
  }
  
  if (virStorageBackendSCSIFindLUs(pool, host)  0) {
 -virReportSystemError(errno,
 - _(Failed to find LUs on host %u), host);
 
 +if (virGetLastError() == NULL)
 
 This condition is never true. And it should only be used if
 we can't fix the function to consistently set an error in all
 cases.
 

Fair enough - just being overly cautious or un-optimistic I suppose

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] iscsi: Check for presence of error before creating new one

2015-03-30 Thread John Ferlan 
If virStorageBackendSCSIFindLUs fails, but the failure has an error
message - the iscsi code didn't honor that creating it's own wonderful
message such as error: Failed to find LUs on host 60: ... - not overly
helpful. Since a few of the called paths generate a message, check for
that before using that generic one.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/storage/storage_backend_iscsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index 079c767..1dac238 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -147,8 +147,9 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
 }
 
 if (virStorageBackendSCSIFindLUs(pool, host)  0) {
-virReportSystemError(errno,
- _(Failed to find LUs on host %u), host);
+if (virGetLastError() == NULL)
+virReportSystemError(errno,
+ _(Failed to find LUs on host %u), host);
 retval = -1;
 }
 
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list