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

[Eric Blake]

On 9/21/19 2:59 PM, Richard W.M. Jones wrote:

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

Yeah, the 'zero' plugin is a lot more compact than the 'null' plugin.
Do we have to ship a separate nbdkit-zero.so, or could we make nbdkit
itself behave as if the zero plugin were in use if no actual plugin is
dlloaded?

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

Even if we require a plugin, always packaging the zero plugin along with
nbdkit seems reasonable (we'd have to tweak the docs a bit to mention
which plugin we settle on as being always available).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital 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.

[Martin Kletzander]

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.

[Richard W.M. Jones]

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 v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.

[Martin Kletzander]

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 v4 07/12] v2v: nbdkit: Add the readahead filter unconditionally if it is available.

[Richard W.M. Jones]

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

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

[Martin Kletzander]

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=MODESet cache MODE, one of writeback (default),
-  writethrough, or unsafe.
-cache-on-read=BOOLSet 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 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







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.

[Richard W.M. Jones]

Also of note in this conversation:

https://github.com/libguestfs/nbdkit/blob/master/docs/nbdkit-probing.pod

which currently doesn't mention filters, but ought to once we settle
on the definitive way to detect them.

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/

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

[Martin Kletzander]

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

I observed the behaviour of the readahead filter with our qcow2
overlay when converting a guest from a vCenter source.  Even when
doing random reads, qemu issues 64K reads which happen to also be the
minimum request size of the readahead filter, so there is no extra
overhead.  When doing the sequential copy the readahead filter
performed better than qemu curl’s readahead because it scaled the
prefetched data appropriately on long contiguous stretches and then
shrunk it back to 64K around fragmented parts of the base image.
---
v2v/nbdkit.ml | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/v2v/nbdkit.ml b/v2v/nbdkit.ml
index 4bbb8f043..b2d77f963 100644
--- a/v2v/nbdkit.ml
+++ b/v2v/nbdkit.ml
@@ -49,6 +49,9 @@ type t = {

  (* nbdkit plugin_name --dump-plugin output. *)
  dump_plugin : (string * string) list;
+
+  (* nbdkit directory containing the filters. *)
+  filterdir : string;
}

(* Check that nbdkit is available and new enough. *)
@@ -105,6 +108,12 @@ let common_create plugin_name plugin_args plugin_env =
  error_unless_nbdkit_min_version dump_config;
  error_unless_nbdkit_compiled_with_selinux dump_config;

+  (* Get the filterdir. *)
+  let filterdir =
+try List.assoc "filterdir" dump_config
+with Not_found ->
+  error (f_"nbdkit --dump-config output did not contain filterdir") in
+
  (* Get the nbdkit plugin_name --dump-plugin output, which also
   * checks that the plugin is available and loadable.
   *)
@@ -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.


+  );
+
  let args = get_args () @ [ plugin_name ] @ plugin_args in

-  { plugin_name; args; env; dump_config; dump_plugin }
+  { plugin_name; args; env; dump_config; dump_plugin; filterdir }

(* VDDK libraries are located under lib32/ or lib64/ relative to the
 * libdir.  Note this is unrelated to Linux multilib or multiarch.
--
2.23.0

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs


signature.asc
Description: PGP signature
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs