qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr)
@ 2016-01-18 15:02 Greg Kurz
  2016-01-18 15:02 ` [Qemu-devel] [PATCH 1/2] 9pfs: " Greg Kurz
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Greg Kurz @ 2016-01-18 15:02 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Greg Kurz, qemu-devel, Aneesh Kumar K.V

Hi,

This series moves all the 9pfs/fsdev code to use error_report(), with the
notable exception of virtfs-proxy-helper, which doesn't need it.

Markus,

Should this patches go through your tree ? Or can they go through my 9p tree
if you ack them ?

Cheers.

--
Greg

---

Greg Kurz (2):
      9pfs: use error_report() instead of fprintf(stderr)
      fsdev: use error_report() instead of fprintf(stderr)


 fsdev/qemu-fsdev.c  |    7 ++++---
 hw/9pfs/9p-handle.c |    5 +++--
 hw/9pfs/9p-local.c  |   15 ++++++++-------
 hw/9pfs/9p-proxy.c  |   12 ++++++------
 hw/9pfs/9p.c        |    2 +-
 5 files changed, 22 insertions(+), 19 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/2] 9pfs: use error_report() instead of fprintf(stderr)
  2016-01-18 15:02 [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr) Greg Kurz
@ 2016-01-18 15:02 ` Greg Kurz
  2016-01-18 16:35   ` Markus Armbruster
  2016-01-18 15:02 ` [Qemu-devel] [PATCH 2/2] fsdev: " Greg Kurz
  2016-01-18 16:39 ` [Qemu-devel] [PATCH 0/2] 9pfs: " Markus Armbruster
  2 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2016-01-18 15:02 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Greg Kurz, qemu-devel, Aneesh Kumar K.V

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/9pfs/9p-handle.c |    5 +++--
 hw/9pfs/9p-local.c  |   15 ++++++++-------
 hw/9pfs/9p-proxy.c  |   12 ++++++------
 hw/9pfs/9p.c        |    2 +-
 4 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c
index 58b77b4c942d..8ba88775a2b6 100644
--- a/hw/9pfs/9p-handle.c
+++ b/hw/9pfs/9p-handle.c
@@ -19,6 +19,7 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include "qemu/xattr.h"
+#include "qemu/error-report.h"
 #include <unistd.h>
 #include <linux/fs.h>
 #ifdef CONFIG_LINUX_MAGIC_H
@@ -655,12 +656,12 @@ static int handle_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
     const char *path = qemu_opt_get(opts, "path");
 
     if (sec_model) {
-        fprintf(stderr, "Invalid argument security_model specified with handle fsdriver\n");
+        error_report("Invalid argument security_model specified with handle fsdriver");
         return -1;
     }
 
     if (!path) {
-        fprintf(stderr, "fsdev: No path specified.\n");
+        error_report("fsdev: No path specified.");
         return -1;
     }
     fse->path = g_strdup(path);
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index bf63eab729ad..9c25ab2db26b 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -20,6 +20,7 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include "qemu/xattr.h"
+#include "qemu/error-report.h"
 #include <libgen.h>
 #include <linux/fs.h>
 #ifdef CONFIG_LINUX_MAGIC_H
@@ -1209,9 +1210,9 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
     const char *path = qemu_opt_get(opts, "path");
 
     if (!sec_model) {
-        fprintf(stderr, "security model not specified, "
-                "local fs needs security model\nvalid options are:"
-                "\tsecurity_model=[passthrough|mapped|none]\n");
+        error_report("security model not specified, local fs needs security model");
+        error_printf("valid options are:"
+                     "\tsecurity_model=[passthrough|mapped-xattr|mapped-file|none]\n");
         return -1;
     }
 
@@ -1225,14 +1226,14 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
     } else if (!strcmp(sec_model, "mapped-file")) {
         fse->export_flags |= V9FS_SM_MAPPED_FILE;
     } else {
-        fprintf(stderr, "Invalid security model %s specified, valid options are"
-                "\n\t [passthrough|mapped-xattr|mapped-file|none]\n",
-                sec_model);
+        error_report("Invalid security model %s specified, valid options are",
+                     sec_model);
+        error_printf("\t [passthrough|mapped-xattr|mapped-file|none]\n");
         return -1;
     }
 
     if (!path) {
-        fprintf(stderr, "fsdev: No path specified.\n");
+        error_report("fsdev: No path specified.");
         return -1;
     }
     fse->path = g_strdup(path);
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 73d00dd74d11..72b9952d7c8b 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -1100,19 +1100,19 @@ static int connect_namedsocket(const char *path)
     struct sockaddr_un helper;
 
     if (strlen(path) >= sizeof(helper.sun_path)) {
-        fprintf(stderr, "Socket name too large\n");
+        error_report("Socket name too large");
         return -1;
     }
     sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (sockfd < 0) {
-        fprintf(stderr, "failed to create socket: %s\n", strerror(errno));
+        error_report("failed to create socket: %s", strerror(errno));
         return -1;
     }
     strcpy(helper.sun_path, path);
     helper.sun_family = AF_UNIX;
     size = strlen(helper.sun_path) + sizeof(helper.sun_family);
     if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) {
-        fprintf(stderr, "failed to connect to %s: %s\n", path, strerror(errno));
+        error_report("failed to connect to %s: %s", path, strerror(errno));
         close(sockfd);
         return -1;
     }
@@ -1128,11 +1128,11 @@ static int proxy_parse_opts(QemuOpts *opts, struct FsDriverEntry *fs)
     const char *sock_fd = qemu_opt_get(opts, "sock_fd");
 
     if (!socket && !sock_fd) {
-        fprintf(stderr, "socket and sock_fd none of the option specified\n");
+        error_report("socket and sock_fd none of the option specified");
         return -1;
     }
     if (socket && sock_fd) {
-        fprintf(stderr, "Both socket and sock_fd options specified\n");
+        error_report("Both socket and sock_fd options specified");
         return -1;
     }
     if (socket) {
@@ -1155,7 +1155,7 @@ static int proxy_init(FsContext *ctx)
     } else {
         sock_id = atoi(ctx->fs_root);
         if (sock_id < 0) {
-            fprintf(stderr, "socket descriptor not initialized\n");
+            error_report("socket descriptor not initialized");
         }
     }
     if (sock_id < 0) {
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 3ff310605cd4..1f3bd12e0d0c 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3370,7 +3370,7 @@ static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
 {
     struct rlimit rlim;
     if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
-        fprintf(stderr, "Failed to get the resource limit\n");
+        error_report("Failed to get the resource limit");
         exit(1);
     }
     open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3);

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/2] fsdev: use error_report() instead of fprintf(stderr)
  2016-01-18 15:02 [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr) Greg Kurz
  2016-01-18 15:02 ` [Qemu-devel] [PATCH 1/2] 9pfs: " Greg Kurz
