Re: [Qemu-devel] [PATCH 02/12] monitor: add Error * argument to monitor_get_fd

[Luiz Capitulino]

On Wed, 19 Sep 2012 16:31:05 +0200
Paolo Bonzini <address@hidden> wrote:

> Differentiate monitor_get_fd and monitor_handle_fd_param by doing
> correct error propagation in the former and its callers.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  dump.c    | 5 +++--
>  monitor.c | 9 ++++++---
>  monitor.h | 2 +-
>  3 file modificati, 10 inserzioni(+), 6 rimozioni(-)
> 
> diff --git a/dump.c b/dump.c
> index 2bf8d8d..cc7aecd 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -824,6 +824,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, 
> bool has_begin,
>      int fd = -1;
>      DumpState *s;
>      int ret;
> +    Error *local_err = NULL;
>  
>      if (has_begin && !has_length) {
>          error_set(errp, QERR_MISSING_PARAMETER, "length");
> @@ -836,9 +837,9 @@ void qmp_dump_guest_memory(bool paging, const char *file, 
> bool has_begin,
>  
>  #if !defined(WIN32)
>      if (strstart(file, "fd:", &p)) {
> -        fd = monitor_get_fd(cur_mon, p);
> +        fd = monitor_get_fd(cur_mon, p, &local_err);
>          if (fd == -1) {
> -            error_set(errp, QERR_FD_NOT_FOUND, p);
> +            error_propagate(errp, local_err);
>              return;
>          }
>      }
> diff --git a/monitor.c b/monitor.c
> index 4901600..b73ae57 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2118,7 +2118,7 @@ static void do_loadvm(Monitor *mon, const QDict *qdict)
>      }
>  }
>  
> -int monitor_get_fd(Monitor *mon, const char *fdname)
> +int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
>  {
>      mon_fd_t *monfd;
>  
> @@ -2139,6 +2139,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
>          return fd;
>      }
>  
> +    error_set(errp, QERR_FD_NOT_FOUND, fdname);

Let's use error_setg() instead, also allows for dropping QERR_FD_NOT_FOUND.

Looks good otherwise.

>      return -1;
>  }
>  
> @@ -2410,12 +2411,14 @@ int monitor_fdset_dup_fd_remove(int dup_fd)
>  int monitor_handle_fd_param(Monitor *mon, const char *fdname)
>  {
>      int fd;
> +    Error *local_err = NULL;
>  
>      if (!qemu_isdigit(fdname[0]) && mon) {
>  
> -        fd = monitor_get_fd(mon, fdname);
> +        fd = monitor_get_fd(mon, fdname, &local_err);
>          if (fd == -1) {
> -            error_report("No file descriptor named %s found", fdname);
> +            qerror_report_err(local_err);
> +            error_free(local_err);
>              return -1;
>          }
>      } else {
> diff --git a/monitor.h b/monitor.h
> index 64c1561..e240c3f 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -66,7 +66,7 @@ int monitor_read_block_device_key(Monitor *mon, const char 
> *device,
>                                    BlockDriverCompletionFunc *completion_cb,
>                                    void *opaque);
>  
> -int monitor_get_fd(Monitor *mon, const char *fdname);
> +int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
>  int monitor_handle_fd_param(Monitor *mon, const char *fdname);
>  
>  void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)