[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCHv2 1/2] char: separate device and system fd handl
From: |
Juan Quintela |
Subject: |
[Qemu-devel] Re: [PATCHv2 1/2] char: separate device and system fd handlers |
Date: |
Tue, 16 Nov 2010 11:24:11 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) |
"Michael S. Tsirkin" <address@hidden> 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 <address@hidden>
> ---
> 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);
> }
> }
- [Qemu-devel] [PATCHv2 0/2] migration: stop dma while VM is stopped, Michael S. Tsirkin, 2010/11/04
- [Qemu-devel] [PATCHv2 1/2] char: separate device and system fd handlers, Michael S. Tsirkin, 2010/11/04
- [Qemu-devel] Re: [PATCHv2 1/2] char: separate device and system fd handlers,
Juan Quintela <=
- [Qemu-devel] [PATCHv2 2/2] tap: mark fd handler as device, Michael S. Tsirkin, 2010/11/04
- [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Juan Quintela, 2010/11/15
- [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Anthony Liguori, 2010/11/15
- [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Michael S. Tsirkin, 2010/11/15
- [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Anthony Liguori, 2010/11/15
- [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Michael S. Tsirkin, 2010/11/15
- [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Anthony Liguori, 2010/11/15
- Re: [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Stefan Hajnoczi, 2010/11/15
- Re: [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Anthony Liguori, 2010/11/15
- Re: [Qemu-devel] Re: [PATCHv2 2/2] tap: mark fd handler as device, Michael S. Tsirkin, 2010/11/15