[Libguestfs] [libnbd PATCH] RFC: api: Add nbd_supports_vsock()

[Laszlo Ersek]

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)
>