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