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

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|