[Qemu-devel] Re: [RFC][PATCH v6 03/23] Make qemu timers available for tools

[Jes Sorensen <Jes.Sorensen@redhat.com>]

On 01/17/11 14:14, Michael Roth wrote:
> diff --git a/qemu-ioh.c b/qemu-ioh.c
> index cc71470..001e7a2 100644
> --- a/qemu-ioh.c
> +++ b/qemu-ioh.c
> @@ -22,7 +22,11 @@
>   * THE SOFTWARE.
>   */
>  #include "qemu-ioh.h"
> +#include "qemu-char.h"
>  #include "qlist.h"
> +#ifdef CONFIG_EVENTFD
> +#include <sys/eventfd.h>
> +#endif
>  
>  /* XXX: fd_read_poll should be suppressed, but an API change is
>     necessary in the character devices to suppress fd_can_read(). */
> @@ -113,3 +117,92 @@ void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
>          }
>      }
>  }
> +
> +#ifndef _WIN32
> +void iothread_event_increment(int *io_thread_fd)

Please split the WIN32 stuff into it's own file, similar to oslib-posix
and oslib-win32.c etc.

> diff --git a/qemu-ioh.h b/qemu-ioh.h
> index 7c6e833..2c714a9 100644
> --- a/qemu-ioh.h
> +++ b/qemu-ioh.h
> @@ -31,4 +31,13 @@ void qemu_get_fdset2(void *ioh_record_list, int *nfds, fd_set *rfds,
>  void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
>                                 const fd_set *wfds, const fd_set *xfds);
>  
> +
> +#ifndef _WIN32
> +void iothread_event_increment(int *io_thread_fd);
> +int iothread_event_init(int *io_thread_fd);
> +#else
> +int win32_event_init(HANDLE *qemu_event_handle);
> +void win32_event_increment(HANDLE *qemu_event_handle);
> +#endif

Can you not do something slightly nicer that allows for those to be the
same prototype for all users? Like define a event_handle_t?

> +
> +#ifndef _WIN32
> +static int io_thread_fd = -1;

Needs splitting into separate files too.

> diff --git a/qemu-tool.h b/qemu-tool.h
> new file mode 100644
> index 0000000..fd693cf
> --- /dev/null
> +++ b/qemu-tool.h
> @@ -0,0 +1,26 @@
> +#ifndef QEMU_TOOL_H
> +#define QEMU_TOOL_H
> +
> +#include "qemu-common.h"
> +
> +#ifdef CONFIG_EVENTFD
> +#include <sys/eventfd.h>
> +#endif
> +
> +typedef void VMStateDescription;
> +typedef int VMStateInfo;
> +
> +#ifndef _WIN32
> +void qemu_event_increment(void);
> +int qemu_event_init(void);
> +#else
> +int qemu_event_init(void);
> +void qemu_event_increment(void);
> +#endif

No matter how long I stare at those prototypes, I fail to see the
difference between the win32 and the posix version :)

Cheers,
Jes