Re: [Libguestfs] [libnbd PATCH 0/3] Simplify nbd_shutdown vs. opt mode
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 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
Re: [Libguestfs] [libnbd PATCH 0/3] Simplify nbd_shutdown vs. opt mode
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. Whether or not we need to have a flag to disable it (ie the opposite sense to the proposed flag) is up to you. For the series: Reviewed-by: Richard W.M. Jones Rich. > Eric Blake (3): > tests: Test behavior of nbd_shutdown during opt mode > api: Add new COVER_OPT_MODE flag to nbd_shutdown > info: Simplify shutdown calls > > generator/API.ml | 21 -- > lib/disconnect.c | 15 > tests/Makefile.am | 5 ++ > tests/shutdown-opt-mode.c | 149 ++ > .gitignore| 1 + > info/list.c | 8 +- > info/main.c | 4 +- > 7 files changed, 188 insertions(+), 15 deletions(-) > create mode 100644 tests/shutdown-opt-mode.c > > -- > 2.41.0 > > ___ > Libguestfs mailing list > Libguestfs@redhat.com > 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 Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [libnbd PATCH 0/3] Simplify nbd_shutdown vs. opt mode
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). Eric Blake (3): tests: Test behavior of nbd_shutdown during opt mode api: Add new COVER_OPT_MODE flag to nbd_shutdown info: Simplify shutdown calls generator/API.ml | 21 -- lib/disconnect.c | 15 tests/Makefile.am | 5 ++ tests/shutdown-opt-mode.c | 149 ++ .gitignore| 1 + info/list.c | 8 +- info/main.c | 4 +- 7 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 tests/shutdown-opt-mode.c -- 2.41.0 ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
