Re: [PATCH 1/2] kvm: libkvm: support for irq routing

[Avi Kivity <avi@redhat.com>]

Sheng Yang wrote:
> On Thursday 15 January 2009 17:59:43 Avi Kivity wrote:
>   
>> Sheng Yang wrote:
>>     
>>> On Thursday 15 January 2009 14:17:16 Sheng Yang wrote:
>>>       
>>>> On Wednesday 14 January 2009 21:53:10 Avi Kivity wrote:
>>>>         
>>>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>>>>           
>>>> Do we need a lock for the table?
>>>>         
>>> And kvm_add_irq_route/kvm_del_irq_route should be generic used, how about
>>> transfer a kvm_irq_routing_entry as parameter?
>>>       
>> These structures + selectors + unions are clumsy.  For libkvm, I'd
>> prefer adding kvm_add_msi() and kvm_del_msi().
>>     
>
> But...
>   
>> +int kvm_add_irq_route(kvm_context_t kvm, int gsi, int irqchip, int pin)
>> +{
>> +#ifdef KVM_CAP_IRQ_ROUTING
>> +       struct kvm_irq_routing *z;
>> +       struct kvm_irq_routing_entry *e;
>> +       int n, size;
>> +
>> +       if (kvm->irq_routes->nr == kvm->nr_allocated_irq_routes) {
>> +               n = kvm->nr_allocated_irq_routes * 2;
>> +               if (n < 64)
>> +                       n = 64;
>> +               size = sizeof(struct kvm_irq_routing);
>> +               size += n * sizeof(*e);
>> +               z = realloc(kvm->irq_routes, size);
>> +               if (!z)
>> +                       return -ENOMEM;
>> +               kvm->nr_allocated_irq_routes = n;
>> +               kvm->irq_routes = z;
>> +       }
>> +       n = kvm->irq_routes->nr++;
>> +       e = &kvm->irq_routes->entries[n];
>> +       memset(e, 0, sizeof(*e));
>> +       e->gsi = gsi;
>> +       e->type = KVM_IRQ_ROUTING_IRQCHIP;
>> +       e->flags = 0;
>> +       e->u.irqchip.irqchip = irqchip;
>> +       e->u.irqchip.pin = pin;
>> +       return 0;
>> +#else
>> +       return -ENOSYS;
>> +#endif
>> +}
>>     
>
> Besides three lines, all can be reused... I don't see the reason for another 
> function...
>
> And the name here is irq_route, I suppose it should be generic used. Can be 
> core function which can be wrapped.
>   

Yes, I guess we can rename it irqchip_route.  When we add msis, both 
this function and the new msi function can share code.

-- 
error compiling committee.c: too many arguments to function