Re: [Libguestfs] [PATCH v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.
On Sat, Sep 21, 2019 at 08:59:54PM +0100, Richard W.M. Jones wrote: On Sat, Sep 21, 2019 at 09:45:16PM +0200, Martin Kletzander wrote: On Sat, Sep 21, 2019 at 03:54:11PM +0100, Richard W.M. Jones wrote: >On Fri, Sep 20, 2019 at 09:33:06AM -0500, Eric Blake wrote: >>Checking for file existence for filters is somewhat less fragile than >>for plugins, because all filters are built in-tree (we've specifically >>documented that we don't provide ABI guarantees for filters, so the only >>sane way to use a filter is to compile it at the same time/version as >>the nbdkit binary that will load it). But you do have a point that >>checking for files is still more fragile than just asking nbdkit whether >>a given filter exists: >> >>$ nbdkit --dump-plugin --filter=nosuch null >>nbdkit: error: cannot open filter 'nosuch': >>/usr/lib64/nbdkit/filters/nbdkit-nosuch-filter.so: cannot open shared >>object file: No such file or directory > >This is definitely the best we can do now, and I've posted a patch >suggesting this change for the nbdkit-probing(1) man page. > >However it's not without a subtle problem: It requires the null plugin >to be present. It's possible to ship nbdkit on its own. On Fedora >try installing just the nbdkit-server package. The null plugin won't >be available so this command will always fail. > >So we do also need to change nbdkit to allow easier probing for >plugins or filters in the long term. > Is it usable without any plugins? Does the null plugin take much space? I wouldn't think so. Would it be too messy to just ship the null plugin unconditionally, even if just for this particular purpose? Right! My original plan was that we could change the API to drop all required callbacks. The "null" plugin would become the plugin which had no callbacks and could therefore be built in to the nbdkit binary. Unfortunately my plan doesn't quite work because the null plugin has a config parameter (nbdkit-zero-plugin is more like this "null" plugin). Oh well. On the other hand any program that relies on such probing to work might depend not only on nbdkit, but also on the null plugin. Indeed, or as you say above we could package one of the regular plugins with the server to guarantee it is always available. Since the probing doesn't work exactly how I thought (as you described in the other thread). So I guess requiring the null plugin is enough until there is --dump-filter or something similar available. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org signature.asc Description: PGP signature ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.
On Sat, Sep 21, 2019 at 09:45:16PM +0200, Martin Kletzander wrote: > On Sat, Sep 21, 2019 at 03:54:11PM +0100, Richard W.M. Jones wrote: > >On Fri, Sep 20, 2019 at 09:33:06AM -0500, Eric Blake wrote: > >>Checking for file existence for filters is somewhat less fragile than > >>for plugins, because all filters are built in-tree (we've specifically > >>documented that we don't provide ABI guarantees for filters, so the only > >>sane way to use a filter is to compile it at the same time/version as > >>the nbdkit binary that will load it). But you do have a point that > >>checking for files is still more fragile than just asking nbdkit whether > >>a given filter exists: > >> > >>$ nbdkit --dump-plugin --filter=nosuch null > >>nbdkit: error: cannot open filter 'nosuch': > >>/usr/lib64/nbdkit/filters/nbdkit-nosuch-filter.so: cannot open shared > >>object file: No such file or directory > > > >This is definitely the best we can do now, and I've posted a patch > >suggesting this change for the nbdkit-probing(1) man page. > > > >However it's not without a subtle problem: It requires the null plugin > >to be present. It's possible to ship nbdkit on its own. On Fedora > >try installing just the nbdkit-server package. The null plugin won't > >be available so this command will always fail. > > > >So we do also need to change nbdkit to allow easier probing for > >plugins or filters in the long term. > > > > Is it usable without any plugins? Does the null plugin take much space? I > wouldn't think so. Would it be too messy to just ship the null plugin > unconditionally, even if just for this particular purpose? Right! My original plan was that we could change the API to drop all required callbacks. The "null" plugin would become the plugin which had no callbacks and could therefore be built in to the nbdkit binary. Unfortunately my plan doesn't quite work because the null plugin has a config parameter (nbdkit-zero-plugin is more like this "null" plugin). Oh well. > On the other hand any program that relies on such probing to work > might depend not only on nbdkit, but also on the null plugin. Indeed, or as you say above we could package one of the regular plugins with the server to guarantee it is always available. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] docs: Suggest how to detect if a filter is installed.
On Sat, Sep 21, 2019 at 09:41:04PM +0200, Martin Kletzander wrote: > On Sat, Sep 21, 2019 at 03:53:45PM +0100, Richard W.M. Jones wrote: > >Thanks: Eric Blake, Martin Kletzander. > >--- > >docs/nbdkit-probing.pod | 11 +++ > >1 file changed, 11 insertions(+) > > > >diff --git a/docs/nbdkit-probing.pod b/docs/nbdkit-probing.pod > >index 94bdfb4..46a7c47 100644 > >--- a/docs/nbdkit-probing.pod > >+++ b/docs/nbdkit-probing.pod > >@@ -80,6 +80,17 @@ installed. A complete shell script which does this is: > > fi > > done > > > >+=head2 Detect if a filter is installed > >+ > >+To find out if a filter is installed (and working) use > >+I<--dump-plugin> with the C plugin and the name of the filter to > >+test: > >+ > >+ nbdkit --dump-plugin --filter=foo null > >+ > >+This will fail with an error and non-zero exit code if the C > >+filter cannot be loaded. > >+ > > Looks good, for people like me it would be also nice to mention that > the above command will list the filter's parameters as well. Hehe, unfortunately it lists the plugin's parameters modulated by the filter's parameters :-( Another reason why we really need a proper dump-filter mode, one day. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.
On Sat, Sep 21, 2019 at 03:54:11PM +0100, Richard W.M. Jones wrote: On Fri, Sep 20, 2019 at 09:33:06AM -0500, Eric Blake wrote: Checking for file existence for filters is somewhat less fragile than for plugins, because all filters are built in-tree (we've specifically documented that we don't provide ABI guarantees for filters, so the only sane way to use a filter is to compile it at the same time/version as the nbdkit binary that will load it). But you do have a point that checking for files is still more fragile than just asking nbdkit whether a given filter exists: $ nbdkit --dump-plugin --filter=nosuch null nbdkit: error: cannot open filter 'nosuch': /usr/lib64/nbdkit/filters/nbdkit-nosuch-filter.so: cannot open shared object file: No such file or directory This is definitely the best we can do now, and I've posted a patch suggesting this change for the nbdkit-probing(1) man page. However it's not without a subtle problem: It requires the null plugin to be present. It's possible to ship nbdkit on its own. On Fedora try installing just the nbdkit-server package. The null plugin won't be available so this command will always fail. So we do also need to change nbdkit to allow easier probing for plugins or filters in the long term. Is it usable without any plugins? Does the null plugin take much space? I wouldn't think so. Would it be too messy to just ship the null plugin unconditionally, even if just for this particular purpose? On the other hand any program that relies on such probing to work might depend not only on nbdkit, but also on the null plugin. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html signature.asc Description: PGP signature ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] docs: Suggest how to detect if a filter is installed.
On Sat, Sep 21, 2019 at 03:53:45PM +0100, Richard W.M. Jones wrote: Thanks: Eric Blake, Martin Kletzander. --- docs/nbdkit-probing.pod | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/nbdkit-probing.pod b/docs/nbdkit-probing.pod index 94bdfb4..46a7c47 100644 --- a/docs/nbdkit-probing.pod +++ b/docs/nbdkit-probing.pod @@ -80,6 +80,17 @@ installed. A complete shell script which does this is: fi done +=head2 Detect if a filter is installed + +To find out if a filter is installed (and working) use +I<--dump-plugin> with the C plugin and the name of the filter to +test: + + nbdkit --dump-plugin --filter=foo null + +This will fail with an error and non-zero exit code if the C +filter cannot be loaded. + Looks good, for people like me it would be also nice to mention that the above command will list the filter's parameters as well. ACK =head1 SEE ALSO L. -- 2.23.0 signature.asc Description: PGP signature ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.
On Fri, Sep 20, 2019 at 09:33:06AM -0500, Eric Blake wrote: > Checking for file existence for filters is somewhat less fragile than > for plugins, because all filters are built in-tree (we've specifically > documented that we don't provide ABI guarantees for filters, so the only > sane way to use a filter is to compile it at the same time/version as > the nbdkit binary that will load it). But you do have a point that > checking for files is still more fragile than just asking nbdkit whether > a given filter exists: > > $ nbdkit --dump-plugin --filter=nosuch null > nbdkit: error: cannot open filter 'nosuch': > /usr/lib64/nbdkit/filters/nbdkit-nosuch-filter.so: cannot open shared > object file: No such file or directory This is definitely the best we can do now, and I've posted a patch suggesting this change for the nbdkit-probing(1) man page. However it's not without a subtle problem: It requires the null plugin to be present. It's possible to ship nbdkit on its own. On Fedora try installing just the nbdkit-server package. The null plugin won't be available so this command will always fail. So we do also need to change nbdkit to allow easier probing for plugins or filters in the long term. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH nbdkit] docs: Suggest how to detect if a filter is installed.
Thanks: Eric Blake, Martin Kletzander. --- docs/nbdkit-probing.pod | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/nbdkit-probing.pod b/docs/nbdkit-probing.pod index 94bdfb4..46a7c47 100644 --- a/docs/nbdkit-probing.pod +++ b/docs/nbdkit-probing.pod @@ -80,6 +80,17 @@ installed. A complete shell script which does this is: fi done +=head2 Detect if a filter is installed + +To find out if a filter is installed (and working) use +I<--dump-plugin> with the C plugin and the name of the filter to +test: + + nbdkit --dump-plugin --filter=foo null + +This will fail with an error and non-zero exit code if the C +filter cannot be loaded. + =head1 SEE ALSO L. -- 2.23.0 ___ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
[Libguestfs] [PATCH nbdkit] server: public: Add nbdkit_parse_* functions for safely parsing integers.
sscanf is sadly not safe (because it doesn't handle integer overflow
correctly), and strto*l functions are a pain to use correctly.
Therefore add some functions to hide the pain of parsing integers from
the command line.
The simpler uses of sscanf and strto*l are replaced. There are still
a few where we are using advanced features of sscanf.
---
docs/nbdkit-plugin.pod | 48 ++
filters/cache/cache.c | 16 +-
filters/cache/cache.h | 2 +-
filters/partition/partition.c | 8 +-
filters/partition/partition.h | 2 +-
filters/retry/retry.c | 14 +-
filters/xz/xz.c | 10 +-
include/nbdkit-common.h | 22 +++
plugins/curl/curl.c | 4 +-
plugins/nbd/nbd-standalone.c| 9 +-
plugins/nbd/nbd.c | 9 +-
plugins/partitioning/partitioning.c | 4 +-
plugins/random/random.c | 4 +-
plugins/ssh/ssh.c | 6 +-
plugins/vddk/vddk.c | 8 +-
server/internal.h | 2 +-
server/main.c | 21 +--
server/nbdkit.syms | 12 ++
server/public.c | 218
server/socket-activation.c | 10 +-
server/test-public.c| 181 ++-
server/usergroup.c | 4 +-
22 files changed, 527 insertions(+), 87 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 2a8..2e1913f 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -1013,6 +1013,54 @@ might be killed by C or segfault).
=head1 PARSING COMMAND LINE PARAMETERS
+=head2 Parsing numbers
+
+There are several functions for parsing numbers. These all deal
+correctly with overflow, out of range and parse errors, and you should
+use them instead of unsafe functions like L, L and
+similar.
+
+ int nbdkit_parse_int (const char *what, const char *str, int *r);
+ int nbdkit_parse_unsigned (const char *what,
+const char *str, unsigned *r);
+ int nbdkit_parse_long (const char *what, const char *str, long *r);
+ int nbdkit_parse_unsigned_long (const char *what,
+ const char *str, unsigned long *r);
+ int nbdkit_parse_int16_t (const char *what,
+ const char *str, int16_t *r);
+ int nbdkit_parse_uint16_t (const char *what,
+const char *str, uint16_t *r);
+ int nbdkit_parse_int32_t (const char *what,
+ const char *str, int32_t *r);
+ int nbdkit_parse_uint32_t (const char *what,
+const char *str, uint32_t *r);
+ int nbdkit_parse_int64_t (const char *what,
+ const char *str, int64_t *r);
+ int nbdkit_parse_uint64_t (const char *what,
+const char *str, uint64_t *r);
+ int nbdkit_parse_ssize_t (const char *what,
+ const char *str, ssize_t *r);
+ int nbdkit_parse_size_t (const char *what,
+ const char *str, size_t *r);
+
+Parse string C into an integer of various types. These functions
+parse a decimal, hexadecimal (C<"0x...">) or octal (C<"0...">) number.
+
+On success the functions return C<0> and set C<*r> to the parsed value
+(unless C<*r == NULL> in which case the result is discarded). On
+error, C is called and the functions return C<-1>.
+
+The C parameter is printed in error messages to provide context.
+It should usually be a short descriptive string of what you are trying
+to parse, eg:
+
+ if (nbdkit_parse_int ("random seed", argv[1], &seed) == -1)
+ return -1;
+
+might print an error:
+
+ random seed: could not parse number: "lalala"
+
=head2 Parsing sizes
Use the C utility function to parse human-readable
diff --git a/filters/cache/cache.c b/filters/cache/cache.c
index 14a3c0a..faf6023 100644
--- a/filters/cache/cache.c
+++ b/filters/cache/cache.c
@@ -70,7 +70,7 @@ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
unsigned blksize;
enum cache_mode cache_mode = CACHE_MODE_WRITEBACK;
int64_t max_size = -1;
-int hi_thresh = 95, lo_thresh = 80;
+unsigned hi_thresh = 95, lo_thresh = 80;
bool cache_on_read = false;
static int cache_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void
*handle, uint32_t flags, int *err);
@@ -129,22 +129,20 @@ cache_config (nbdkit_next_config *next, void *nxdata,
return 0;
}
else if (strcmp (key, "cache-high-threshold") == 0) {
-if (sscanf (value, "%d", &hi_thresh) != 1) {
- nbdkit_error ("invalid cache-high-threshold parameter: %s", value);
+if (nbdkit_parse_unsigned ("cache-high-threshold",
+ value, &hi_thresh) == -1)
return -1;
-}
-if (hi_thresh <= 0) {
+if (hi_thresh == 0) {
nbdkit_error ("cache-high-threshold must be greater than zero");
return -1;
}
return 0;
