[Libguestfs] [nbdkit PATCH] filters: Improve docs on .prepare prerequisites
Eric Blake
eblake at redhat.com
Wed Jul 22 01:43:22 UTC 2020
On 7/9/20 2:20 PM, Eric Blake wrote:
>>> For C<.prepare>, the value of C<readonly> is the same as was passed to
>>> -C<.open>, declaring how this filter will be used.
>>> +C<.open>, declaring how this filter will be used. When calling plugin
>>> +functions during C<.prepare>, the filter must ensure that prerequisite
>>> +functions have succeeded (for example, calling C<.pwrite>) requires
>>> +checking both C<.get_size> and C<.can_write>); while these
>>> +prerequisites are automatically handled in most other cases thanks to
>>> +client negotiation, the timing of C<.prepare> comes before client
>>> +negotiation has completed.
>>
>> I think this isn't sufficient. I think a filter which does:
>>
>> int64_t my_filter_get_size () { return size; }
>> int my_filter_prepare (int readonly) { return 0; }
>>
>> will fail as h->exportsize is only updated by a call to
>> next_ops->get_size. This is basically what the tar filter was doing
>> on the second connection (before I fixed it).
>
> True. But I'm not sure how best to word it. Ultimately, a filter may
> choose to bypass any part of the negotiation (such as overriding
> .can_write or .get_size because it plans on giving a different answer),
> and where it does not later ask for the same underlying functionality
> from the plugin, that's not a problem. So the real rule is more like:
> for any action that you plan to use the underlying plugin for, you must
> ultimately go through the same negotiation prerequisites, even if by the
> time you are in .get_ready you have enough information to short circuit
> some of that information. If nothing else, since the tar filter plans
> on using .pread for each handle, but only reads sizes during the first
> handle, subsequent handles should do a cursory check of .get_size to
> ensure the underlying plugin hasn't resized behind tar's back.
>
> I'll give this a couple more days of thought to see if I can derive
> better wording to go in the docs.
Here's what I finally pushed:
From fcb322c4e87a926607393349d7b9b0e9a1533fda Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake at redhat.com>
Date: Thu, 9 Jul 2020 11:36:03 -0500
Subject: [nbdkit PATCH] filters: Improve docs on .prepare prerequisites
Filters have a requirement to ensure prerequisite functions are called
before using backend I/O functions, in order to avoid triggering
assertions in backend.c that perform sanity checking on each request.
When a filter has no .prepare and no hard-coded overrides of .can_FOO,
this happens automatically, but it is worth documenting cases where
.prepare must call these prerequisites manually (such as when .prepare
does I/O, or .get_size returns a hard-coded size).
See also: https://bugzilla.redhat.com/show_bug.cgi?id=1855330,
https://www.redhat.com/archives/libguestfs/2020-July/msg00041.html
Signed-off-by: Eric Blake <eblake at redhat.com>
---
docs/nbdkit-filter.pod | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod
index acac3e50..32208b4c 100644
--- a/docs/nbdkit-filter.pod
+++ b/docs/nbdkit-filter.pod
@@ -383,15 +383,27 @@ connection (C<.finalize>).
For example if you need to scan the underlying disk to check for a
partition table, you could do it in your C<.prepare> method (calling
-the plugin's C<.pread> method via C<next_ops>). Or if you need to
-cleanly update superblock data in the image on close you can do it in
-your C<.finalize> method (calling the plugin's C<.pwrite> method).
-Doing these things in the filter's C<.open> or C<.close> method is not
-possible.
+the plugin's C<.get_size> and C<.pread> methods via C<next_ops>). Or
+if you need to cleanly update superblock data in the image on close
+you can do it in your C<.finalize> method (calling the plugin's
+C<.pwrite> method). Doing these things in the filter's C<.open> or
+C<.close> method is not possible.
For C<.prepare>, the value of C<readonly> is the same as was passed to
C<.open>, declaring how this filter will be used.
+Note that nbdkit performs sanity checking on requests made to the
+underlying plugin; for example, C<next_ops-E<gt>pread> cannot be
+called on a given connection unless C<next_ops-E<gt>get_size> has
+first been called at least once in the same connection (to ensure the
+read requests are in bounds), and C<next_ops-E<gt>pwrite> further
+requires an earlier successful call to C<next_ops-E<gt>can_write>. In
+many filters, these prerequisites will be automatically called during
+client negotiation phase, but there are cases where a filter overrides
+query functions or makes I/O calls into the plugin before handshaking
+is complete, where the filter needs to make those prerequisite calls
+manually during C<.prepare>.
+
There is no C<next_ops-E<gt>prepare> or C<next_ops-E<gt>finalize>.
Unlike other filter methods, prepare and finalize are not chained
through the C<next_ops> structure. Instead the core nbdkit server
--
2.27.0
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
More information about the Libguestfs
mailing list