[Libguestfs] [PATCH v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.
Martin Kletzander
mkletzan at redhat.com
Fri Sep 20 19:40:17 UTC 2019
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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190920/21c4dbe6/attachment.sig>
More information about the Libguestfs
mailing list