@ 2016-01-18 15:02 ` Greg Kurz
  2016-01-18 16:39 ` [Qemu-devel] [PATCH 0/2] 9pfs: " Markus Armbruster
  2 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2016-01-18 15:02 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Greg Kurz, qemu-devel, Aneesh Kumar K.V

Only fix the code that gets built into QEMU.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 fsdev/qemu-fsdev.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index ccfec139ab2b..55e2f7a0fb58 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -17,6 +17,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/config-file.h"
+#include "qemu/error-report.h"
 
 static QTAILQ_HEAD(FsDriverEntry_head, FsDriverListEntry) fsdriver_entries =
     QTAILQ_HEAD_INITIALIZER(fsdriver_entries);
@@ -40,7 +41,7 @@ int qemu_fsdev_add(QemuOpts *opts)
     bool ro = qemu_opt_get_bool(opts, "readonly", 0);
 
     if (!fsdev_id) {
-        fprintf(stderr, "fsdev: No id specified\n");
+        error_report("fsdev: No id specified");
         return -1;
     }
 
@@ -52,11 +53,11 @@ int qemu_fsdev_add(QemuOpts *opts)
         }
 
         if (i == ARRAY_SIZE(FsDrivers)) {
-            fprintf(stderr, "fsdev: fsdriver %s not found\n", fsdriver);
+            error_report("fsdev: fsdriver %s not found", fsdriver);
             return -1;
         }
     } else {
-        fprintf(stderr, "fsdev: No fsdriver specified\n");
+        error_report("fsdev: No fsdriver specified");
         return -1;
     }
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] 9pfs: use error_report() instead of fprintf(stderr)
  2016-01-18 15:02 ` [Qemu-devel] [PATCH 1/2] 9pfs: " Greg Kurz
@ 2016-01-18 16:35   ` Markus Armbruster
  2016-01-19 11:25     ` Greg Kurz
  0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2016-01-18 16:35 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-devel, Aneesh Kumar K.V

Greg Kurz <gkurz@linux.vnet.ibm.com> writes:

> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>  hw/9pfs/9p-handle.c |    5 +++--
>  hw/9pfs/9p-local.c  |   15 ++++++++-------
>  hw/9pfs/9p-proxy.c  |   12 ++++++------
>  hw/9pfs/9p.c        |    2 +-
>  4 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c
> index 58b77b4c942d..8ba88775a2b6 100644
> --- a/hw/9pfs/9p-handle.c
> +++ b/hw/9pfs/9p-handle.c
> @@ -19,6 +19,7 @@
>  #include <sys/socket.h>
>  #include <sys/un.h>
>  #include "qemu/xattr.h"
> +#include "qemu/error-report.h"
>  #include <unistd.h>
>  #include <linux/fs.h>
>  #ifdef CONFIG_LINUX_MAGIC_H
> @@ -655,12 +656,12 @@ static int handle_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
>      const char *path = qemu_opt_get(opts, "path");
>  
>      if (sec_model) {
> -        fprintf(stderr, "Invalid argument security_model specified with handle fsdriver\n");
> +        error_report("Invalid argument security_model specified with handle fsdriver");
>          return -1;
>      }
>  
>      if (!path) {
> -        fprintf(stderr, "fsdev: No path specified.\n");
> +        error_report("fsdev: No path specified.");

Recommend to drop the period while there.

>          return -1;
>      }
>      fse->path = g_strdup(path);
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index bf63eab729ad..9c25ab2db26b 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -20,6 +20,7 @@
>  #include <sys/socket.h>
>  #include <sys/un.h>
>  #include "qemu/xattr.h"
> +#include "qemu/error-report.h"
>  #include <libgen.h>
>  #include <linux/fs.h>
>  #ifdef CONFIG_LINUX_MAGIC_H
> @@ -1209,9 +1210,9 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
>      const char *path = qemu_opt_get(opts, "path");
>  
>      if (!sec_model) {
> -        fprintf(stderr, "security model not specified, "
> -                "local fs needs security model\nvalid options are:"
> -                "\tsecurity_model=[passthrough|mapped|none]\n");
> +        error_report("security model not specified, local fs needs security model");
> +        error_printf("valid options are:"
> +                     "\tsecurity_model=[passthrough|mapped-xattr|mapped-file|none]\n");
>          return -1;
>      }
>  
> @@ -1225,14 +1226,14 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
>      } else if (!strcmp(sec_model, "mapped-file")) {
>          fse->export_flags |= V9FS_SM_MAPPED_FILE;
>      } else {
> -        fprintf(stderr, "Invalid security model %s specified, valid options are"
> -                "\n\t [passthrough|mapped-xattr|mapped-file|none]\n",
> -                sec_model);
> +        error_report("Invalid security model %s specified, valid options are",
> +                     sec_model);
> +        error_printf("\t [passthrough|mapped-xattr|mapped-file|none]\n");

Neater:

        error_report("Invalid security model %s specified", sec_model);
        error_printf("valid options are;"
                     "\t[passthrough|mapped-xattr|mapped-file|none]\n");

>          return -1;
>      }
>  
>      if (!path) {
> -        fprintf(stderr, "fsdev: No path specified.\n");
> +        error_report("fsdev: No path specified.");

Recommend to drop the period while there.

>          return -1;
>      }
>      fse->path = g_strdup(path);
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index 73d00dd74d11..72b9952d7c8b 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -1100,19 +1100,19 @@ static int connect_namedsocket(const char *path)
>      struct sockaddr_un helper;
>  
>      if (strlen(path) >= sizeof(helper.sun_path)) {
> -        fprintf(stderr, "Socket name too large\n");
> +        error_report("Socket name too large");

"too long" would be clearer, I think.

>          return -1;
>      }
>      sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
>      if (sockfd < 0) {
> -        fprintf(stderr, "failed to create socket: %s\n", strerror(errno));
> +        error_report("failed to create socket: %s", strerror(errno));
>          return -1;
>      }
>      strcpy(helper.sun_path, path);
>      helper.sun_family = AF_UNIX;
>      size = strlen(helper.sun_path) + sizeof(helper.sun_family);
>      if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) {
> -        fprintf(stderr, "failed to connect to %s: %s\n", path, strerror(errno));
> +        error_report("failed to connect to %s: %s", path, strerror(errno));
>          close(sockfd);
>          return -1;
>      }
> @@ -1128,11 +1128,11 @@ static int proxy_parse_opts(QemuOpts *opts, struct FsDriverEntry *fs)
>      const char *sock_fd = qemu_opt_get(opts, "sock_fd");
>  
>      if (!socket && !sock_fd) {
> -        fprintf(stderr, "socket and sock_fd none of the option specified\n");
> +        error_report("socket and sock_fd none of the option specified");

"Must specify either socket or sock_fd" would be clearer.

>          return -1;
>      }
>      if (socket && sock_fd) {
> -        fprintf(stderr, "Both socket and sock_fd options specified\n");
> +        error_report("Both socket and sock_fd options specified");
>          return -1;
>      }
>      if (socket) {
> @@ -1155,7 +1155,7 @@ static int proxy_init(FsContext *ctx)
>      } else {
>          sock_id = atoi(ctx->fs_root);
>          if (sock_id < 0) {
> -            fprintf(stderr, "socket descriptor not initialized\n");
> +            error_report("socket descriptor not initialized");
>          }
>      }
>      if (sock_id < 0) {
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 3ff310605cd4..1f3bd12e0d0c 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3370,7 +3370,7 @@ static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
>  {
>      struct rlimit rlim;
>      if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
> -        fprintf(stderr, "Failed to get the resource limit\n");
> +        error_report("Failed to get the resource limit");
>          exit(1);
>      }
>      open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr)
  2016-01-18 15:02 [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr) Greg Kurz
  2016-01-18 15:02 ` [Qemu-devel] [PATCH 1/2] 9pfs: " Greg Kurz
  2016-01-18 15:02 ` [Qemu-devel] [PATCH 2/2] fsdev: " Greg Kurz
@ 2016-01-18 16:39 ` Markus Armbruster
  2016-01-19 11:28   ` Greg Kurz
  2 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2016-01-18 16:39 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-devel, Aneesh Kumar K.V

Greg Kurz <gkurz@linux.vnet.ibm.com> writes:

> Hi,
>
> This series moves all the 9pfs/fsdev code to use error_report(), with the
> notable exception of virtfs-proxy-helper, which doesn't need it.
>
> Markus,
>
> Should this patches go through your tree ? Or can they go through my 9p tree
> if you ack them ?

It can certainly go through your tree!

My tree is meant for crosscutting error work.  I also offer it
maintainers who prefer to leave the pull request to me.

PATCH 1 could use a bit of polish, and I encourage you to respin.  But
it's not wrong, therefore

Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] 9pfs: use error_report() instead of fprintf(stderr)
  2016-01-18 16:35   ` Markus Armbruster
