Re: [Qemu-devel] [PATCH] 9p: Print error hints if option parsing fails

[Greg Kurz]

On Sat, 14 Sep 2019 20:59:46 +0200
Markus Armbruster  wrote:

> Greg Kurz  writes:
> 
> > Option parsing fonctions are called with _fatal, which
> 
> functions
> 
> > causes error_setg() to call exit() and the hints are never
> > printed.
> >
> > Use an intermediate error object so that exit() happens in
> > error_propagate() after error_append_hint() could be called.
> 
> Hmm.
> 
> Code that creates error objects should not need to know how they are
> handled.
> 

Agreed.

> Your patch shows that error_append_hint() requires error_propagate() to
> work regardless of how the error is handled.  We should amend
> error_append_hint()'s contract in error.h to spell this out, and search
> the tree for more misuse of error_append_hint().

Sure. I'll take care of that.

Cheers,

--
Greg

Re: [Qemu-devel] [PATCH] 9p: Print error hints if option parsing fails

[Markus Armbruster]

Greg Kurz  writes:

> Option parsing fonctions are called with _fatal, which

functions

> causes error_setg() to call exit() and the hints are never
> printed.
>
> Use an intermediate error object so that exit() happens in
> error_propagate() after error_append_hint() could be called.

Hmm.

Code that creates error objects should not need to know how they are
handled.

Your patch shows that error_append_hint() requires error_propagate() to
work regardless of how the error is handled.  We should amend
error_append_hint()'s contract in error.h to spell this out, and search
the tree for more misuse of error_append_hint().

[Qemu-devel] [PATCH] 9p: Print error hints if option parsing fails

[Greg Kurz]

Option parsing fonctions are called with _fatal, which
causes error_setg() to call exit() and the hints are never
printed.

Use an intermediate error object so that exit() happens in
error_propagate() after error_append_hint() could be called.

Signed-off-by: Greg Kurz 
---
 hw/9pfs/9p-local.c |   11 +++
 hw/9pfs/9p-proxy.c |   11 +++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 6f7309f4e691..2c80d2ac30f5 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1486,8 +1486,9 @@ static int local_parse_opts(QemuOpts *opts, FsDriverEntry 
*fse, Error **errp)
 Error *local_err = NULL;
 
 if (!sec_model) {
-error_setg(errp, "security_model property not set");
-error_append_security_model_hint(errp);
+error_setg(_err, "security_model property not set");
+error_append_security_model_hint(_err);
+error_propagate(errp, local_err);
 return -1;
 }
 
@@ -1501,8 +1502,10 @@ static int local_parse_opts(QemuOpts *opts, 
FsDriverEntry *fse, Error **errp)
 } else if (!strcmp(sec_model, "mapped-file")) {
 fse->export_flags |= V9FS_SM_MAPPED_FILE;
 } else {
-error_setg(errp, "invalid security_model property '%s'", sec_model);
-error_append_security_model_hint(errp);
+error_setg(_err, "invalid security_model property '%s'",
+   sec_model);
+error_append_security_model_hint(_err);
+error_propagate(errp, local_err);
 return -1;
 }
 
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 97ab9c58a573..5de5e53702f7 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -1126,15 +1126,18 @@ static int proxy_parse_opts(QemuOpts *opts, 
FsDriverEntry *fs, Error **errp)
 {
 const char *socket = qemu_opt_get(opts, "socket");
 const char *sock_fd = qemu_opt_get(opts, "sock_fd");
+Error *local_err = NULL;
 
 if (!socket && !sock_fd) {
-error_setg(errp, "both socket and sock_fd properties are missing");
-error_append_socket_sockfd_hint(errp);
+error_setg(_err, "both socket and sock_fd properties are 
missing");
+error_append_socket_sockfd_hint(_err);
+error_propagate(errp, local_err);
 return -1;
 }
 if (socket && sock_fd) {
-error_setg(errp, "both socket and sock_fd properties are set");
-error_append_socket_sockfd_hint(errp);
+error_setg(_err, "both socket and sock_fd properties are set");
+error_append_socket_sockfd_hint(_err);
+error_propagate(errp, local_err);
 return -1;
 }
 if (socket) {