On 09/01/22 22:46, Richard W.M. Jones wrote:
> On Thu, Sep 01, 2022 at 03:38:34PM -0500, Eric Blake wrote:
>> On Thu, Sep 01, 2022 at 05:29:21PM +0100, Richard W.M. Jones wrote:
>>> On Wed, Aug 31, 2022 at 09:39:20AM -0500, Eric Blake wrote:
>>>> Add a new control knob nbd_set_request_meta_context(), modeled after
>>>> the existing nbd_set_request_structured_replies(), to make it possible
>>>> to skip the NBD_OPT_SET_META_CONTEXT half of the two-command sequence
>>>> currently performed in nbd_opt_go() and nbd_opt_info(). Also add a
>>>> counterpart nbd_get_request_meta_context() for symmetry.
>>>>
>>>
>>> This really needs an example.
>>
>> Will do. Sounds like I have enough incremental things to improve that
>> it will be worth a v3 series, although I'll continue to let more
>> reviews come on in the rest of the series before I post that.
>
> I did look at all the patches in the series and didn't have any other
> comments.
>
> TBH I don't mind if you want to post another round or just push
> something. We've got plenty of time to test and fix things up before
> the next release.
I'd be OK with the reviewed front of the series being merged, and then a
v3 of the tail being posted (if that's not too much burden for Eric).
I'm between patches #3 and #4, so if Eric agrees, I could resume my
review with *v3* of patch#4.
I can also continue reviewing this version if that's preferred.
(The main reason I'm trying to review this series meticulously is to
learn about libnbd for the long term; it's a learning investment on my
part. It's not because I insist that the series not be merged without my
approval! :))
Laszlo
>
>> I'm wondering whether it is easier to modify the existing
>> examples/list-exports.c, or to write a second example. Over the
>> course of this series, as more and more APIs are added, the example
>> can cover more steps. By the end of the series, it would look
>> something like:
>>
>> // error checking omitted here for brevity, but I'll probably add it in the
>> example
>> nbd = nbd_create ();
>> nbd_set_tls (nbd, LIBNBD_TLS_DISABLE); // we'll manually enable tls later
>> nbd_set_request_structured_reply (nbd, false); // we'll manually request SR
>> later
>> nbd_set_request_meta_context (nbd, false); // we'll do this ourselves
>> nbd_set_opt_mode (nbd, true);
>> nbd_connect_...();
>> if (!nbd_aio_is_negotiating (nbd))
>> fatal ("server does not support listing");
>> // Determine which exports (if any) are exposed without TLS
>> printf ("unencrypted exports:\n");
>> nbd_opt_list (nbd, callback_exports);
>> // Now enable TLS, and try again
>> if (nbd_opt_starttls (nbd) != 0)
>> fatal ("server does not support TLS");
>> printf ("encrypted exports:\n");
>> nbd_opt_list (nbd, callback_exports);
>> printf ("which export to learn more about?");
>> get user's choice...
>> nbd_set_export_name (nbd, name);
>> if (nbd_opt_structured_reply (nbd) == 1) {
>> printf ("metacontexts available:");
>> nbd_opt_list_meta_context (nbd, callback_contexts);
>> while (1) {
>> printf ("which context to request, or -1 to finish connecting");
>> get user's choice
>> if (choice == -1) break;
>> nbd_add_meta_context (nbd, choice);
>> }
>> nbd_opt_set_meta_context (nbd, callback_silent);
>> }
>> else
>> printf ("metacontexts not available\n");
>> nbd_opt_go (nbd);
>
> I don't have a preference, but the thing to bear in mind is that
> people often copy examples verbatim.
>
> So it's best to make examples be as simple and clear as possible, and
> especially not contain any "gotchas" -- eg code that if copied without
> understanding will make things slow or complicated. More examples are
> fine when they demonstrate distinct things.
>
>>>> @@ -3246,6 +3299,10 @@ 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. *)
>>>> + "set_request_meta_context", (1, 14);
>>>> + "get_request_meta_context", (1, 14);
>>>
>>> I think this should be 1.15.x .. 1.16, since 1.14 is already out.
>>
>> You're right. Fixed for the nbd_supports_vsock() patch, since that one
>> will beat this series in.
>
> Rich.
>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
