Re: [Libguestfs] [nbdkit PATCH] nbd: Another libnbd version bump
On 8/15/19 10:37 AM, Richard W.M. Jones wrote:
> On Thu, Aug 15, 2019 at 09:27:14AM -0500, Eric Blake wrote:
>> The 0.9.8 release breaks API, requiring a number of changes:
>> - Use symbolic constants instead of magic numbers/open-coded strings
>> (well, the string for "base:allocation" was present before this
>> libnbd bump)
>> - Change callbacks to drop the valid_flag parameter
>> - Add _is to nbd_read_only call
>> - Drop the _callback suffix on nbd_aio_FOO calls
>> - Add a struct for managing callback/user_data at once
>
> Seems reasonable.
>
>> @@ -160,11 +160,12 @@ nbdplug_config (const char *key, const char *value)
>> if (strcasecmp (value, "require") == 0 ||
>> strcasecmp (value, "required") == 0 ||
>> strcasecmp (value, "force") == 0)
>> - tls = 2;
>> + tls = LIBNBD_TLS_REQUIRE;
>> else {
>> - tls = nbdkit_parse_bool (value);
>> - if (tls == -1)
>> + r = nbdkit_parse_bool (value);
>> + if (r == -1)
>> exit (EXIT_FAILURE);
>> + tls = r ? LIBNBD_TLS_ALLOW : LIBNBD_TLS_DISABLE;
For this patch, it was straight conversion. If we change things, that
belongs in a separate patch.
>
> Our feedback was the LIBNBD_TLS_ALLOW was really bad (I'm unconvinced
> because I prefer my stuff to work and TLS very often doesn't). Do you
> think we should use REQUIRE here as well?
The existing behavior of 'nbd tls=...' matches the 'nbdkit --tls=...'
behavior. Whether to keep both options as tri-state, or whether to make
'--tls=on'/'--tls=yes' default to strict, and add yet another spelling
for '--tls=optional' as something that is harder to spell for the
specific case when you are willing to open yourself to
man-in-the-middle, may have merit.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit PATCH] nbd: Another libnbd version bump
On Thu, Aug 15, 2019 at 09:27:14AM -0500, Eric Blake wrote:
> The 0.9.8 release breaks API, requiring a number of changes:
> - Use symbolic constants instead of magic numbers/open-coded strings
> (well, the string for "base:allocation" was present before this
> libnbd bump)
> - Change callbacks to drop the valid_flag parameter
> - Add _is to nbd_read_only call
> - Drop the _callback suffix on nbd_aio_FOO calls
> - Add a struct for managing callback/user_data at once
Seems reasonable.
> @@ -160,11 +160,12 @@ nbdplug_config (const char *key, const char *value)
> if (strcasecmp (value, "require") == 0 ||
> strcasecmp (value, "required") == 0 ||
> strcasecmp (value, "force") == 0)
> - tls = 2;
> + tls = LIBNBD_TLS_REQUIRE;
> else {
> - tls = nbdkit_parse_bool (value);
> - if (tls == -1)
> + r = nbdkit_parse_bool (value);
> + if (r == -1)
> exit (EXIT_FAILURE);
> + tls = r ? LIBNBD_TLS_ALLOW : LIBNBD_TLS_DISABLE;
Our feedback was the LIBNBD_TLS_ALLOW was really bad (I'm unconvinced
because I prefer my stuff to work and TLS very often doesn't). Do you
think we should use REQUIRE here as well?
Rich.
--
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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [nbdkit PATCH] nbd: Another libnbd version bump
Oh that was an ACK whatever you decide :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
