Re: [Qemu-devel] [PATCH v3 4/5] socket: Add num connections to qio_net_listener_open_sync()

[Juan Quintela]

Eric Blake <address@hidden> wrote:
> On 8/20/19 5:48 AM, Juan Quintela wrote:
>> Reviewed-by: Daniel P. Berrangé <address@hidden>
>> Signed-off-by: Juan Quintela <address@hidden>
>> ---
>>  blockdev-nbd.c            | 2 +-
>>  chardev/char-socket.c     | 2 +-
>>  include/io/net-listener.h | 2 ++
>>  io/net-listener.c         | 3 ++-
>>  migration/socket.c        | 2 +-
>>  qemu-nbd.c                | 2 +-
>>  ui/vnc.c                  | 4 ++--
>>  7 files changed, 10 insertions(+), 7 deletions(-)
>
> Just now noticing this one, even though the pull request is already sent...
>
>> 
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index 7a71da447f..c621686131 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -101,7 +101,7 @@ void nbd_server_start(SocketAddress *addr, const char 
>> *tls_creds,
>>      qio_net_listener_set_name(nbd_server->listener,
>>                                "nbd-listener");
>>  
>> -    if (qio_net_listener_open_sync(nbd_server->listener, addr, errp) < 0) {
>> +    if (qio_net_listener_open_sync(nbd_server->listener, addr, 1, errp) < 
>> 0) {
>>          goto error;
>>      }
>
> Does this interfere with the ability to have more than one client
> connect to an NBD server during pull-mode incremental backup?  Or can
> you still have multiple simultaneous clients, provided that the server
> has finished accepting the connection from the first before the second
> one starts?

It is exactly the same than the old code.  Old code always use one.  We
need to have more than one for multifd.

Once told that, if the connections don't start "very" simultaneosly
(i..e. With multifd we start <num channels> connections in paraller),
you will never notice that the backlog is one (sie of queue of pending
connections nowadays).


>
>> +++ b/qemu-nbd.c
>> @@ -1054,7 +1054,7 @@ int main(int argc, char **argv)
>>      server = qio_net_listener_new();
>>      if (socket_activation == 0) {
>>          saddr = nbd_build_socket_address(sockpath, bindto, port);
>> -        if (qio_net_listener_open_sync(server, saddr, &local_err) < 0) {
>> +        if (qio_net_listener_open_sync(server, saddr, 1, &local_err) < 0) {
>
> Here, 'qemu-nbd -e $n' allows up to $n simultaneous clients.  Should we
> be feeding in that number, instead of a hard-coded 1, to make it easier
> for those clients to connect simultaneously?
>
> We can make such changes as a followup patch.

>From the man page:

       The backlog argument defines the maximum length to which the  queue  of
       pending  connections  for  sockfd  may  grow.

So, except if you plan to start multiples connections at the same time,
you don't care.  And if the old code worked, this one makes no
difference.

To explain multifd problem, I was creating 10 threads, and each launched
a connect.  On the receiving side, I only got 8 connections, 2 of them
were missing.

Later, Juan.