"Michael S. Tsirkin" <m...@redhat.com> wrote: > Create separate lists for system and device fd handlers. > Device handlers will not run while vm is stopped. > By default all fds are assumed system so they will > keep running as before. > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > --- > qemu-char.h | 6 +++- > qemu-kvm.c | 2 +- > qemu-tool.c | 10 +++++ > vl.c | 117 > ++++++++++++++++++++++++++++++++++++++--------------------- > 4 files changed, 92 insertions(+), 43 deletions(-) > > diff --git a/qemu-char.h b/qemu-char.h > index 18ad12b..ad09f56 100644 > --- a/qemu-char.h > +++ b/qemu-char.h > @@ -101,7 +101,11 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd); > extern int term_escape_char; > > /* async I/O support */ > - > +int qemu_set_fd_handler3(bool device, int fd, > + IOCanReadHandler *fd_read_poll, > + IOHandler *fd_read, > + IOHandler *fd_write, > + void *opaque);
No this horror. Can't we just create: qemu_set_device_handler() qemu_set_system_handler() or whatever named you like? qemu_set_fd_handler2 is already bad enough, adding another one just make things worse in my humble opinion. > +int qemu_set_fd_handler3(bool device, > + int fd, > IOCanReadHandler *fd_read_poll, > IOHandler *fd_read, > IOHandler *fd_write, > void *opaque) > { > + IOHandlerRecordList *list; > IOHandlerRecord *ioh; > > + list = device ? &device_io_handlers: &system_io_handlers; > + If you are going to use this, passing list paramenter instead of device looks like a much better option. It would indeed make things go better. > if (!fd_read && !fd_write) { > - QLIST_FOREACH(ioh, &io_handlers, next) { > + QLIST_FOREACH(ioh, list, next) { > if (ioh->fd == fd) { > ioh->deleted = 1; > break; > } > } > } else { > - QLIST_FOREACH(ioh, &io_handlers, next) { > + QLIST_FOREACH(ioh, list, next) { > if (ioh->fd == fd) > goto found; > } > ioh = qemu_mallocz(sizeof(IOHandlerRecord)); > - QLIST_INSERT_HEAD(&io_handlers, ioh, next); > + QLIST_INSERT_HEAD(list, ioh, next); > found: > ioh->fd = fd; > ioh->fd_read_poll = fd_read_poll; > @@ -998,6 +1003,19 @@ int qemu_set_fd_handler2(int fd, > return 0; > } > > + > +/* XXX: fd_read_poll should be suppressed, but an API change is > + necessary in the character devices to suppress fd_can_read(). */ > +int qemu_set_fd_handler2(int fd, > + IOCanReadHandler *fd_read_poll, > + IOHandler *fd_read, > + IOHandler *fd_write, > + void *opaque) > +{ > + return qemu_set_fd_handler3(false, fd, fd_read_poll, fd_read, fd_write, > + opaque); > +} > + > int qemu_set_fd_handler(int fd, > IOHandler *fd_read, > IOHandler *fd_write, > @@ -1242,9 +1260,52 @@ void qemu_system_powerdown_request(void) > qemu_notify_event(); > } > > -void main_loop_wait(int nonblocking) > +static inline int get_ioh_fds(IOHandlerRecordList *list, > + int nfds, fd_set *rfds, fd_set *wfds) > { > IOHandlerRecord *ioh; > + QLIST_FOREACH(ioh, list, next) { > + if (ioh->deleted) > + continue; > + if (ioh->fd_read && > + (!ioh->fd_read_poll || > + ioh->fd_read_poll(ioh->opaque) != 0)) { > + FD_SET(ioh->fd, rfds); > + if (ioh->fd > nfds) > + nfds = ioh->fd; > + } > + if (ioh->fd_write) { > + FD_SET(ioh->fd, wfds); > + if (ioh->fd > nfds) > + nfds = ioh->fd; > + } > + } > + return nfds; > +} > + > +static inline void call_ioh_fds(IOHandlerRecordList *list, > + fd_set *rfds, fd_set *wfds) > +{ > + IOHandlerRecord *ioh, *pioh; > + > + QLIST_FOREACH_SAFE(ioh, list, next, pioh) { > + if (ioh->deleted) { > + QLIST_REMOVE(ioh, next); > + qemu_free(ioh); > + continue; > + } > + if (ioh->fd_read && FD_ISSET(ioh->fd, rfds)) { > + ioh->fd_read(ioh->opaque); > + if (!(ioh->fd_read_poll && ioh->fd_read_poll(ioh->opaque))) > + FD_CLR(ioh->fd, rfds); > + } > + if (ioh->fd_write && FD_ISSET(ioh->fd, wfds)) { > + ioh->fd_write(ioh->opaque); > + } > + } > +} > +void main_loop_wait(int nonblocking) > +{ > fd_set rfds, wfds, xfds; > int ret, nfds; > struct timeval tv; > @@ -1260,26 +1321,13 @@ void main_loop_wait(int nonblocking) > os_host_main_loop_wait(&timeout); > > /* poll any events */ > - /* XXX: separate device handlers from system ones */ > nfds = -1; > FD_ZERO(&rfds); > FD_ZERO(&wfds); > FD_ZERO(&xfds); > - QLIST_FOREACH(ioh, &io_handlers, next) { > - if (ioh->deleted) > - continue; > - if (ioh->fd_read && > - (!ioh->fd_read_poll || > - ioh->fd_read_poll(ioh->opaque) != 0)) { > - FD_SET(ioh->fd, &rfds); > - if (ioh->fd > nfds) > - nfds = ioh->fd; > - } > - if (ioh->fd_write) { > - FD_SET(ioh->fd, &wfds); > - if (ioh->fd > nfds) > - nfds = ioh->fd; > - } > + nfds = get_ioh_fds(&system_io_handlers, nfds, &rfds, &wfds); > + if (vm_running) { > + nfds = get_ioh_fds(&device_io_handlers, nfds, &rfds, &wfds); > } > > tv.tv_sec = timeout / 1000; > @@ -1291,22 +1339,9 @@ void main_loop_wait(int nonblocking) > ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv); > qemu_mutex_lock_iothread(); > if (ret > 0) { > - IOHandlerRecord *pioh; > - > - QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) { > - if (ioh->deleted) { > - QLIST_REMOVE(ioh, next); > - qemu_free(ioh); > - continue; > - } > - if (ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) { > - ioh->fd_read(ioh->opaque); > - if (!(ioh->fd_read_poll && ioh->fd_read_poll(ioh->opaque))) > - FD_CLR(ioh->fd, &rfds); > - } > - if (ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) { > - ioh->fd_write(ioh->opaque); > - } > + call_ioh_fds(&system_io_handlers, &rfds, &wfds); > + if (vm_running) { > + call_ioh_fds(&device_io_handlers, &rfds, &wfds); > } > }