[Libguestfs] [PATCH nbdkit] rust: Add implementation of after_fork() method

Richard W.M. Jones rjones at redhat.com
Fri Apr 21 10:04:26 UTC 2023


On Fri, Apr 21, 2023 at 11:32:31AM +0200, Laszlo Ersek wrote:
> 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 now indented it in my copy.

Rich.

> 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 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


More information about the Libguestfs mailing list