Eric Blake
2019-Nov-22 20:55 UTC
Re: [Libguestfs] [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
On 11/22/19 1:53 PM, Richard W.M. Jones wrote:> To avoid breaking existing plugins, Python plugins wishing to use > version 2 of the API must opt in by declaring: > > def api_version(): > return 2 > > (Plugins which do not do this are assumed to want API version 1).Could we also permit the python code to declare a global variable instead of a function? But a function is just fine (and easier to integrate the way we do all our other callbacks).> +++ b/plugins/python/example.py > @@ -34,6 +34,13 @@ import errno > disk = bytearray(1024 * 1024) > > > +# There are several variants of the API. nbdkit will call this > +# function first to determine which one you want to use. This is the > +# latest version at the time this example was written. > +def api_version(): > + return 2Matches the C counterpart of #define NBDKIT_API_VERSION 2 at the top of a plugin.> + > + > # This just prints the extra command line parameters, but real plugins > # should parse them and reject any unknown parameters. > def config(key, value): > @@ -54,20 +61,20 @@ def get_size(h): > return len(disk) > > > -def pread(h, count, offset): > +def pread(h, count, offset, flags): > global disk > return disk[offset:offset+count]Do we really want to be passing 'flags' as an integer that the python script must decode? Could we instead pass the flags as named kwargs? For pread, there are no defined flags, so that would mean we stick with def pread(h, count, offset):> > > -def pwrite(h, buf, offset): > +def pwrite(h, buf, offset, flags):but for pwrite, it would look like: def pwrite(h, buf, offset, fua=False):> > -def zero(h, count, offset, may_trim): > +def zero(h, count, offset, flags):and for zero (once fast zero is supported later in the series), it could look like: def zero(h, count, offset, may_trim=False, fua=False, fast=False): The user could also write: def zero(h, count, offset, **flags) and manually extract key/value pairs out of the flags, to be robust to unknown flags (although our API somewhat promises that we never pass flags to the data-handling callbacks unless the can_FOO callbacks already indicated that the plugin was willing to support the flag)> +++ b/plugins/python/nbdkit-python-plugin.pod > @@ -82,6 +82,19 @@ I<--dump-plugin> option, eg: > python_version=3.7.0 > python_pep_384_abi_version=3 > > +=head2 API versions > + > +The nbdkit API has evolved and new versions are released periodically. > +To ensure backwards compatibility plugins have to opt in to the new > +version. From Python you do this by declaring a function in your > +module: > + > + def api_version(): > + return 2 > + > +(where 2 is the latest version at the time this documentation was > +written). All newly written Python modules must have this function. > + > =head2 Executable script > > If you want you can make the script executable and include a "shebang" > @@ -120,6 +133,10 @@ nbdkit-plugin(3).Unrelated side topic: in your recent addition of eval.sh, you wondered if we should promote it to a full-blown plugin rather than just an example script. But reading 'man nbdkit-sh-plugin', there is no mention of turning an executable script into a full-blown plugin via a shebang, the way that python documents it. [I guess 'man nbdkit' sort of mentions it under Shebang scripts]> - def pwrite(h, buf, offset): > + def pwrite(h, buf, offset, flags): > length = len (buf) > # no return value > > The body of your C<pwrite> function should write the C<buf> string to > the disk. You should write C<count> bytes to the disk starting at > -C<offset>. > +C<offset>. C<flags> may contain C<nbdkit.FLAG_FUA>.Should we mention that FLAG_FUA is only set if the python callback can_fua returned 1? Or is that somewhat redundant with the fact that we already document that someone writing a python plugin should be familiar with the docs for C plugins, and that guarantee is already in the C plugin docs?> - def zero(h, count, offset, may_trim): > + def zero(h, count, offset, flags): > # no return value > > -The body of your C<zero> function should ensure that C<count> bytes > -of the disk, starting at C<offset>, will read back as zero. If > -C<may_trim> is true, the operation may be optimized as a trim as long > -as subsequent reads see zeroes. > +The body of your C<zero> function should ensure that C<count> bytes of > +the disk, starting at C<offset>, will read back as zero. C<flags> is > +a bitmask which may include C<nbdkit.FLAG_MAY_TRIM>, > +C<nbdkit.FLAG_FUA>, C<nbdkit.FLAG_FAST_ZERO>.Well, technically FAST_ZERO can't be set until later in the series when you plumb in the can_fast_zero callback... :)> static int > -py_pwrite (void *handle, const void *buf, > - uint32_t count, uint64_t offset) > +py_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, > + uint32_t flags) > { > PyObject *obj = handle; > PyObject *fn; > @@ -515,9 +546,19 @@ py_pwrite (void *handle, const void *buf, > if (callback_defined ("pwrite", &fn)) { > PyErr_Clear (); > > - r = PyObject_CallFunction (fn, "ONL", obj, > - PyByteArray_FromStringAndSize (buf, count), > - offset, NULL); > + switch (py_api_version) { > + case 1: > + r = PyObject_CallFunction (fn, "ONL", obj, > + PyByteArray_FromStringAndSize (buf, count), > + offset, NULL);Here, we could assert (flags == 0) (the FUA flag should not be set if the plugin uses v1 API).> @@ -565,7 +614,15 @@ py_trim (void *handle, uint32_t count, uint64_t offset) > if (callback_defined ("trim", &fn)) { > PyErr_Clear (); > > - r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL); > + switch (py_api_version) { > + case 1: > + r = PyObject_CallFunction (fn, "OiL", obj, count, offset, NULL); > + break;Again, we may want to assert that flags does not include FUA here.> static int > -py_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) > +py_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) > { > PyObject *obj = handle; > PyObject *fn; > @@ -590,9 +647,20 @@ py_zero (void *handle, uint32_t count, uint64_t offset, int may_trim) > PyErr_Clear (); > > last_error = 0; > - r = PyObject_CallFunction (fn, "OiLO", > - obj, count, offset, > - may_trim ? Py_True : Py_False, NULL); > + switch (py_api_version) { > + case 1: { > + int may_trim = flags & NBDKIT_FLAG_MAY_TRIM;and maybe assert that no other flags are set. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Richard W.M. Jones
2019-Nov-22 21:07 UTC
[Libguestfs] Shebang sh plugins (was: Re: [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.)
On Fri, Nov 22, 2019 at 02:55:31PM -0600, Eric Blake wrote:> Unrelated side topic: in your recent addition of eval.sh, you > wondered if we should promote it to a full-blown plugin rather than > just an example script. But reading 'man nbdkit-sh-plugin', there > is no mention of turning an executable script into a full-blown > plugin via a shebang, the way that python documents it. [I guess > 'man nbdkit' sort of mentions it under Shebang scripts]I believe it's not possible to do it for sh plugins. For (eg) python plugins it works like this: #!/usr/sbin/nbdkit python -> runs nbdkit python <script name> -> <script name> is interpreted as magic script= parameter -> the python plugin works by loading the script using PyRun_SimpleFileEx which interprets the contents of the script as python code However for shell it doesn't work: #!/usr/sbin/nbdkit sh -> runs nbdkit sh <script name> -> <script name> is interpreted as magic script= parameter -> the sh plugin works by actually executing the script -> executing the script repeats the steps over from the top, causing an infinite loop If you can think of a way to make this work it would be a useful feature IMO. Rich. -- 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
Richard W.M. Jones
2019-Nov-22 21:14 UTC
Re: [Libguestfs] [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
On Fri, Nov 22, 2019 at 02:55:31PM -0600, Eric Blake wrote:> On 11/22/19 1:53 PM, Richard W.M. Jones wrote: > >To avoid breaking existing plugins, Python plugins wishing to use > >version 2 of the API must opt in by declaring: > > > > def api_version(): > > return 2 > > > >(Plugins which do not do this are assumed to want API version 1). > > Could we also permit the python code to declare a global variable > instead of a function? But a function is just fine (and easier to > integrate the way we do all our other callbacks).I couldn't work out how to do this, plus we have the callback_defined function which makes this easy, so yes, I did the easy thing :-)> >@@ -54,20 +61,20 @@ def get_size(h): > > return len(disk) > >-def pread(h, count, offset): > >+def pread(h, count, offset, flags): > > global disk > > return disk[offset:offset+count] > > Do we really want to be passing 'flags' as an integer that the > python script must decode?While I'm no Python programmer, it does look as if existing Python core APIs are happy to use bitmasks, eg: https://docs.python.org/3/library/os.html#os.open and this is also easy to implement and keeps it similar to the C API.> >- def pwrite(h, buf, offset): > >+ def pwrite(h, buf, offset, flags): > > length = len (buf) > > # no return value > > The body of your C<pwrite> function should write the C<buf> string to > > the disk. You should write C<count> bytes to the disk starting at > >-C<offset>. > >+C<offset>. C<flags> may contain C<nbdkit.FLAG_FUA>. > > Should we mention that FLAG_FUA is only set if the python callback > can_fua returned 1? Or is that somewhat redundant with the fact > that we already document that someone writing a python plugin should > be familiar with the docs for C plugins, and that guarantee is > already in the C plugin docs?I think it would be better if we simply referred back to the C documentation to avoid duplication. Also an advantage of using bitmasks. That does require a rather larger change to the documentation though.> > static int > >-py_pwrite (void *handle, const void *buf, > >- uint32_t count, uint64_t offset) > >+py_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, > >+ uint32_t flags) > > { > > PyObject *obj = handle; > > PyObject *fn; > >@@ -515,9 +546,19 @@ py_pwrite (void *handle, const void *buf, > > if (callback_defined ("pwrite", &fn)) { > > PyErr_Clear (); > >- r = PyObject_CallFunction (fn, "ONL", obj, > >- PyByteArray_FromStringAndSize (buf, count), > >- offset, NULL); > >+ switch (py_api_version) { > >+ case 1: > >+ r = PyObject_CallFunction (fn, "ONL", obj, > >+ PyByteArray_FromStringAndSize (buf, count), > >+ offset, NULL); > > Here, we could assert (flags == 0) (the FUA flag should not be set > if the plugin uses v1 API).Is that true? The plugin asserts that it wants the v1 API, but we are still using the v2 C API, whatever the plugin asks for? I'm cautious about adding asserts unless they can never happen now or in the future, because we don't really want an assert() happening on a customer site running virt-v2v with the rhv-upload Python plugin. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Nir Soffer
2019-Nov-22 21:20 UTC
Re: [Libguestfs] [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
On Fri, Nov 22, 2019 at 10:55 PM Eric Blake <eblake@redhat.com> wrote:> > On 11/22/19 1:53 PM, Richard W.M. Jones wrote: > > To avoid breaking existing plugins, Python plugins wishing to use > > version 2 of the API must opt in by declaring: > > > > def api_version(): > > return 2 > > > > (Plugins which do not do this are assumed to want API version 1). > > Could we also permit the python code to declare a global variable > instead of a function? But a function is just fine (and easier to > integrate the way we do all our other callbacks). > > > > +++ b/plugins/python/example.py > > @@ -34,6 +34,13 @@ import errno > > disk = bytearray(1024 * 1024) > > > > > > +# There are several variants of the API. nbdkit will call this > > +# function first to determine which one you want to use. This is the > > +# latest version at the time this example was written. > > +def api_version(): > > + return 2 > > Matches the C counterpart of #define NBDKIT_API_VERSION 2 at the top of > a plugin.This is the same thing I was thinking about. This makes it more clear that the api version is constant, and not something the plugin should change while it is being used. Same for all can_xxx functions, NBD does not support changing any of these anyway after negotiation. v2 is an opportunity to do this change if we want to. ...> > -def pread(h, count, offset): > > +def pread(h, count, offset, flags): > > global disk > > return disk[offset:offset+count] > > Do we really want to be passing 'flags' as an integer that the python > script must decode? Could we instead pass the flags as named kwargs? > For pread, there are no defined flags, so that would mean we stick withI was going to comment about the same thing.> > def pread(h, count, offset): > > > > > > > -def pwrite(h, buf, offset): > > +def pwrite(h, buf, offset, flags): > > but for pwrite, it would look like: > > def pwrite(h, buf, offset, fua=False): > > > > > -def zero(h, count, offset, may_trim): > > +def zero(h, count, offset, flags): > > and for zero (once fast zero is supported later in the series), it could > look like: > > def zero(h, count, offset, may_trim=False, fua=False, fast=False):This is nicer - but not extensible.> The user could also write: > > def zero(h, count, offset, **flags)This is less nice, but easy to extend without adding v3. But I would call it **options instead.> > and manually extract key/value pairs out of the flags, to be robust to > unknown flags (although our API somewhat promises that we never pass > flags to the data-handling callbacks unless the can_FOO callbacks > already indicated that the plugin was willing to support the flag)... Nir
Nir Soffer
2019-Nov-22 21:23 UTC
Re: [Libguestfs] [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
On Fri, Nov 22, 2019 at 11:14 PM Richard W.M. Jones <rjones@redhat.com> wrote:> > On Fri, Nov 22, 2019 at 02:55:31PM -0600, Eric Blake wrote: > > On 11/22/19 1:53 PM, Richard W.M. Jones wrote: > > >To avoid breaking existing plugins, Python plugins wishing to use > > >version 2 of the API must opt in by declaring: > > > > > > def api_version(): > > > return 2 > > > > > >(Plugins which do not do this are assumed to want API version 1). > > > > Could we also permit the python code to declare a global variable > > instead of a function? But a function is just fine (and easier to > > integrate the way we do all our other callbacks). > > I couldn't work out how to do this, plus we have the callback_defined > function which makes this easy, so yes, I did the easy thing :-) > > > >@@ -54,20 +61,20 @@ def get_size(h): > > > return len(disk) > > >-def pread(h, count, offset): > > >+def pread(h, count, offset, flags): > > > global disk > > > return disk[offset:offset+count] > > > > Do we really want to be passing 'flags' as an integer that the > > python script must decode? > > While I'm no Python programmer, it does look as if existing Python > core APIs are happy to use bitmasks, eg: > > https://docs.python.org/3/library/os.html#os.open > > and this is also easy to implement and keeps it similar to the C API.Lot of python code is a thin wrapper over C code, and low level details like flags leak out to python, so this is a reasonable API. But having more pythonic API can make python developer little happier when working with this code. I don't know if it is worth the extra effort. Nir
Eric Blake
2019-Nov-22 21:30 UTC
Re: [Libguestfs] [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
On 11/22/19 3:14 PM, Richard W.M. Jones wrote:>>> @@ -54,20 +61,20 @@ def get_size(h): >>> return len(disk) >>> -def pread(h, count, offset): >>> +def pread(h, count, offset, flags): >>> global disk >>> return disk[offset:offset+count] >> >> Do we really want to be passing 'flags' as an integer that the >> python script must decode? > > While I'm no Python programmer, it does look as if existing Python > core APIs are happy to use bitmasks, eg: > > https://docs.python.org/3/library/os.html#os.open > > and this is also easy to implement and keeps it similar to the C API.Fair enough. I'm just making sure that our Python interface is at least idiomatic to a Python programmer, rather than blatantly being a naive translation of a C programmer.> >>> - def pwrite(h, buf, offset): >>> + def pwrite(h, buf, offset, flags): >>> length = len (buf) >>> # no return value >>> The body of your C<pwrite> function should write the C<buf> string to >>> the disk. You should write C<count> bytes to the disk starting at >>> -C<offset>. >>> +C<offset>. C<flags> may contain C<nbdkit.FLAG_FUA>. >> >> Should we mention that FLAG_FUA is only set if the python callback >> can_fua returned 1? Or is that somewhat redundant with the fact >> that we already document that someone writing a python plugin should >> be familiar with the docs for C plugins, and that guarantee is >> already in the C plugin docs? > > I think it would be better if we simply referred back to the C > documentation to avoid duplication. Also an advantage of using > bitmasks. That does require a rather larger change to the > documentation though.Pointing to the C docs, and focusing on just the bindings-induced differences, is fine.> >>> static int >>> -py_pwrite (void *handle, const void *buf, >>> - uint32_t count, uint64_t offset) >>> +py_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, >>> + uint32_t flags) >>> { >>> PyObject *obj = handle; >>> PyObject *fn; >>> @@ -515,9 +546,19 @@ py_pwrite (void *handle, const void *buf, >>> if (callback_defined ("pwrite", &fn)) { >>> PyErr_Clear (); >>> - r = PyObject_CallFunction (fn, "ONL", obj, >>> - PyByteArray_FromStringAndSize (buf, count), >>> - offset, NULL); >>> + switch (py_api_version) { >>> + case 1: >>> + r = PyObject_CallFunction (fn, "ONL", obj, >>> + PyByteArray_FromStringAndSize (buf, count), >>> + offset, NULL); >> >> Here, we could assert (flags == 0) (the FUA flag should not be set >> if the plugin uses v1 API). > > Is that true? The plugin asserts that it wants the v1 API, but we are > still using the v2 C API, whatever the plugin asks for?Hmm. If the user implements a 'can_fua' callback that returns 1 even though they forgot to declare 'api_version', then the flag can indeed be set. Perhaps that means our 'can_fua' C wrapper should have a version 1/2 difference in behavior (in v1, raise an error reminding the user to fix their plugin to declare 'api_version', in v2 pass back the result), so that we could indeed then use the assert with a guarantee that it will not trigger on an arbitrary plugin.> > I'm cautious about adding asserts unless they can never happen now or > in the future, because we don't really want an assert() happening on a > customer site running virt-v2v with the rhv-upload Python plugin.Understood. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Eric Blake
2019-Nov-22 21:35 UTC
Re: [Libguestfs] [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
On 11/22/19 3:20 PM, Nir Soffer wrote:>>> +# There are several variants of the API. nbdkit will call this >>> +# function first to determine which one you want to use. This is the >>> +# latest version at the time this example was written. >>> +def api_version(): >>> + return 2 >> >> Matches the C counterpart of #define NBDKIT_API_VERSION 2 at the top of >> a plugin. > > This is the same thing I was thinking about. This makes it more clear > that the api > version is constant, and not something the plugin should change while > it is being used.Hmm - api_version() really is constant for the entire life of nbdkit. We call it exactly once. Figuring out how to read a Python variable instead of calling a function would be slightly more in line with the fact that in C code it is a #define constant rather than a function pointer callback. But it is that much more glue code to figure out how to check for a python global variable, compared to the glue code we already have for calling a python function.> > Same for all can_xxx functions, NBD does not support changing any of > these anyway > after negotiation.While can_xxx functions are somewhat dynamic (we only call them once per connection, but connection A can be readonly while connection B is read-write, changing the can_write result, for example). So those have to remain functions.>> and for zero (once fast zero is supported later in the series), it could >> look like: >> >> def zero(h, count, offset, may_trim=False, fua=False, fast=False): > > This is nicer - but not extensible.Why not? Any future flag additions would still appear as new key=value kwargs.> >> The user could also write: >> >> def zero(h, count, offset, **flags) > > This is less nice, but easy to extend without adding v3. But I would > call it **options > instead.I really need to post my patches where I played with kwargs, for contrast to this version. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Nir Soffer
2019-Nov-22 23:58 UTC
Re: [Libguestfs] Shebang sh plugins (was: Re: [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.)
On Fri, Nov 22, 2019 at 11:07 PM Richard W.M. Jones <rjones@redhat.com> wrote:> > On Fri, Nov 22, 2019 at 02:55:31PM -0600, Eric Blake wrote: > > Unrelated side topic: in your recent addition of eval.sh, you > > wondered if we should promote it to a full-blown plugin rather than > > just an example script. But reading 'man nbdkit-sh-plugin', there > > is no mention of turning an executable script into a full-blown > > plugin via a shebang, the way that python documents it. [I guess > > 'man nbdkit' sort of mentions it under Shebang scripts] > > I believe it's not possible to do it for sh plugins. > > For (eg) python plugins it works like this: > > #!/usr/sbin/nbdkit python > -> runs nbdkit python <script name> > -> <script name> is interpreted as magic script= parameter > -> the python plugin works by loading the script using > PyRun_SimpleFileEx which interprets the contents of the > script as python code > > However for shell it doesn't work: > > #!/usr/sbin/nbdkit sh > -> runs nbdkit sh <script name> > -> <script name> is interpreted as magic script= parameter > -> the sh plugin works by actually executing the script > -> executing the script repeats the steps over from the top, > causing an infinite loop > > If you can think of a way to make this work it would be a useful > feature IMO.Even if we can find a way, it sounds too complicated, and I don't see the use case. Nir
On 11/22/19 3:07 PM, Richard W.M. Jones wrote:> On Fri, Nov 22, 2019 at 02:55:31PM -0600, Eric Blake wrote: >> Unrelated side topic: in your recent addition of eval.sh, you >> wondered if we should promote it to a full-blown plugin rather than >> just an example script. But reading 'man nbdkit-sh-plugin', there >> is no mention of turning an executable script into a full-blown >> plugin via a shebang, the way that python documents it. [I guess >> 'man nbdkit' sort of mentions it under Shebang scripts] > > I believe it's not possible to do it for sh plugins. > > For (eg) python plugins it works like this: > > #!/usr/sbin/nbdkit python > -> runs nbdkit python <script name> > -> <script name> is interpreted as magic script= parameter > -> the python plugin works by loading the script using > PyRun_SimpleFileEx which interprets the contents of the > script as python code > > However for shell it doesn't work: > > #!/usr/sbin/nbdkit sh > -> runs nbdkit sh <script name> > -> <script name> is interpreted as magic script= parameter > -> the sh plugin works by actually executing the script > -> executing the script repeats the steps over from the top, > causing an infinite loop > > If you can think of a way to make this work it would be a useful > feature IMO.Hmm. Right now, we document that: nbdkit sh file requires 'file' to be executable, and that we call exec*("file", { "file", "op", ... }) and that the interpreter need not be sh (file can start with any #! to run a script that will be executed by another interpreter). In order to get a shebang redirect to work, though, it seems like we must NOT directly re-execute the file, but instead force the file to be parsed by "/bin/sh" (or have some other means of identifying exactly which interpreter to use, outside of the normal #! means), where we would invoke exec*("/bin/sh", {"file", "op", ...}) and thereby skip the kernel's attempt to re-execute under a different interpreter. Perhaps we could get this by teaching the sh plugin to read() the first line of the script; if it starts with #! and contains 'nbdkit sh', then we force exec through "sh" (hmm, is there a way to force exec through bash instead?). Otherwise, we exec through the file-name as-is. Then our sh plugin would support two modes: the existing mode (we exec any interpreter by letting the kernel interpret the #! line, but you can't get shebang forwarding over to nbdkit), and the new mode (we exec a shell directly to bypass kernel shebang interpretation, but now you can write a shell script that gets shebang forwarding over to nbdkit). It's also a bummer that different platforms vary on how #! lines are parsed - you can't portably pass more than a single argument, so there is no portable way to write '#!/path/to/nbdkit sh shell=/bin/bash' as a convenient way to choose which shell to use when exec'ing the script while bypassing the #!, short of having nbdkit reimplement what GNU coreutils recently added as '/bin/env -S' (and that reimplementation would have to be in nbdkit proper, not in the sh plugin, because such a shebang line would result in 'nbdkit' 'sh shell=/bin/bash' 'file' 'args', but there is no '/path/to/nbdkit-plugin-sh shell=/bin/bash.so' plugin to be loaded). If we choose to make the sh plugin smart enough to parse line 1 to see if it is a shebang containing the substring 'nbdkit sh', we could also make it parse line 2 to pick up some magic comment line of 'SHELL=/bin/bash' as the binary to exec. That's a bit more complicated to document, but might serve to let us use the sh plugin for the two separate modes documented above (mode 1 to run an arbitrary executable file which may use shebang to call out its own interpreter, mode 2 to run an arbitrary file with a fixed interpreter allowing that file to start with a shebang to call out to nbdkit). Or maybe we create two different plugins (keep the existing 'sh' plugin with existing semantics, and add a new 'shell' plugin that shares 99% of the code but forces execution with a fixed shell). The other thing to consider is what brought this topic up - a question of whether plugins/sh/eval.sh is worth a conversion into a standalone plugin. With other languages, our choice is between: nbdkit python script ./script (where #! reinvokes nbdkit python script) But we don't have a way to write: nbdkit script where nbdkit recognizes that 'script' is not a .so file, but IS a file that calls out python in its #!, and therefore attempts to load the python.so plugin. The question with the eval.sh plugin is if we can write: nbdkit eval pread=... and have nbdkit figure out that 'eval' is not a plugin but IS a file with a #! calling out 'nbdkit sh', and therefore load the 'sh' plugin with 'script=eval'. In other words, maybe it's time to teach nbdkit general magic on how to handle the plugin argument (if an .so is found then load it; if not, then inspect the file to see which .so to use instead). Or maybe we just write a C form of a plugin named 'eval' that does the same things as what 'sh eval.sh' does, sharing as much code as possible, but where you have no shebang form because you never pass a script= argument to the eval plugin. All interesting thoughts, but I'm not sure I got us any closer to a nice conclusion. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Seemingly Similar Threads
- Re: [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
- Re: [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
- [PATCH nbdkit v2 03/10] python: Implement nbdkit API version 2.
- [PATCH nbdkit v3 2/7] python: Implement nbdkit API version 2.
- [PATCH nbdkit v3 0/7] Implement nbdkit API v2 for Python plugins.