[Qemu-devel] Re: [patch 1/2] qemu: sem/thread helpers

[Anthony Liguori <aliguori@us.ibm.com>]

Marcelo Tosatti wrote:
> Index: qemu/Makefile.target
> ===================================================================
> --- qemu.orig/Makefile.target
> +++ qemu/Makefile.target
> @@ -500,7 +500,7 @@ endif #CONFIG_BSD_USER
>  # System emulator target
>  ifndef CONFIG_USER_ONLY
>
> -OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o
> +OBJS=vl.o qemu-thread.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o
>  # virtio has to be here due to weird dependency between PCI and virtio-net.
>  # need to fix this properly
>  OBJS+=virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
> Index: qemu/vl.c
> ===================================================================
> --- qemu.orig/vl.c
> +++ qemu/vl.c
> @@ -36,6 +36,7 @@
>  #include "gdbstub.h"
>  #include "qemu-timer.h"
>  #include "qemu-char.h"
> +#include "qemu-thread.h"
>  #include "cache-utils.h"
>  #include "block.h"
>  #include "audio/audio.h"
> @@ -263,6 +264,8 @@ static QEMUTimer *nographic_timer;
>
>  uint8_t qemu_uuid[16];
>
> +QemuSem qemu_sem;
> +
>  /***********************************************************/
>  /* x86 ISA bus support */
>
> @@ -3650,7 +3653,10 @@ void main_loop_wait(int timeout)
>          slirp_select_fill(&nfds, &rfds, &wfds, &xfds);
>      }
>  #endif
> +
> +    qemu_sem_unlock(&qemu_sem);
>      ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
> +    qemu_sem_lock(&qemu_sem);
>   

I've looked at this a number of times, so I feel comfortable with this, 
but I think it's subtle enough that it deserves a comment.

main_loop_wait() *must* not assume any global state is consistent across 
select() invocations.  We should make that very clear as it's a detail 
that can be easily missed.

Regards,

Anthony Liguori