1. Patchew
  2. QEMU
  3. [PATCH v3 00/15] Misc cleanups
  4. View patch

[PATCH v3 07/15] qga: throw an Error in ga_channel_open()

marcandre.lureau@redhat.com posted 15 patches 2 years, 8 months ago
There is a newer version of this series
[PATCH v3 07/15] qga: throw an Error in ga_channel_open()
Posted by marcandre.lureau@redhat.com 2 years, 8 months ago 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Allow for a single point of error reporting, and further refactoring.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qga/channel-posix.c | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index a996858e24..039e1ddcb2 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -119,8 +119,9 @@ static int ga_channel_client_add(GAChannel *c, int fd)
 }
 
 static gboolean ga_channel_open(GAChannel *c, const gchar *path,
-                                GAChannelMethod method, int fd)
+                                GAChannelMethod method, int fd, Error **errp)
 {
+    ERRP_GUARD();
     int ret;
     c->method = method;
 
@@ -133,21 +134,20 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
 #endif
                            );
         if (fd == -1) {
-            g_critical("error opening channel: %s", strerror(errno));
+            error_setg_errno(errp, errno, "error opening channel");
             return false;
         }
 #ifdef CONFIG_SOLARIS
         ret = ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI);
         if (ret == -1) {
-            g_critical("error setting event mask for channel: %s",
-                       strerror(errno));
+            error_setg_errno(errp, errno, "error setting event mask for channel");
             close(fd);
             return false;
         }
 #endif
         ret = ga_channel_client_add(c, fd);
         if (ret) {
-            g_critical("error adding channel to main loop");
+            error_setg(errp, "error adding channel to main loop");
             close(fd);
             return false;
         }
