Re: [PATCH 7/7] libiscsi: fix iscsi pool error path

[Mike Christie <michaelc@cs.wisc.edu>]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

Chris Wright wrote:
> * Mike Christie (michaelc@cs.wisc.edu) wrote:
>>>> -	if (q->queue == ERR_PTR(-ENOMEM))
>>>> +	if (IS_ERR(q->queue)) {
>>> This indeed solves the problem I had underlined before, but
>>> introduces a new one. Right now the only error returned by kfifo_init
>>> is -ENOMEM. This may however change in the future, and then the
>>> above code would silently convert the other error code to -ENOMEM.
>>> This might make it difficult to trace errors.
>> What do you mean by converting other errors codes to -ENOMEM?
> 
> Jean means the goto label...it will jump to essentially return -ENOMEM.
> 

Ah, ok, I can agree with passing up errors in general. I did the 
attached patch to propogate the error code upwards. It does not really 
make much difference now. The iscsi pool init caller just checks for 
error or no error and does not even print out the error. It will not 
make tracing errors easier or more difficult. To be useful, we would 
have to propgate the error higher to userspace so the user can see it. I 
do not think I have time to get that in the current feature window, so 
that would have to wait unless you have patches I can test now.

[-- Attachment #2: libiscsi-propogate-pool-error.patch --]
[-- Type: text/plain, Size: 1460 bytes --]

libiscsi: propogate errors from kfifo_init to iscsi_pool_init callers

Don't convert kfifo_init's error code to -ENOMEM when passing the
error to the iscsi_pool_init caller.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 6896283..cf86d59 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1984,6 +1984,7 @@ int
 iscsi_pool_init(struct iscsi_pool *q, int max, void ***items, int item_size)
 {
 	int i, num_arrays = 1;
+	int rc = -ENOMEM;
 
 	memset(q, 0, sizeof(*q));
 
@@ -1995,20 +1996,21 @@ iscsi_pool_init(struct iscsi_pool *q, int max, void ***items, int item_size)
 		num_arrays++;
 	q->pool = kzalloc(num_arrays * max * sizeof(void*), GFP_KERNEL);
 	if (q->pool == NULL)
-		return -ENOMEM;
+		return rc;
 
 	q->queue = kfifo_init((void*)q->pool, max * sizeof(void*),
 			      GFP_KERNEL, NULL);
 	if (IS_ERR(q->queue)) {
 		q->queue = NULL;
-		goto enomem;
+		rc = PTR_ERR(q->queue);		
+		goto fail;
 	}
 
 	for (i = 0; i < max; i++) {
 		q->pool[i] = kzalloc(item_size, GFP_KERNEL);
 		if (q->pool[i] == NULL) {
 			q->max = i;
-			goto enomem;
+			goto fail;
 		}
 		__kfifo_put(q->queue, (void*)&q->pool[i], sizeof(void*));
 	}
@@ -2020,9 +2022,9 @@ iscsi_pool_init(struct iscsi_pool *q, int max, void ***items, int item_size)
 
 	return 0;
 
-enomem:
+fail:
 	iscsi_pool_free(q);
-	return -ENOMEM;
+	return rc;
 }
 EXPORT_SYMBOL_GPL(iscsi_pool_init);