Re: [PATCH 3/4] monitor: add more *_locked() functions

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Allow flushing and printing to the monitor while mon->mon_lock is
> held.  This will help cleaning up the locking of mon->mux_out and
> mon->suspend_cnt.

Actually, the patch adds just monitor_puts_locked().
monitor_flush_locked() it merely gives external linkage.  Suggest

  monitor: More *_locked() functions

  Being able to print to the monitor and flush while mon->mon_lock is
  held will help cleaning up the locking of mon->mux_out and
  mon->suspend_cnt in the next commit.  Give monitor_flush_locked()
  external linkage, and factor monitor_puts_locked() out of
  monitor_puts().
  
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/monitor/monitor.h |  3 +++
>  monitor/monitor.c         | 14 ++++++++------
>  2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 033390f69917..965f5d545003 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -40,6 +40,9 @@ void monitor_flush(Monitor *mon);
>  int monitor_set_cpu(Monitor *mon, int cpu_index);
>  int monitor_get_cpu_index(Monitor *mon);
>  
> +int monitor_puts_locked(Monitor *mon, const char *str);
> +void monitor_flush_locked(Monitor *mon);
> +
>  void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp);
>  
>  void monitor_read_command(MonitorHMP *mon, int show_prompt);
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 7080d2da8ec6..20e33e28d20d 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -154,8 +154,6 @@ static inline bool monitor_is_hmp_non_interactive(const 
> Monitor *mon)
>      return !monitor_uses_readline(container_of(mon, MonitorHMP, common));
>  }
>  
> -static void monitor_flush_locked(Monitor *mon);
> -
>  static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
>                                    void *opaque)
>  {
> @@ -168,7 +166,7 @@ static gboolean monitor_unblocked(void *do_not_use, 
> GIOCondition cond,
>  }
>  
>  /* Caller must hold mon->mon_lock */
> -static void monitor_flush_locked(Monitor *mon)
> +void monitor_flush_locked(Monitor *mon)
>  {
>      int rc;
>      size_t len;
> @@ -207,12 +205,11 @@ void monitor_flush(Monitor *mon)
>  }
>  
>  /* flush at every end of line */
> -int monitor_puts(Monitor *mon, const char *str)
> +int monitor_puts_locked(Monitor *mon, const char *str)
>  {
>      int i;
>      char c;
>  
> -    qemu_mutex_lock(&mon->mon_lock);
>      for (i = 0; str[i]; i++) {
>          c = str[i];
>          if (c == '\n') {
> @@ -223,11 +220,16 @@ int monitor_puts(Monitor *mon, const char *str)
>              monitor_flush_locked(mon);
>          }
>      }
> -    qemu_mutex_unlock(&mon->mon_lock);
>  
>      return i;
>  }
>  
> +int monitor_puts(Monitor *mon, const char *str)
> +{
> +    QEMU_LOCK_GUARD(&mon->mon_lock);
> +    return monitor_puts_locked(mon, str);
> +}
> +
>  int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
>  {
>      char *buf;

Prefereably with a clarified commit message
Reviewed-by: Markus Armbruster <armbru@redhat.com>