@ 2016-01-19 11:25     ` Greg Kurz
  0 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2016-01-19 11:25 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, Aneesh Kumar K.V

On Mon, 18 Jan 2016 17:35:25 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> 
> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> > ---

I agree to all your suggestions. Thanks !

> >  hw/9pfs/9p-handle.c |    5 +++--
> >  hw/9pfs/9p-local.c  |   15 ++++++++-------
> >  hw/9pfs/9p-proxy.c  |   12 ++++++------
> >  hw/9pfs/9p.c        |    2 +-
> >  4 files changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/hw/9pfs/9p-handle.c b/hw/9pfs/9p-handle.c
> > index 58b77b4c942d..8ba88775a2b6 100644
> > --- a/hw/9pfs/9p-handle.c
> > +++ b/hw/9pfs/9p-handle.c
> > @@ -19,6 +19,7 @@
> >  #include <sys/socket.h>
> >  #include <sys/un.h>
> >  #include "qemu/xattr.h"
> > +#include "qemu/error-report.h"
> >  #include <unistd.h>
> >  #include <linux/fs.h>
> >  #ifdef CONFIG_LINUX_MAGIC_H
> > @@ -655,12 +656,12 @@ static int handle_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
> >      const char *path = qemu_opt_get(opts, "path");
> >  
> >      if (sec_model) {
> > -        fprintf(stderr, "Invalid argument security_model specified with handle fsdriver\n");
> > +        error_report("Invalid argument security_model specified with handle fsdriver");
> >          return -1;
> >      }
> >  
> >      if (!path) {
> > -        fprintf(stderr, "fsdev: No path specified.\n");
> > +        error_report("fsdev: No path specified.");  
> 
> Recommend to drop the period while there.
> 
> >          return -1;
> >      }
> >      fse->path = g_strdup(path);
> > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> > index bf63eab729ad..9c25ab2db26b 100644
> > --- a/hw/9pfs/9p-local.c
> > +++ b/hw/9pfs/9p-local.c
> > @@ -20,6 +20,7 @@
> >  #include <sys/socket.h>
> >  #include <sys/un.h>
> >  #include "qemu/xattr.h"
> > +#include "qemu/error-report.h"
> >  #include <libgen.h>
> >  #include <linux/fs.h>
> >  #ifdef CONFIG_LINUX_MAGIC_H
> > @@ -1209,9 +1210,9 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
> >      const char *path = qemu_opt_get(opts, "path");
> >  
> >      if (!sec_model) {
> > -        fprintf(stderr, "security model not specified, "
> > -                "local fs needs security model\nvalid options are:"
> > -                "\tsecurity_model=[passthrough|mapped|none]\n");
> > +        error_report("security model not specified, local fs needs security model");
> > +        error_printf("valid options are:"
> > +                     "\tsecurity_model=[passthrough|mapped-xattr|mapped-file|none]\n");
> >          return -1;
> >      }
> >  
> > @@ -1225,14 +1226,14 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
> >      } else if (!strcmp(sec_model, "mapped-file")) {
> >          fse->export_flags |= V9FS_SM_MAPPED_FILE;
> >      } else {
> > -        fprintf(stderr, "Invalid security model %s specified, valid options are"
> > -                "\n\t [passthrough|mapped-xattr|mapped-file|none]\n",
> > -                sec_model);
> > +        error_report("Invalid security model %s specified, valid options are",
> > +                     sec_model);
> > +        error_printf("\t [passthrough|mapped-xattr|mapped-file|none]\n");  
> 
> Neater:
> 
>         error_report("Invalid security model %s specified", sec_model);
>         error_printf("valid options are;"
>                      "\t[passthrough|mapped-xattr|mapped-file|none]\n");
> 
> >          return -1;
> >      }
> >  
> >      if (!path) {
> > -        fprintf(stderr, "fsdev: No path specified.\n");
> > +        error_report("fsdev: No path specified.");  
> 
> Recommend to drop the period while there.
> 
> >          return -1;
> >      }
> >      fse->path = g_strdup(path);
> > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> > index 73d00dd74d11..72b9952d7c8b 100644
> > --- a/hw/9pfs/9p-proxy.c
> > +++ b/hw/9pfs/9p-proxy.c
> > @@ -1100,19 +1100,19 @@ static int connect_namedsocket(const char *path)
> >      struct sockaddr_un helper;
> >  
> >      if (strlen(path) >= sizeof(helper.sun_path)) {
> > -        fprintf(stderr, "Socket name too large\n");
> > +        error_report("Socket name too large");  
> 
> "too long" would be clearer, I think.
> 
> >          return -1;
> >      }
> >      sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
> >      if (sockfd < 0) {
> > -        fprintf(stderr, "failed to create socket: %s\n", strerror(errno));
> > +        error_report("failed to create socket: %s", strerror(errno));
> >          return -1;
> >      }
> >      strcpy(helper.sun_path, path);
> >      helper.sun_family = AF_UNIX;
> >      size = strlen(helper.sun_path) + sizeof(helper.sun_family);
> >      if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) {
> > -        fprintf(stderr, "failed to connect to %s: %s\n", path, strerror(errno));
> > +        error_report("failed to connect to %s: %s", path, strerror(errno));
> >          close(sockfd);
> >          return -1;
> >      }
> > @@ -1128,11 +1128,11 @@ static int proxy_parse_opts(QemuOpts *opts, struct FsDriverEntry *fs)
> >      const char *sock_fd = qemu_opt_get(opts, "sock_fd");
> >  
> >      if (!socket && !sock_fd) {
> > -        fprintf(stderr, "socket and sock_fd none of the option specified\n");
> > +        error_report("socket and sock_fd none of the option specified");  
> 
> "Must specify either socket or sock_fd" would be clearer.
> 
> >          return -1;
> >      }
> >      if (socket && sock_fd) {
> > -        fprintf(stderr, "Both socket and sock_fd options specified\n");
> > +        error_report("Both socket and sock_fd options specified");
> >          return -1;
> >      }
> >      if (socket) {
> > @@ -1155,7 +1155,7 @@ static int proxy_init(FsContext *ctx)
> >      } else {
> >          sock_id = atoi(ctx->fs_root);
> >          if (sock_id < 0) {
> > -            fprintf(stderr, "socket descriptor not initialized\n");
> > +            error_report("socket descriptor not initialized");
> >          }
> >      }
> >      if (sock_id < 0) {
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 3ff310605cd4..1f3bd12e0d0c 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -3370,7 +3370,7 @@ static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
> >  {
> >      struct rlimit rlim;
> >      if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) {
> > -        fprintf(stderr, "Failed to get the resource limit\n");
> > +        error_report("Failed to get the resource limit");
> >          exit(1);
> >      }
> >      open_fd_hw = rlim.rlim_cur - MIN(400, rlim.rlim_cur/3);  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr)
  2016-01-18 16:39 ` [Qemu-devel] [PATCH 0/2] 9pfs: " Markus Armbruster
