2:40 p.m.
On Fri, Sep 20, 2019 at 09:33:06AM -0500, Eric Blake wrote:
On 9/20/19 9:04 AM, Martin Kletzander wrote:
> On Fri, Sep 20, 2019 at 10:28:18AM +0100, Richard W.M. Jones wrote:
>> The readahead filter is a self-configuring filter that makes
>> sequential reads faster when the plugin is slow (and all of the
>> plugins we use here are always slow).
>>
>> @@ -133,9 +142,17 @@ let common_create plugin_name plugin_args
>> plugin_env =
>> if have_selinux then ( (* label the socket so qemu can open
>> it *)
>> add_arg "--selinux-label"; add_arg
>> "system_u:object_r:svirt_socket_t:s0"
>> );
>> +
>> + (* Adding the readahead filter is always a win for our access
>> + * patterns. However if it doesn't exist don't worry.
>> + *)
>> + if Sys.file_exists (filterdir // "nbdkit-readahead-filter.so") then
(
>> + add_arg "--filter"; add_arg "readahead"
>
> Just out of curiosity, wouldn't it be cleaner, at least in the future,
> to add an
> option in nbdkit to report filters it can load? Of course it would require
> newer nbdkit in the long run.
>
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
Oh, this is way better than my:
nbdkit --filter=nosuch memory size=1M --run true
nbdkit: error: cannot open filter 'nosuch':
/usr/lib64/nbdkit/filters/nbdkit-nosuch-filter.so: cannot open shared
object file: No such file or directory
$ echo $?
1
$ nbdkit --dump-plugin --filter=cache null
path=/usr/lib64/nbdkit/plugins/nbdkit-null-plugin.so
name=null
...
$ echo $?
0
Similarly for --help. But notice:
$ diff -u <(nbdkit --dump-plugin --filter=cache null) \
<(nbdkit --dump-plugin null)
$ diff -u <(nbdkit --help --filter=cache null) <(nbdkit --help null)
--- /dev/fd/63 2019-09-20 09:31:43.184428823 -0500
+++ /dev/fd/62 2019-09-20 09:31:43.185428824 -0500
@@ -23,15 +23,6 @@
Please read the nbdkit(1) manual page for full usage.
-filter: cache (nbdkit caching filter)
-(/usr/lib64/nbdkit/filters/nbdkit-cache-filter.so)
-cache=MODE Set cache MODE, one of writeback (default),
- writethrough, or unsafe.
-cache-on-read=BOOL Set to true to cache on reads (default false).
-cache-max-size=SIZE Set maximum space used by cache.
-cache-high-threshold=PCT Percentage of max size where reclaim begins.
-cache-low-threshold=PCT Percentage of max size where reclaim ends.
-
But this is still better because it also allows checking for the filter
parameters had they been changed.
In this case I feel like there is no need to add anything else, I did not know
that this already exists. I don't even know if this is really needed, I was
just curious if it would be nicer.
plugin: null
(/usr/lib64/nbdkit/plugins/nbdkit-null-plugin.so)
size=<SIZE> Size of the backing disk
So we could probably improve --dump-plugin to ALSO show details about
the filter (such as a second path=), the way --help does, or even add a
'nbdkit --dump-filter filtername' to work in isolation. But while you'd
have to wait for a future nbdkit release to get more details from
--dump-plugin, and/or a way to list all available filters (think search
permissions on a directory), you already have a way with current nbdkit
to check existence of a filter (think read permissions on a directory).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org