Re: [Qemu-devel] [PATCH 3/3] char-socket: delay setting fd-pass feature until connected

[Marc-André Lureau]

Hi

On Tue, Jul 17, 2018 at 3:01 PM, Daniel P. Berrangé <address@hidden> wrote:
> On Tue, Jul 17, 2018 at 02:52:39PM +0200, Marc-André Lureau wrote:
>> Wait for QIO channel connection completion, and check the feature set
>> by QIO. This fixes setting fd-pass chardev feature on
>> SOCKET_ADDRESS_FD where fd has AF_UNIX.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>>  chardev/char-socket.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index 6daa8d003a..7387e632d4 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -789,6 +789,9 @@ static int tcp_chr_new_client(Chardev *chr, 
>> QIOChannelSocket *sioc)
>>
>>      qio_channel_set_blocking(s->ioc, false, NULL);
>>
>> +    if (qio_channel_has_feature(s->ioc, QIO_CHANNEL_FEATURE_FD_PASS)) {
>> +        qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
>> +    }
>
> I really don't like this approach as it has unpleasant async/race prone
> semantics or the external users of the chardev.
>
> With the current approach they know that once the chardev is created,
> the features have been initialized.
>
> With this approach, the features are only initialized once the client
> connection has been completed, or once the server has started listening,
> which may or may not be the case once the chardev constructor completes.

Ok, what about augmenting the feature set then?:

keep the current qemu_chr_set_feature() in contructor for
ADDRESS_TYPE_UNIX, and add the feature in new_client() for
ADDRESS_TYPE_FD?

>
>>      /* be isn't opened until we get a connection */
>>      *be_opened = false;
>> --
>> 2.18.0.129.ge3331758f1
>>
>
> 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 :|
>



-- 
Marc-André Lureau