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

[Daniel P . Berrangé]

On Mon, Jul 16, 2018 at 06:57:48PM +0200, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 4, 2018 at 2:08 PM, Daniel P. Berrangé <address@hidden> wrote:
> > On Wed, Jul 04, 2018 at 01:53:03PM +0200, Paolo Bonzini wrote:
> >> 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?
> >
> > That potentially executes asynchronously in the backend and vhostuser
> > checks it when it first resolves the chardev ID. IOW the point where
> > vhostuser checks, all we have is the SocketAddress struct - in some
> > case we might be lucky & have connected, but we can't assume it :-(
> 
> In which case is it executed asynchronously?
> 
> Chardev creation is happening before vhost-user net,
> qemu_chardev_new() and qemu_char_open() calls
> qio_channel_socket_connect_sync().

The qmp_chardev_open_socket() method is not guaranteed to complete
synchronously. If the "reconnect" property is set everything happens
asynchronously, so we're not able to set the feature flag by the time
qemu_char_open() returns.


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