@@ -159,7 +159,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
         assert(fd < 0);
         fd = qemu_open_old(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
         if (fd == -1) {
-            g_critical("error opening channel: %s", strerror(errno));
+            error_setg_errno(errp, errno, "error opening channel");
             return false;
         }
         tcgetattr(fd, &tio);
@@ -180,7 +180,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
         tcsetattr(fd, TCSANOW, &tio);
         ret = ga_channel_client_add(c, fd);
         if (ret) {
-            g_critical("error adding channel to main loop");
+            error_setg(errp, "error adding channel to main loop");
             close(fd);
             return false;
         }
@@ -188,12 +188,8 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
     }
     case GA_CHANNEL_UNIX_LISTEN: {
         if (fd < 0) {
-            Error *local_err = NULL;
-
-            fd = unix_listen(path, &local_err);
-            if (local_err != NULL) {
-                g_critical("%s", error_get_pretty(local_err));
-                error_free(local_err);
+            fd = unix_listen(path, errp);
+            if (fd < 0) {
                 return false;
             }
         }
@@ -202,24 +198,19 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
     }
     case GA_CHANNEL_VSOCK_LISTEN: {
         if (fd < 0) {
-            Error *local_err = NULL;
             SocketAddress *addr;
             char *addr_str;
 
             addr_str = g_strdup_printf("vsock:%s", path);
-            addr = socket_parse(addr_str, &local_err);
+            addr = socket_parse(addr_str, errp);
             g_free(addr_str);
-            if (local_err != NULL) {
-                g_critical("%s", error_get_pretty(local_err));
-                error_free(local_err);
+            if (!addr) {
                 return false;
             }
 
-            fd = socket_listen(addr, 1, &local_err);
+            fd = socket_listen(addr, 1, errp);
             qapi_free_SocketAddress(addr);
-            if (local_err != NULL) {
-                g_critical("%s", error_get_pretty(local_err));
-                error_free(local_err);
+            if (fd < 0) {
                 return false;
             }
         }
@@ -227,7 +218,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
         break;
     }
     default:
-        g_critical("error binding/listening to specified socket");
+        error_setg(errp, "error binding/listening to specified socket");
         return false;
     }
 
@@ -272,12 +263,14 @@ GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize *count)
 GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
                           int listen_fd, GAChannelCallback cb, gpointer opaque)
 {
+    Error *err = NULL;
     GAChannel *c = g_new0(GAChannel, 1);
     c->event_cb = cb;
     c->user_data = opaque;
 
-    if (!ga_channel_open(c, path, method, listen_fd)) {
-        g_critical("error opening channel");
+    if (!ga_channel_open(c, path, method, listen_fd, &err)) {
+        g_critical("%s", error_get_pretty(err));
+        error_free(err);
         ga_channel_free(c);
         return NULL;
     }
-- 
2.36.1
Re: [PATCH v3 07/15] qga: throw an Error in ga_channel_open()
Posted by Markus Armbruster 2 years, 8 months ago 
marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Allow for a single point of error reporting, and further refactoring.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qga/channel-posix.c | 43 ++++++++++++++++++-------------------------
>  1 file changed, 18 insertions(+), 25 deletions(-)
>
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index a996858e24..039e1ddcb2 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -119,8 +119,9 @@ static int ga_channel_client_add(GAChannel *c, int fd)
>  }
>  
>  static gboolean ga_channel_open(GAChannel *c, const gchar *path,
> -                                GAChannelMethod method, int fd)
> +                                GAChannelMethod method, int fd, Error **errp)
>  {
> +    ERRP_GUARD();

I can't see where this is needed.

>      int ret;
>      c->method = method;
>  
> @@ -133,21 +134,20 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>  #endif
>                             );
>          if (fd == -1) {
> -            g_critical("error opening channel: %s", strerror(errno));
> +            error_setg_errno(errp, errno, "error opening channel");
>              return false;
>          }
>  #ifdef CONFIG_SOLARIS
>          ret = ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI);
>          if (ret == -1) {
> -            g_critical("error setting event mask for channel: %s",
> -                       strerror(errno));
> +            error_setg_errno(errp, errno, "error setting event mask for channel");
>              close(fd);
>              return false;
>          }
>  #endif
>          ret = ga_channel_client_add(c, fd);
>          if (ret) {
> -            g_critical("error adding channel to main loop");
> +            error_setg(errp, "error adding channel to main loop");
>              close(fd);
>              return false;
>          }
> @@ -159,7 +159,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>          assert(fd < 0);
>          fd = qemu_open_old(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
>          if (fd == -1) {
> -            g_critical("error opening channel: %s", strerror(errno));
> +            error_setg_errno(errp, errno, "error opening channel");
>              return false;
>          }
>          tcgetattr(fd, &tio);
> @@ -180,7 +180,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>          tcsetattr(fd, TCSANOW, &tio);
>          ret = ga_channel_client_add(c, fd);
>          if (ret) {
> -            g_critical("error adding channel to main loop");
> +            error_setg(errp, "error adding channel to main loop");
>              close(fd);
>              return false;
>          }
> @@ -188,12 +188,8 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>      }
>      case GA_CHANNEL_UNIX_LISTEN: {
>          if (fd < 0) {
> -            Error *local_err = NULL;
> -
> -            fd = unix_listen(path, &local_err);
> -            if (local_err != NULL) {
> -                g_critical("%s", error_get_pretty(local_err));
> -                error_free(local_err);
> +            fd = unix_listen(path, errp);
> +            if (fd < 0) {
>                  return false;
>              }
>          }
> @@ -202,24 +198,19 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>      }
>      case GA_CHANNEL_VSOCK_LISTEN: {
>          if (fd < 0) {
> -            Error *local_err = NULL;
>              SocketAddress *addr;
>              char *addr_str;
>  
>              addr_str = g_strdup_printf("vsock:%s", path);
> -            addr = socket_parse(addr_str, &local_err);
> +            addr = socket_parse(addr_str, errp);
>              g_free(addr_str);
> -            if (local_err != NULL) {
> -                g_critical("%s", error_get_pretty(local_err));
> -                error_free(local_err);
> +            if (!addr) {
>                  return false;
>              }
>  
> -            fd = socket_listen(addr, 1, &local_err);
> +            fd = socket_listen(addr, 1, errp);
>              qapi_free_SocketAddress(addr);
> -            if (local_err != NULL) {
> -                g_critical("%s", error_get_pretty(local_err));
> -                error_free(local_err);
> +            if (fd < 0) {
>                  return false;
>              }
>          }
> @@ -227,7 +218,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
>          break;
>      }
>      default:
> -        g_critical("error binding/listening to specified socket");
> +        error_setg(errp, "error binding/listening to specified socket");
>          return false;
>      }
>  
> @@ -272,12 +263,14 @@ GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize *count)
>  GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
>                            int listen_fd, GAChannelCallback cb, gpointer opaque)
>  {
> +    Error *err = NULL;
>      GAChannel *c = g_new0(GAChannel, 1);
>      c->event_cb = cb;
>      c->user_data = opaque;
>  
> -    if (!ga_channel_open(c, path, method, listen_fd)) {
> -        g_critical("error opening channel");
> +    if (!ga_channel_open(c, path, method, listen_fd, &err)) {
> +        g_critical("%s", error_get_pretty(err));
> +        error_free(err);
>          ga_channel_free(c);
>          return NULL;
>      }

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.