Re: [Qemu-devel] [Spice-devel] [PATCH 20/21] spice-core: allow an interface to be in AIO context

[Alon Levy]

On 11/18/2013 02:25 PM, Marc-André Lureau wrote:
> The Spice block driver must be able complete operations within a AIO
> context only.
> 
> Spice is currently only running within the main loop, and doesn't allow
> the block driver to complete operations, such as flush during migration.
> 
> This patch allows a Spice interface to be associated with a different
> context. Currently, the interface user_data is simply used to
> differentiate main loop from AIO, but could later be used to associate
> an interface with a particular thread.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  include/ui/qemu-spice.h |  2 +-
>  qemu-char.c             |  2 +-
>  spice-qemu-char.c       |  9 +++----
>  ui/spice-core.c         | 62 
> +++++++++++++++++++++++++++++++++++++++++++------
>  4 files changed, 62 insertions(+), 13 deletions(-)
> 
> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
> index a93b4b2..d5ba702 100644
> --- a/include/ui/qemu-spice.h
> +++ b/include/ui/qemu-spice.h
> @@ -48,7 +48,7 @@ int qemu_spice_migrate_info(const char *hostname, int port, 
> int tls_port,
>  void do_info_spice_print(Monitor *mon, const QObject *data);
>  void do_info_spice(Monitor *mon, QObject **ret_data);
>  
> -CharDriverState *qemu_chr_open_spice_vmc(const char *type);
> +CharDriverState *qemu_chr_open_spice_vmc(const char *type, bool aio);
>  #if SPICE_SERVER_VERSION >= 0x000c02
>  CharDriverState *qemu_chr_open_spice_port(const char *name);
>  void qemu_spice_register_ports(void);
> diff --git a/qemu-char.c b/qemu-char.c
> index 418dc69..bfac7bf 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3747,7 +3747,7 @@ ChardevReturn *qmp_chardev_add(const char *id, 
> ChardevBackend *backend,
>  #endif
>  #ifdef CONFIG_SPICE
>      case CHARDEV_BACKEND_KIND_SPICEVMC:
> -        chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
> +        chr = qemu_chr_open_spice_vmc(backend->spicevmc->type, false);
>          break;
>      case CHARDEV_BACKEND_KIND_SPICEPORT:
>          chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 16439c5..421f7de 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -248,7 +248,7 @@ static void print_allowed_subtypes(void)
>      fprintf(stderr, "\n");
>  }
>  
> -static CharDriverState *chr_open(const char *subtype)
> +static CharDriverState *chr_open(const char *subtype, bool aio)
>  {
>      CharDriverState *chr;
>      SpiceCharDriver *s;
> @@ -257,6 +257,7 @@ static CharDriverState *chr_open(const char *subtype)
>      s = g_malloc0(sizeof(SpiceCharDriver));
>      s->chr = chr;
>      s->active = false;
> +    s->sin.base.user_data = (void*)aio;
>      s->sin.subtype = g_strdup(subtype);
>      chr->opaque = s;
>      chr->chr_write = spice_chr_write;
> @@ -271,7 +272,7 @@ static CharDriverState *chr_open(const char *subtype)
>      return chr;
>  }
>  
> -CharDriverState *qemu_chr_open_spice_vmc(const char *type)
> +CharDriverState *qemu_chr_open_spice_vmc(const char *type, bool aio)
>  {
>      const char **psubtype = spice_server_char_device_recognized_subtypes();
>  
> @@ -291,7 +292,7 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
>          return NULL;
>      }
>  
> -    return chr_open(type);
> +    return chr_open(type, aio);
>  }
>  
>  #if SPICE_SERVER_VERSION >= 0x000c02
> @@ -305,7 +306,7 @@ CharDriverState *qemu_chr_open_spice_port(const char 
> *name)
>          return NULL;
>      }
>  
> -    chr = chr_open("port");
> +    chr = chr_open("port", false);
>      s = chr->opaque;
>      s->sin.portname = g_strdup(name);
>  
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index e4d533d..0f69630 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -53,34 +53,64 @@ static QemuThread me;
>  
>  struct SpiceTimer {
>      QEMUTimer *timer;
> +    QEMUBH *bh;
>      QTAILQ_ENTRY(SpiceTimer) next;
>  };
>  static QTAILQ_HEAD(, SpiceTimer) timers = QTAILQ_HEAD_INITIALIZER(timers);
>  
> +#if SPICE_INTERFACE_CORE_MAJOR >= 2
> +static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque, 
> SpiceBaseInstance *sin)
> +#else
>  static SpiceTimer *timer_add(SpiceTimerFunc func, void *opaque)
> +#endif
>  {
>      SpiceTimer *timer;
>  
>      timer = g_malloc0(sizeof(*timer));
> -    timer->timer = timer_new_ms(QEMU_CLOCK_REALTIME, func, opaque);
> +
> +#if SPICE_INTERFACE_CORE_MAJOR >= 2
> +    bool aio = sin ? !!sin->user_data : false;

Shouldn't there be a cast there:
(bool)sin->user_data
?


> +    if (aio) {
> +        fprintf(stderr, "AIO doesn't have timers yet, using BH\n");
> +        timer->bh = qemu_bh_new(func, opaque);
> +    } else
> +#endif
> +    {
> +        timer->timer = timer_new_ms(QEMU_CLOCK_REALTIME, func, opaque);
> +    }
> +
>      QTAILQ_INSERT_TAIL(&timers, timer, next);
> +
>      return timer;
>  }
>  
>  static void timer_start(SpiceTimer *timer, uint32_t ms)
>  {
> -    timer_mod(timer->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + ms);
> +    if (timer->bh) {
> +        qemu_bh_schedule_idle(timer->bh); /* at least every 10ms, see 
> async.c */
> +    } else {
> +        timer_mod(timer->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + ms);
> +    }
>  }
>  
>  static void timer_cancel(SpiceTimer *timer)
>  {
> -    timer_del(timer->timer);
> +    if (timer->bh) {
> +        qemu_bh_cancel(timer->bh);
> +    } else {
> +        timer_del(timer->timer);
> +    }
>  }
>  
>  static void timer_remove(SpiceTimer *timer)
>  {
> -    timer_del(timer->timer);
> -    timer_free(timer->timer);
> +    if (timer->bh) {
> +        qemu_bh_delete(timer->bh);
> +    } else {
> +        timer_del(timer->timer);
> +        timer_free(timer->timer);
> +    }
> +
>      QTAILQ_REMOVE(&timers, timer, next);
>      g_free(timer);
>  }
> @@ -89,6 +119,7 @@ struct SpiceWatch {
>      int fd;
>      int event_mask;
>      SpiceWatchFunc func;
> +    bool aio;
>      void *opaque;
>      QTAILQ_ENTRY(SpiceWatch) next;
>  };
> @@ -118,10 +149,19 @@ static void watch_update_mask(SpiceWatch *watch, int 
> event_mask)
>      if (watch->event_mask & SPICE_WATCH_EVENT_WRITE) {
>          on_write = watch_write;
>      }
> -    qemu_set_fd_handler(watch->fd, on_read, on_write, watch);
> +
> +    if (watch->aio) {
> +        qemu_aio_set_fd_handler(watch->fd, on_read, on_write, watch);
> +    } else {
> +        qemu_set_fd_handler(watch->fd, on_read, on_write, watch);
> +    }
>  }
>  
> +#if SPICE_INTERFACE_CORE_MAJOR >= 2
> +static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, 
> void *opaque, SpiceBaseInstance *sin)
> +#else
>  static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, 
> void *opaque)
> +#endif
>  {
>      SpiceWatch *watch;
>  
> @@ -129,6 +169,10 @@ static SpiceWatch *watch_add(int fd, int event_mask, 
> SpiceWatchFunc func, void *
>      watch->fd     = fd;
>      watch->func   = func;
>      watch->opaque = opaque;
> +#if SPICE_INTERFACE_CORE_MAJOR >= 2
> +    watch->aio    = sin ? !!sin->user_data : false;
> +#endif
> +
>      QTAILQ_INSERT_TAIL(&watches, watch, next);
>  
>      watch_update_mask(watch, event_mask);
> @@ -137,7 +181,11 @@ static SpiceWatch *watch_add(int fd, int event_mask, 
> SpiceWatchFunc func, void *
>  
>  static void watch_remove(SpiceWatch *watch)
>  {
> -    qemu_set_fd_handler(watch->fd, NULL, NULL, NULL);
> +    if (watch->aio) {
> +        qemu_aio_set_fd_handler(watch->fd, NULL, NULL, NULL);
> +    } else {
> +        qemu_set_fd_handler(watch->fd, NULL, NULL, NULL);
> +    }
>      QTAILQ_REMOVE(&watches, watch, next);
>      g_free(watch);
>  }
>