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

[Marc-André Lureau]

Hi

On Mon, Jul 16, 2018 at 7:03 PM, Daniel P. Berrangé <address@hidden> wrote:
> 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.

Shouldn't reconnect= and fd= arguments be incompatible? Does it make
sense to try a reconnect on the same fd?




-- 
Marc-André Lureau