qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 11/11] sockets: Make abstract UnixSocketAddress depend on


From: Eric Blake
Subject: Re: [PATCH v2 11/11] sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUX
Date: Mon, 2 Nov 2020 08:12:57 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1

On 11/2/20 3:44 AM, Markus Armbruster wrote:
> The abstract socket namespace is a non-portable Linux extension.  An
> attempt to use it elsewhere should fail with ENOENT (the abstract
> address looks like a "" pathname, which does not resolve).  We report
> this failure like
> 
>     Failed to connect socket abc: No such file or directory
> 
> Tolerable, although ENOTSUP would be better.
> 
> However, introspection lies: it has @abstract regardless of host
> support.  Easy enough to fix: since Linux provides them since 2.2,
> 'if': 'defined(CONFIG_LINUX)' should do.
> 
> The above failure becomes
> 
>     Parameter 'backend.data.addr.data.abstract' is unexpected
> 
> I consider this an improvement.
> 

Commit message lacks mention of the fact that we are now explicitly not
outputting 'strict' for non-abstract sockets (in fact, that change could
be squashed in 9/11 if you wanted to do it there).  But as this cleans
up the code I mentioned in 9/11, I'll leave it up to Dan if the commit
message needs a tweak; the end result is fine if we don't feel like a v3
spin just for moving hunks around.

Reviewed-by: Eric Blake <eblake@redhat.com>

> +++ b/chardev/char-socket.c
> @@ -444,14 +444,20 @@ static char *qemu_chr_socket_address(SocketChardev *s, 
> const char *prefix)
>          break;
>      case SOCKET_ADDRESS_TYPE_UNIX:
>      {
> +        const char *tight = "", *abstract = "";
>          UnixSocketAddress *sa = &s->addr->u.q_unix;
>  
> -        return g_strdup_printf("%sunix:%s%s%s%s", prefix,
> -                               s->addr->u.q_unix.path,
> -                               sa->has_abstract && sa->abstract
> -                               ? ",abstract" : "",
> -                               sa->has_tight && sa->tight
> -                               ? ",tight" : "",

Unconditional output if tight is true (which is its stated default)...

> +#ifdef CONFIG_LINUX
> +        if (sa->has_abstract && sa->abstract) {
> +            abstract = ",abstract";
> +            if (sa->has_tight && sa->tight) {
> +                tight = ",tight";
> +            }
> +        }
> +#endif
> +
> +        return g_strdup_printf("%sunix:%s%s%s%s", prefix, sa->path,
> +                               abstract, tight,

...vs. the now-nicer conditional where tight is only present if abstract.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]