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

[Daniel P . Berrangé]

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

Previously vhostuser poked directly in the QemuOpts for the chardev
which I changed to this feature based approach in

  commit 0a73336d96397c80881219d080518fac6f1ecacb
  Author: Daniel P. Berrange <address@hidden>
  Date:   Fri Oct 7 13:18:34 2016 +0100

    net: don't poke at chardev internal QemuOpts

This whole mess is merely so that vhostuser can print out an error
message if you point it to a chardev that isn't a UNIX socket. If
anything I'm half inclined to just delete that upfront error check
entirely. 

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