Re: [Qemu-devel] [PATCH 07/18] migration: make writes blocking

[Markus Armbruster]

Juan Quintela <address@hidden> writes:

> Markus Armbruster <address@hidden> wrote:
>> Juan Quintela <address@hidden> writes:
>>
>>> Markus Armbruster <address@hidden> wrote:
>>>> Juan Quintela <address@hidden> writes:
>>>>
>>>>> Move all the writes to the migration_thread, and make writings
>>>>> blocking.  Notice that are still using the iothread for everything
>>>>> that we do.
>>>> [...]
>>>>> diff --git a/qemu-sockets.c b/qemu-sockets.c
>>>>> index cfed9c5..61b6e95 100644
>>>>> --- a/qemu-sockets.c
>>>>> +++ b/qemu-sockets.c
>>>>> @@ -276,9 +276,6 @@ static int inet_connect_addr(struct addrinfo *addr, 
>>>>> bool *in_progress,
>>>>>          return -1;
>>>>>      }
>>>>>      qemu_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
>>>>> -    if (connect_state != NULL) {
>>>>> -        socket_set_nonblock(sock);
>>>>> -    }
>>>>>      /* connect to peer */
>>>>>      do {
>>>>>          rc = 0;
>>>>> @@ -732,7 +729,6 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
>>>>>          connect_state = g_malloc0(sizeof(*connect_state));
>>>>>          connect_state->callback = callback;
>>>>>          connect_state->opaque = opaque;
>>>>> -        socket_set_nonblock(sock);
>>>>>      }
>>>>>
>>>>>      memset(&un, 0, sizeof(un));
>>>>
>>>> Doesn't this break inet_nonblocking_connect() and
>>>> unix_nonblocking_connect()?
>>>>
>>>> In your cover letter, you wrote:
>>>>
>>>>     Note: Writes has become blocking, and I have to change the "remove"
>>>>     the feature now in qemu-sockets.c.  Checked that migration was the
>>>>     only user of that feature.  If new users appear, they just need to add
>>>>     the socket_set_nonblock() by hand.
>>>>
>>>> Yes, migration-{tcp,unix} are their only users, but if they want a
>>>> blocking socket now, why not use inet_connect() and unix_connect()?
>>>>
>>>> New users can't "just add socket_set_nonblock()".  They'd have to add it
>>>> right where you deleted it: between qemu_socket() and connect().  Else
>>>> the connect() is blocking.
>>>
>>> Grrr.
>>>
>>> So, is there any way to make a connection that is non-blocking, but then
>>> writes are blocking?
>>
>> Which operations on the migration socket do you need to block, and which
>> ones do you need not to block?
>
> connect: not blocking (done on the iothread)
> writes: blocking, done in the migration thread.
>
> I think thet socket_set_block() that paolo says is the right solution.

Sounds good.

By the way, I probably would've missed this had you not pointed to it in
the cover letter.  Smart move on your part.