Re: [Qemu-devel] [PATCH] chardev: unconditionally set FD_PASS feature for socket type=fd

[Paolo Bonzini]

On 04/07/2018 13:11, Daniel P. Berrangé wrote:
> The vhostuser network backend requires the chardev it is using to have
> the FD passing feature. It checks this upfront when initializing the
> network backend and reports an error if not set.
> 
> The socket chardev has to set the FD_PASS feature during early
> initialization to satisfy the vhostuser backend, and at this point
> the socket has not been initialized. It is thus unable to do a live
> check on the socket to see if it supports FD passing (aka is a UNIX
> socket). As a result it has to blindly set FD_PASS feature based
> solely on info in the SocketAddress struct, such as address type.
> 
> Unfortunately libvirt wishes to use FD passing to provide the UNIX
> domain socket listener, and as a result the FD_PASS feature is no
> longer set, which breaks vhostuser's checks, despite the fact that
> FD passing will in fact work later.
> 
> This unconditionally sets FD_PASS feature for any socket address
> which has type==fd. Thus will be wrong if the passed in FD was not
> a UNIX socket, but if an attempt is later made to use FD passing
> we'll still get an error reported by the QIOChannelSocket class.
> So the effective of setting the chardev FD_PASS feature early
> is merely to delay error reporting.

Could you query with getsockopt or getsockname in tcp_chr_connect?

Paolo

> Signed-off-by: Daniel P. Berrangé <address@hidden>
> ---
>  chardev/char-socket.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 17519ec589..b495d6a851 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -990,8 +990,18 @@ static void qmp_chardev_open_socket(Chardev *chr,
>      s->addr = addr = socket_address_flatten(sock->addr);
>  
>      qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
> -    /* TODO SOCKET_ADDRESS_FD where fd has AF_UNIX */
> -    if (addr->type == SOCKET_ADDRESS_TYPE_UNIX) {
> +    /*
> +     * We can't tell at this point if the "fd" we're passed is
> +     * a UNIX socket or not, so can't reliably set the
> +     * FD_PASS feature. vhost-user, however, checks for this
> +     * feature early before we've even created the I/O channel,
> +     * so we can't wait until later to set the feature. Thus
> +     * we optimistically set the FD_PASS feature. If the passed
> +     * in "fd" is not a UNIX socket, there will be an error
> +     * reported later anyway.
> +     */
> +    if (addr->type == SOCKET_ADDRESS_TYPE_UNIX ||
> +        addr->type == SOCKET_ADDRESS_TYPE_FD) {
>          qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
>      }
>  
>