[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 2/5] char: Introduce char_set/remove_fd_handlers
From: |
Gerd Hoffmann |
Subject: |
[Qemu-devel] Re: [PATCH 2/5] char: Introduce char_set/remove_fd_handlers() |
Date: |
Wed, 12 Jan 2011 10:11:23 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101208 Red Hat/3.1.7-3.el6_0 Thunderbird/3.1.7 |
Hi,
Moving the handlers to a separate struct is clearly a incremental
cleanup which can follow later. Using enable/disable flags will
probably simplify the interfaces for the non-blocking mode and thus
simplify the whole patch series so I think this should be done now.
Agree -- but it looks to be a big patch. I have some initial work done,
and hence am not converting anything other than unix/tcp backends. The
proposed interface here is local to this file, and just a couple of
callers. No big deal to change it once this is in.
(The struct for that will look like:
struct fd_handler {
int fd;
IOHandler *read;
IOHandler *write;
IOCanReadHandler *read_poll;
bool read_enabled, write_enabled, read_poll_enabled;
void (*set_read_handler)(IOHandler *read_handler);
void (*set_write_handler)(IOHandler *write_handler);
void (*set_readpoll_handler)(IOCanReadHandler *read_poll_handler);
}
No, that isn't what I have in mind ...
I'm thinking more about this:
(1) IOHandlerRecord gets variables to enable/disable the handlers
(a bunch of bools, a bitmask, whatever). They default to enabled
to maintain backward compatibility.
(2) One or more functions are added to enable/disable the handlers,
something like qemu_fd_check_read(int fd, bool enable);
(3) main_loop_wait() will check whenever the handler is actually
enabled before adding it to the file handle set for the select()
call.
This shouldn't be that big. And with this initial stuff in place you
can go forward with the non-blocking stuff. No need to pass around the
write handler or have callbacks just to enable/disable the checking for
a writable fd, the non-blocking code can just call
qemu_fd_check_write(fd, true/false) to do it.
Additional cleanups possible:
(1) switch everybody who registers/unregisters handlers to the new
enable/disable interface.
(2) move the function pointers from IOHandlerRecord to a separate
struct (say IOHandlerOps).
Also: we also want to be able to select() on all fds so that we can
detect disconnection events as they happen. So we also need an array
somewhere.)
I think you can't do that without switching from select() to poll().
cheers,
Gerd
- Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers, (continued)
- Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers, Amit Shah, 2011/01/12
- Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers, Michael Roth, 2011/01/12
- Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers, Blue Swirl, 2011/01/12
- Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers, Amit Shah, 2011/01/13
- Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers, Blue Swirl, 2011/01/13
[Qemu-devel] [PATCH 2/5] char: Introduce char_set/remove_fd_handlers(), Amit Shah, 2011/01/11
[Qemu-devel] [PATCH 3/5] char: Add framework for a 'write unblocked' callback, Amit Shah, 2011/01/11
[Qemu-devel] [PATCH 4/5] char: Update send_all() to handle nonblocking chardev write requests, Amit Shah, 2011/01/11
[Qemu-devel] [PATCH 5/5] char: Equip the unix/tcp backend to handle nonblocking writes, Amit Shah, 2011/01/11