On Thu, Sep 01, 2022 at 08:07:33AM -0500, Eric Blake wrote:
> On Thu, Sep 01, 2022 at 08:11:17AM +0200, Laszlo Ersek wrote:
> > 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.)
>
> How about the following wording:
>
> Returns true if libnbd was compiled with support for the C<AF_VSOCK>
> family of sockets, or false if not. Note that on the Linux
> operating system, this returns true if there is compile-time
> support, but you may still need runtime support for some aspects
> of AF_VSOCK usage; for example, use of C<VMADDR_CID_LOCAL> as the
> server name requires that the I<vsock_loopback> kernel module is
> loaded.
You can use paragraphs (and even subheadings) in the POD
here if you want.
Anyway, looks good with the enhanced documentation so:
Reviewed-by: Richard W.M. Jones <[email protected]>
Rich.
>
> >
> > So with the longdesc expansion:
> >
> > Acked-by: Laszlo Ersek <[email protected]>
>
> Thanks; I'll wait a bit to see if you like my updated wording first.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
> _______________________________________________
> Libguestfs mailing list
> [email protected]
> https://listman.redhat.com/mailman/listinfo/libguestfs
--
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
_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs
