Re: [Libguestfs] [PATCH nbdkit] rust: Add implementation of after_fork() method
On Fri, Apr 21, 2023 at 06:30:52AM -0700, alan somers wrote:
> On Fri, Apr 21, 2023 at 2:32 AM 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() -> Result {
> > > 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.
>
> That patch looks good from first glance. Have you tried running it yet?
Yes it works fine. I will push it shortly.
I didn't look at the other part of the user's bug report.
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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] rust: Add implementation of after_fork() method
On Fri, Apr 21, 2023 at 2:32 AM 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() -> Result {
> > 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.
That patch looks good from first glance. Have you tried running it yet?
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [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
___
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
Re: [Libguestfs] [PATCH nbdkit] rust: Add implementation of after_fork() method
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() -> Result {
> > 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 { unreachable!() }
> > }
> >
> > +static mut AFTER_FORK: fn() -> Result<()> = unreachable::config_complete;
> > static mut CONFIG: fn(k: , v: ) -> Result<()> =
> > unreachable::config;
> > static mut CONFIG_COMPLETE: fn() -> Result<()> =
> > unreachable::config_complete;
> > static mut CONFIG_HELP: Vec = 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 c_int>,
> > pub preconnect: Option c_int>,
> > pub get_ready: Option c_int>,
> > +pub after_fork: Option 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
Re: [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() -> Result {
> 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 { unreachable!() }
> }
>
> +static mut AFTER_FORK: fn() -> Result<()> = unreachable::config_complete;
> static mut CONFIG: fn(k: , v: ) -> Result<()> = unreachable::config;
> static mut CONFIG_COMPLETE: fn() -> Result<()> =
> unreachable::config_complete;
> static mut CONFIG_HELP: Vec = 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 c_int>,
> pub preconnect: Option c_int>,
> pub get_ready: Option c_int>,
> +pub after_fork: Option 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(, count: u32, offset: u64) -> Result<()>;
> fn can_cache() -> Result;
> fn can_extents() -> Result;
> 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:
