August 2019 - Libguestfs - libguestfs List Archives

[PATCH libnbd 0/3] One API and small documentation changes.
by Richard W.M. Jones 06 Aug '19

06 Aug '19

One API change, some small documentation changes.

2 6

[PATCH] Rust bindings: Implement Event features
by Hiroyuki Katsura 06 Aug '19

06 Aug '19

This patch includes: - Event callback handlers - Tests related to events(410-430) --- generator/rust.ml | 38 ++++++- rust/src/base.rs | 24 +++-- rust/src/error.rs | 8 +- rust/src/event.rs | 158 ++++++++++++++++++++++++++++ rust/src/lib.rs | 2 + rust/tests/040_create_multiple.rs | 2 +- rust/tests/410_close_event.rs | 38 +++++++ rust/tests/420_log_messages.rs | 60 +++++++++++ rust/tests/430_progress_messages.rs | 61 +++++++++++ 9 files changed, 381 insertions(+), 10 deletions(-) create mode 100644 rust/src/event.rs create mode 100644 rust/tests/410_close_event.rs create mode 100644 rust/tests/420_log_messages.rs create mode 100644 rust/tests/430_progress_messages.rs diff --git a/generator/rust.ml b/generator/rust.ml index a1735602c..1f5cefa62 100644 --- a/generator/rust.ml +++ b/generator/rust.ml @@ -72,6 +72,42 @@ extern \"C\" { } "; + (* event enum *) + pr "\n"; + pr "pub enum Event {\n"; + List.iter ( + fun (name, _) -> + pr " %s,\n" (snake2caml name) + ) events; + pr "}\n\n"; + + pr "impl Event {\n"; + pr " pub fn to_u64(&self) -> u64 {\n"; + pr " match self {\n"; + List.iter ( + fun (name, i) -> + pr " Event::%s => %d,\n" (snake2caml name) i + ) events; + pr " }\n"; + pr " }\n"; + pr " pub fn from_bitmask(bitmask: u64) -> Option<Event> {\n"; + pr " match bitmask {\n"; + List.iter ( + fun (name, i) -> + pr " %d => Some(Event::%s),\n" i (snake2caml name) + ) events; + pr " _ => None,\n"; + pr " }\n"; + pr " }\n"; + pr "}\n\n"; + + pr "pub const EVENT_ALL: [Event; %d] = [\n" (List.length events); + List.iter ( + fun (name, _) -> + pr " Event::%s,\n" (snake2caml name) + ) events; + pr "];\n"; + List.iter ( fun { s_camel_name = name; s_name = c_name; s_cols = cols } -> pr "\n"; @@ -356,7 +392,7 @@ extern \"C\" { pr "}\n"; - pr "impl Handle {\n"; + pr "impl<'a> Handle<'a> {\n"; List.iter ( fun ({ name = name; shortdesc = shortdesc; longdesc = longdesc; style = (ret, args, optargs) } as f) -> diff --git a/rust/src/base.rs b/rust/src/base.rs index 02ad33535..0c6a3bdba 100644 --- a/rust/src/base.rs +++ b/rust/src/base.rs @@ -17,6 +17,10 @@ */ use crate::error; +use crate::event; +use crate::guestfs; +use std::collections; +use std::sync; #[allow(non_camel_case_types)] #[repr(C)] @@ -34,31 +38,37 @@ extern "C" { const GUESTFS_CREATE_NO_ENVIRONMENT: i64 = 1; const GUESTFS_CREATE_NO_CLOSE_ON_EXIT: i64 = 2; -pub struct Handle { +pub struct Handle<'a> { pub(crate) g: *mut guestfs_h, + pub(crate) callbacks: collections::HashMap< + event::EventHandle, + sync::Arc<Fn(guestfs::Event, event::EventHandle, &[u8], &[u64]) + 'a>, + >, } -impl Handle { - pub fn create() -> Result<Handle, error::Error> { +impl<'a> Handle<'a> { + pub fn create() -> Result<Handle<'a>, error::Error> { let g = unsafe { guestfs_create() }; if g.is_null() { Err(error::Error::Create) } else { - Ok(Handle { g }) + let callbacks = collections::HashMap::new(); + Ok(Handle { g, callbacks }) } } - pub fn create_flags(flags: CreateFlags) -> Result<Handle, error::Error> { + pub fn create_flags(flags: CreateFlags) -> Result<Handle<'a>, error::Error> { let g = unsafe { guestfs_create_flags(flags.to_libc_int()) }; if g.is_null() { Err(error::Error::Create) } else { - Ok(Handle { g }) + let callbacks = collections::HashMap::new(); + Ok(Handle { g, callbacks }) } } } -impl Drop for Handle { +impl<'a> Drop for Handle<'a> { fn drop(&mut self) { unsafe { guestfs_close(self.g) } } diff --git a/rust/src/error.rs b/rust/src/error.rs index 705ee1735..ce444e199 100644 --- a/rust/src/error.rs +++ b/rust/src/error.rs @@ -20,6 +20,7 @@ use crate::base; use crate::utils; use std::convert; use std::ffi; +use std::io; use std::os::raw::{c_char, c_int}; use std::str; @@ -41,6 +42,7 @@ pub enum Error { API(APIError), IllegalString(ffi::NulError), Utf8Error(str::Utf8Error), + UnixError(io::Error, &'static str), Create, } @@ -56,7 +58,11 @@ impl convert::From<str::Utf8Error> for Error { } } -impl base::Handle { +pub(crate) fn unix_error(operation: &'static str) -> Error { + Error::UnixError(io::Error::last_os_error(), operation) +} + +impl<'a> base::Handle<'a> { pub(crate) fn get_error_from_handle(&self, operation: &'static str) -> Error { let c_msg = unsafe { guestfs_last_error(self.g) }; let message = unsafe { utils::char_ptr_to_string(c_msg).unwrap() }; diff --git a/rust/src/event.rs b/rust/src/event.rs new file mode 100644 index 000000000..942feec69 --- /dev/null +++ b/rust/src/event.rs @@ -0,0 +1,158 @@ +/* libguestfs Rust bindings + * Copyright (C) 2019 Hiroyuki Katsura <hiroyuki.katsura.0513(a)gmail.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +use crate::base; +use crate::error; +use crate::guestfs; +use std::ffi; +use std::os::raw::{c_char, c_void}; +use std::slice; +use std::sync; + +type GuestfsEventCallback = extern "C" fn( + *const base::guestfs_h, + *const c_void, + u64, + i32, + i32, + *const i8, + usize, + *const u64, + usize, +); + +#[link(name = "guestfs")] +extern "C" { + fn guestfs_set_event_callback( + g: *const base::guestfs_h, + cb: GuestfsEventCallback, + event_bitmask: u64, + flags: i32, + opaque: *const c_void, + ) -> i32; + fn guestfs_delete_event_callback(g: *const base::guestfs_h, eh: i32); + fn guestfs_event_to_string(bitmask: u64) -> *const c_char; + fn free(buf: *const c_void); +} + +#[derive(Hash, PartialEq, Eq)] +pub struct EventHandle { + eh: i32, +} + +pub type Callback = FnMut(guestfs::Event, EventHandle, &[i8], &[u64]); + +fn events_to_bitmask(v: &[guestfs::Event]) -> u64 { + let mut r = 0u64; + for x in v.iter() { + r |= x.to_u64(); + } + r +} + +pub fn event_to_string(events: &[guestfs::Event]) -> Result<String, error::Error> { + let bitmask = events_to_bitmask(events); + + let r = unsafe { guestfs_event_to_string(bitmask) }; + if r.is_null() { + Err(error::unix_error("event_to_string")) + } else { + let s = unsafe { ffi::CStr::from_ptr(r) }; + let s = s.to_str()?.to_string(); + unsafe { free(r as *const c_void) }; + Ok(s) + } +} + +/* -- Why Not Box<Callback> but Arc<Callback> (in struct base::Handle)? -- + * Assume that there are more than threads. While callback is running, + * if a thread frees the handle, automatically the buffer is freed if Box<Callback> + * is used. Therefore Arc<Callback> is used. + */ + +impl<'a> base::Handle<'a> { + pub fn set_event_callback<C: 'a>( + &mut self, + callback: C, + events: &[guestfs::Event], + ) -> Result<EventHandle, error::Error> + where + C: Fn(guestfs::Event, EventHandle, &[u8], &[u64]), + { + extern "C" fn trampoline<C>( + _g: *const base::guestfs_h, + opaque: *const c_void, + event: u64, + event_handle: i32, + _flags: i32, + buf: *const c_char, + buf_len: usize, + array: *const u64, + array_len: usize, + ) where + C: Fn(guestfs::Event, EventHandle, &[u8], &[u64]), + { + // trampoline function + // c.f. https://s3.amazonaws.com/temp.michaelfbryan.com/callbacks/index.html + + let event = match guestfs::Event::from_bitmask(event) { + Some(x) => x, + None => panic!("Failed to parse bitmask: {}", event), + }; + let eh = EventHandle { eh: event_handle }; + let buf = unsafe { slice::from_raw_parts(buf as *const u8, buf_len) }; + let array = unsafe { slice::from_raw_parts(array, array_len) }; + + let callback_ptr = unsafe { &*(opaque as *const sync::Arc<C>) }; + let callback = sync::Arc::clone(&callback_ptr); + callback(event, eh, buf, array) + } + let callback = sync::Arc::<C>::new(callback); + let event_bitmask = events_to_bitmask(events); + + let eh = { + // Weak::into_raw is nightly. + // In order to make sure that callback is freed when handle is freed, + // lifetime is explicitly declared. + let ptr: &'a sync::Arc<C> = Box::leak(Box::new(callback.clone())); + unsafe { + guestfs_set_event_callback( + self.g, + trampoline::<C>, + event_bitmask, + 0, + ptr as *const sync::Arc<C> as *const c_void, + ) + } + }; + if eh == -1 { + return Err(self.get_error_from_handle("set_event_callback")); + } + self.callbacks.insert(EventHandle { eh }, callback); + + Ok(EventHandle { eh }) + } + + pub fn delete_event_callback(&mut self, eh: EventHandle) -> Result<(), error::Error> { + unsafe { + guestfs_delete_event_callback(self.g, eh.eh); + } + self.callbacks.remove(&eh); + Ok(()) + } +} diff --git a/rust/src/lib.rs b/rust/src/lib.rs index cc41a99f8..81adef2a3 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -18,9 +18,11 @@ mod base; mod error; +mod event; mod guestfs; mod utils; pub use crate::base::*; pub use crate::error::*; +pub use crate::event::*; pub use crate::guestfs::*; diff --git a/rust/tests/040_create_multiple.rs b/rust/tests/040_create_multiple.rs index cc93554a3..970c988af 100644 --- a/rust/tests/040_create_multiple.rs +++ b/rust/tests/040_create_multiple.rs @@ -18,7 +18,7 @@ extern crate guestfs; -fn create() -> guestfs::Handle { +fn create<'a>() -> guestfs::Handle<'a> { match guestfs::Handle::create() { Ok(g) => g, Err(e) => panic!("fail: {:?}", e), diff --git a/rust/tests/410_close_event.rs b/rust/tests/410_close_event.rs new file mode 100644 index 000000000..308471098 --- /dev/null +++ b/rust/tests/410_close_event.rs @@ -0,0 +1,38 @@ +/* libguestfs Rust bindings + * Copyright (C) 2019 Hiroyuki Katsura <hiroyuki.katsura.0513(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +extern crate guestfs; + +use std::sync::{Arc, Mutex}; + +#[test] +fn close_event() { + let close_invoked = Arc::new(Mutex::new(0)); + { + let mut g = guestfs::Handle::create().expect("create"); + g.set_event_callback( + |_, _, _, _| { + let mut data = (&close_invoked).lock().unwrap(); + *data += 1; + }, + &[guestfs::Event::Close], + ) + .unwrap(); + } + assert_eq!(*((&close_invoked).lock().unwrap()), 1); +} diff --git a/rust/tests/420_log_messages.rs b/rust/tests/420_log_messages.rs new file mode 100644 index 000000000..1e9627ca7 --- /dev/null +++ b/rust/tests/420_log_messages.rs @@ -0,0 +1,60 @@ +/* libguestfs Rust bindings + * Copyright (C) 2019 Hiroyuki Katsura <hiroyuki.katsura.0513(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +extern crate guestfs; + +use std::str; +use std::sync::{Arc, Mutex}; + +#[test] +fn log_messages() { + let close_invoked = Arc::new(Mutex::new(0)); + { + let mut g = guestfs::Handle::create().expect("create"); + g.set_event_callback( + |ev, _, buf, array| { + let mut data = (&close_invoked).lock().unwrap(); + *data += 1; + + let event = guestfs::event_to_string(&[ev]).unwrap(); + + let buf = str::from_utf8(buf).unwrap(); + let array = array + .into_iter() + .map(|x| format!("{}", x)) + .collect::<Vec<String>>() + .join(","); + + eprintln!("event logged: event={} buf={} array={}", event, buf, array) + }, + &[ + guestfs::Event::Appliance, + guestfs::Event::Library, + guestfs::Event::Warning, + guestfs::Event::Trace, + ], + ) + .unwrap(); + + g.set_trace(true).unwrap(); + g.set_verbose(true).unwrap(); + g.add_drive_ro("/dev/null").unwrap(); + g.set_autosync(true).unwrap(); + } + assert!(*((&close_invoked).lock().unwrap()) > 0); +} diff --git a/rust/tests/430_progress_messages.rs b/rust/tests/430_progress_messages.rs new file mode 100644 index 000000000..a1d33aff7 --- /dev/null +++ b/rust/tests/430_progress_messages.rs @@ -0,0 +1,61 @@ +/* libguestfs Rust bindings + * Copyright (C) 2019 Hiroyuki Katsura <hiroyuki.katsura.0513(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +extern crate guestfs; + +use std::default::Default; +use std::sync::{Arc, Mutex}; + +#[test] +fn progress_messages() { + let callback_invoked = Arc::new(Mutex::new(0)); + { + let mut g = guestfs::Handle::create().expect("create"); + g.add_drive("/dev/null", Default::default()).unwrap(); + g.launch().unwrap(); + + let eh = g + .set_event_callback( + |_, _, _, _| { + let mut data = (&callback_invoked).lock().unwrap(); + *data += 1; + }, + &[guestfs::Event::Progress], + ) + .unwrap(); + assert_eq!("ok", g.debug("progress", &["5"]).unwrap()); + assert!(*(&callback_invoked).lock().unwrap() > 0); + + *(&callback_invoked).lock().unwrap() = 0; + g.delete_event_callback(eh).unwrap(); + assert_eq!("ok", g.debug("progress", &["5"]).unwrap()); + assert_eq!(*(&callback_invoked).lock().unwrap(), 0); + + g.set_event_callback( + |_, _, _, _| { + let mut data = (&callback_invoked).lock().unwrap(); + *data += 1; + }, + &[guestfs::Event::Progress], + ) + .unwrap(); + assert_eq!("ok", g.debug("progress", &["5"]).unwrap()); + assert!(*(&callback_invoked).lock().unwrap() > 0); + } + assert!(*(&callback_invoked).lock().unwrap() > 0); +} -- 2.20.1 (Apple Git-117)

5 13

Versioning of rust bindings
by Hiroyuki Katsura 06 Aug '19

06 Aug '19

In the last patch, I let the version of Rust bindings be the same as the version of libguestfs. However, I came to think it is not preferable in terms of the custom of Rust crate versioning. After a version of a crate is published to crates.io, no one can republish the crate with the same version. This means that when there are some mistakes in the rust bindings, after they are fixed, it is required to add the version to publish it. I'm not sure about the versioning of libguestfs, but I believe that it is not preferable that the versioning issue of rust bindings affects the whole versioning of libguestfs. Though it is available to hack SemanticVersioning by using suffixes(as we discussed before), maybe it is not preferable like 1.40.0-ver1. So, I think the versioning of rust bindings should be independent of the one of libguestfs. Do you agree, or not? Regards, Hiroyuki

3 2

[nbdkit PATCH 0/3] More responsive shutdown
by Eric Blake 06 Aug '19

06 Aug '19

We noticed while writing various libnbd tests that when the delay filter is in use, there are scenarios where we had to resort to SIGKILL to get rid of nbdkit, because it was non-responsive to SIGINT. I'm still trying to figure out the best way to add testsuite coverage of this, but already proved to myself that it works from the command line, under two scenarios that both used to cause long delays: 1. Early client death: nbdkit -U - -vf --filter=delay null size=1m rdelay=10 --run \ 'timeout 1s nbdsh --connect "nbd+unix:///?socket=$unixsocket" -c "h.pread(1,0)"' Pre-patch, the server detects the death of the client, but the worker thread stays busy for the remaining 9 seconds before nbdkit can finally exit. Post-patch, the server exits right after the client. 2. Early server death: timeout 1s nbdkit -U - -vf --filter=delay null size=1m rdelay=10 --run \ 'nbdsh --connect "nbd+unix:///?socket=$unixsocket" -c "h.pread(1,0)"' Pre-patch, the server reacts to the signal and kills the client, but the worker thread stays busy for the remaining 9 seconds before nbdkit can finally exit. Post-patch, the server is able to finalize right after the signal. Use of --run in the above tests lets you test things in one command line, but to some extent hides the longevity of the nbdkit process (you get the shell prompt back when the main thread exits, even though the detatched threads are still around); if you avoid --run and actually keep nbdkit in the foreground in one terminal and use nbdsh in a different terminal, choosing which terminal gets ^C, the effects are a bit more apparent. Patch 3 needs porting to any platform lacking ppoll. I have ideas for that port, but don't want to spend time on it before knowing for sure we need the port. And the fact that the pre-patch tests show output after the shell prompt returns means we still have cases where our detached threads are executing past the point where the main thread has tried to unload the plugin, which is never a nice thing. We may still have more work ahead of us to ensure that we don't unload an in-use plugin. Eric Blake (3): server: Add threadlocal_get_conn server: Add pipe for tracking disconnects server: Add and use nbdkit_nanosleep docs/nbdkit-plugin.pod | 28 +++++++++++++++++ configure.ac | 1 + include/nbdkit-common.h | 1 + server/internal.h | 3 ++ filters/delay/delay.c | 14 ++------- filters/rate/rate.c | 10 +++---- server/connections.c | 66 ++++++++++++++++++++++++++++++++++++++++- server/public.c | 61 +++++++++++++++++++++++++++++++++++++ server/threadlocal.c | 22 +++++++++++++- server/nbdkit.syms | 1 + 10 files changed, 188 insertions(+), 19 deletions(-) -- 2.20.1

2 6

[libnbd PATCH] lib: Always return cookie once command is queued
by Eric Blake 05 Aug '19

05 Aug '19

Although rare, it is possible that nbd_internal_run(cmd_issue) will report failure (perhaps because the server disconnected at the wrong time), even though we have queued the user's command. If we have a valid cookie, we MUST return it for the sake of users that will be calling nbd_aio_command_complete, as otherwise the user has no idea what cookie to wait on. Ignoring the state machine failure is okay; the whole idea of an asynch command is that the user will be calling more APIs to track the eventual completion, and will eventually learn that the state machine has moved to DEAD. --- lib/rw.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/rw.c b/lib/rw.c index 51ee691..6d37daa 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -229,6 +229,12 @@ nbd_internal_command_common (struct nbd_handle *h, /* Add the command to the end of the queue. Kick the state machine * if there is no other command being processed, otherwise, it will * be handled automatically on a future cycle around to READY. + * Beyond this point, we have to return a cookie to the user, since + * we are queuing the command, even if kicking the state machine + * detects a failure. Not reporting a state machine failure here is + * okay - any caller of an async command will be calling more API to + * await results, and will eventually learn that the machine has + * moved on to DEAD at that time. */ h->in_flight++; if (h->cmds_to_issue != NULL) { @@ -240,7 +246,7 @@ nbd_internal_command_common (struct nbd_handle *h, h->cmds_to_issue = h->cmds_to_issue_tail = cmd; if (nbd_internal_is_state_ready (get_next_state (h)) && nbd_internal_run (h, cmd_issue) == -1) - return -1; + nbd_internal_debug (h, "command queued, ignoring state machine failure"); } return cmd->cookie; -- 2.20.1

2 1

Re: [Libguestfs] nbdkit random seek performance
by Richard W.M. Jones 04 Aug '19

04 Aug '19

On Thu, Aug 01, 2019 at 03:44:31PM -0700, ivo welch wrote: > hi richard---arthur and I are working with nbdkit v1.12.3 on qemu/kvm. > > we found that our linux (ubuntu 16.04 32-bit) boot time from a local .img > file went from about 10 seconds to about 3 minutes when using the nbdkit > file plugin instead of directly connecting qemu to the file. on further > inspection with bonnie++, we think the problem may be related to poor > random seek performance: In general I would expect some slowdown between using qemu directly against a file and using qemu -> nbd driver -> nbdkit -> file, but not nearly as much as you observed. This is running bonnie++ inside the guest? Are qemu & nbdkit running on the same machine? Are you using a TCP port or a Unix domain socket? (If TCP you may want to consider commits d088bf45, 0721aa9579 and 3842a080c). Which version of qemu is it? Rich. > DIRECT FROM QEMU TO .IMG FILE: > ------------------------------------------------------------------------------------------------------------------------------ > Version 1.97 ------Sequential Output------ --Sequential Input- > --Random- > Concurrency 1 -Per Chr- --Block-- -Rewrite- -Per Chr- --Block-- > --Seeks-- > Machine Size K/sec %CP K/sec %CP K/sec %CP K/sec %CP K/sec %CP /sec > %CP > TEST 4G 109468 11 58405 16 180613 33 > 3204 147 > Latency 7125ms 5812ms 131ms > 101ms > ------------------------------------------------------------------------------------------------------------------------------ > ------------------------------------------------------------------------------------------------------------------------------ > > > > NBDKIT: > ------------------------------------------------------------------------------------------------------------------------------ > 1.97,1.97,TEST,1,1564697809,4G,,,,109468,11,58405,16,,,180613,33,3204,147,,,,,,,,,,,,,,,,,,,7125ms,5812ms,,131ms,101ms,,,,,, > Version 1.97 ------Sequential Output------ --Sequential Input- > --Random- > Concurrency 1 -Per Chr- --Block-- -Rewrite- -Per Chr- --Block-- > --Seeks-- > Machine Size K/sec %CP K/sec %CP K/sec %CP K/sec %CP K/sec %CP /sec > %CP > TEST 4G 105761 11 58992 14 178472 30 > 39.3 6 > Latency 8166ms 3683ms 187ms > 1197ms > ------------------------------------------------------------------------------------------------------------------------------ > ------------------------------------------------------------------------------------------------------------------------------ > > > > NBDKIT (-fv): > ------------------------------------------------------------------------------------------------------------------------------ > 1.97,1.97,TEST,1,1564698256,4G,,,,105761,11,58992,14,,,178472,30,39.3,6,,,,,,,,,,,,,,,,,,,8166ms,3683ms,,187ms,1197ms,,,,,, > Version 1.97 ------Sequential Output------ --Sequential Input- > --Random- > Concurrency 1 -Per Chr- --Block-- -Rewrite- -Per Chr- --Block-- > --Seeks-- > Machine Size K/sec %CP K/sec %CP K/sec %CP K/sec %CP K/sec %CP /sec > %CP > TEST 4G 104941 10 56501 14 160557 28 > 39.9 6 > Latency 15318ms 5200ms 101ms > 1152ms > > 1.97,1.97,TEST,1,1564695086,4G,,,,104941,10,56501,14,,,160557,28,39.9,6,,,,,,,,,,,,,,,,,,,15318ms,5200ms,,101ms,1152ms,,,,,, > ------------------------------------------------------------------------------------------------------------------------------ > ------------------------------------------------------------------------------------------------------------------------------ > > > is this an error on our part? do you have any recommendations on what we > could investigate next? > > regards, > > /iaw > > > -- > Ivo Welch (ivo.welch(a)ucla.edu) > http://www.ivo-welch.info/ -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org

2 2

[PATCH libnbd] generator: Generate typedefs automatically for Closure arguments.
by Richard W.M. Jones 03 Aug '19

03 Aug '19

For example nbd_set_debug takes a callback function. Previously this was defined explicitly inside the function parameters. This commit defines a new public typedef: typedef int (*nbd_debug_callback) (unsigned valid_flag, void *user_data, const char *context, const char *msg); and then uses the typedef like this: extern int nbd_set_debug_callback (struct nbd_handle *h, nbd_debug_callback debug_callback, void *debug_callback_user_data); (Previously typedefs were available, but they were written by hand and only used internally to the library.) This change necessitates that we uniquely name all of our closures across methods (the same-named closure is required to have the same cbargs). I took this opportunity to rename some, so especially completion callbacks now have the type nbd_completion_callback. The generator also checks they are named uniquely. This does not change the C API or ABI. --- generator/generator | 93 +++++++++++++++++++++++------ generator/states-reply-simple.c | 12 ++-- generator/states-reply-structured.c | 40 ++++++------- generator/states-reply.c | 8 +-- generator/states.c | 8 +-- lib/aio.c | 12 ++-- lib/debug.c | 12 ++-- lib/handle.c | 6 +- lib/internal.h | 20 ++----- lib/rw.c | 61 +++++++++++-------- 10 files changed, 165 insertions(+), 107 deletions(-) diff --git a/generator/generator b/generator/generator index 2a952cc..6f89792 100755 --- a/generator/generator +++ b/generator/generator @@ -924,7 +924,7 @@ Return the state of the debug flag on this handle."; "set_debug_callback", { default_call with - args = [ Closure { cbname="debug_fn"; + args = [ Closure { cbname="debug"; cbargs=[String "context"; String "msg"] } ]; ret = RErr; shortdesc = "set the debug callback"; @@ -1731,7 +1731,7 @@ C<nbd_pread>."; "aio_pread_callback", { default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset"; - Closure { cbname="callback"; + Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -1778,7 +1778,7 @@ documented in C<nbd_pread_structured>."; UInt64 "offset"; UInt "status"; Mutable (Int "error")] }; - Closure { cbname="callback"; + Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -1814,7 +1814,7 @@ C<nbd_pwrite>."; "aio_pwrite_callback", { default_call with args = [ BytesPersistIn ("buf", "count"); UInt64 "offset"; - Closure { cbname="callback"; + Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -1870,7 +1870,7 @@ Parameters behave as documented in C<nbd_flush>."; "aio_flush_callback", { default_call with - args = [ Closure { cbname="callback"; + args = [ Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -1904,7 +1904,7 @@ Parameters behave as documented in C<nbd_trim>."; "aio_trim_callback", { default_call with args = [ UInt64 "count"; UInt64 "offset"; - Closure { cbname="callback"; + Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -1938,7 +1938,7 @@ Parameters behave as documented in C<nbd_cache>."; "aio_cache_callback", { default_call with args = [ UInt64 "count"; UInt64 "offset"; - Closure { cbname="callback"; + Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -1972,7 +1972,7 @@ Parameters behave as documented in C<nbd_zero>."; "aio_zero_callback", { default_call with args = [ UInt64 "count"; UInt64 "offset"; - Closure { cbname="callback"; + Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -2017,7 +2017,7 @@ Parameters behave as documented in C<nbd_block_status>."; ArrayAndLen (UInt32 "entries", "nr_entries"); Mutable (Int "error")] }; - Closure { cbname="callback"; + Closure { cbname="completion"; cbargs=[Mutable (Int "error")] }; Flags "flags" ]; ret = RInt64; @@ -2363,6 +2363,25 @@ let rec group_by = function | (day, x) :: rest -> (day, [x]) :: group_by rest +let uniq ?(cmp = Pervasives.compare) xs = + let rec loop acc = function + | [] -> acc + | [x] -> x :: acc + | x :: (y :: _ as xs) when cmp x y = 0 -> + loop acc xs + | x :: (y :: _ as xs) -> + loop (x :: acc) xs + in + List.rev (loop [] xs) + +(* This is present in OCaml 4.04, so we can remove it when + * we depend on OCaml >= 4.04. + *) +let sort_uniq ?(cmp = Pervasives.compare) xs = + let xs = List.sort cmp xs in + let xs = uniq ~cmp xs in + xs + let chan = ref Pervasives.stdout let pr fs = ksprintf (fun str -> output_string !chan str) fs @@ -3100,6 +3119,30 @@ let () = name ) handle_calls; + (* Closures must be uniquely named across all calls. *) + let () = + let all_args = + List.flatten (List.map (fun (_, { args }) -> args) handle_calls) in + let h = Hashtbl.create 13 in + List.iter ( + function + | Closure { cbname; cbargs } -> + (try + (* If we've already added this name to the hash, check + * closure args are identical. + *) + let other_cbargs = Hashtbl.find h cbname in + if cbargs <> other_cbargs then + failwithf "%s: Closure has different arguments across methods" + cbname + with Not_found -> + (* Otherwise add it to the hash. *) + Hashtbl.add h cbname cbargs + ) + | _ -> () + ) all_args + in + (* !may_set_error is incompatible with permitted_states != [] because * an incorrect state will result in set_error being called by the * generated wrapper. It is also incompatible with RUint. @@ -3170,7 +3213,8 @@ let rec name_of_arg = function | BytesOut (n, len) -> [n; len] | BytesPersistIn (n, len) -> [n; len] | BytesPersistOut (n, len) -> [n; len] -| Closure { cbname } -> [cbname; sprintf "%s_user_data" cbname ] +| Closure { cbname } -> + [ sprintf "%s_callback" cbname; sprintf "%s_user_data" cbname ] | Flags n -> [n] | Int n -> [n] | Int64 n -> [n] @@ -3232,12 +3276,8 @@ let rec print_arg_list ?(handle = false) ?(valid_flag = false) if types then pr "size_t "; pr "%s" len | Closure { cbname; cbargs } -> - if types then ( - pr "int (*%s) " cbname; - print_arg_list ~valid_flag:true ~user_data:true cbargs; - ) - else - pr "%s" cbname; + if types then pr "nbd_%s_callback " cbname; + pr "%s_callback" cbname; pr ", "; if types then pr "void *"; pr "%s_user_data" cbname @@ -3297,6 +3337,24 @@ let print_extern name args ret = print_call name args ret; pr ";\n" +(* Callback typedefs in <libnbd.h> *) +let print_closure_typedefs () = + let all_cls = + List.map ( + fun (_, { args }) -> + filter_map (function Closure cl -> Some cl | _ -> None) args + ) handle_calls in + let all_cls = List.flatten all_cls in + let cmp { cbname = n1 } { cbname = n2 } = compare n1 n2 in + let unique_cls = sort_uniq ~cmp all_cls in + List.iter ( + fun { cbname; cbargs } -> + pr "typedef int (*nbd_%s_callback) " cbname; + print_arg_list ~valid_flag:true ~user_data:true cbargs; + pr ";\n"; + ) unique_cls; + pr "\n" + let print_extern_and_define name args ret = let name_upper = String.uppercase_ascii name in print_extern name args ret; @@ -3355,6 +3413,7 @@ let generate_include_libnbd_h () = pr "extern int nbd_get_errno (void);\n"; pr "#define LIBNBD_HAVE_NBD_GET_ERRNO 1\n"; pr "\n"; + print_closure_typedefs (); List.iter ( fun (name, { args; ret }) -> print_extern_and_define name args ret ) handle_calls; @@ -4845,7 +4904,7 @@ let print_ocaml_binding (name, { args; ret }) = pr " if (%s_user_data == NULL) caml_raise_out_of_memory ();\n" cbname; pr " caml_register_generational_global_root (%s_user_data);\n" cbname; pr " *%s_user_data = %sv;\n" cbname cbname; - pr " const void *%s = %s_%s_wrapper;\n" cbname name cbname + pr " const void *%s_callback = %s_%s_wrapper;\n" cbname name cbname | OCamlArg (Flags _) -> assert false (* see above *) | OCamlArg (Int n) -> pr " int %s = Int_val (%sv);\n" n n diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index e0fd71d..9b249ab 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -60,16 +60,16 @@ case 0: /* guaranteed by START */ assert (cmd); - if (cmd->cb.fn.read) { + if (cmd->cb.fn.chunk) { int error = 0; assert (cmd->error == 0); - if (cmd->cb.fn.read (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, - cmd->cb.fn_user_data, - cmd->data, cmd->count, - cmd->offset, LIBNBD_READ_DATA, &error) == -1) + if (cmd->cb.fn.chunk (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, + cmd->cb.fn_user_data, + cmd->data, cmd->count, + cmd->offset, LIBNBD_READ_DATA, &error) == -1) cmd->error = error ? error : EPROTO; - cmd->cb.fn.read = NULL; /* because we've freed it */ + cmd->cb.fn.chunk = NULL; /* because we've freed it */ } SET_NEXT_STATE (%^FINISH_COMMAND); diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index ff5b727..cdd9f10 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -304,7 +304,7 @@ valid_flags (struct nbd_handle *h) offset, cmd->offset, cmd->count); return 0; } - if (cmd->type == NBD_CMD_READ && cmd->cb.fn.read) { + if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk) { int scratch = error; unsigned valid = valid_flags (h); @@ -312,13 +312,13 @@ valid_flags (struct nbd_handle *h) * current error rather than any earlier one. If the callback fails * without setting errno, then use the server's error below. */ - if (cmd->cb.fn.read (valid, cmd->cb.fn_user_data, - cmd->data + (offset - cmd->offset), - 0, offset, LIBNBD_READ_ERROR, &scratch) == -1) + if (cmd->cb.fn.chunk (valid, cmd->cb.fn_user_data, + cmd->data + (offset - cmd->offset), + 0, offset, LIBNBD_READ_ERROR, &scratch) == -1) if (cmd->error == 0) cmd->error = scratch; if (valid & LIBNBD_CALLBACK_FREE) - cmd->cb.fn.read = NULL; /* because we've freed it */ + cmd->cb.fn.chunk = NULL; /* because we've freed it */ } } @@ -398,18 +398,18 @@ valid_flags (struct nbd_handle *h) offset = be64toh (h->sbuf.sr.payload.offset_data.offset); assert (cmd); /* guaranteed by CHECK */ - if (cmd->cb.fn.read) { + if (cmd->cb.fn.chunk) { int error = cmd->error; unsigned valid = valid_flags (h); - if (cmd->cb.fn.read (valid, cmd->cb.fn_user_data, - cmd->data + (offset - cmd->offset), - length - sizeof offset, offset, + if (cmd->cb.fn.chunk (valid, cmd->cb.fn_user_data, + cmd->data + (offset - cmd->offset), + length - sizeof offset, offset, LIBNBD_READ_DATA, &error) == -1) if (cmd->error == 0) cmd->error = error ? error : EPROTO; if (valid & LIBNBD_CALLBACK_FREE) - cmd->cb.fn.read = NULL; /* because we've freed it */ + cmd->cb.fn.chunk = NULL; /* because we've freed it */ } SET_NEXT_STATE (%FINISH); @@ -463,18 +463,18 @@ valid_flags (struct nbd_handle *h) * them as an extension, and this works even when length == 0. */ memset (cmd->data + offset, 0, length); - if (cmd->cb.fn.read) { + if (cmd->cb.fn.chunk) { int error = cmd->error; unsigned valid = valid_flags (h); - if (cmd->cb.fn.read (valid, cmd->cb.fn_user_data, - cmd->data + offset, length, - cmd->offset + offset, - LIBNBD_READ_HOLE, &error) == -1) + if (cmd->cb.fn.chunk (valid, cmd->cb.fn_user_data, + cmd->data + offset, length, + cmd->offset + offset, + LIBNBD_READ_HOLE, &error) == -1) if (cmd->error == 0) cmd->error = error ? error : EPROTO; if (valid & LIBNBD_CALLBACK_FREE) - cmd->cb.fn.read = NULL; /* because we've freed it */ + cmd->cb.fn.chunk = NULL; /* because we've freed it */ } SET_NEXT_STATE(%FINISH); @@ -548,10 +548,10 @@ valid_flags (struct nbd_handle *h) if (cmd->type == NBD_CMD_BLOCK_STATUS && cmd->cb.fn.extent) cmd->cb.fn.extent (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, NULL, 0, NULL, 0, NULL); - if (cmd->type == NBD_CMD_READ && cmd->cb.fn.read) - cmd->cb.fn.read (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, - NULL, 0, 0, 0, NULL); - cmd->cb.fn.read = NULL; + if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk) + cmd->cb.fn.chunk (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, + NULL, 0, 0, 0, NULL); + cmd->cb.fn.chunk = NULL; SET_NEXT_STATE (%^FINISH_COMMAND); } else { diff --git a/generator/states-reply.c b/generator/states-reply.c index 078d67f..09adfed 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -168,14 +168,14 @@ save_reply_state (struct nbd_handle *h) retire = cmd->type == NBD_CMD_DISC; /* Notify the user */ - if (cmd->cb.callback) { + if (cmd->cb.completion) { int error = cmd->error; int r; assert (cmd->type != NBD_CMD_DISC); - r = cmd->cb.callback (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, - cmd->cb.user_data, &error); - cmd->cb.callback = NULL; /* because we've freed it */ + r = cmd->cb.completion (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, + cmd->cb.user_data, &error); + cmd->cb.completion = NULL; /* because we've freed it */ switch (r) { case -1: if (error) diff --git a/generator/states.c b/generator/states.c index 654e4c8..9ed57ae 100644 --- a/generator/states.c +++ b/generator/states.c @@ -121,14 +121,14 @@ void abort_commands (struct nbd_handle *h, bool retire = cmd->type == NBD_CMD_DISC; next = cmd->next; - if (cmd->cb.callback) { + if (cmd->cb.completion) { int error = cmd->error ? cmd->error : ENOTCONN; int r; assert (cmd->type != NBD_CMD_DISC); - r = cmd->cb.callback (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, - cmd->cb.user_data, &error); - cmd->cb.callback = NULL; /* because we've freed it */ + r = cmd->cb.completion (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, + cmd->cb.user_data, &error); + cmd->cb.completion = NULL; /* because we've freed it */ switch (r) { case -1: if (error) diff --git a/lib/aio.c b/lib/aio.c index 1c11dbd..c141de6 100644 --- a/lib/aio.c +++ b/lib/aio.c @@ -35,12 +35,12 @@ nbd_internal_retire_and_free_command (struct command *cmd) if (cmd->type == NBD_CMD_BLOCK_STATUS && cmd->cb.fn.extent) cmd->cb.fn.extent (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, NULL, 0, NULL, 0, NULL); - if (cmd->type == NBD_CMD_READ && cmd->cb.fn.read) - cmd->cb.fn.read (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, - NULL, 0, 0, 0, NULL); - if (cmd->cb.callback) - cmd->cb.callback (LIBNBD_CALLBACK_FREE, cmd->cb.user_data, - NULL); + if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk) + cmd->cb.fn.chunk (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, + NULL, 0, 0, 0, NULL); + if (cmd->cb.completion) + cmd->cb.completion (LIBNBD_CALLBACK_FREE, cmd->cb.user_data, + NULL); free (cmd); } diff --git a/lib/debug.c b/lib/debug.c index 7784bd9..f4b374d 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -40,12 +40,12 @@ nbd_unlocked_get_debug (struct nbd_handle *h) int nbd_unlocked_set_debug_callback (struct nbd_handle *h, - debug_fn debug_fn, void *data) + nbd_debug_callback debug_callback, void *data) { - if (h->debug_fn) + if (h->debug_callback) /* ignore return value */ - h->debug_fn (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); - h->debug_fn = debug_fn; + h->debug_callback (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); + h->debug_callback = debug_callback; h->debug_data = data; return 0; } @@ -76,9 +76,9 @@ nbd_internal_debug (struct nbd_handle *h, const char *fs, ...) if (r == -1) goto out; - if (h->debug_fn) + if (h->debug_callback) /* ignore return value */ - h->debug_fn (LIBNBD_CALLBACK_VALID, h->debug_data, context, msg); + h->debug_callback (LIBNBD_CALLBACK_VALID, h->debug_data, context, msg); else fprintf (stderr, "libnbd: debug: %s: %s\n", context ? : "unknown", msg); out: diff --git a/lib/handle.c b/lib/handle.c index a9ade3d..1360079 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -105,9 +105,9 @@ nbd_close (struct nbd_handle *h) return; /* Free user callbacks first. */ - if (h->debug_fn) - h->debug_fn (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); - h->debug_fn = NULL; + if (h->debug_callback) + h->debug_callback (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); + h->debug_callback = NULL; free (h->bs_entries); for (m = h->meta_contexts; m != NULL; m = m_next) { diff --git a/lib/internal.h b/lib/internal.h index 90ce6aa..d85fb4b 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -47,7 +47,6 @@ struct meta_context; struct socket; struct command; -typedef int (*debug_fn) (unsigned, void *, const char *, const char *); struct nbd_handle { /* Lock protecting concurrent access to the handle. */ @@ -80,7 +79,7 @@ struct nbd_handle { /* For debugging. */ bool debug; - debug_fn debug_fn; + nbd_debug_callback debug_callback; void *debug_data; /* State machine. @@ -248,23 +247,14 @@ struct socket { const struct socket_ops *ops; }; -typedef int (*extent_fn) (unsigned valid_flag, void *user_data, - const char *metacontext, uint64_t offset, - uint32_t *entries, size_t nr_entries, int *error); -typedef int (*read_fn) (unsigned valid_flag, void *user_data, - const void *buf, size_t count, - uint64_t offset, unsigned status, int *error); -typedef int (*callback_fn) (unsigned valid_flag, void *user_data, - int *error); - struct command_cb { union { - extent_fn extent; - read_fn read; + nbd_extent_callback extent; + nbd_chunk_callback chunk; } fn; void *fn_user_data; /* associated with one of the fn callbacks above */ - callback_fn callback; - void *user_data; /* associated with the callback function */ + nbd_completion_callback completion; + void *user_data; /* associated with the completion callback */ }; struct command { diff --git a/lib/rw.c b/lib/rw.c index 382cfb9..51ee691 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -60,12 +60,13 @@ nbd_unlocked_pread (struct nbd_handle *h, void *buf, int nbd_unlocked_pread_structured (struct nbd_handle *h, void *buf, size_t count, uint64_t offset, - read_fn read, void *user_data, uint32_t flags) + nbd_chunk_callback chunk, void *user_data, + uint32_t flags) { int64_t cookie; cookie = nbd_unlocked_aio_pread_structured (h, buf, count, offset, - read, user_data, flags); + chunk, user_data, flags); if (cookie == -1) return -1; @@ -145,7 +146,8 @@ nbd_unlocked_zero (struct nbd_handle *h, int nbd_unlocked_block_status (struct nbd_handle *h, uint64_t count, uint64_t offset, - extent_fn extent, void *user_data, uint32_t flags) + nbd_extent_callback extent, void *user_data, + uint32_t flags) { int64_t cookie; @@ -255,10 +257,11 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf, int64_t nbd_unlocked_aio_pread_callback (struct nbd_handle *h, void *buf, size_t count, uint64_t offset, - callback_fn callback, void *user_data, + nbd_completion_callback completion, + void *user_data, uint32_t flags) { - struct command_cb cb = { .callback = callback, .user_data = user_data, }; + struct command_cb cb = { .completion = completion, .user_data = user_data, }; /* We could silently accept flag DF, but it really only makes sense * with callbacks, because otherwise there is no observable change @@ -276,11 +279,11 @@ nbd_unlocked_aio_pread_callback (struct nbd_handle *h, void *buf, int64_t nbd_unlocked_aio_pread_structured (struct nbd_handle *h, void *buf, size_t count, uint64_t offset, - read_fn read, void *user_data, + nbd_chunk_callback chunk, void *user_data, uint32_t flags) { return nbd_unlocked_aio_pread_structured_callback (h, buf, count, offset, - read, user_data, + chunk, user_data, NULL, NULL, flags); } @@ -288,15 +291,15 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h, void *buf, int64_t nbd_unlocked_aio_pread_structured_callback (struct nbd_handle *h, void *buf, size_t count, uint64_t offset, - read_fn read, + nbd_chunk_callback chunk, void *read_user_data, - callback_fn callback, + nbd_completion_callback completion, void *callback_user_data, uint32_t flags) { - struct command_cb cb = { .fn.read = read, + struct command_cb cb = { .fn.chunk = chunk, .fn_user_data = read_user_data, - .callback = callback, + .completion = completion, .user_data = callback_user_data, }; if ((flags & ~LIBNBD_CMD_FLAG_DF) != 0) { @@ -326,10 +329,11 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void *buf, int64_t nbd_unlocked_aio_pwrite_callback (struct nbd_handle *h, const void *buf, size_t count, uint64_t offset, - callback_fn callback, void *user_data, + nbd_completion_callback completion, + void *user_data, uint32_t flags) { - struct command_cb cb = { .callback = callback, .user_data = user_data, }; + struct command_cb cb = { .completion = completion, .user_data = user_data, }; if (nbd_unlocked_read_only (h) == 1) { set_error (EINVAL, "server does not support write operations"); @@ -358,10 +362,12 @@ nbd_unlocked_aio_flush (struct nbd_handle *h, uint32_t flags) } int64_t -nbd_unlocked_aio_flush_callback (struct nbd_handle *h, callback_fn callback, - void *user_data, uint32_t flags) +nbd_unlocked_aio_flush_callback (struct nbd_handle *h, + nbd_completion_callback completion, + void *user_data, + uint32_t flags) { - struct command_cb cb = { .callback = callback, .user_data = user_data, }; + struct command_cb cb = { .completion = completion, .user_data = user_data, }; if (nbd_unlocked_can_flush (h) != 1) { set_error (EINVAL, "server does not support flush operations"); @@ -388,10 +394,11 @@ nbd_unlocked_aio_trim (struct nbd_handle *h, int64_t nbd_unlocked_aio_trim_callback (struct nbd_handle *h, uint64_t count, uint64_t offset, - callback_fn callback, void *user_data, + nbd_completion_callback completion, + void *user_data, uint32_t flags) { - struct command_cb cb = { .callback = callback, .user_data = user_data, }; + struct command_cb cb = { .completion = completion, .user_data = user_data, }; if (nbd_unlocked_read_only (h) == 1) { set_error (EINVAL, "server does not support write operations"); @@ -428,10 +435,11 @@ nbd_unlocked_aio_cache (struct nbd_handle *h, int64_t nbd_unlocked_aio_cache_callback (struct nbd_handle *h, uint64_t count, uint64_t offset, - callback_fn callback, void *user_data, + nbd_completion_callback completion, + void *user_data, uint32_t flags) { - struct command_cb cb = { .callback = callback, .user_data = user_data, }; + struct command_cb cb = { .completion = completion, .user_data = user_data, }; /* Actually according to the NBD protocol document, servers do exist * that support NBD_CMD_CACHE but don't advertise the @@ -462,10 +470,11 @@ nbd_unlocked_aio_zero (struct nbd_handle *h, int64_t nbd_unlocked_aio_zero_callback (struct nbd_handle *h, uint64_t count, uint64_t offset, - callback_fn callback, void *user_data, + nbd_completion_callback completion, + void *user_data, uint32_t flags) { - struct command_cb cb = { .callback = callback, .user_data = user_data, }; + struct command_cb cb = { .completion = completion, .user_data = user_data, }; if (nbd_unlocked_read_only (h) == 1) { set_error (EINVAL, "server does not support write operations"); @@ -495,7 +504,7 @@ nbd_unlocked_aio_zero_callback (struct nbd_handle *h, int64_t nbd_unlocked_aio_block_status (struct nbd_handle *h, uint64_t count, uint64_t offset, - extent_fn extent, void *user_data, + nbd_extent_callback extent, void *user_data, uint32_t flags) { return nbd_unlocked_aio_block_status_callback (h, count, offset, @@ -507,15 +516,15 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h, int64_t nbd_unlocked_aio_block_status_callback (struct nbd_handle *h, uint64_t count, uint64_t offset, - extent_fn extent, + nbd_extent_callback extent, void *extent_user_data, - callback_fn callback, + nbd_completion_callback completion, void *callback_user_data, uint32_t flags) { struct command_cb cb = { .fn.extent = extent, .fn_user_data = extent_user_data, - .callback = callback, + .completion = completion, .user_data = callback_user_data }; if (!h->structured_replies) { -- 2.22.0

2 1

[nbdkit PATCH 0/3] sh plugin fixes
by Eric Blake 02 Aug '19

02 Aug '19

I'm pushing the first one as blatantly obvious. The second one is also simple enough, but not enough of a bug for me to push tonight. The third is something I noticed while working on sh, but is really more about docs vs. plugins in general. There, we could either change the code to match the docs (breaking backwards behavior for a plugin that set .errno_is_preserved=2) [what my patch did], or change the docs to match the code (mention that any non-zero value will do). Preference? Eric Blake (3): sh: Fix flags when none are present sh: Avoid setenv after fork plugins: Match docs for .errno_is_preserved docs/nbdkit-plugin.pod | 16 ++++++++++++++++ plugins/sh/call.c | 3 --- plugins/sh/sh.c | 8 ++++++++ server/plugins.c | 2 +- tests/test-shell.sh | 15 +++++++++++++++ 5 files changed, 40 insertions(+), 4 deletions(-) -- 2.20.1

2 6

[nbdkit PATCH 0/8] fd leak safety
by Eric Blake 01 Aug '19

01 Aug '19

There's enough here to need a review; some of it probably needs backporting to stable-1.12. This probably breaks tests on Haiku or other platforms that have not been as on-the-ball about atomic CLOEXEC; feel free to report issues that arise, and I'll help come up with workarounds (even if we end up leaving a rare fd leak on less-capable systems). Meanwhile, I'm still working on my promise to add an nbdkit_nanosleep for use in the delay and stat filters, and which makes nbdkit more responsive to ^C where it currently waits for a full delay to expire before exiting. (Nothing like finding several other pre-requisite bugs to fix first before getting to my real goal...) Eric Blake (8): rate: Pass through delay failures server: Don't leave uninit variable on failure server: Add test for nbdkit_read_password Revert "RHEL 5: Define O_CLOEXEC and SOCK_CLOEXEC." cow, cache: Better mkostemp fallback server: Atomically set CLOEXEC on all fds filters: Set CLOEXEC on files opened during .config rate: Atomically set CLOEXEC on fds server/internal.h | 8 ---- filters/cache/blk.c | 19 ++++++++- filters/cow/blk.c | 19 ++++++++- filters/log/log.c | 20 ++++++++- filters/rate/rate.c | 26 ++++++++---- filters/stats/stats.c | 18 +++++++- filters/xz/xzfile.c | 4 -- plugins/example2/example2.c | 4 -- plugins/file/file.c | 4 -- plugins/streaming/streaming.c | 4 -- server/connections.c | 1 + server/quit.c | 5 ++- server/sockets.c | 7 ++-- server/test-utils.c | 79 +++++++++++++++++++++++++++++++++-- server/utils.c | 10 ++++- 15 files changed, 182 insertions(+), 46 deletions(-) -- 2.20.1

3 20

[PATCH] v2v: -i vmx: Use scp -T option if available to unbreak scp (RHBZ#1733168).
by Richard W.M. Jones 01 Aug '19

01 Aug '19

Tested using: cd v2v LIBGUESTFS_BACKEND=direct ../run virt-v2v -i vmx -it ssh "ssh://localhost/$PWD/test-v2v-i-vmx-1.vmx" -o null -v -x and manually examining the debug output. Thanks: Ming Xie, Jakub Jelen. --- v2v/input_vmx.ml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/v2v/input_vmx.ml b/v2v/input_vmx.ml index 5441bccb9..4a82a867f 100644 --- a/v2v/input_vmx.ml +++ b/v2v/input_vmx.ml @@ -61,6 +61,13 @@ let server_of_uri { Xml.uri_server } = let path_of_uri { Xml.uri_path } = match uri_path with None -> assert false | Some p -> p +let scp_supports_T_option = lazy ( + let cmd = "LANG=C scp -T |& grep \"unknown option\"" in + if verbose () then + eprintf "%s\n%!" cmd; + Sys.command cmd <> 0 +) + (* 'scp' a remote file into a temporary local file, returning the path * of the temporary local file. *) @@ -68,8 +75,9 @@ let scp_from_remote_to_temporary uri tmpdir filename = let localfile = tmpdir // filename in let cmd = - sprintf "scp%s%s %s%s:%s %s" + sprintf "scp%s%s%s %s%s:%s %s" (if verbose () then "" else " -q") + (if Lazy.force scp_supports_T_option then " -T" else "") (match port_of_uri uri with | None -> "" | Some port -> sprintf " -P %d" port) -- 2.22.0

2 1