Re: [Qemu-devel] [PATCH 2/2] monitor: maintain at most one G_IO_OUT watch

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

Laszlo Ersek <lersek@redhat.com> writes:

> When monitor_flush() is invoked repeatedly outside the monitor_unblocked()
> callback, for example from tlb_info() -> ... -> print_pte(), several
> watches may be added for the same event.
>
> This is no problem per se because the extra monitor_unblocked() callbacks
> are harmless if mon->outbuf is empty, the watches will be removed
> gradually. However a big number of watches can grow "gpollfds" without
> limit in glib_pollfds_fill(), triggering a -1/EINVAL condition in
> g_poll().
>
> Keep at most one such watch, by following the pattern observable in eg.
> commits c874ea97 and c3d6b96e. The change has no effect when
> monitor_unblocked() calls monitor_flush() (when the watch can either be
> removed or renewed 1-for-1), but non-callback contexts won't create an
> additional watch when the monitor already has one.
>
> Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=970047
>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>  monitor.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 2ba7876..de24b2c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -189,6 +189,7 @@ struct Monitor {
>      int suspend_cnt;
>      bool skip_flush;
>      QString *outbuf;
> +    guint watch;
>      ReadLineState *rs;
>      MonitorControl *mc;
>      CPUState *mon_cpu;
> @@ -263,7 +264,10 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>  static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
>                                    void *opaque)
>  {
> -    monitor_flush(opaque);
> +    Monitor *mon = opaque;
> +
> +    mon->watch = 0;
> +    monitor_flush(mon);
>      return FALSE;
>  }
>  
> @@ -294,7 +298,10 @@ void monitor_flush(Monitor *mon)
>              QDECREF(mon->outbuf);
>              mon->outbuf = tmp;
>          }
> -        qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
> +        if (mon->watch == 0) {
> +            mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT,
> +                                               monitor_unblocked, mon);
> +        }
>      }
>  }
>  
> -- 
> 1.7.1