Richard W.M. Jones
2023-Apr-21 09:20 UTC
[Libguestfs] [PATCH nbdkit] rust: Add implementation of after_fork() method
See: https://github.com/libguestfs/nbdkit/issues/21
Tested by applying the following patch to the example plugin and
running it with verbose enabled:
--- a/plugins/rust/examples/ramdisk.rs
+++ b/plugins/rust/examples/ramdisk.rs
@@ -43,6 +43,12 @@ struct RamDisk {
}
impl Server for RamDisk {
+ fn after_fork() -> Result<()> {
+ // A place to start background threads.
+ eprintln!("forked");
+ Ok(())
+ }
+
fn get_size(&self) -> Result<i64> {
Ok(DISK.lock().unwrap().len() as i64)
}
@@ -76,4 +82,4 @@ impl Server for RamDisk {
}
}
-plugin!(RamDisk {thread_model, write_at});
+plugin!(RamDisk {thread_model, write_at, after_fork});
---
plugins/rust/src/lib.rs | 23 ++++++++++++++++-
plugins/rust/tests/common/mod.rs | 1 +
plugins/rust/tests/full_featured.rs | 39 +++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/plugins/rust/src/lib.rs b/plugins/rust/src/lib.rs
index a5b88e851..fc7b28454 100644
--- a/plugins/rust/src/lib.rs
+++ b/plugins/rust/src/lib.rs
@@ -143,6 +143,7 @@ mod unreachable {
pub(super) fn thread_model() -> Result<ThreadModel> {
unreachable!() }
}
+static mut AFTER_FORK: fn() -> Result<()> =
unreachable::config_complete;
static mut CONFIG: fn(k: &str, v: &str) -> Result<()> =
unreachable::config;
static mut CONFIG_COMPLETE: fn() -> Result<()> =
unreachable::config_complete;
static mut CONFIG_HELP: Vec<u8> = Vec::new();
@@ -315,6 +316,16 @@ impl ExtentHandle {
mod ffi {
use super::*;
+ pub(super) extern fn after_fork() -> c_int {
+ match unsafe { AFTER_FORK() } {
+ Ok(()) => 0,
+ Err(e) => {
+ set_error(e);
+ -1
+ }
+ }
+ }
+
macro_rules! can_method {
( $meth:ident ) => {
pub(super) extern fn $meth(h: *mut c_void) -> c_int {
@@ -584,6 +595,12 @@ mod ffi {
// We want the argument names to show up without underscores in the API docs
#[allow(unused_variables)]
pub trait Server {
+ /// This optional callback is called before the server starts serving.
+ ///
+ /// It is called after the server forks and changes directory. If
+ /// a plugin needs to create background threads it should do so here.
+ fn after_fork() -> Result<()> where Self: Sized { unimplemented!()
}
+
/// Indicates that the client intends to make further accesses to the given
/// data region.
///
@@ -900,6 +917,7 @@ macro_rules! opt_method {
#[doc(hidden)]
#[derive(Default)]
pub struct Builder {
+ pub after_fork: bool,
pub cache: bool,
pub can_cache: bool,
pub can_extents: bool,
@@ -938,6 +956,7 @@ impl Builder {
CONFIG_COMPLETE = S::config_complete;
DUMP_PLUGIN = S::dump_plugin;
GET_READY = S::get_ready;
+ AFTER_FORK = S::after_fork;
LOAD = S::load;
OPEN = S::open;
PRECONNECT = S::preconnect;
@@ -1021,7 +1040,8 @@ impl Builder {
thread_model: opt_method!(self, thread_model),
can_fast_zero: opt_method!(self, can_fast_zero),
preconnect: opt_method!(self, preconnect),
- get_ready: opt_method!(self, get_ready)
+ get_ready: opt_method!(self, get_ready),
+ after_fork: opt_method!(self, after_fork)
};
// Leak the memory to C. NBDKit will never give it back.
Box::into_raw(Box::new(plugin))
@@ -1191,6 +1211,7 @@ pub struct Plugin {
pub can_fast_zero: Option<extern fn (h: *mut c_void) -> c_int>,
pub preconnect: Option<extern fn(readonly: c_int) -> c_int>,
pub get_ready: Option<extern fn() -> c_int>,
+ pub after_fork: Option<extern fn() -> c_int>,
}
/// Register your plugin with NBDKit.
diff --git a/plugins/rust/tests/common/mod.rs b/plugins/rust/tests/common/mod.rs
index de26c89fc..77c987a1f 100644
--- a/plugins/rust/tests/common/mod.rs
+++ b/plugins/rust/tests/common/mod.rs
@@ -50,6 +50,7 @@ mock!{
pub Server {}
#[allow(dead_code)]
impl Server for Server {
+ fn after_fork() -> Result<()> where Self: Sized;
fn cache(&self, count: u32, offset: u64) -> Result<()>;
fn can_cache(&self) -> Result<CacheFlags>;
fn can_extents(&self) -> Result<bool>;
diff --git a/plugins/rust/tests/full_featured.rs
b/plugins/rust/tests/full_featured.rs
index d5f02e064..87a0256e0 100644
--- a/plugins/rust/tests/full_featured.rs
+++ b/plugins/rust/tests/full_featured.rs
@@ -46,6 +46,7 @@ static mut PLUGIN: Option<*const Plugin> = None;
static INIT: Once = Once::new();
plugin!(MockServer{
+ after_fork,
cache,
can_cache,
can_extents,
@@ -539,6 +540,44 @@ mod get_ready {
}
}
+mod after_fork {
+ use super::*;
+
+ #[test]
+ fn einval() {
+ initialize();
+ let _m = MOCK_SERVER_MTX.lock().unwrap();
+
+ let after_fork_ctx = MockServer::after_fork_context();
+ after_fork_ctx.expect()
+ .returning(|| Err(Error::new(libc::EINVAL, "foo is
required")));
+
+ let pluginp = unsafe { PLUGIN.unwrap()};
+ let plugin = unsafe {&*pluginp};
+
+ let after_fork = plugin.after_fork.as_ref().unwrap();
+ assert_eq!( -1, after_fork());
+ ERRNO.with(|e| assert_eq!(libc::EINVAL, *e.borrow()));
+ ERRMSG.with(|e| assert_eq!("foo is required", *e.borrow()));
+ }
+
+ #[test]
+ fn ok() {
+ initialize();
+ let _m = MOCK_SERVER_MTX.lock().unwrap();
+
+ let after_fork_ctx = MockServer::after_fork_context();
+ after_fork_ctx.expect()
+ .returning(|| Ok(()));
+
+ let pluginp = unsafe { PLUGIN.unwrap()};
+ let plugin = unsafe {&*pluginp};
+
+ let after_fork = plugin.after_fork.as_ref().unwrap();
+ assert_eq!( 0, after_fork());
+ }
+}
+
can_method!(is_rotational, expect_is_rotational);
unload_like!(load, load_context);
--
2.39.2
Laszlo Ersek
2023-Apr-21 09:32 UTC
[Libguestfs] [PATCH nbdkit] rust: Add implementation of after_fork() method
On 4/21/23 11:20, Richard W.M. Jones wrote:> See: https://github.com/libguestfs/nbdkit/issues/21 > > Tested by applying the following patch to the example plugin and > running it with verbose enabled: > > --- a/plugins/rust/examples/ramdisk.rs > +++ b/plugins/rust/examples/ramdisk.rs > @@ -43,6 +43,12 @@ struct RamDisk { > } > > impl Server for RamDisk { > + fn after_fork() -> Result<()> { > + // A place to start background threads. > + eprintln!("forked"); > + Ok(()) > + } > + > fn get_size(&self) -> Result<i64> { > Ok(DISK.lock().unwrap().len() as i64) > } > @@ -76,4 +82,4 @@ impl Server for RamDisk { > } > } > > -plugin!(RamDisk {thread_model, write_at}); > +plugin!(RamDisk {thread_model, write_at, after_fork});I *think* diffs embedded in commit messages are best quoted somehow (or at least indented); I vaguely recall "naked" diffs in the commit message confusing git-am. I've not written a line of Rust thus far, so I'll let Alan review the patch. Laszlo> --- > plugins/rust/src/lib.rs | 23 ++++++++++++++++- > plugins/rust/tests/common/mod.rs | 1 + > plugins/rust/tests/full_featured.rs | 39 +++++++++++++++++++++++++++++ > 3 files changed, 62 insertions(+), 1 deletion(-) > > diff --git a/plugins/rust/src/lib.rs b/plugins/rust/src/lib.rs > index a5b88e851..fc7b28454 100644 > --- a/plugins/rust/src/lib.rs > +++ b/plugins/rust/src/lib.rs > @@ -143,6 +143,7 @@ mod unreachable { > pub(super) fn thread_model() -> Result<ThreadModel> { unreachable!() } > } > > +static mut AFTER_FORK: fn() -> Result<()> = unreachable::config_complete; > static mut CONFIG: fn(k: &str, v: &str) -> Result<()> = unreachable::config; > static mut CONFIG_COMPLETE: fn() -> Result<()> = unreachable::config_complete; > static mut CONFIG_HELP: Vec<u8> = Vec::new(); > @@ -315,6 +316,16 @@ impl ExtentHandle { > mod ffi { > use super::*; > > + pub(super) extern fn after_fork() -> c_int { > + match unsafe { AFTER_FORK() } { > + Ok(()) => 0, > + Err(e) => { > + set_error(e); > + -1 > + } > + } > + } > + > macro_rules! can_method { > ( $meth:ident ) => { > pub(super) extern fn $meth(h: *mut c_void) -> c_int { > @@ -584,6 +595,12 @@ mod ffi { > // We want the argument names to show up without underscores in the API docs > #[allow(unused_variables)] > pub trait Server { > + /// This optional callback is called before the server starts serving. > + /// > + /// It is called after the server forks and changes directory. If > + /// a plugin needs to create background threads it should do so here. > + fn after_fork() -> Result<()> where Self: Sized { unimplemented!() } > + > /// Indicates that the client intends to make further accesses to the given > /// data region. > /// > @@ -900,6 +917,7 @@ macro_rules! opt_method { > #[doc(hidden)] > #[derive(Default)] > pub struct Builder { > + pub after_fork: bool, > pub cache: bool, > pub can_cache: bool, > pub can_extents: bool, > @@ -938,6 +956,7 @@ impl Builder { > CONFIG_COMPLETE = S::config_complete; > DUMP_PLUGIN = S::dump_plugin; > GET_READY = S::get_ready; > + AFTER_FORK = S::after_fork; > LOAD = S::load; > OPEN = S::open; > PRECONNECT = S::preconnect; > @@ -1021,7 +1040,8 @@ impl Builder { > thread_model: opt_method!(self, thread_model), > can_fast_zero: opt_method!(self, can_fast_zero), > preconnect: opt_method!(self, preconnect), > - get_ready: opt_method!(self, get_ready) > + get_ready: opt_method!(self, get_ready), > + after_fork: opt_method!(self, after_fork) > }; > // Leak the memory to C. NBDKit will never give it back. > Box::into_raw(Box::new(plugin)) > @@ -1191,6 +1211,7 @@ pub struct Plugin { > pub can_fast_zero: Option<extern fn (h: *mut c_void) -> c_int>, > pub preconnect: Option<extern fn(readonly: c_int) -> c_int>, > pub get_ready: Option<extern fn() -> c_int>, > + pub after_fork: Option<extern fn() -> c_int>, > } > > /// Register your plugin with NBDKit. > diff --git a/plugins/rust/tests/common/mod.rs b/plugins/rust/tests/common/mod.rs > index de26c89fc..77c987a1f 100644 > --- a/plugins/rust/tests/common/mod.rs > +++ b/plugins/rust/tests/common/mod.rs > @@ -50,6 +50,7 @@ mock!{ > pub Server {} > #[allow(dead_code)] > impl Server for Server { > + fn after_fork() -> Result<()> where Self: Sized; > fn cache(&self, count: u32, offset: u64) -> Result<()>; > fn can_cache(&self) -> Result<CacheFlags>; > fn can_extents(&self) -> Result<bool>; > diff --git a/plugins/rust/tests/full_featured.rs b/plugins/rust/tests/full_featured.rs > index d5f02e064..87a0256e0 100644 > --- a/plugins/rust/tests/full_featured.rs > +++ b/plugins/rust/tests/full_featured.rs > @@ -46,6 +46,7 @@ static mut PLUGIN: Option<*const Plugin> = None; > static INIT: Once = Once::new(); > > plugin!(MockServer{ > + after_fork, > cache, > can_cache, > can_extents, > @@ -539,6 +540,44 @@ mod get_ready { > } > } > > +mod after_fork { > + use super::*; > + > + #[test] > + fn einval() { > + initialize(); > + let _m = MOCK_SERVER_MTX.lock().unwrap(); > + > + let after_fork_ctx = MockServer::after_fork_context(); > + after_fork_ctx.expect() > + .returning(|| Err(Error::new(libc::EINVAL, "foo is required"))); > + > + let pluginp = unsafe { PLUGIN.unwrap()}; > + let plugin = unsafe {&*pluginp}; > + > + let after_fork = plugin.after_fork.as_ref().unwrap(); > + assert_eq!( -1, after_fork()); > + ERRNO.with(|e| assert_eq!(libc::EINVAL, *e.borrow())); > + ERRMSG.with(|e| assert_eq!("foo is required", *e.borrow())); > + } > + > + #[test] > + fn ok() { > + initialize(); > + let _m = MOCK_SERVER_MTX.lock().unwrap(); > + > + let after_fork_ctx = MockServer::after_fork_context(); > + after_fork_ctx.expect() > + .returning(|| Ok(())); > + > + let pluginp = unsafe { PLUGIN.unwrap()}; > + let plugin = unsafe {&*pluginp}; > + > + let after_fork = plugin.after_fork.as_ref().unwrap(); > + assert_eq!( 0, after_fork()); > + } > +} > + > can_method!(is_rotational, expect_is_rotational); > > unload_like!(load, load_context);
Richard W.M. Jones
2023-Apr-21 10:07 UTC
[Libguestfs] [PATCH nbdkit] rust: Add implementation of after_fork() method
On Fri, Apr 21, 2023 at 10:20:35AM +0100, Richard W.M. Jones wrote:> impl Server for RamDisk { > + fn after_fork() -> Result<()> { > + // A place to start background threads. > + eprintln!("forked");Alan, this reminds me that there is no binding for nbdkit_debug. I think there should be as it is very useful! Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit