Re: [Qemu-devel] [PATCH 2/7] iohandlers: Introduce a new API

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/22/2011 07:28 AM, Avi Kivity wrote:
> On 02/22/2011 12:18 PM, Amit Shah wrote:
>> Introduce a new iohandler api that doesn't have multiple callbacks.
>> Instead, a single callback and a mask of events that got set will be
>> passed on to the handler.  This will ease our transition to a poll()
>> interface instead of the current select() that happens on the fds.
>>
>>
>> +
>> +/* iohandler masks */
>> +#define IOH_MASK_CAN_READ    (1U<<  0)
>> +#define IOH_MASK_READ        (1U<<  1)
>> +#define IOH_MASK_WRITE        (1U<<  2)
>> +
>> +typedef int IOAllHandler(void *opaque, unsigned int mask);
>
> Strange name.
>
> Drop the opaque, instead put the IOHandler in there (or maybe the 
> CharDev?) and use container_of().

You know, I'm not there that this is automatically better.

>> +
>> +int assign_iohandler(int fd, IOAllHandler *handler, unsigned int mask,
>> +                     void *opaque);
>> +int remove_iohandler(int fd);
>> +int update_fd_mask(int fd, unsigned int mask);
>> +int get_fd_mask(int fd, unsigned int *mask);
>> +
>
> iohandler_init(IOHandler *ioh, int fd, IOEventHandler *handler, 
> unsigned mask)
> iohandler_del(IOHandler *ioh)
> iohandler_set_event_mask(IOHandler *ioh, unsigned mask)
> iohandler_event_mask(IOHandler *ioh)
>
> No opaques, use an object as a key so you don't have to search for it.

I don't think there should be a setter/getter for the event mask.  Just 
delete the handler and re-add it.

Why is there an IOHandler and an IOEventHandler argument for init?

Regards,

Anthony Liguori