Libguestfs - Aug 2019 - Re: [nbdkit PATCH 1/2] rust: Implement can_cache

On 8/16/19 12:08 PM, Eric Blake wrote:
> Implementing extents requires some coordination for the Rust code to
> call back into libnbdkit; I'm not familiar with Rust enough to do
> that. But with placeholders for those slots, implementing
> can_cache/cache is trivial.  This improves the situation mentioned in
> commit 031fae85.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> +++ b/plugins/rust/src/lib.rs
> @@ -93,6 +93,15 @@ pub struct Plugin {
>      pub magic_config_key: *const c_char,
> 
>      pub can_multi_conn: Option<extern fn (h: *mut c_void) ->
c_int>,
> +
> +    // Slots for extents functions, which needs more integration.
> +    _can_extents: Option<extern fn ()>,
> +    _extents: Option<extern fn ()>,
Here, I was just copying what we had already done for the v1 fields that
should remain unused (see above, in _pread_old for example).  But is
there a saner way to write a Rust struct that reserves the space
required by the C ABI, but where the field MUST be left as NULL and not
populated with Some(...) by the end user?  Perhaps by marking the field
const, while the rest of the struct is used as a mutable?  (Doesn't
change this patch, so much as a cleanup we could apply on top to prevent
all of our _named fields from being overwritten after the initial
construction with default values).

And of course, if you want to actually implement extents, and figure out
how to expose C-based nbdkit functions to be called by Rust code, be my
guest (for .zero, we need nbdkit_set_error(), for extents, we need
nbdkit_add_extent(), and then we have several other utility functions
like nbdkit_realpath() that could be useful).

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

On 8/16/19 1:01 PM, Eric Blake wrote:
> On 8/16/19 12:08 PM, Eric Blake wrote:
>> Implementing extents requires some coordination for the Rust code to
>> call back into libnbdkit; I'm not familiar with Rust enough to do
>> that. But with placeholders for those slots, implementing
>> can_cache/cache is trivial.  This improves the situation mentioned in
>> commit 031fae85.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
> And of course, if you want to actually implement extents, and figure out
> how to expose C-based nbdkit functions to be called by Rust code, be my
> guest (for .zero, we need nbdkit_set_error(), for extents, we need
> nbdkit_add_extent(), and then we have several other utility functions
> like nbdkit_realpath() that could be useful).
Also, we are lacking a tests/*rust* usage; it would be nice if 'make
check' covered a Rust plugin (more than just running 'nbdkit
plugins/rust/target/release/examples/libramdisk.so').

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

On Fri, Aug 16, 2019 at 01:05:58PM -0500, Eric Blake
wrote:
> On 8/16/19 1:01 PM, Eric Blake wrote:
> > On 8/16/19 12:08 PM, Eric Blake wrote:
> >> Implementing extents requires some coordination for the Rust code
to
> >> call back into libnbdkit; I'm not familiar with Rust enough to
do
> >> that. But with placeholders for those slots, implementing
> >> can_cache/cache is trivial.  This improves the situation mentioned
in
> >> commit 031fae85.
> >>
> >> Signed-off-by: Eric Blake <eblake@redhat.com>
> >> ---
> 
> > And of course, if you want to actually implement extents, and figure
out
> > how to expose C-based nbdkit functions to be called by Rust code, be
my
> > guest (for .zero, we need nbdkit_set_error(), for extents, we need
> > nbdkit_add_extent(), and then we have several other utility functions
> > like nbdkit_realpath() that could be useful).
> 
> Also, we are lacking a tests/*rust* usage; it would be nice if 'make
> check' covered a Rust plugin (more than just running 'nbdkit
> plugins/rust/target/release/examples/libramdisk.so').
We certainly do need tests for the Rust plugin.  Actually placing them
in tests/ might be difficult because Rust assumes a build environment
(called cargo) and that requires a bunch of files.

Since we already have cargo files it's probably better to put the
tests for this plugin next to the plugin.  Unfortunately an additional
complication is that we probably can't use the tests facility of cargo
because we need to go through building a shared library and running it
under the nbdkit wrapper.  Perhaps we can add them as further examples
and wire up 'make check' to run them?  It's not especially elegant
but
here we are.

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