@ 2016-01-19 11:28   ` Greg Kurz
  2016-01-19 12:19     ` Markus Armbruster
  0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2016-01-19 11:28 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, Aneesh Kumar K.V

On Mon, 18 Jan 2016 17:39:28 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
> 
> > Hi,
> >
> > This series moves all the 9pfs/fsdev code to use error_report(), with the
> > notable exception of virtfs-proxy-helper, which doesn't need it.
> >
> > Markus,
> >
> > Should this patches go through your tree ? Or can they go through my 9p tree
> > if you ack them ?  
> 
> It can certainly go through your tree!
> 
> My tree is meant for crosscutting error work.  I also offer it
> maintainers who prefer to leave the pull request to me.
> 
> PATCH 1 could use a bit of polish, and I encourage you to respin.  But
> it's not wrong, therefore
> 
> Series
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Thanks Markus !

I've respun the series with your suggestions. Is it expected I repost to
qemu-devel before doing a pull request ?

Cheers.

--
Greg

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr)
  2016-01-19 11:28   ` Greg Kurz
@ 2016-01-19 12:19     ` Markus Armbruster
  0 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2016-01-19 12:19 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-devel, Aneesh Kumar K.V

Greg Kurz <gkurz@linux.vnet.ibm.com> writes:

> On Mon, 18 Jan 2016 17:39:28 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Greg Kurz <gkurz@linux.vnet.ibm.com> writes:
>> 
>> > Hi,
>> >
>> > This series moves all the 9pfs/fsdev code to use error_report(), with the
>> > notable exception of virtfs-proxy-helper, which doesn't need it.
>> >
>> > Markus,
>> >
>> > Should this patches go through your tree ? Or can they go through my 9p tree
>> > if you ack them ?  
>> 
>> It can certainly go through your tree!
>> 
>> My tree is meant for crosscutting error work.  I also offer it
>> maintainers who prefer to leave the pull request to me.
>> 
>> PATCH 1 could use a bit of polish, and I encourage you to respin.  But
>> it's not wrong, therefore
>> 
>> Series
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> 
>
> Thanks Markus !
>
> I've respun the series with your suggestions. Is it expected I repost to
> qemu-devel before doing a pull request ?

Maintainers sometimes skip posting trivially revised patches for
additional review before doing a pull request.  Posting them is never
wrong, though.

When in doubt, post :)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-01-19 12:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-18 15:02 [Qemu-devel] [PATCH 0/2] 9pfs: fsdev: use error_report() instead of fprintf(stderr) Greg Kurz
2016-01-18 15:02 ` [Qemu-devel] [PATCH 1/2] 9pfs: " Greg Kurz
2016-01-18 16:35   ` Markus Armbruster
2016-01-19 11:25     ` Greg Kurz
2016-01-18 15:02 ` [Qemu-devel] [PATCH 2/2] fsdev: " Greg Kurz
2016-01-18 16:39 ` [Qemu-devel] [PATCH 0/2] 9pfs: " Markus Armbruster
2016-01-19 11:28   ` Greg Kurz
2016-01-19 12:19     ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).