7:27 a.m.
Add missing test for socket activation and fix the server socket to use larger
backlog, allowing concurrent connections.
Nir Soffer (2):
tests: Test nbd_connect_systemd_socket_activation
socket-activation: Allow multiple connections
generator/states-connect-socket-activation.c | 2 +-
tests/Makefile.am | 5 ++
tests/connect-systemd-socket-activation.c | 83 ++++++++++++++++++++
3 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 tests/connect-systemd-socket-activation.c
--
2.31.1
7:27 a.m.
New subject: [PATCH libnbd 1/2] tests: Test nbd_connect_systemd_socket_activation
Test connecting using systemd socket activation, and using the handle
URI to connect additional handles.
The tests reveals a deadlock when closing the first handle owning the
nbdkit server. Closing the first handle last avoids this issue.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
tests/Makefile.am | 5 ++
tests/connect-systemd-socket-activation.c | 83 +++++++++++++++++++++++
2 files changed, 88 insertions(+)
create mode 100644 tests/connect-systemd-socket-activation.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7f00f6f..1451fb8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -192,6 +192,7 @@ check_PROGRAMS += \
opt-list \
opt-info \
opt-list-meta \
+ connect-systemd-socket-activation \
connect-unix \
connect-tcp \
connect-tcp6 \
@@ -235,6 +236,7 @@ TESTS += \
opt-list \
opt-info \
opt-list-meta \
+ connect-systemd-socket-activation \
connect-unix \
connect-tcp \
connect-tcp6 \
@@ -424,6 +426,9 @@ opt_info_LDADD = $(top_builddir)/lib/libnbd.la
opt_list_meta_SOURCES = opt-list-meta.c
opt_list_meta_LDADD = $(top_builddir)/lib/libnbd.la
+connect_systemd_socket_activation_SOURCES = connect-systemd-socket-activation.c
+connect_systemd_socket_activation_LDADD = $(top_builddir)/lib/libnbd.la
+
connect_unix_SOURCES = connect-unix.c
connect_unix_LDADD = $(top_builddir)/lib/libnbd.la
diff --git a/tests/connect-systemd-socket-activation.c
b/tests/connect-systemd-socket-activation.c
new file mode 100644
index 0000000..d64319a
--- /dev/null
+++ b/tests/connect-systemd-socket-activation.c
@@ -0,0 +1,83 @@
+/* NBD client library in userspace
+ * Copyright (C) 2021 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Test connecting using systemd socket activation. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <libnbd.h>
+
+#define CONNECTIONS 4
+
+int
+main (int argc, char *argv[])
+{
+ char *args[] = {"nbdkit", "-f", "memory",
"size=1m", NULL};
+ struct nbd_handle *nbd[CONNECTIONS] = {0};
+ char *uri = NULL;
+ int result = EXIT_FAILURE;
+
+ printf("Connecting handle 0 with systemd socket actication\n");
+
+ nbd[0] = nbd_create ();
+ if (nbd[0] == NULL)
+ goto out;
+
+ if (nbd_connect_systemd_socket_activation (nbd[0], args) == -1)
+ goto out;
+
+ uri = nbd_get_uri (nbd[0]);
+ if (uri == NULL)
+ goto out;
+
+ printf("Using URI: %s\n", uri);
+
+ for (int i = 1; i < CONNECTIONS; i++) {
+ printf ("Connecting handle %d to URI\n", i);
+
+ nbd[i] = nbd_create ();
+ if (nbd[i] == NULL)
+ goto out;
+
+ if (nbd_connect_uri (nbd[i], uri) == -1)
+ goto out;
+ }
+
+ printf ("All handles connected\n");
+ result = EXIT_SUCCESS;
+
+out:
+ if (result == EXIT_FAILURE)
+ fprintf (stderr, "%s\n", nbd_get_error ());
+
+ free (uri);
+
+ /* Closing the first connection deadlocks, so we close the additional
+ * connections first. */
+ for (int i = CONNECTIONS - 1; i >= 0; i--) {
+ if (nbd[i]) {
+ printf ("Closing handle %d\n", i);
+ nbd_close (nbd[i]);
+ }
+ }
+
+ exit (result);
+}
--
2.31.1
7:58 a.m.
New subject: [PATCH libnbd 1/2] tests: Test nbd_connect_systemd_socket_activation
On Sun, Nov 07, 2021 at 03:27:46PM +0200, Nir Soffer wrote:
Test connecting using systemd socket activation, and using the
handle
URI to connect additional handles.
The tests reveals a deadlock when closing the first handle owning the
nbdkit server. Closing the first handle last avoids this issue.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
tests/Makefile.am | 5 ++
tests/connect-systemd-socket-activation.c | 83 +++++++++++++++++++++++
2 files changed, 88 insertions(+)
create mode 100644 tests/connect-systemd-socket-activation.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7f00f6f..1451fb8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -192,6 +192,7 @@ check_PROGRAMS += \
opt-list \
opt-info \
opt-list-meta \
+ connect-systemd-socket-activation \
connect-unix \
connect-tcp \
connect-tcp6 \
@@ -235,6 +236,7 @@ TESTS += \
opt-list \
opt-info \
opt-list-meta \
+ connect-systemd-socket-activation \
connect-unix \
connect-tcp \
connect-tcp6 \
@@ -424,6 +426,9 @@ opt_info_LDADD = $(top_builddir)/lib/libnbd.la
opt_list_meta_SOURCES = opt-list-meta.c
opt_list_meta_LDADD = $(top_builddir)/lib/libnbd.la
+connect_systemd_socket_activation_SOURCES = connect-systemd-socket-activation.c
+connect_systemd_socket_activation_LDADD = $(top_builddir)/lib/libnbd.la
+
connect_unix_SOURCES = connect-unix.c
connect_unix_LDADD = $(top_builddir)/lib/libnbd.la
diff --git a/tests/connect-systemd-socket-activation.c
b/tests/connect-systemd-socket-activation.c
new file mode 100644
index 0000000..d64319a
--- /dev/null
+++ b/tests/connect-systemd-socket-activation.c
@@ -0,0 +1,83 @@
+/* NBD client library in userspace
+ * Copyright (C) 2021 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Test connecting using systemd socket activation. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <libnbd.h>
+
+#define CONNECTIONS 4
+
+int
+main (int argc, char *argv[])
+{
+ char *args[] = {"nbdkit", "-f", "memory",
"size=1m", NULL};
+ struct nbd_handle *nbd[CONNECTIONS] = {0};
+ char *uri = NULL;
+ int result = EXIT_FAILURE;
The test needs to skip if nbdkit and/or the memory plugin is not
available. This can be done using:
#include "requires.h"
...
requires ("nbdkit --version");
requires ("nbdkit memory --version");
+ printf("Connecting handle 0 with systemd socket
actication\n");
+
+ nbd[0] = nbd_create ();
+ if (nbd[0] == NULL)
+ goto out;
+
+ if (nbd_connect_systemd_socket_activation (nbd[0], args) == -1)
+ goto out;
+
+ uri = nbd_get_uri (nbd[0]);
+ if (uri == NULL)
+ goto out;
+
+ printf("Using URI: %s\n", uri);
I'm not sure what nbd_get_uri should show here - possibly it *ought*
to return NULL (since there's no URI that should connect back to a
server launched with systemd-socket-activation). But later ...
+ for (int i = 1; i < CONNECTIONS; i++) {
+ printf ("Connecting handle %d to URI\n", i);
+
+ nbd[i] = nbd_create ();
+ if (nbd[i] == NULL)
+ goto out;
+
+ if (nbd_connect_uri (nbd[i], uri) == -1)
+ goto out;
+ }
Now I'm intrigued. I'm guessing the URI exposes the internal socket
we create (/tmp/libnbdXXXXXX/sock) for SSA. Which sounds like an
"interesting" corner of the API. It also prevents us from changing
this in future -- eg. to use socketpair or a socket in the anonymous
space.
Also this test needs to check for nbd_can_multi_conn >= 1 before
actually doing this (although nbdkit will always return it).
+ printf ("All handles connected\n");
+ result = EXIT_SUCCESS;
+
+out:
+ if (result == EXIT_FAILURE)
+ fprintf (stderr, "%s\n", nbd_get_error ());
+
+ free (uri);
+
+ /* Closing the first connection deadlocks, so we close the additional
+ * connections first. */
And this seems like a bear-trap.
I support the idea of making this work, but I'm not sure that doing it
this way is a good API.
Rich.
+ for (int i = CONNECTIONS - 1; i >= 0; i--) {
+ if (nbd[i]) {
+ printf ("Closing handle %d\n", i);
+ nbd_close (nbd[i]);
+ }
+ }
+
+ exit (result);
+}
--
2.31.1
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
8:52 a.m.
New subject: [PATCH libnbd 1/2] tests: Test nbd_connect_systemd_socket_activation
On Sun, Nov 7, 2021 at 3:58 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
On Sun, Nov 07, 2021 at 03:27:46PM +0200, Nir Soffer wrote:
> Test connecting using systemd socket activation, and using the handle
> URI to connect additional handles.
>
> The tests reveals a deadlock when closing the first handle owning the
> nbdkit server. Closing the first handle last avoids this issue.
>
> Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
> ---
> tests/Makefile.am | 5 ++
> tests/connect-systemd-socket-activation.c | 83 +++++++++++++++++++++++
> 2 files changed, 88 insertions(+)
> create mode 100644 tests/connect-systemd-socket-activation.c
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 7f00f6f..1451fb8 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -192,6 +192,7 @@ check_PROGRAMS += \
> opt-list \
> opt-info \
> opt-list-meta \
> + connect-systemd-socket-activation \
> connect-unix \
> connect-tcp \
> connect-tcp6 \
> @@ -235,6 +236,7 @@ TESTS += \
> opt-list \
> opt-info \
> opt-list-meta \
> + connect-systemd-socket-activation \
> connect-unix \
> connect-tcp \
> connect-tcp6 \
> @@ -424,6 +426,9 @@ opt_info_LDADD = $(top_builddir)/lib/libnbd.la
> opt_list_meta_SOURCES = opt-list-meta.c
> opt_list_meta_LDADD = $(top_builddir)/lib/libnbd.la
>
> +connect_systemd_socket_activation_SOURCES = connect-systemd-socket-activation.c
> +connect_systemd_socket_activation_LDADD = $(top_builddir)/lib/libnbd.la
> +
> connect_unix_SOURCES = connect-unix.c
> connect_unix_LDADD = $(top_builddir)/lib/libnbd.la
>
> diff --git a/tests/connect-systemd-socket-activation.c
b/tests/connect-systemd-socket-activation.c
> new file mode 100644
> index 0000000..d64319a
> --- /dev/null
> +++ b/tests/connect-systemd-socket-activation.c
> @@ -0,0 +1,83 @@
> +/* NBD client library in userspace
> + * Copyright (C) 2021 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/* Test connecting using systemd socket activation. */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include <libnbd.h>
> +
> +#define CONNECTIONS 4
> +
> +int
> +main (int argc, char *argv[])
> +{
> + char *args[] = {"nbdkit", "-f", "memory",
"size=1m", NULL};
> + struct nbd_handle *nbd[CONNECTIONS] = {0};
> + char *uri = NULL;
> + int result = EXIT_FAILURE;
The test needs to skip if nbdkit and/or the memory plugin is not
available. This can be done using:
#include "requires.h"
...
requires ("nbdkit --version");
requires ("nbdkit memory --version");
Thanks, I'll add it.
> + printf("Connecting handle 0 with systemd socket
actication\n");
> +
> + nbd[0] = nbd_create ();
> + if (nbd[0] == NULL)
> + goto out;
> +
> + if (nbd_connect_systemd_socket_activation (nbd[0], args) == -1)
> + goto out;
> +
> + uri = nbd_get_uri (nbd[0]);
> + if (uri == NULL)
> + goto out;
> +
> + printf("Using URI: %s\n", uri);
I'm not sure what nbd_get_uri should show here - possibly it *ought*
to return NULL (since there's no URI that should connect back to a
server launched with systemd-socket-activation). But later ...
> + for (int i = 1; i < CONNECTIONS; i++) {
> + printf ("Connecting handle %d to URI\n", i);
> +
> + nbd[i] = nbd_create ();
> + if (nbd[i] == NULL)
> + goto out;
> +
> + if (nbd_connect_uri (nbd[i], uri) == -1)
> + goto out;
> + }
Now I'm intrigued. I'm guessing the URI exposes the internal socket
we create (/tmp/libnbdXXXXXX/sock) for SSA.
This is the case:
$ tests/connect-systemd-socket-activation
Connecting handle 0 with systemd socket activation
Using URI: nbd+unix:///?socket=/tmp/libnbd9BNa21/sock
Connecting handle 1 to URI
Which sounds like an
"interesting" corner of the API. It also prevents us from changing
this in future -- eg. to use socketpair or a socket in the anonymous
space.
Yes we don't document that socket activation uses unix socket, and
using this may break in future versions.
I think this the user should be in control here. For example:
nbd_connect_systemd_socket_activation(h, args, LIBNBD_SA_UNIX)
If the user does not care about the type of the socket:
nbd_connect_systemd_socket_activation(h, args, LIBNBD_SA_AUTO)
Also this test needs to check for nbd_can_multi_conn >= 1 before
actually doing this (although nbdkit will always return it).
Sure, although this is not an example code, just a test. We can cut
corners to simplify the test.
> + printf ("All handles connected\n");
> + result = EXIT_SUCCESS;
> +
> +out:
> + if (result == EXIT_FAILURE)
> + fprintf (stderr, "%s\n", nbd_get_error ());
> +
> + free (uri);
> +
> + /* Closing the first connection deadlocks, so we close the additional
> + * connections first. */
And this seems like a bear-trap.
I suspect this is caused by multiple bugs:
- libnbd sends SIGTERM and wait - if the server does not terminate, it
will block forever.
- nbdkit does not terminate if there are active connections? (I did
not investigate yet)
So this is not an API issue, just rough edges in libnbd.
I support the idea of making this work, but I'm not sure that
doing it
this way is a good API.
All this can be done by the user outside of libnbd, but using socket activation
in linbd makes this really easy to use.
Rich.
> + for (int i = CONNECTIONS - 1; i >= 0; i--) {
> + if (nbd[i]) {
> + printf ("Closing handle %d\n", i);
> + nbd_close (nbd[i]);
> + }
> + }
> +
> + exit (result);
> +}
> --
> 2.31.1
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
3:38 a.m.
New subject: [PATCH libnbd 1/2] tests: Test nbd_connect_systemd_socket_activation
I had a think about this.
nbd_connect_systemd_socket_activation uses SSA to pass in a listening
socket (not a connected socket), so it is expected that the child
calls accept(2) at least once on the socket passed in, and presumably
anticipated that the child could call accept(2) multiple times.
It therefore makes sense that other clients might use the SSA socket.
But using nbd_get_uri to obtain this "secret" socket is an unexpected
side effect of adding that API.
Since it's a listening socket, that rules out future use of socketpair(2)
(because that only creates connected sockets, and as far as I can tell
there is no way to pass connected sockets through SSA).
We might possibly change the implementation in future to use an
abstract socket (a Linux extension). Those are in theory expressible
as NBD URIs, although it's unlikely that any C library can handle such
URIs properly, certainly not libxml2.
So I think we can do this as proposed, BUT:
- It needs to be documented in the nbd_connect_systemd_socket_activation
man page.
- Documentation needs to state that if nbd_get_uri() returns NULL or
nbd_can_multi_conn() returns <= 0, then multi-conn isn't supported.
- Need to also document how to close these shared sockets without
deadlocking.
I'm still wondering if some kind of nbd_share_systemd_socket_activation()
call might be implemented. It would return the "secret" socket path,
plus perhaps set a flag on the handle to note that the handle is
shared and somehow fix the shutdown path. But I'm not sure how to
make that work, and if it's just a more clumsy version of the above.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
7:27 a.m.
New subject: [PATCH libnbd 2/2] socket-activation: Allow multiple connections
When using qemu-nbd with systemd socket activation, trying to connect
additional clients concurrently fails with:
nbd_connect_uri: connect: server backlog overflowed, see
https://bugzilla.redhat.com/1925045: Resource temporarily
unavailable
But this bug was fixed in qemu-nbd long time ago; turns out that libnbd
has the same bug, creating the server socket with:
listen(s, 1);
Use SOMAXCONN so we can have multiple connections for better
performance. I think this can be very useful for nbdcopy.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
generator/states-connect-socket-activation.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/generator/states-connect-socket-activation.c
b/generator/states-connect-socket-activation.c
index 800d963..2abf61b 100644
--- a/generator/states-connect-socket-activation.c
+++ b/generator/states-connect-socket-activation.c
@@ -147,7 +147,7 @@ STATE_MACHINE {
return 0;
}
- if (listen (s, 1) == -1) {
+ if (listen (s, SOMAXCONN) == -1) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "listen");
close (s);
--
2.31.1
7:59 a.m.
New subject: [PATCH libnbd 2/2] socket-activation: Allow multiple connections
On Sun, Nov 07, 2021 at 03:27:47PM +0200, Nir Soffer wrote:
When using qemu-nbd with systemd socket activation, trying to
connect
additional clients concurrently fails with:
nbd_connect_uri: connect: server backlog overflowed, see
https://bugzilla.redhat.com/1925045: Resource temporarily
unavailable
But this bug was fixed in qemu-nbd long time ago; turns out that libnbd
has the same bug, creating the server socket with:
listen(s, 1);
Use SOMAXCONN so we can have multiple connections for better
performance. I think this can be very useful for nbdcopy.
Apart from the problems in API mentioned in the other review, this
patch ordering isn't bisectable.
What's the motivation for this patch series?
Rich.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
generator/states-connect-socket-activation.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/generator/states-connect-socket-activation.c
b/generator/states-connect-socket-activation.c
index 800d963..2abf61b 100644
--- a/generator/states-connect-socket-activation.c
+++ b/generator/states-connect-socket-activation.c
@@ -147,7 +147,7 @@ STATE_MACHINE {
return 0;
}
- if (listen (s, 1) == -1) {
+ if (listen (s, SOMAXCONN) == -1) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "listen");
close (s);
--
2.31.1
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
8:22 a.m.
New subject: [PATCH libnbd 2/2] socket-activation: Allow multiple connections
On Sun, Nov 7, 2021 at 4:00 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
On Sun, Nov 07, 2021 at 03:27:47PM +0200, Nir Soffer wrote:
> When using qemu-nbd with systemd socket activation, trying to connect
> additional clients concurrently fails with:
>
> nbd_connect_uri: connect: server backlog overflowed, see
> https://bugzilla.redhat.com/1925045: Resource temporarily
> unavailable
>
> But this bug was fixed in qemu-nbd long time ago; turns out that libnbd
> has the same bug, creating the server socket with:
>
> listen(s, 1);
>
> Use SOMAXCONN so we can have multiple connections for better
> performance. I think this can be very useful for nbdcopy.
Apart from the problems in API mentioned in the other review, this
patch ordering isn't bisectable.
What's the motivation for this patch series?
Why is it not bissectable?
In general, I like to add first tests before I modify code. In the best case
the test also reproduces the issue. Having a failing test in the build is
better than having a bug.
In this case the test is too simple, we need to use multiple threads to
reproduce the issue.
We can switch the order if you don't like this approach.
Nir
8:26 a.m.
New subject: [PATCH libnbd 2/2] socket-activation: Allow multiple connections
On Sun, Nov 07, 2021 at 04:22:26PM +0200, Nir Soffer wrote:
On Sun, Nov 7, 2021 at 4:00 PM Richard W.M. Jones
<rjones(a)redhat.com> wrote:
>
> On Sun, Nov 07, 2021 at 03:27:47PM +0200, Nir Soffer wrote:
> > When using qemu-nbd with systemd socket activation, trying to connect
> > additional clients concurrently fails with:
> >
> > nbd_connect_uri: connect: server backlog overflowed, see
> > https://bugzilla.redhat.com/1925045: Resource temporarily
> > unavailable
> >
> > But this bug was fixed in qemu-nbd long time ago; turns out that libnbd
> > has the same bug, creating the server socket with:
> >
> > listen(s, 1);
> >
> > Use SOMAXCONN so we can have multiple connections for better
> > performance. I think this can be very useful for nbdcopy.
>
> Apart from the problems in API mentioned in the other review, this
> patch ordering isn't bisectable.
>
> What's the motivation for this patch series?
Why is it not bissectable?
Doesn't the test fail after the first patch is applied and
before the second one?
Rich.
In general, I like to add first tests before I modify code. In the
best case
the test also reproduces the issue. Having a failing test in the build is
better than having a bug.
In this case the test is too simple, we need to use multiple threads to
reproduce the issue.
We can switch the order if you don't like this approach.
Nir
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
8:54 a.m.
New subject: [PATCH libnbd 2/2] socket-activation: Allow multiple connections
On Sun, Nov 7, 2021 at 4:26 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
On Sun, Nov 07, 2021 at 04:22:26PM +0200, Nir Soffer wrote:
> On Sun, Nov 7, 2021 at 4:00 PM Richard W.M. Jones <rjones(a)redhat.com> wrote:
> >
> > On Sun, Nov 07, 2021 at 03:27:47PM +0200, Nir Soffer wrote:
> > > When using qemu-nbd with systemd socket activation, trying to connect
> > > additional clients concurrently fails with:
> > >
> > > nbd_connect_uri: connect: server backlog overflowed, see
> > > https://bugzilla.redhat.com/1925045: Resource temporarily
> > > unavailable
> > >
> > > But this bug was fixed in qemu-nbd long time ago; turns out that libnbd
> > > has the same bug, creating the server socket with:
> > >
> > > listen(s, 1);
> > >
> > > Use SOMAXCONN so we can have multiple connections for better
> > > performance. I think this can be very useful for nbdcopy.
> >
> > Apart from the problems in API mentioned in the other review, this
> > patch ordering isn't bisectable.
> >
> > What's the motivation for this patch series?
>
> Why is it not bissectable?
Doesn't the test fail after the first patch is applied and
before the second one?
No, it requires using concurrent connections to fail. I have an application
using this that fails reliably without the fix.
1457
days inactive
1458
days old
9 comments
2 participants
participants (2)
-
Nir Soffer -
Richard W.M. Jones