[Libguestfs] [PATCH v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.

[Eric Blake]

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
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.
-
 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190920/eed9cdd4/attachment.sig>