[Qemu-devel] [PATCH] vhost-scsi: Improve error reporting for invalid vhostfd

[Qemu-devel] [PATCH] vhost-scsi: Improve error reporting for invalid vhostfd

[Markus Armbruster]

We get two error messages: one from monitor_handle_fd_param2(), and
another one from vhost_scsi_realize().  The second one gets suppressed
in QMP context.

That's because monitor_handle_fd_param() calls qerror_report_err().
Calling qerror_report_err() is always inappropriate in realize
methods, because it doesn't return the Error object.  It either
reports the error to stderr or the human monitor, or it stores it in
the QMP monitor, where it makes the QMP command fail even when the
realize method ignores the error and succeeds.  Fortunately,
vhost_scsi_realize() doesn't do that.

Fix by switching to monitor_handle_fd_param2().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi/vhost-scsi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index dcb2bc5..567f350 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -214,9 +214,11 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     }
 
     if (vs->conf.vhostfd) {
-        vhostfd = monitor_handle_fd_param(cur_mon, vs->conf.vhostfd);
+        vhostfd = monitor_handle_fd_param2(cur_mon, vs->conf.vhostfd, &err);
         if (vhostfd == -1) {
-            error_setg(errp, "vhost-scsi: unable to parse vhostfd");
+            error_setg(errp, "vhost-scsi: unable to parse vhostfd: %s",
+                       error_get_pretty(err));
+            error_free(err);
             return;
         }
     } else {
-- 
1.9.3

Re: [Qemu-devel] [PATCH] vhost-scsi: Improve error reporting for invalid vhostfd

[Paolo Bonzini]

On 09/02/2015 11:52, Markus Armbruster wrote:
> We get two error messages: one from monitor_handle_fd_param2(), and
> another one from vhost_scsi_realize().  The second one gets suppressed
> in QMP context.
> 
> That's because monitor_handle_fd_param() calls qerror_report_err().
> Calling qerror_report_err() is always inappropriate in realize
> methods, because it doesn't return the Error object.  It either
> reports the error to stderr or the human monitor, or it stores it in
> the QMP monitor, where it makes the QMP command fail even when the
> realize method ignores the error and succeeds.  Fortunately,
> vhost_scsi_realize() doesn't do that.
> 
> Fix by switching to monitor_handle_fd_param2().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/scsi/vhost-scsi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index dcb2bc5..567f350 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -214,9 +214,11 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
>      }
>  
>      if (vs->conf.vhostfd) {
> -        vhostfd = monitor_handle_fd_param(cur_mon, vs->conf.vhostfd);
> +        vhostfd = monitor_handle_fd_param2(cur_mon, vs->conf.vhostfd, &err);
>          if (vhostfd == -1) {
> -            error_setg(errp, "vhost-scsi: unable to parse vhostfd");
> +            error_setg(errp, "vhost-scsi: unable to parse vhostfd: %s",
> +                       error_get_pretty(err));
> +            error_free(err);
>              return;
>          }
>      } else {
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>