[Libguestfs] [nbdkit PATCH 2/2] rust: Add support for dynamic .thread_model
Richard W.M. Jones
rjones at redhat.com
Fri Aug 16 17:29:03 UTC 2019
On Fri, Aug 16, 2019 at 12:08:11PM -0500, Eric Blake wrote:
> We do not promise API stability for non-C languages; this is an API
> break as follows: instead of calling plugin_init with a static model,
> you can now populate .thread_model in the Plugin struct, with a
> default to Parallel. As in C, the model is still chosen at .load time
> (at most, making it a function allows you to alter it based on
> configuration), and not something that can change per-connection.
>
> Since all existing Rust plugins will have already thought about thread
> models, they can convert their existing model into the new
> .thread_model field (and thus, I don't feel too bad making Parallel
> the default, even if it is not always the safest).
>
> And something I learned the hard way: enum ThreadModel _must_ be
> #[repr(C)], otherwise Rust will compile a return type of this enum
> into a 1-byte return, even though C expects sign-extension into a
> 4-byte return; the garbage left in the other three bytes (at least on
> x86 architecture) would generally render .thread_model broken
> otherwise.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>
> plugins/rust/nbdkit-rust-plugin.pod | 19 ++++++++++++++-----
> plugins/rust/examples/ramdisk.rs | 8 ++++++--
> plugins/rust/src/lib.rs | 10 +++++++---
> 3 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/plugins/rust/nbdkit-rust-plugin.pod b/plugins/rust/nbdkit-rust-plugin.pod
> index 930829cc..61c2e0ba 100644
> --- a/plugins/rust/nbdkit-rust-plugin.pod
> +++ b/plugins/rust/nbdkit-rust-plugin.pod
> @@ -37,14 +37,19 @@ compatible with the C struct used by C plugins.
> use nbdkit::ThreadModel::*;
>
> #[no_mangle]
> + extern fn myplugin_thread_model () -> ThreadModel {
> + Serialize_AllRequests
> + }
> +
> + //... more functions
> +
> pub extern fn plugin_init () -> *const Plugin {
> // Plugin name.
> let name = "myplugin\0"
> as *const str as *const [c_char] as *const c_char;
>
> - // Create a mutable plugin, setting the 5 required fields.
> + // Create a mutable plugin, setting the 4 required fields.
> let mut plugin = Plugin::new (
> - Serialize_All_Requests,
> name,
> myplugin_open,
> myplugin_get_size,
> @@ -53,6 +58,7 @@ compatible with the C struct used by C plugins.
> // Update any other fields as required.
> plugin.close = Some (myplugin_close);
> plugin.pwrite = Some (myplugin_pwrite);
> + plugin.thread_model = Some (myplugin_thread_model);
>
> // Return the pointer.
> let plugin = Box::new(plugin);
> @@ -74,9 +80,12 @@ L<nbdkit-plugin(3)>):
>
> =head2 Threads
>
> -The first parameter of C<Plugin::new> is the thread model, which can
> -be one of the values in the table below. For more information on
> -thread models, see L<nbdkit-plugin(3)/THREADS>.
> +One of the members in C<Plugin> returned by C<plugin_init> is
> +C<thread_model>, which must return one of the values in the table
> +below. For more information on thread models, see
> +L<nbdkit-plugin(3)/THREADS>. If this optional function is not
> +provided, the thread model defaults to
> +C<nbdkit::ThreadModel::Parallel>.
>
> =over 4
>
> diff --git a/plugins/rust/examples/ramdisk.rs b/plugins/rust/examples/ramdisk.rs
> index 9d0af0ee..e9462fca 100644
> --- a/plugins/rust/examples/ramdisk.rs
> +++ b/plugins/rust/examples/ramdisk.rs
> @@ -53,6 +53,10 @@ struct Handle {
> _not_used: i32,
> }
>
> +extern fn ramdisk_thread_model () -> ThreadModel {
> + Parallel
> +}
> +
> extern fn ramdisk_open (_readonly: c_int) -> *mut c_void {
> let h = Handle {_not_used: 0};
> let h = Box::new(h);
> @@ -98,9 +102,8 @@ pub extern fn plugin_init () -> *const Plugin {
> // https://github.com/rust-lang/rfcs/issues/400
> let name = "ramdisk\0" as *const str as *const [c_char] as *const c_char;
>
> - // Create a mutable plugin, setting the 5 required fields.
> + // Create a mutable plugin, setting the 4 required fields.
> let mut plugin = Plugin::new (
> - Parallel,
> name,
> ramdisk_open,
> ramdisk_get_size,
> @@ -109,6 +112,7 @@ pub extern fn plugin_init () -> *const Plugin {
> // Update any other fields as required.
> plugin.close = Some (ramdisk_close);
> plugin.pwrite = Some (ramdisk_pwrite);
> + plugin.thread_model = Some (ramdisk_thread_model);
>
> // Return the pointer.
> let plugin = Box::new(plugin);
> diff --git a/plugins/rust/src/lib.rs b/plugins/rust/src/lib.rs
> index 25af2fe6..e369ecbe 100644
> --- a/plugins/rust/src/lib.rs
> +++ b/plugins/rust/src/lib.rs
> @@ -36,6 +36,7 @@ use std::os::raw::{c_char, c_int, c_void};
> // function must return.
> #[repr(C)]
> pub struct Plugin {
> + // Do not modify these three fields directly.
> _struct_size: u64,
> _api_version: c_int,
> _thread_model: c_int,
> @@ -102,8 +103,11 @@ pub struct Plugin {
> pub cache: Option<extern fn (h: *mut c_void,
> count: u32, offset: u64,
> flags: u32) -> c_int>,
> +
> + pub thread_model: Option<extern fn () -> ThreadModel>,
> }
>
> +#[repr(C)]
> pub enum ThreadModel {
> SerializeConnections = 0,
> SerializeAllRequests = 1,
> @@ -112,8 +116,7 @@ pub enum ThreadModel {
> }
>
> impl Plugin {
> - pub fn new (thread_model: ThreadModel,
> - name: *const c_char,
> + pub fn new (name: *const c_char,
> open: extern fn (c_int) -> *mut c_void,
> get_size: extern fn (*mut c_void) -> i64,
> pread: extern fn (h: *mut c_void, buf: *mut c_char,
> @@ -122,7 +125,7 @@ impl Plugin {
> Plugin {
> _struct_size: mem::size_of::<Plugin>() as u64,
> _api_version: 2,
> - _thread_model: thread_model as c_int,
> + _thread_model: ThreadModel::Parallel as c_int,
> name: name,
> longname: std::ptr::null(),
> version: std::ptr::null(),
> @@ -159,6 +162,7 @@ impl Plugin {
> _extents: None,
> can_cache: None,
> cache: None,
> + thread_model: None,
Indentation?
> }
> }
> }
Anyway this looks fine to me. If it compiles and runs there's
not much that can go wrong, so ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
More information about the Libguestfs
mailing list