Eric Blake
2022-Aug-31 19:51 UTC
[Libguestfs] [libnbd PATCH] RFC: api: Add nbd_supports_vsock()
Similar to nbd_supports_tls(), it is nice to know from a feature probe
whether we are likely to have VSOCK support before even trying more
expensive APIs like nbd_connect_uri("nbd+vsock://...").
---
https://bugzilla.redhat.com/show_bug.cgi?id=2069558 documents a case
where AF_VSOCK is compiled, but vsock still doesn't work because
vsock_loopback is not loaded. Should we make nbd_supports_vsock()
return true only when ALL aspects of vsock are known to be working?
---
generator/API.ml | 25 ++++++++++++++++++++++---
lib/handle.c | 17 +++++++++++++++++
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/generator/API.ml b/generator/API.ml
index b377b9f..7ec85ca 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -1393,7 +1393,8 @@ "connect_uri", {
=item C<nbds+vsock:>
Connect over the C<AF_VSOCK> transport, without or with
-TLS respectively.
+TLS respectively. You can use L<nbd_supports_vsock(3)> to
+see if this build of libnbd supports C<AF_VSOCK>.
=back
@@ -1494,7 +1495,8 @@ "connect_uri", {
see_also = [URLLink
"https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md";
Link "aio_connect_uri";
Link "set_export_name"; Link "set_tls";
- Link "set_opt_mode"; Link "get_uri"];
+ Link "set_opt_mode"; Link "get_uri";
+ Link "supports_vsock"; Link
"supports_uri"];
};
"connect_unix", {
@@ -1522,8 +1524,12 @@ "connect_vsock", {
C<cid> should be C<2> (to connect to the host), and C<port>
might be
C<10809> or another port number assigned to you by the host
administrator.
+
+Not all systems support C<AF_VSOCK>; to determine if libnbd was
+built on a system with vsock support, see L<nbd_supports_vsock(3)>.
" ^ blocking_connect_call_description;
- see_also = [Link "aio_connect_vsock"; Link
"set_opt_mode"];
+ see_also = [Link "aio_connect_vsock"; Link
"set_opt_mode";
+ Link "supports_vsock"];
};
"connect_tcp", {
@@ -3061,6 +3067,16 @@ "supports_tls", {
see_also = [Link "set_tls"];
};
+ "supports_vsock", {
+ default_call with
+ args = []; ret = RBool; is_locked = false; may_set_error = false;
+ shortdesc = "true if libnbd was compiled with support for
AF_VSOCK";
+ longdesc = "\
+Returns true if libnbd was compiled with support for the AF_VSOCK family
+of sockets, or false if not.";
+ see_also = [Link "connect_vsock"; Link "connect_uri"];
+ };
+
"supports_uri", {
default_call with
args = []; ret = RBool; is_locked = false; may_set_error = false;
@@ -3233,6 +3249,9 @@ let first_version "set_request_block_size",
(1, 12);
"get_request_block_size", (1, 12);
+ (* Added in 1.13.x development cycle, will be stable and supported in 1.14.
*)
+ "supports_vsock", (1, 14);
+
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
"get_tls_certificates", (1, ??);
diff --git a/lib/handle.c b/lib/handle.c
index 8713e18..ac64fe9 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -28,6 +28,12 @@
#include <sys/types.h>
#include <sys/wait.h>
+#ifdef HAVE_LINUX_VM_SOCKETS_H
+#include <linux/vm_sockets.h>
+#elif HAVE_SYS_VSOCK_H
+#include <sys/vsock.h>
+#endif
+
#include "internal.h"
static void
@@ -483,6 +489,17 @@ nbd_unlocked_supports_tls (struct nbd_handle *h)
#endif
}
+/* NB: is_locked = false, may_set_error = false. */
+int
+nbd_unlocked_supports_vsock (struct nbd_handle *h)
+{
+#ifdef AF_VSOCK
+ return 1;
+#else
+ return 0;
+#endif
+}
+
/* NB: is_locked = false, may_set_error = false. */
int
nbd_unlocked_supports_uri (struct nbd_handle *h)
--
2.37.2
Laszlo Ersek
2022-Sep-01 06:11 UTC
[Libguestfs] [libnbd PATCH] RFC: api: Add nbd_supports_vsock()
On 08/31/22 21:51, Eric Blake wrote:> Similar to nbd_supports_tls(), it is nice to know from a feature probe > whether we are likely to have VSOCK support before even trying more > expensive APIs like nbd_connect_uri("nbd+vsock://..."). > > --- > > https://bugzilla.redhat.com/show_bug.cgi?id=2069558 documents a case > where AF_VSOCK is compiled, but vsock still doesn't work because > vsock_loopback is not loaded. Should we make nbd_supports_vsock() > return true only when ALL aspects of vsock are known to be working? > --- > generator/API.ml | 25 ++++++++++++++++++++++--- > lib/handle.c | 17 +++++++++++++++++ > 2 files changed, 39 insertions(+), 3 deletions(-)I think the nbd_supports_tls(3) manual is a good example to follow here: "Returns true if libnbd was compiled with gnutls which is required to support TLS encryption, or false if not." It says "necessary" but does not say "sufficient", and the distinction is clear, too. So I think the present patch is good, but we should perhaps add a hint to longdesc about the necessary kernel modules. Rich always says that error reports and such should recommend actions for the user, and I think extending longdesc with kernel module hints should cover that. (Making nbd_supports_vsock() check for the kernel module, but *not* reporting it human-readably as the failure reason, would be too obscure IMO.) So with the longdesc expansion: Acked-by: Laszlo Ersek <lersek at redhat.com> Laszlo> > diff --git a/generator/API.ml b/generator/API.ml > index b377b9f..7ec85ca 100644 > --- a/generator/API.ml > +++ b/generator/API.ml > @@ -1393,7 +1393,8 @@ "connect_uri", { > =item C<nbds+vsock:> > > Connect over the C<AF_VSOCK> transport, without or with > -TLS respectively. > +TLS respectively. You can use L<nbd_supports_vsock(3)> to > +see if this build of libnbd supports C<AF_VSOCK>. > > =back > > @@ -1494,7 +1495,8 @@ "connect_uri", { > see_also = [URLLink "https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md"; > Link "aio_connect_uri"; > Link "set_export_name"; Link "set_tls"; > - Link "set_opt_mode"; Link "get_uri"]; > + Link "set_opt_mode"; Link "get_uri"; > + Link "supports_vsock"; Link "supports_uri"]; > }; > > "connect_unix", { > @@ -1522,8 +1524,12 @@ "connect_vsock", { > C<cid> should be C<2> (to connect to the host), and C<port> might be > C<10809> or another port number assigned to you by the host > administrator. > + > +Not all systems support C<AF_VSOCK>; to determine if libnbd was > +built on a system with vsock support, see L<nbd_supports_vsock(3)>. > " ^ blocking_connect_call_description; > - see_also = [Link "aio_connect_vsock"; Link "set_opt_mode"]; > + see_also = [Link "aio_connect_vsock"; Link "set_opt_mode"; > + Link "supports_vsock"]; > }; > > "connect_tcp", { > @@ -3061,6 +3067,16 @@ "supports_tls", { > see_also = [Link "set_tls"]; > }; > > + "supports_vsock", { > + default_call with > + args = []; ret = RBool; is_locked = false; may_set_error = false; > + shortdesc = "true if libnbd was compiled with support for AF_VSOCK"; > + longdesc = "\ > +Returns true if libnbd was compiled with support for the AF_VSOCK family > +of sockets, or false if not."; > + see_also = [Link "connect_vsock"; Link "connect_uri"]; > + }; > + > "supports_uri", { > default_call with > args = []; ret = RBool; is_locked = false; may_set_error = false; > @@ -3233,6 +3249,9 @@ let first_version > "set_request_block_size", (1, 12); > "get_request_block_size", (1, 12); > > + (* Added in 1.13.x development cycle, will be stable and supported in 1.14. *) > + "supports_vsock", (1, 14); > + > (* These calls are proposed for a future version of libnbd, but > * have not been added to any released version so far. > "get_tls_certificates", (1, ??); > diff --git a/lib/handle.c b/lib/handle.c > index 8713e18..ac64fe9 100644 > --- a/lib/handle.c > +++ b/lib/handle.c > @@ -28,6 +28,12 @@ > #include <sys/types.h> > #include <sys/wait.h> > > +#ifdef HAVE_LINUX_VM_SOCKETS_H > +#include <linux/vm_sockets.h> > +#elif HAVE_SYS_VSOCK_H > +#include <sys/vsock.h> > +#endif > + > #include "internal.h" > > static void > @@ -483,6 +489,17 @@ nbd_unlocked_supports_tls (struct nbd_handle *h) > #endif > } > > +/* NB: is_locked = false, may_set_error = false. */ > +int > +nbd_unlocked_supports_vsock (struct nbd_handle *h) > +{ > +#ifdef AF_VSOCK > + return 1; > +#else > + return 0; > +#endif > +} > + > /* NB: is_locked = false, may_set_error = false. */ > int > nbd_unlocked_supports_uri (struct nbd_handle *h) >