[Lguest] [PATCH] lguest: Make sure the interrupt is allocated correctly by lguest_setup_irq (ie check the return value of irq_alloc_desc_at for -ENOMEM)

[Stratos Psomadakis]

Sorry for the delay, but I was very busy the last month.

The [RESEND PATCH] which was supposed to fix the issues that you 
mentioned was 'wrong'. I'll resend it, and hopefully it'll be ok.

On 05/21/2011 07:33 AM, Rusty Russell wrote:
>> I think we only have to check for -ENOMEM, since all the other return values
>> are harmless (ie -EEXIST) and -EINVAL cannot be returned (normally).
> We should probably pass back any error we receive, however, for
> simplicity, futureproofing and clarity.
>
>> -void lguest_setup_irq(unsigned int irq)
>> +int lguest_setup_irq(unsigned int irq)
>>   {
>> -	irq_alloc_desc_at(irq, 0);
>> +	int err = 0;
>> +
>> +	err = irq_alloc_desc_at(irq, 0);
>> +	if (err  == -ENOMEM)
>> +		goto out;
>> +	
>>   	irq_set_chip_and_handler_name(irq,&lguest_irq_controller,
>>   				      handle_level_irq, "level");
>> +
>> +out:
>> +	return err;
>>   }
About this comment, we pass back any error we receive anyway. We just 
check for -ENOMEM, so that we don't call irq_set_chip_and_handler_name 
if we run out of memory.

However, after some searching and looking at code I think that:
1) We could just do err = irq_alloc_desc_at, without checking for 
-ENOMEM, and just pass the error code to lg_find_vq. 
irq_set_chip_and_handler_name can handle this situation without problems 
(ie no descriptor for the given irq).
2) We should only check for -ENOMEM in lg_find_vq. Other error codes 
either don't ever get returned (ie -EINVAL), or they're 'valid', like 
-EEXIST, since we don't free the irq descs when deleting a virtqueue. If 
we call setup_irq again irq_alloc_desc_at will return -EEXIST I think.

Am I right?

Thanks,

-- 
Stratos Psomadakis
<psomas at ece.ntua.gr>