[PATCH v3 2/5] util/qemu-sockets: Refactor setting client sockopts into a separate function

Juraj Marcin posted 5 patches 8 months, 1 week ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v3 2/5] util/qemu-sockets: Refactor setting client sockopts into a separate function
Posted by Juraj Marcin 8 months, 1 week ago
From: Juraj Marcin <jmarcin@redhat.com>

This is done in preparation for enabling the SO_KEEPALIVE support for
server sockets and adding settings for more TCP keep-alive socket
options.

Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
---
 util/qemu-sockets.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 77477c1cd5..d15f6aa4b0 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -205,6 +205,22 @@ static int try_bind(int socket, InetSocketAddress *saddr, struct addrinfo *e)
 #endif
 }
 
+static int inet_set_sockopts(int sock, InetSocketAddress *saddr, Error **errp)
+{
+    if (saddr->keep_alive) {
+        int keep_alive = 1;
+        int ret = setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
+                             &keep_alive, sizeof(keep_alive));
+
+        if (ret < 0) {
+            error_setg_errno(errp, errno,
+                             "Unable to set keep-alive option on socket");
+            return -1;
+        }
+    }
+    return 0;
+}
+
 static int inet_listen_saddr(InetSocketAddress *saddr,
                              int port_offset,
                              int num,
@@ -475,16 +491,9 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error **errp)
         return sock;
     }
 
-    if (saddr->keep_alive) {
-        int val = 1;
-        int ret = setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
-                             &val, sizeof(val));
-
-        if (ret < 0) {
-            error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
-            close(sock);
-            return -1;
-        }
+    if (inet_set_sockopts(sock, saddr, errp)) {
+        close(sock);
+        return -1;
     }
 
     return sock;
-- 
2.48.1
Re: [PATCH v3 2/5] util/qemu-sockets: Refactor setting client sockopts into a separate function
Posted by Daniel P. Berrangé 8 months, 1 week ago
On Tue, Apr 08, 2025 at 01:25:01PM +0200, Juraj Marcin wrote:
> From: Juraj Marcin <jmarcin@redhat.com>
> 
> This is done in preparation for enabling the SO_KEEPALIVE support for
> server sockets and adding settings for more TCP keep-alive socket
> options.
> 
> Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
> ---
>  util/qemu-sockets.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 77477c1cd5..d15f6aa4b0 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -205,6 +205,22 @@ static int try_bind(int socket, InetSocketAddress *saddr, struct addrinfo *e)
>  #endif
>  }
>  
> +static int inet_set_sockopts(int sock, InetSocketAddress *saddr, Error **errp)
> +{
> +    if (saddr->keep_alive) {
> +        int keep_alive = 1;
> +        int ret = setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
> +                             &keep_alive, sizeof(keep_alive));
> +
> +        if (ret < 0) {
> +            error_setg_errno(errp, errno,
> +                             "Unable to set keep-alive option on socket");
> +            return -1;
> +        }
> +    }
> +    return 0;
> +}
> +
>  static int inet_listen_saddr(InetSocketAddress *saddr,
>                               int port_offset,
>                               int num,
> @@ -475,16 +491,9 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error **errp)
>          return sock;
>      }
>  
> -    if (saddr->keep_alive) {
> -        int val = 1;
> -        int ret = setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
> -                             &val, sizeof(val));
> -
> -        if (ret < 0) {
> -            error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
> -            close(sock);
> -            return -1;
> -        }
> +    if (inet_set_sockopts(sock, saddr, errp)) {

Since this returns -1 on error, by convention we check "< 0", reserving
positive return values for future non-error scenarios.

> +        close(sock);
> +        return -1;
>      }
>  
>      return sock;
> -- 
> 2.48.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|