Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
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(-)
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;
}
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 7ca5d97af3..8c7c9da567 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1160,7 +1160,7 @@ static int qmp_chardev_open_socket_server(Chardev *chr,
qio_net_listener_set_name(s->listener, name);
g_free(name);
- if (qio_net_listener_open_sync(s->listener, s->addr, errp) < 0) {
+ if (qio_net_listener_open_sync(s->listener, s->addr, 1, errp) < 0) {
object_unref(OBJECT(s->listener));
s->listener = NULL;
return -1;
diff --git a/include/io/net-listener.h b/include/io/net-listener.h
index 8081ac58a2..fb101703e3 100644
--- a/include/io/net-listener.h
+++ b/include/io/net-listener.h
@@ -95,6 +95,7 @@ void qio_net_listener_set_name(QIONetListener *listener,
* qio_net_listener_open_sync:
* @listener: the network listener object
* @addr: the address to listen on
+ * @num: the amount of expected connections
* @errp: pointer to a NULL initialized error object
*
* Synchronously open a listening connection on all
@@ -104,6 +105,7 @@ void qio_net_listener_set_name(QIONetListener *listener,
*/
int qio_net_listener_open_sync(QIONetListener *listener,
SocketAddress *addr,
+ int num,
Error **errp);
/**
diff --git a/io/net-listener.c b/io/net-listener.c
index dc81150318..5d8a226872 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -62,6 +62,7 @@ static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
int qio_net_listener_open_sync(QIONetListener *listener,
SocketAddress *addr,
+ int num,
Error **errp)
{
QIODNSResolver *resolver = qio_dns_resolver_get_instance();
@@ -82,7 +83,7 @@ int qio_net_listener_open_sync(QIONetListener *listener,
for (i = 0; i < nresaddrs; i++) {
QIOChannelSocket *sioc = qio_channel_socket_new();
- if (qio_channel_socket_listen_sync(sioc, resaddrs[i], 1,
+ if (qio_channel_socket_listen_sync(sioc, resaddrs[i], num,
err ? NULL : &err) == 0) {
success = true;
diff --git a/migration/socket.c b/migration/socket.c
index 98efdc0286..e63f5e1612 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -181,7 +181,7 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
qio_net_listener_set_name(listener, "migration-socket-listener");
- if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+ if (qio_net_listener_open_sync(listener, saddr, 1, errp) < 0) {
object_unref(OBJECT(listener));
return;
}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 049645491d..83b6c32d73 100644
--- a/qemu-nbd.c
+++ 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) {
object_unref(OBJECT(server));
error_report_err(local_err);
exit(EXIT_FAILURE);
diff --git a/ui/vnc.c b/ui/vnc.c
index 4812ed29d0..258461f814 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3762,7 +3762,7 @@ static int vnc_display_listen(VncDisplay *vd,
qio_net_listener_set_name(vd->listener, "vnc-listen");
for (i = 0; i < nsaddr; i++) {
if (qio_net_listener_open_sync(vd->listener,
- saddr[i],
+ saddr[i], 1,
errp) < 0) {
return -1;
}
@@ -3777,7 +3777,7 @@ static int vnc_display_listen(VncDisplay *vd,
qio_net_listener_set_name(vd->wslistener, "vnc-ws-listen");
for (i = 0; i < nwsaddr; i++) {
if (qio_net_listener_open_sync(vd->wslistener,
- wsaddr[i],
+ wsaddr[i], 1,
errp) < 0) {
return -1;
}
--
2.21.0
On 8/20/19 5:48 AM, Juan Quintela wrote:
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> 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?
> +++ 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.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Eric Blake <eblake@redhat.com> wrote:
> On 8/20/19 5:48 AM, Juan Quintela wrote:
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> 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.
On Wed, Sep 04, 2019 at 03:19:21PM +0200, Juan Quintela wrote:
> Eric Blake <eblake@redhat.com> wrote:
> > On 8/20/19 5:48 AM, Juan Quintela wrote:
> >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >> 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).
If incremental backup needs multiple concurrent connections, then
you certainly *do* want to increase this value to something other
than 1, or you will get random failures. As Juan says, this is a
pre-existing problem with NBD though.
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 :|
© 2016 - 2026 Red Hat, Inc.