On Fri, Sep 01, 2023 at 10:28:52PM +0100, Richard W.M. Jones wrote: > On Tue, Aug 29, 2023 at 05:20:40PM -0500, Eric Blake wrote: > > While working on a larger set of patches to make nbdinfo favor > > NBD_OPT_INFO over NBD_OPT_GO where possible (which requires use of > > nbd_set_opt_mode(,true) in more cases), I noticed that it got unwieldy > > to have to pick the correct shutdown function in all code paths. So I > > propose making the API smarter, by adding an opt-in flag that does the > > right thing on my behalf. > > > > If you have an idea for a better name for the flag, or think this > > functionality should be enabled by default, let me know. Part of the > > reason for choosing a new flag is that it becomes a compile-time > > witness of whether nbd_shutdown has the desired capability (if we > > allow it to auto-opt_abort without a flag, it's harder to tell whether > > we are running against an older libnbd where it errors out instead). > > My feeling is this should be enabled by default, as that does the > right thing by default.
Makes sense. Certainly easier to write nbd_shutdown(nbd, 0) than nbd_shutdown(nbd, LIBNBD_SHUTDOWN_COVER_OPT_MODE). > > Whether or not we need to have a flag to disable it (ie the opposite > sense to the proposed flag) is up to you. At this point, there are so few affected callers (most programs don't use nbd_set_opt_mode, and nbdinfo is patched by this series), so for now I won't bother to add a witness flag; but I will go ahead and backport the fix to stable branches. Adding a flag in a followup patch (with the opposite sense of NOT shutting down if still in opt mode - mainly as the witness of the feature existing) is still an option. > > For the series: > Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> I'll reply again with commit ids once I've amended patch 2 and 3; I've also just replied with a separate mail (oops, I didn't title it 4/3) with the reasons behind this series in the first place, before I can finish checking in the tail end of the 64-bit extension series. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
