[Libguestfs] [PATCH nbdkit] server: Implement extents/can_extents calls for plugins and filters.

[Richard W.M. Jones]

On Tue, Mar 12, 2019 at 07:51:57AM -0500, Eric Blake wrote:
> On 3/12/19 6:20 AM, Richard W.M. Jones wrote:
> > This tentative commit implements extents/can_extents, roughly as
> > discussed in the previous thread here:
> > 
> > https://www.redhat.com/archives/libguestfs/2019-March/msg00017.html
> > 
> > I can't say that I'm a big fan of having the plugin allocate an
> > extents array.  There are no other plugin callbacks currently where we
> > require the plugin to allocate complex data structures (or indeed do
> > any allocation at all).  The interface is complex and error prone for
> > plugin writers.  However I can't at the moment think of a better way
> > to do it.
> 
> Can we at least provide a utility function that the plugin can call to
> obtain an array containing N extents, where nbdkit does the
> malloc()/free() rather than mixing malloc() in plugin and free() in
> nbdkit?  Among other things, doing this would let us switch to a
> different allocation engine (pool-based, maybe?) without a super-close
> coupling where plugins and nbdkit have to share the same allocator.
> 
> nbdkit        plugin
> receive client BLOCK_STATUS call
> call plugin.extents
>               call nbdkit_extents_array(N)
> allocate an array of N extents
>               populate the array and return
> process the array
> 
> 
> The only other thing I can think of is that nbdkit could have a function
> that the plugin must call one or more times per extent, before
> returning, and where nbdkit collects the extents itself based on the
> sequence of calls rather than making the plugin itself manage an array.
> 
> nbdkit        plugin
> receive client BLOCK_STATUS call
> initialize array
> call plugin.extents
>               call nbdkit_extent_add()
> add extent to array
>               call nbdkit_extent_add()
> add extent to array
>               ...
>               return
> process the array

Yes I agree with both these points.

In the second case a concrete implementation might look like
this (without error checking):

   /* plugin method */
   int extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags,
                void *extents_h)
   {
     nbdkit_extent_add (extents_h, offset, count, NBDKIT_EXTENT_TYPE_DATA);
     nbdkit_extent_add (extents_h, offset+count, 1024, NBDKIT_EXTENT_TYPE_HOLE);
     return 0;
   }

   /* filter method */
   int extents (struct nbdkit_next_ops *next_ops, void *nxdata,
                void *handle, uint32_t count, uint64_t offset, uint32_t flags,
                void *extents_h, int *err)
   {
      if (next_ops->extents (...) == -1) return -1;
      /* call adjust_offset_fn on each extent */
      nbdkit_extent_foreach (extents_h, adjust_offset_fn);
      return 0;
   }

If you think that looks reasonable I'll see if I can come up with an
actual patch for this this afternoon.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top