[PATCH nbdkit] rust: Add implementation of after_fork() method
by Richard W.M. Jones
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
1 year, 7 months
[libnbd PATCH 0/3] start wrapping generated C source code harder at 80 chars
by Laszlo Ersek
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
This is my first stab at wrapping generated C source code "harder" at 80
characters. The first two patches only tweak "scripts/git.orderfile".
The last patch modifies the generator. I'm posting the last patch
effectively in isolation (from any other generator patches) so we can
agree on the approach up-front, before I do more work.
Laszlo
Laszlo Ersek (3):
scripts/git.orderfile: drop "generator/generator"
scripts/git.orderfile: put *.mli first among generator files
generator: indent C argument list 2 spaces relative to function
designator
generator/C.mli | 3 +-
generator/C.ml | 33 ++++++++++++++++----
generator/GoLang.ml | 4 +--
scripts/git.orderfile | 2 +-
4 files changed, 32 insertions(+), 10 deletions(-)
base-commit: 86820dbab49723d8ce3182fb0b94f3006f151b57
1 year, 7 months
[libnbd PATCH v2] tests/requires: wrap source code at 80 characters
by Laszlo Ersek
Embedding a shell script in a multi-line C string literal is an exercise
in pain. I can see why the original author (whom I shall not look up with
git-blame :) ) went for the easy route. Still, we want the source code to
fit in 80 columns.
At Eric's suggestion, also:
- replace the non-portable control operator "|&" with the portable
redirection operator "2>&1" and the portable control operator "|";
- replace the "if ...; then exit 1; else exit 0; fi" constructs with "!
...".
Note: in my "interop/test-suite.log", I see
> SKIP: interop-qemu-nbd-tls-certs
> ================================
>
> requires test -d /home/lacos/src/v2v/libnbd/tests/pki
> Test skipped because prerequisite is missing or not working.
> SKIP interop-qemu-nbd-tls-certs (exit status: 77)
>
> SKIP: interop-qemu-nbd-tls-psk
> ==============================
>
> requires test -f /home/lacos/src/v2v/libnbd/tests/keys.psk
> Test skipped because prerequisite is missing or not working.
> SKIP interop-qemu-nbd-tls-psk (exit status: 77)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
Notes:
v2:
- v1: https://listman.redhat.com/archives/libguestfs/2023-April/031298.html
- incorporate Eric's recommendations (commit message, code)
tests/requires.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/requires.c b/tests/requires.c
index 199e30605473..2b7031532fe2 100644
--- a/tests/requires.c
+++ b/tests/requires.c
@@ -57,7 +57,8 @@ requires_qemu_nbd_tls_support (const char *qemu_nbd)
* interested in the error message that it prints.
*/
snprintf (cmd, sizeof cmd,
- "if %s --object tls-creds-x509,id=tls0 |& grep -sq 'TLS credentials support requires GNUTLS'; then exit 1; else exit 0; fi",
+ "! %s --object tls-creds-x509,id=tls0 2>&1 \\\n"
+ " | grep -sq 'TLS credentials support requires GNUTLS'\n",
qemu_nbd);
requires (cmd);
}
@@ -72,7 +73,8 @@ requires_qemu_nbd_tls_psk_support (const char *qemu_nbd)
* interested in the error message that it prints.
*/
snprintf (cmd, sizeof cmd,
- "if %s --object tls-creds-psk,id=tls0 / |& grep -sq 'invalid object type'; then exit 1; else exit 0; fi",
+ "! %s --object tls-creds-psk,id=tls0 / 2>&1 \\\n"
+ " | grep -sq 'invalid object type'\n",
qemu_nbd);
requires (cmd);
}
base-commit: a458f0644de0fa541b055299fe6a58e1f42d0073
1 year, 7 months
[libnbd PATCH 00/18] wrap hand-written source code at 80 characters
by Laszlo Ersek
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2172516
This series wraps the non-generated C-language source code (*.c and *.h
files) at 80 characters.
"ocaml/helpers.c" remains overlong, but I couldn't find a way to wrap
it: its single overlong line contains the comment
/* For how we're getting the exception name, see:
* https://github.com/libguestfs/libguestfs/blob/5d94be2583d557cfc7f8a8cfee7...
*/
and even if I truncate the blob hash to 12 nibbles, the line remains too
long.
The following files are also too wide:
include/libnbd.h
lib/api.c
lib/states-run.c
lib/states.c
lib/states.h
lib/unlocked.h
ocaml/nbd-c.c
python/libnbdmod.c
python/methods.h
but they are all generated; we'll have to discuss them separately.
Laszlo
Laszlo Ersek (18):
examples/copy-libev: wrap source code at 80 characters
examples/glib-main-loop: wrap source code at 80 characters
examples/strict-structured-reads: wrap source code at 80 characters
fuse/nbdfuse: wrap source code at 80 characters
generator/states: wrap source code at 80 characters
interop/{dirty-bitmap,structured-read}: wrap the source code at 80
chars
interop/interop: wrap source code at 80 characters
lib/internal.h: wrap source code at 80 characters
lib/opt: wrap source code at 80 characters
python/handle: wrap source code at 80 characters
tests/closure-lifetimes: wrap source code at 80 characters
tests/connect-uri: wrap source code at 80 characters
tests/meta-base-allocation: wrap source code at 80 characters
tests/newstyle-limited: wrap source code at 80 characters
tests/oldstyle: wrap source code at 80 characters
tests/requires: wrap source code at 80 characters
ublk/nbdublk: wrap source code at 80 characters
ublk/tgt: wrap source code at 80 characters
examples/copy-libev.c | 8 ++--
examples/glib-main-loop.c | 6 ++-
examples/strict-structured-reads.c | 11 +++--
fuse/nbdfuse.c | 4 +-
generator/states-magic.c | 3 +-
generator/states-newstyle-opt-go.c | 12 +++--
interop/dirty-bitmap.c | 12 ++---
interop/interop.c | 3 +-
interop/structured-read.c | 15 +++----
lib/internal.h | 7 ++-
lib/opt.c | 8 ++--
python/handle.c | 4 +-
tests/closure-lifetimes.c | 47 ++++++++------------
tests/connect-uri.c | 3 +-
tests/meta-base-allocation.c | 13 +++---
tests/newstyle-limited.c | 8 ++--
tests/oldstyle.c | 8 ++--
tests/requires.c | 16 ++++++-
ublk/nbdublk.c | 4 +-
ublk/tgt.c | 6 ++-
20 files changed, 106 insertions(+), 92 deletions(-)
base-commit: 424420a55bab1a8ec9acffe684e0bbd0dc4ec45d
1 year, 7 months
[PATCH nbdkit] server: Fix exit with parent behaviour if flag is not specified
by Richard W.M. Jones
Commit 6b03ec8ad6 ("common: Move exit-with-parent code from include/
to utils/") was supposed to be a largely neutral refactoring of the
--exit-with-parent flag. However I made a mistake in one hunk of that
patch:
-#ifdef HAVE_EXIT_WITH_PARENT
- if (exit_with_parent) {
- if (set_exit_with_parent () == -1) {
- perror ("nbdkit: --exit-with-parent");
- exit (EXIT_FAILURE);
- }
+ if (set_exit_with_parent () == -1) {
+ perror ("nbdkit: --exit-with-parent");
+ exit (EXIT_FAILURE);
}
-#endif
by removing the test for exit_with_parent. The effect is similar to
if we always specified --exit-with-parent when nbdkit is running in
the foreground.
Somewhat surprisingly this didn't have any noticable effect. That's
for a few of reasons: (1) Mostly if you don't fork then you should be
using --exit-with-parent (and the bulk of the tests do this). (2) If
we do fork into the background then the PR_SET_PDEATHSIG flag is
cleared by Linux across the fork. (3) It actually _did_ break Windows
because there set_exit_with_parent calls abort, but for unrelated
reasons our Windows CI was broken (and never tested running
nbdkit.exe).
Fix this with the obvious adjustment.
This broke nbdkit 1.32 and 1.34.
Fixes: commit 6b03ec8ad672e5956b41cbacae9c307e5acd786b
---
server/main.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/server/main.c b/server/main.c
index c3b9bf384..1df5d69ac 100644
--- a/server/main.c
+++ b/server/main.c
@@ -574,9 +574,11 @@ main (int argc, char *argv[])
/* Implement --exit-with-parent early in case plugin initialization
* takes a long time and the parent exits during that time.
*/
- if (set_exit_with_parent () == -1) {
- perror ("nbdkit: --exit-with-parent");
- exit (EXIT_FAILURE);
+ if (exit_with_parent) {
+ if (set_exit_with_parent () == -1) {
+ perror ("nbdkit: --exit-with-parent");
+ exit (EXIT_FAILURE);
+ }
}
/* If the user has mixed up -p/--run/-s/-U/--vsock options, then
--
2.39.2
1 year, 7 months
[PATCH libnbd v2] README: Document additional packages
by Nir Soffer
When building from git we need autoconf, automake and libtool.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
Changes sinve v1:
- Remove `,` between package namses (Laszlo)
README.md | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/README.md b/README.md
index c7166613..7eed0e31 100644
--- a/README.md
+++ b/README.md
@@ -32,10 +32,17 @@ ## License
very liberal license.
## Building from source
+Building from source requires additional packages. On rpm based system
+use:
+
+```
+dnf install autoconf automake libtool
+```
+
To build from git:
```
autoreconf -i
./configure
--
2.39.2
1 year, 7 months
[PATCH libnbd] README: Document additional packages
by Nir Soffer
When building from git we need autoconf, automake and libtool.
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
README.md | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/README.md b/README.md
index c7166613..42a187c0 100644
--- a/README.md
+++ b/README.md
@@ -32,10 +32,17 @@ ## License
very liberal license.
## Building from source
+Building from source requires additional packages. On rpm based system
+use:
+
+```
+dnf install autoconf, automake, libtool
+```
+
To build from git:
```
autoreconf -i
./configure
--
2.39.2
1 year, 7 months
