Re: [Libguestfs] [nbdkit PATCH 2/2] rust: Add support for dynamic .thread_model

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(a)redhat.com&gt; --- 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,

} } }