Rust binding for QAPI and qemu-ga QMP handler examples

[RFC v3 12/32] rust: provide a common crate for QEMU

Posted by marcandre.lureau@redhat.com 4 years ago

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This crates provides common bindings and facilities for QEMU C API
shared by various projects.

Most importantly, it defines the conversion traits used to convert from
C to Rust types. Those traits are largely adapted from glib-rs, since
those have proved to be very flexible, and should guide us to bind
further QEMU types such as QOM. If glib-rs becomes a dependency, we
should consider adopting glib translate traits. For QAPI, we need a
smaller subset.

Cargo.lock is checked-in, as QEMU produces end-of-chain binaries, and it
is desirable to track the exact set of packages that are involved in
managed builds.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 Cargo.lock                   |  63 +++++
 Cargo.toml                   |   4 +-
 rust/common/Cargo.toml       |  11 +
 rust/common/src/error.rs     | 113 ++++++++
 rust/common/src/ffi.rs       |  93 +++++++
 rust/common/src/lib.rs       |  21 ++
 rust/common/src/qemu.rs      | 101 ++++++++
 rust/common/src/qmp.rs       |   0
 rust/common/src/translate.rs | 482 +++++++++++++++++++++++++++++++++++
 9 files changed, 887 insertions(+), 1 deletion(-)
 create mode 100644 Cargo.lock
 create mode 100644 rust/common/Cargo.toml
 create mode 100644 rust/common/src/error.rs
 create mode 100644 rust/common/src/ffi.rs
 create mode 100644 rust/common/src/lib.rs
 create mode 100644 rust/common/src/qemu.rs
 create mode 100644 rust/common/src/qmp.rs
 create mode 100644 rust/common/src/translate.rs

diff --git a/Cargo.lock b/Cargo.lock
new file mode 100644
index 0000000000..8dc2dd9da7
--- /dev/null
+++ b/Cargo.lock
@@ -0,0 +1,63 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 3
+
+[[package]]
+name = "autocfg"
+version = "1.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a"
+
+[[package]]
+name = "bitflags"
+version = "1.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693"
+
+[[package]]
+name = "cc"
+version = "1.0.70"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d26a6ce4b6a484fa3edb70f7efa6fc430fd2b87285fe8b84304fd0936faa0dc0"
+
+[[package]]
+name = "cfg-if"
+version = "1.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
+
+[[package]]
+name = "common"
+version = "0.1.0"
+dependencies = [
+ "libc",
+ "nix",
+]
+
+[[package]]
+name = "libc"
+version = "0.2.101"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3cb00336871be5ed2c8ed44b60ae9959dc5b9f08539422ed43f09e34ecaeba21"
+
+[[package]]
+name = "memoffset"
+version = "0.6.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "59accc507f1338036a0477ef61afdae33cde60840f4dfe481319ce3ad116ddf9"
+dependencies = [
+ "autocfg",
+]
+
+[[package]]
+name = "nix"
+version = "0.20.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "df8e5e343312e7fbeb2a52139114e9e702991ef9c2aea6817ff2440b35647d56"
+dependencies = [
+ "bitflags",
+ "cc",
+ "cfg-if",
+ "libc",
+ "memoffset",
+]
diff --git a/Cargo.toml b/Cargo.toml
index c4b464ff15..14131eed3c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,2 +1,4 @@
 [workspace]
-members = []
+members = [
+  "rust/common",
+]
diff --git a/rust/common/Cargo.toml b/rust/common/Cargo.toml
new file mode 100644
index 0000000000..6c240447f3
--- /dev/null
+++ b/rust/common/Cargo.toml
@@ -0,0 +1,11 @@
+[package]
+name = "common"
+version = "0.1.0"
+edition = "2018"
+publish = false
+
+[dependencies]
+libc = "0.2.92"
+
+[target."cfg(unix)".dependencies]
+nix = "0.20.0"
diff --git a/rust/common/src/error.rs b/rust/common/src/error.rs
new file mode 100644
index 0000000000..f166ac42ea
--- /dev/null
+++ b/rust/common/src/error.rs
@@ -0,0 +1,113 @@
+use std::{self, ffi::CString, fmt, io, ptr};
+
+use crate::translate::*;
+use crate::{ffi, qemu};
+
+/// Common error type for QEMU and related projects.
+#[derive(Debug)]
+pub enum Error {
+    /// A generic error with file and line location.
+    FailedAt(String, &'static str, u32),
+    /// An IO error.
+    Io(io::Error),
+    #[cfg(unix)]
+    /// A nix error.
+    Nix(nix::Error),
+}
+
+/// Alias for a `Result` with the error type for QEMU.
+pub type Result<T> = std::result::Result<T, Error>;
+
+impl Error {
+    fn message(&self) -> String {
+        use Error::*;
+        match self {
+            FailedAt(msg, _, _) => msg.into(),
+            Io(io) => format!("IO error: {}", io),
+            #[cfg(unix)]
+            Nix(nix) => format!("Nix error: {}", nix),
+        }
+    }
+
+    fn location(&self) -> Option<(&'static str, u32)> {
+        use Error::*;
+        match self {
+            FailedAt(_, file, line) => Some((file, *line)),
+            Io(_) => None,
+            #[cfg(unix)]
+            Nix(_) => None,
+        }
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use Error::*;
+        match self {
+            FailedAt(msg, file, line) => write!(f, "{} ({}:{})", msg, file, line),
+            _ => write!(f, "{}", self.message()),
+        }
+    }
+}
+
+impl From<io::Error> for Error {
+    fn from(val: io::Error) -> Self {
+        Error::Io(val)
+    }
+}
+
+#[cfg(unix)]
+impl From<nix::Error> for Error {
+    fn from(val: nix::Error) -> Self {
+        Error::Nix(val)
+    }
+}
+
+impl QemuPtrDefault for Error {
+    type QemuType = *mut ffi::Error;
+}
+
+impl<'a> ToQemuPtr<'a, *mut ffi::Error> for Error {
+    type Storage = qemu::CError;
+
+    fn to_qemu_none(&'a self) -> Stash<'a, *mut ffi::Error, Self> {
+        let err = self.to_qemu_full();
+
+        Stash(err, unsafe { from_qemu_full(err) })
+    }
+
+    fn to_qemu_full(&self) -> *mut ffi::Error {
+        let cmsg =
+            CString::new(self.message()).expect("ToQemuPtr<Error>: unexpected '\0' character");
+        let mut csrc = CString::new("").unwrap();
+        let (src, line) = self.location().map_or((ptr::null(), 0_i32), |loc| {
+            csrc = CString::new(loc.0).expect("ToQemuPtr<Error>:: unexpected '\0' character");
+            (csrc.as_ptr() as *const libc::c_char, loc.1 as i32)
+        });
+        let func = ptr::null();
+
+        let mut err: *mut ffi::Error = ptr::null_mut();
+        unsafe {
+            ffi::error_setg_internal(
+                &mut err as *mut *mut _,
+                src,
+                line,
+                func,
+                cmsg.as_ptr() as *const libc::c_char,
+            );
+            err
+        }
+    }
+}
+
+/// Convenience macro to build a [`Error::FailedAt`] error.
+///
+/// Returns a `Result::Err` with the file:line location.
+/// (the error can then be converted to a QEMU `ffi::Error`)
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! err {
+    ($msg:expr) => {
+        Err(Error::FailedAt($msg.into(), file!(), line!()))
+    };
+}
diff --git a/rust/common/src/ffi.rs b/rust/common/src/ffi.rs
new file mode 100644
index 0000000000..82818d503a
--- /dev/null
+++ b/rust/common/src/ffi.rs
@@ -0,0 +1,93 @@
+//! Bindings to the raw low-level C API commonly provided by QEMU projects.
+//!
+//! Manual bindings to C API availabe when linking QEMU projects.
+//! It includes minimal glib allocation functions too, since it's the default
+//! allocator used by QEMU, and we don't depend on glib-rs crate yet).
+//!
+//! Higher-level Rust-friendly bindings are provided by different modules.
+
+use libc::{c_char, c_void, size_t};
+
+extern "C" {
+    pub fn g_malloc0(n_bytes: size_t) -> *mut c_void;
+    pub fn g_free(ptr: *mut c_void);
+    pub fn g_strndup(str: *const c_char, n: size_t) -> *mut c_char;
+}
+
+#[repr(C)]
+pub struct QObject(c_void);
+
+impl ::std::fmt::Debug for QObject {
+    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
+        f.debug_struct(&format!("QObject @ {:?}", self as *const _))
+            .finish()
+    }
+}
+
+#[repr(C)]
+pub struct QNull(c_void);
+
+impl ::std::fmt::Debug for QNull {
+    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
+        f.debug_struct(&format!("QNull @ {:?}", self as *const _))
+            .finish()
+    }
+}
+
+#[repr(C)]
+pub struct Error(c_void);
+
+impl ::std::fmt::Debug for Error {
+    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
+        f.debug_struct(&format!("Error @ {:?}", self as *const _))
+            .finish()
+    }
+}
+
+extern "C" {
+    pub fn error_setg_internal(
+        errp: *mut *mut Error,
+        src: *const c_char,
+        line: i32,
+        func: *const c_char,
+        fmt: *const c_char,
+        ...
+    );
+    pub fn error_get_pretty(err: *const Error) -> *const c_char;
+    pub fn error_free(err: *mut Error);
+}
+
+/// Wrap a QMP hanlder.
+#[macro_export]
+macro_rules! qmp {
+    // the basic return value variant
+    ($e:expr, $errp:ident, $errval:expr) => {{
+        assert!(!$errp.is_null());
+        unsafe {
+            *$errp = std::ptr::null_mut();
+        }
+
+        match $e {
+            Ok(val) => val,
+            Err(err) => unsafe {
+                *$errp = err.to_qemu_full();
+                $errval
+            },
+        }
+    }};
+    // the ptr return value variant
+    ($e:expr, $errp:ident) => {{
+        assert!(!$errp.is_null());
+        unsafe {
+            *$errp = std::ptr::null_mut();
+        }
+
+        match $e {
+            Ok(val) => val.to_qemu_full().into(),
+            Err(err) => unsafe {
+                *$errp = err.to_qemu_full();
+                std::ptr::null_mut()
+            },
+        }
+    }};
+}
diff --git a/rust/common/src/lib.rs b/rust/common/src/lib.rs
new file mode 100644
index 0000000000..4de826bc2e
--- /dev/null
+++ b/rust/common/src/lib.rs
@@ -0,0 +1,21 @@
+//! Common code for QEMU
+//!
+//! This crates provides common bindings and facilities for QEMU C API shared by
+//! various projects. Most importantly, it defines the conversion traits used to
+//! convert from C to Rust types. Those traits are largely adapted from glib-rs,
+//! since those have prooven to be very flexible, and should guide us to bind
+//! further QEMU types such as QOM. If glib-rs becomes a dependency, we should
+//! consider adopting glib translate traits. For QAPI, we need a smaller subset.
+
+pub use libc;
+
+mod error;
+pub use error::*;
+
+mod qemu;
+pub use qemu::*;
+
+mod translate;
+pub use translate::*;
+
+pub mod ffi;
diff --git a/rust/common/src/qemu.rs b/rust/common/src/qemu.rs
new file mode 100644
index 0000000000..dd01c6d92d
--- /dev/null
+++ b/rust/common/src/qemu.rs
@@ -0,0 +1,101 @@
+use std::{ffi::CStr, ptr, str};
+
+use crate::{ffi, translate};
+use translate::{FromQemuPtrFull, FromQemuPtrNone, QemuPtrDefault, Stash, ToQemuPtr};
+
+/// A type representing an owned C QEMU Error.
+pub struct CError(ptr::NonNull<ffi::Error>);
+
+impl translate::FromQemuPtrFull<*mut ffi::Error> for CError {
+    unsafe fn from_qemu_full(ptr: *mut ffi::Error) -> Self {
+        assert!(!ptr.is_null());
+        Self(ptr::NonNull::new_unchecked(ptr))
+    }
+}
+
+impl CError {
+    pub fn pretty(&self) -> &str {
+        unsafe {
+            let pretty = ffi::error_get_pretty(self.0.as_ptr());
+            let bytes = CStr::from_ptr(pretty).to_bytes();
+            str::from_utf8(bytes)
+                .unwrap_or_else(|err| str::from_utf8(&bytes[..err.valid_up_to()]).unwrap())
+        }
+    }
+}
+
+impl Drop for CError {
+    fn drop(&mut self) {
+        unsafe { ffi::error_free(self.0.as_ptr()) }
+    }
+}
+
+/// QObject (JSON object)
+#[derive(Clone, Debug)]
+pub struct QObject;
+
+impl QemuPtrDefault for QObject {
+    type QemuType = *mut ffi::QObject;
+}
+
+impl FromQemuPtrFull<*mut ffi::QObject> for QObject {
+    #[inline]
+    unsafe fn from_qemu_full(_ffi: *mut ffi::QObject) -> Self {
+        unimplemented!()
+    }
+}
+
+impl FromQemuPtrNone<*const ffi::QObject> for QObject {
+    #[inline]
+    unsafe fn from_qemu_none(_ffi: *const ffi::QObject) -> Self {
+        unimplemented!()
+    }
+}
+
+impl<'a> ToQemuPtr<'a, *mut ffi::QObject> for QObject {
+    type Storage = ();
+
+    #[inline]
+    fn to_qemu_none(&self) -> Stash<'a, *mut ffi::QObject, QObject> {
+        unimplemented!()
+    }
+    #[inline]
+    fn to_qemu_full(&self) -> *mut ffi::QObject {
+        unimplemented!()
+    }
+}
+
+/// QNull (JSON null)
+#[derive(Clone, Debug)]
+pub struct QNull;
+
+impl QemuPtrDefault for QNull {
+    type QemuType = *mut ffi::QNull;
+}
+
+impl FromQemuPtrFull<*mut ffi::QObject> for QNull {
+    #[inline]
+    unsafe fn from_qemu_full(_ffi: *mut ffi::QObject) -> Self {
+        unimplemented!()
+    }
+}
+
+impl FromQemuPtrNone<*const ffi::QObject> for QNull {
+    #[inline]
+    unsafe fn from_qemu_none(_ffi: *const ffi::QObject) -> Self {
+        unimplemented!()
+    }
+}
+
+impl<'a> ToQemuPtr<'a, *mut ffi::QNull> for QNull {
+    type Storage = ();
+
+    #[inline]
+    fn to_qemu_none(&self) -> Stash<'a, *mut ffi::QNull, QNull> {
+        unimplemented!()
+    }
+    #[inline]
+    fn to_qemu_full(&self) -> *mut ffi::QNull {
+        unimplemented!()
+    }
+}
diff --git a/rust/common/src/qmp.rs b/rust/common/src/qmp.rs
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/rust/common/src/translate.rs b/rust/common/src/translate.rs
new file mode 100644
index 0000000000..315e14fa25
--- /dev/null
+++ b/rust/common/src/translate.rs
@@ -0,0 +1,482 @@
+// largely adapted from glib-rs
+// we don't depend on glib-rs as this brings a lot more code that we may not need
+// and also because there are issues with the conversion traits for our ffi::*mut.
+use libc::{c_char, size_t};
+use std::ffi::{CStr, CString};
+use std::ptr;
+
+use crate::ffi;
+
+/// A pointer.
+pub trait Ptr: Copy + 'static {
+    fn is_null(&self) -> bool;
+    fn from<X>(ptr: *mut X) -> Self;
+    fn to<X>(self) -> *mut X;
+}
+
+impl<T: 'static> Ptr for *const T {
+    #[inline]
+    fn is_null(&self) -> bool {
+        (*self).is_null()
+    }
+
+    #[inline]
+    fn from<X>(ptr: *mut X) -> *const T {
+        ptr as *const T
+    }
+
+    #[inline]
+    fn to<X>(self) -> *mut X {
+        self as *mut X
+    }
+}
+
+impl<T: 'static> Ptr for *mut T {
+    #[inline]
+    fn is_null(&self) -> bool {
+        (*self).is_null()
+    }
+
+    #[inline]
+    fn from<X>(ptr: *mut X) -> *mut T {
+        ptr as *mut T
+    }
+
+    #[inline]
+    fn to<X>(self) -> *mut X {
+        self as *mut X
+    }
+}
+
+/// Macro to declare a `NewPtr` struct.
+///
+/// A macro to declare a newtype for pointers, to workaround that *T are not
+/// defined in our binding crates, and allow foreign traits implementations.
+/// (this is used by qapi-gen bindings)
+#[allow(unused_macros)]
+#[macro_export]
+#[doc(hidden)]
+macro_rules! new_ptr {
+    () => {
+        #[derive(Copy, Clone)]
+        pub struct NewPtr<P: Ptr>(pub P);
+
+        impl<P: Ptr> Ptr for NewPtr<P> {
+            #[inline]
+            fn is_null(&self) -> bool {
+                self.0.is_null()
+            }
+
+            #[inline]
+            fn from<X>(ptr: *mut X) -> Self {
+                NewPtr(P::from(ptr))
+            }
+
+            #[inline]
+            fn to<X>(self) -> *mut X {
+                self.0.to()
+            }
+        }
+    };
+}
+
+/// Provides the default pointer type to be used in some container conversions.
+///
+/// It's `*mut c_char` for `String`, `*mut ffi::GuestInfo` for `GuestInfo`...
+pub trait QemuPtrDefault {
+    type QemuType: Ptr;
+}
+
+impl QemuPtrDefault for String {
+    type QemuType = *mut c_char;
+}
+
+/// A Stash contains the temporary storage and a pointer into it.
+///
+/// The pointer is valid for the lifetime of the `Stash`. As the lifetime of the
+/// `Stash` returned from `to_qemu_none` is at least the enclosing statement,
+/// you can avoid explicitly binding the stash in most cases and just take the
+/// pointer out of it:
+///
+/// ```ignore
+///     pub fn set_device_name(&self, name: &str) {
+///         unsafe {
+///             ffi::qemu_device_set_name(self.pointer, name.to_qemu_none().0)
+///         }
+///     }
+/// ```
+pub struct Stash<'a, P: Copy, T: ?Sized + ToQemuPtr<'a, P>>(
+    pub P,
+    pub <T as ToQemuPtr<'a, P>>::Storage,
+);
+
+/// Translate to a pointer.
+pub trait ToQemuPtr<'a, P: Copy> {
+    type Storage;
+
+    /// The pointer in the `Stash` is only valid for the lifetime of the `Stash`.
+    fn to_qemu_none(&'a self) -> Stash<'a, P, Self>;
+
+    /// Transfer the ownership to the ffi.
+    fn to_qemu_full(&self) -> P {
+        unimplemented!();
+    }
+}
+
+impl<'a, P: Ptr, T: ToQemuPtr<'a, P>> ToQemuPtr<'a, P> for Option<T> {
+    type Storage = Option<<T as ToQemuPtr<'a, P>>::Storage>;
+
+    #[inline]
+    fn to_qemu_none(&'a self) -> Stash<'a, P, Option<T>> {
+        self.as_ref()
+            .map_or(Stash(Ptr::from::<()>(ptr::null_mut()), None), |s| {
+                let s = s.to_qemu_none();
+                Stash(s.0, Some(s.1))
+            })
+    }
+
+    #[inline]
+    fn to_qemu_full(&self) -> P {
+        self.as_ref()
+            .map_or(Ptr::from::<()>(ptr::null_mut()), ToQemuPtr::to_qemu_full)
+    }
+}
+
+impl<'a, P: Ptr, T: ToQemuPtr<'a, P>> ToQemuPtr<'a, P> for Box<T> {
+    type Storage = <T as ToQemuPtr<'a, P>>::Storage;
+
+    #[inline]
+    fn to_qemu_none(&'a self) -> Stash<'a, P, Box<T>> {
+        let s = self.as_ref().to_qemu_none();
+        Stash(s.0, s.1)
+    }
+
+    #[inline]
+    fn to_qemu_full(&self) -> P {
+        ToQemuPtr::to_qemu_full(self.as_ref())
+    }
+}
+
+impl<'a> ToQemuPtr<'a, *mut c_char> for String {
+    type Storage = CString;
+
+    #[inline]
+    fn to_qemu_none(&self) -> Stash<'a, *mut c_char, String> {
+        let tmp = CString::new(&self[..])
+            .expect("String::ToQemuPtr<*mut c_char>: unexpected '\0' character");
+        Stash(tmp.as_ptr() as *mut c_char, tmp)
+    }
+
+    #[inline]
+    fn to_qemu_full(&self) -> *mut c_char {
+        unsafe { ffi::g_strndup(self.as_ptr() as *const c_char, self.len() as size_t) }
+    }
+}
+
+/// Translate from a pointer type, without taking ownership.
+pub trait FromQemuPtrNone<P: Ptr>: Sized {
+    /// # Safety
+    ///
+    /// `ptr` must be a valid pointer. It is not referenced after the call.
+    unsafe fn from_qemu_none(ptr: P) -> Self;
+}
+
+/// Translate from a pointer type, taking ownership.
+pub trait FromQemuPtrFull<P: Ptr>: Sized {
+    /// # Safety
+    ///
+    /// `ptr` must be a valid pointer. Ownership is transferred.
+    unsafe fn from_qemu_full(ptr: P) -> Self;
+}
+
+/// See [`FromQemuPtrNone`](trait.FromQemuPtrNone.html).
+#[inline]
+#[allow(clippy::missing_safety_doc)]
+pub unsafe fn from_qemu_none<P: Ptr, T: FromQemuPtrNone<P>>(ptr: P) -> T {
+    FromQemuPtrNone::from_qemu_none(ptr)
+}
+
+/// See [`FromQemuPtrFull`](trait.FromQemuPtrFull.html).
+#[inline]
+#[allow(clippy::missing_safety_doc)]
+pub unsafe fn from_qemu_full<P: Ptr, T: FromQemuPtrFull<P>>(ptr: P) -> T {
+    FromQemuPtrFull::from_qemu_full(ptr)
+}
+
+impl<P: Ptr, T: FromQemuPtrNone<P>> FromQemuPtrNone<P> for Option<T> {
+    #[inline]
+    unsafe fn from_qemu_none(ptr: P) -> Option<T> {
+        if ptr.is_null() {
+            None
+        } else {
+            Some(from_qemu_none(ptr))
+        }
+    }
+}
+
+impl<P: Ptr, T: FromQemuPtrFull<P>> FromQemuPtrFull<P> for Option<T> {
+    #[inline]
+    unsafe fn from_qemu_full(ptr: P) -> Option<T> {
+        if ptr.is_null() {
+            None
+        } else {
+            Some(from_qemu_full(ptr))
+        }
+    }
+}
+
+impl FromQemuPtrNone<*const c_char> for String {
+    #[inline]
+    unsafe fn from_qemu_none(ptr: *const c_char) -> Self {
+        assert!(!ptr.is_null());
+        String::from_utf8_lossy(CStr::from_ptr(ptr).to_bytes()).into_owned()
+    }
+}
+
+impl FromQemuPtrFull<*mut c_char> for String {
+    #[inline]
+    unsafe fn from_qemu_full(ptr: *mut c_char) -> Self {
+        let res = from_qemu_none(ptr as *const _);
+        ffi::g_free(ptr as *mut _);
+        res
+    }
+}
+
+#[doc(hidden)]
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! vec_ffi_wrapper {
+    ($ffi:ident) => {
+        #[allow(non_camel_case_types)]
+        pub struct $ffi(*mut qapi_ffi::$ffi);
+
+        impl Drop for $ffi {
+            fn drop(&mut self) {
+                let mut list = self.0;
+                unsafe {
+                    while !list.is_null() {
+                        let next = (*list).next;
+                        Box::from_raw(list);
+                        list = next;
+                    }
+                }
+            }
+        }
+
+        impl From<NewPtr<*mut qapi_ffi::$ffi>> for *mut qapi_ffi::$ffi {
+            fn from(p: NewPtr<*mut qapi_ffi::$ffi>) -> Self {
+                p.0
+            }
+        }
+    };
+}
+
+#[doc(hidden)]
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! impl_vec_scalars_to_qemu {
+    ($rs:ty, $ffi:ident) => {
+        impl<'a> ToQemuPtr<'a, NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
+            type Storage = $ffi;
+
+            #[inline]
+            fn to_qemu_none(&self) -> Stash<NewPtr<*mut qapi_ffi::$ffi>, Self> {
+                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
+                for value in self.iter().rev() {
+                    let b = Box::new(qapi_ffi::$ffi {
+                        next: list,
+                        value: *value,
+                    });
+                    list = Box::into_raw(b);
+                }
+                Stash(NewPtr(list), $ffi(list))
+            }
+
+            #[inline]
+            fn to_qemu_full(&self) -> NewPtr<*mut qapi_ffi::$ffi> {
+                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
+                unsafe {
+                    for value in self.iter().rev() {
+                        let l = ffi::g_malloc0(std::mem::size_of::<qapi_ffi::$ffi>())
+                            as *mut qapi_ffi::$ffi;
+                        (*l).next = list;
+                        (*l).value = *value;
+                        list = l;
+                    }
+                }
+                NewPtr(list)
+            }
+        }
+    };
+}
+
+#[doc(hidden)]
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! impl_vec_scalars_from_qemu {
+    ($rs:ty, $ffi:ident, $free_ffi:ident) => {
+        impl FromQemuPtrFull<NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
+            #[inline]
+            unsafe fn from_qemu_full(ffi: NewPtr<*mut qapi_ffi::$ffi>) -> Self {
+                let ret = from_qemu_none(NewPtr(ffi.0 as *const _));
+                qapi_ffi::$free_ffi(ffi.0);
+                ret
+            }
+        }
+
+        impl FromQemuPtrNone<NewPtr<*const qapi_ffi::$ffi>> for Vec<$rs> {
+            #[inline]
+            unsafe fn from_qemu_none(ffi: NewPtr<*const qapi_ffi::$ffi>) -> Self {
+                let mut ret = vec![];
+                let mut it = ffi.0;
+                while !it.is_null() {
+                    let e = &*it;
+                    ret.push(e.value);
+                    it = e.next;
+                }
+                ret
+            }
+        }
+    };
+}
+
+#[doc(hidden)]
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! impl_vec_to_qemu {
+    ($rs:ty, $ffi:ident) => {
+        // impl doesn't use only types from inside the current crate
+        // impl QemuPtrDefault for Vec<$rs> {
+        //     type QemuType = NewPtr<*mut qapi_ffi::$ffi>;
+        // }
+
+        impl<'a> ToQemuPtr<'a, NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
+            type Storage = ($ffi, Vec<Stash<'a, <$rs as QemuPtrDefault>::QemuType, $rs>>);
+
+            #[inline]
+            fn to_qemu_none(&self) -> Stash<NewPtr<*mut qapi_ffi::$ffi>, Self> {
+                let stash_vec: Vec<_> = self.iter().rev().map(ToQemuPtr::to_qemu_none).collect();
+                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
+                for stash in &stash_vec {
+                    let b = Box::new(qapi_ffi::$ffi {
+                        next: list,
+                        value: Ptr::to(stash.0),
+                    });
+                    list = Box::into_raw(b);
+                }
+                Stash(NewPtr(list), ($ffi(list), stash_vec))
+            }
+
+            #[inline]
+            fn to_qemu_full(&self) -> NewPtr<*mut qapi_ffi::$ffi> {
+                let v: Vec<_> = self.iter().rev().map(ToQemuPtr::to_qemu_full).collect();
+                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
+                unsafe {
+                    for val in v {
+                        let l = ffi::g_malloc0(std::mem::size_of::<qapi_ffi::$ffi>())
+                            as *mut qapi_ffi::$ffi;
+                        (*l).next = list;
+                        (*l).value = val;
+                        list = l;
+                    }
+                }
+                NewPtr(list)
+            }
+        }
+    };
+}
+
+#[doc(hidden)]
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! impl_vec_from_qemu {
+    ($rs:ty, $ffi:ident, $free_ffi:ident) => {
+        impl FromQemuPtrFull<NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
+            #[inline]
+            unsafe fn from_qemu_full(ffi: NewPtr<*mut qapi_ffi::$ffi>) -> Self {
+                let ret = from_qemu_none(NewPtr(ffi.0 as *const _));
+                qapi_ffi::$free_ffi(ffi.0);
+                ret
+            }
+        }
+
+        impl FromQemuPtrNone<NewPtr<*const qapi_ffi::$ffi>> for Vec<$rs> {
+            #[inline]
+            unsafe fn from_qemu_none(ffi: NewPtr<*const qapi_ffi::$ffi>) -> Self {
+                let mut ret = vec![];
+                let mut it = ffi.0;
+                while !it.is_null() {
+                    let e = &*it;
+                    ret.push(from_qemu_none(e.value as *const _));
+                    it = e.next;
+                }
+                ret
+            }
+        }
+    };
+}
+
+/// A macro to help the implementation of `Vec<T>` translations.
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! vec_type {
+    (Vec<$rs:ty>, $ffi:ident, $free_ffi:ident, 0) => {
+        vec_ffi_wrapper!($ffi);
+        impl_vec_from_qemu!($rs, $ffi, $free_ffi);
+        impl_vec_to_qemu!($rs, $ffi);
+    };
+    (Vec<$rs:ty>, $ffi:ident, $free_ffi:ident, 1) => {
+        vec_ffi_wrapper!($ffi);
+        impl_vec_scalars_from_qemu!($rs, $ffi, $free_ffi);
+        impl_vec_scalars_to_qemu!($rs, $ffi);
+    };
+}
+
+/// A macro to implement [`ToQemuPtr`] as boxed scalars
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! impl_to_qemu_scalar_boxed {
+    ($ty:ty) => {
+        impl<'a> ToQemuPtr<'a, *mut $ty> for $ty {
+            type Storage = Box<$ty>;
+
+            fn to_qemu_none(&'a self) -> Stash<'a, *mut $ty, Self> {
+                let mut box_ = Box::new(*self);
+                Stash(&mut *box_, box_)
+            }
+
+            fn to_qemu_full(&self) -> *mut $ty {
+                unsafe {
+                    let ptr = ffi::g_malloc0(std::mem::size_of::<$ty>()) as *mut _;
+                    *ptr = *self;
+                    ptr
+                }
+            }
+        }
+    };
+}
+
+/// A macro to implement [`FromQemuPtrNone`] for scalar pointers.
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! impl_from_qemu_none_scalar {
+    ($ty:ty) => {
+        impl FromQemuPtrNone<*const $ty> for $ty {
+            unsafe fn from_qemu_none(ptr: *const $ty) -> Self {
+                *ptr
+            }
+        }
+    };
+}
+
+macro_rules! impl_scalar_boxed {
+    ($($t:ident)*) => (
+        $(
+            impl_to_qemu_scalar_boxed!($t);
+            impl_from_qemu_none_scalar!($t);
+        )*
+    )
+}
+
+// the only built-in used so far, feel free to add more as needed
+impl_scalar_boxed!(bool i64 f64);
-- 
2.33.0.113.g6c40894d24

Re: [RFC v3 12/32] rust: provide a common crate for QEMU

Posted by Alistair Francis 3 years, 12 months ago

On Tue, Sep 7, 2021 at 10:41 PM <marcandre.lureau@redhat.com> wrote:
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> This crates provides common bindings and facilities for QEMU C API
> shared by various projects.
>
> Most importantly, it defines the conversion traits used to convert from
> C to Rust types. Those traits are largely adapted from glib-rs, since
> those have proved to be very flexible, and should guide us to bind
> further QEMU types such as QOM. If glib-rs becomes a dependency, we
> should consider adopting glib translate traits. For QAPI, we need a
> smaller subset.
>
> Cargo.lock is checked-in, as QEMU produces end-of-chain binaries, and it
> is desirable to track the exact set of packages that are involved in
> managed builds.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  Cargo.lock                   |  63 +++++
>  Cargo.toml                   |   4 +-
>  rust/common/Cargo.toml       |  11 +
>  rust/common/src/error.rs     | 113 ++++++++
>  rust/common/src/ffi.rs       |  93 +++++++
>  rust/common/src/lib.rs       |  21 ++
>  rust/common/src/qemu.rs      | 101 ++++++++
>  rust/common/src/qmp.rs       |   0
>  rust/common/src/translate.rs | 482 +++++++++++++++++++++++++++++++++++
>  9 files changed, 887 insertions(+), 1 deletion(-)
>  create mode 100644 Cargo.lock
>  create mode 100644 rust/common/Cargo.toml
>  create mode 100644 rust/common/src/error.rs
>  create mode 100644 rust/common/src/ffi.rs
>  create mode 100644 rust/common/src/lib.rs
>  create mode 100644 rust/common/src/qemu.rs
>  create mode 100644 rust/common/src/qmp.rs
>  create mode 100644 rust/common/src/translate.rs
>
> diff --git a/Cargo.lock b/Cargo.lock
> new file mode 100644
> index 0000000000..8dc2dd9da7
> --- /dev/null
> +++ b/Cargo.lock
> @@ -0,0 +1,63 @@
> +# This file is automatically @generated by Cargo.
> +# It is not intended for manual editing.
> +version = 3
> +
> +[[package]]
> +name = "autocfg"
> +version = "1.0.1"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a"
> +
> +[[package]]
> +name = "bitflags"
> +version = "1.2.1"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693"
> +
> +[[package]]
> +name = "cc"
> +version = "1.0.70"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "d26a6ce4b6a484fa3edb70f7efa6fc430fd2b87285fe8b84304fd0936faa0dc0"
> +
> +[[package]]
> +name = "cfg-if"
> +version = "1.0.0"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
> +
> +[[package]]
> +name = "common"
> +version = "0.1.0"
> +dependencies = [
> + "libc",
> + "nix",
> +]
> +
> +[[package]]
> +name = "libc"
> +version = "0.2.101"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "3cb00336871be5ed2c8ed44b60ae9959dc5b9f08539422ed43f09e34ecaeba21"
> +
> +[[package]]
> +name = "memoffset"
> +version = "0.6.4"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "59accc507f1338036a0477ef61afdae33cde60840f4dfe481319ce3ad116ddf9"
> +dependencies = [
> + "autocfg",
> +]
> +
> +[[package]]
> +name = "nix"
> +version = "0.20.1"
> +source = "registry+https://github.com/rust-lang/crates.io-index"
> +checksum = "df8e5e343312e7fbeb2a52139114e9e702991ef9c2aea6817ff2440b35647d56"
> +dependencies = [
> + "bitflags",
> + "cc",
> + "cfg-if",
> + "libc",
> + "memoffset",
> +]
> diff --git a/Cargo.toml b/Cargo.toml
> index c4b464ff15..14131eed3c 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -1,2 +1,4 @@
>  [workspace]
> -members = []
> +members = [
> +  "rust/common",
> +]
> diff --git a/rust/common/Cargo.toml b/rust/common/Cargo.toml
> new file mode 100644
> index 0000000000..6c240447f3
> --- /dev/null
> +++ b/rust/common/Cargo.toml
> @@ -0,0 +1,11 @@
> +[package]
> +name = "common"
> +version = "0.1.0"
> +edition = "2018"
> +publish = false
> +
> +[dependencies]
> +libc = "0.2.92"
> +
> +[target."cfg(unix)".dependencies]
> +nix = "0.20.0"
> diff --git a/rust/common/src/error.rs b/rust/common/src/error.rs
> new file mode 100644
> index 0000000000..f166ac42ea
> --- /dev/null
> +++ b/rust/common/src/error.rs
> @@ -0,0 +1,113 @@
> +use std::{self, ffi::CString, fmt, io, ptr};
> +
> +use crate::translate::*;

It's not uncommon to ban wildcard imports that aren't preludes as it
can make it confusing to read. Does QEMU have a stance on that type of
thing?

> +use crate::{ffi, qemu};
> +
> +/// Common error type for QEMU and related projects.
> +#[derive(Debug)]
> +pub enum Error {
> +    /// A generic error with file and line location.
> +    FailedAt(String, &'static str, u32),
> +    /// An IO error.
> +    Io(io::Error),
> +    #[cfg(unix)]
> +    /// A nix error.
> +    Nix(nix::Error),
> +}
> +
> +/// Alias for a `Result` with the error type for QEMU.
> +pub type Result<T> = std::result::Result<T, Error>;

I think this is very confusing. Rust developers expect `Result` to be
the one from `std::result`, it would be better to call this
`QEMUResult`

> +
> +impl Error {
> +    fn message(&self) -> String {
> +        use Error::*;

Do we need this here? Why not put it at the top of the file?

> +        match self {
> +            FailedAt(msg, _, _) => msg.into(),
> +            Io(io) => format!("IO error: {}", io),
> +            #[cfg(unix)]
> +            Nix(nix) => format!("Nix error: {}", nix),
> +        }
> +    }
> +
> +    fn location(&self) -> Option<(&'static str, u32)> {
> +        use Error::*;
> +        match self {
> +            FailedAt(_, file, line) => Some((file, *line)),
> +            Io(_) => None,
> +            #[cfg(unix)]
> +            Nix(_) => None,
> +        }
> +    }
> +}
> +
> +impl fmt::Display for Error {
> +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> +        use Error::*;
> +        match self {
> +            FailedAt(msg, file, line) => write!(f, "{} ({}:{})", msg, file, line),
> +            _ => write!(f, "{}", self.message()),
> +        }
> +    }
> +}
> +
> +impl From<io::Error> for Error {
> +    fn from(val: io::Error) -> Self {
> +        Error::Io(val)
> +    }
> +}
> +
> +#[cfg(unix)]
> +impl From<nix::Error> for Error {
> +    fn from(val: nix::Error) -> Self {
> +        Error::Nix(val)
> +    }
> +}
> +
> +impl QemuPtrDefault for Error {
> +    type QemuType = *mut ffi::Error;
> +}
> +
> +impl<'a> ToQemuPtr<'a, *mut ffi::Error> for Error {
> +    type Storage = qemu::CError;
> +
> +    fn to_qemu_none(&'a self) -> Stash<'a, *mut ffi::Error, Self> {
> +        let err = self.to_qemu_full();
> +
> +        Stash(err, unsafe { from_qemu_full(err) })
> +    }
> +
> +    fn to_qemu_full(&self) -> *mut ffi::Error {
> +        let cmsg =
> +            CString::new(self.message()).expect("ToQemuPtr<Error>: unexpected '\0' character");
> +        let mut csrc = CString::new("").unwrap();
> +        let (src, line) = self.location().map_or((ptr::null(), 0_i32), |loc| {
> +            csrc = CString::new(loc.0).expect("ToQemuPtr<Error>:: unexpected '\0' character");
> +            (csrc.as_ptr() as *const libc::c_char, loc.1 as i32)
> +        });
> +        let func = ptr::null();
> +
> +        let mut err: *mut ffi::Error = ptr::null_mut();
> +        unsafe {
> +            ffi::error_setg_internal(
> +                &mut err as *mut *mut _,
> +                src,
> +                line,
> +                func,
> +                cmsg.as_ptr() as *const libc::c_char,
> +            );
> +            err
> +        }
> +    }
> +}
> +
> +/// Convenience macro to build a [`Error::FailedAt`] error.
> +///
> +/// Returns a `Result::Err` with the file:line location.
> +/// (the error can then be converted to a QEMU `ffi::Error`)
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! err {
> +    ($msg:expr) => {
> +        Err(Error::FailedAt($msg.into(), file!(), line!()))
> +    };
> +}
> diff --git a/rust/common/src/ffi.rs b/rust/common/src/ffi.rs
> new file mode 100644
> index 0000000000..82818d503a
> --- /dev/null
> +++ b/rust/common/src/ffi.rs
> @@ -0,0 +1,93 @@
> +//! Bindings to the raw low-level C API commonly provided by QEMU projects.
> +//!
> +//! Manual bindings to C API availabe when linking QEMU projects.

s/availabe/available/g

> +//! It includes minimal glib allocation functions too, since it's the default
> +//! allocator used by QEMU, and we don't depend on glib-rs crate yet).
> +//!
> +//! Higher-level Rust-friendly bindings are provided by different modules.
> +
> +use libc::{c_char, c_void, size_t};
> +
> +extern "C" {
> +    pub fn g_malloc0(n_bytes: size_t) -> *mut c_void;
> +    pub fn g_free(ptr: *mut c_void);
> +    pub fn g_strndup(str: *const c_char, n: size_t) -> *mut c_char;
> +}

We can get there from the glib/glib_sys crate:
https://gtk-rs.org/gtk-rs-core/stable/latest/docs/glib_sys/fn.g_malloc0.html

If we only plan on using these 3 I think this approach is fine, but
something to keep in mind if we use more glib functions.

> +
> +#[repr(C)]
> +pub struct QObject(c_void);
> +
> +impl ::std::fmt::Debug for QObject {
> +    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
> +        f.debug_struct(&format!("QObject @ {:?}", self as *const _))
> +            .finish()
> +    }
> +}
> +
> +#[repr(C)]
> +pub struct QNull(c_void);
> +
> +impl ::std::fmt::Debug for QNull {
> +    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
> +        f.debug_struct(&format!("QNull @ {:?}", self as *const _))
> +            .finish()
> +    }
> +}
> +
> +#[repr(C)]
> +pub struct Error(c_void);
> +
> +impl ::std::fmt::Debug for Error {
> +    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
> +        f.debug_struct(&format!("Error @ {:?}", self as *const _))
> +            .finish()
> +    }
> +}
> +
> +extern "C" {
> +    pub fn error_setg_internal(
> +        errp: *mut *mut Error,
> +        src: *const c_char,
> +        line: i32,
> +        func: *const c_char,
> +        fmt: *const c_char,
> +        ...
> +    );
> +    pub fn error_get_pretty(err: *const Error) -> *const c_char;
> +    pub fn error_free(err: *mut Error);
> +}
> +
> +/// Wrap a QMP hanlder.

handler

> +#[macro_export]
> +macro_rules! qmp {
> +    // the basic return value variant
> +    ($e:expr, $errp:ident, $errval:expr) => {{
> +        assert!(!$errp.is_null());
> +        unsafe {
> +            *$errp = std::ptr::null_mut();
> +        }
> +
> +        match $e {
> +            Ok(val) => val,
> +            Err(err) => unsafe {
> +                *$errp = err.to_qemu_full();
> +                $errval
> +            },
> +        }
> +    }};
> +    // the ptr return value variant
> +    ($e:expr, $errp:ident) => {{
> +        assert!(!$errp.is_null());
> +        unsafe {
> +            *$errp = std::ptr::null_mut();
> +        }
> +
> +        match $e {
> +            Ok(val) => val.to_qemu_full().into(),
> +            Err(err) => unsafe {
> +                *$errp = err.to_qemu_full();
> +                std::ptr::null_mut()
> +            },
> +        }
> +    }};
> +}

It would be a good idea to document why this code is safe

Alistair

> diff --git a/rust/common/src/lib.rs b/rust/common/src/lib.rs
> new file mode 100644
> index 0000000000..4de826bc2e
> --- /dev/null
> +++ b/rust/common/src/lib.rs
> @@ -0,0 +1,21 @@
> +//! Common code for QEMU
> +//!
> +//! This crates provides common bindings and facilities for QEMU C API shared by
> +//! various projects. Most importantly, it defines the conversion traits used to
> +//! convert from C to Rust types. Those traits are largely adapted from glib-rs,
> +//! since those have prooven to be very flexible, and should guide us to bind
> +//! further QEMU types such as QOM. If glib-rs becomes a dependency, we should
> +//! consider adopting glib translate traits. For QAPI, we need a smaller subset.
> +
> +pub use libc;
> +
> +mod error;
> +pub use error::*;
> +
> +mod qemu;
> +pub use qemu::*;
> +
> +mod translate;
> +pub use translate::*;
> +
> +pub mod ffi;
> diff --git a/rust/common/src/qemu.rs b/rust/common/src/qemu.rs
> new file mode 100644
> index 0000000000..dd01c6d92d
> --- /dev/null
> +++ b/rust/common/src/qemu.rs
> @@ -0,0 +1,101 @@
> +use std::{ffi::CStr, ptr, str};
> +
> +use crate::{ffi, translate};
> +use translate::{FromQemuPtrFull, FromQemuPtrNone, QemuPtrDefault, Stash, ToQemuPtr};
> +
> +/// A type representing an owned C QEMU Error.
> +pub struct CError(ptr::NonNull<ffi::Error>);
> +
> +impl translate::FromQemuPtrFull<*mut ffi::Error> for CError {
> +    unsafe fn from_qemu_full(ptr: *mut ffi::Error) -> Self {
> +        assert!(!ptr.is_null());
> +        Self(ptr::NonNull::new_unchecked(ptr))
> +    }
> +}
> +
> +impl CError {
> +    pub fn pretty(&self) -> &str {
> +        unsafe {
> +            let pretty = ffi::error_get_pretty(self.0.as_ptr());
> +            let bytes = CStr::from_ptr(pretty).to_bytes();
> +            str::from_utf8(bytes)
> +                .unwrap_or_else(|err| str::from_utf8(&bytes[..err.valid_up_to()]).unwrap())
> +        }
> +    }
> +}
> +
> +impl Drop for CError {
> +    fn drop(&mut self) {
> +        unsafe { ffi::error_free(self.0.as_ptr()) }
> +    }
> +}
> +
> +/// QObject (JSON object)
> +#[derive(Clone, Debug)]
> +pub struct QObject;
> +
> +impl QemuPtrDefault for QObject {
> +    type QemuType = *mut ffi::QObject;
> +}
> +
> +impl FromQemuPtrFull<*mut ffi::QObject> for QObject {
> +    #[inline]
> +    unsafe fn from_qemu_full(_ffi: *mut ffi::QObject) -> Self {
> +        unimplemented!()
> +    }
> +}
> +
> +impl FromQemuPtrNone<*const ffi::QObject> for QObject {
> +    #[inline]
> +    unsafe fn from_qemu_none(_ffi: *const ffi::QObject) -> Self {
> +        unimplemented!()
> +    }
> +}
> +
> +impl<'a> ToQemuPtr<'a, *mut ffi::QObject> for QObject {
> +    type Storage = ();
> +
> +    #[inline]
> +    fn to_qemu_none(&self) -> Stash<'a, *mut ffi::QObject, QObject> {
> +        unimplemented!()
> +    }
> +    #[inline]
> +    fn to_qemu_full(&self) -> *mut ffi::QObject {
> +        unimplemented!()
> +    }
> +}
> +
> +/// QNull (JSON null)
> +#[derive(Clone, Debug)]
> +pub struct QNull;
> +
> +impl QemuPtrDefault for QNull {
> +    type QemuType = *mut ffi::QNull;
> +}
> +
> +impl FromQemuPtrFull<*mut ffi::QObject> for QNull {
> +    #[inline]
> +    unsafe fn from_qemu_full(_ffi: *mut ffi::QObject) -> Self {
> +        unimplemented!()
> +    }
> +}
> +
> +impl FromQemuPtrNone<*const ffi::QObject> for QNull {
> +    #[inline]
> +    unsafe fn from_qemu_none(_ffi: *const ffi::QObject) -> Self {
> +        unimplemented!()
> +    }
> +}
> +
> +impl<'a> ToQemuPtr<'a, *mut ffi::QNull> for QNull {
> +    type Storage = ();
> +
> +    #[inline]
> +    fn to_qemu_none(&self) -> Stash<'a, *mut ffi::QNull, QNull> {
> +        unimplemented!()
> +    }
> +    #[inline]
> +    fn to_qemu_full(&self) -> *mut ffi::QNull {
> +        unimplemented!()
> +    }
> +}
> diff --git a/rust/common/src/qmp.rs b/rust/common/src/qmp.rs
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/rust/common/src/translate.rs b/rust/common/src/translate.rs
> new file mode 100644
> index 0000000000..315e14fa25
> --- /dev/null
> +++ b/rust/common/src/translate.rs
> @@ -0,0 +1,482 @@
> +// largely adapted from glib-rs
> +// we don't depend on glib-rs as this brings a lot more code that we may not need
> +// and also because there are issues with the conversion traits for our ffi::*mut.
> +use libc::{c_char, size_t};
> +use std::ffi::{CStr, CString};
> +use std::ptr;
> +
> +use crate::ffi;
> +
> +/// A pointer.
> +pub trait Ptr: Copy + 'static {
> +    fn is_null(&self) -> bool;
> +    fn from<X>(ptr: *mut X) -> Self;
> +    fn to<X>(self) -> *mut X;
> +}
> +
> +impl<T: 'static> Ptr for *const T {
> +    #[inline]
> +    fn is_null(&self) -> bool {
> +        (*self).is_null()
> +    }
> +
> +    #[inline]
> +    fn from<X>(ptr: *mut X) -> *const T {
> +        ptr as *const T
> +    }
> +
> +    #[inline]
> +    fn to<X>(self) -> *mut X {
> +        self as *mut X
> +    }
> +}
> +
> +impl<T: 'static> Ptr for *mut T {
> +    #[inline]
> +    fn is_null(&self) -> bool {
> +        (*self).is_null()
> +    }
> +
> +    #[inline]
> +    fn from<X>(ptr: *mut X) -> *mut T {
> +        ptr as *mut T
> +    }
> +
> +    #[inline]
> +    fn to<X>(self) -> *mut X {
> +        self as *mut X
> +    }
> +}
> +
> +/// Macro to declare a `NewPtr` struct.
> +///
> +/// A macro to declare a newtype for pointers, to workaround that *T are not
> +/// defined in our binding crates, and allow foreign traits implementations.
> +/// (this is used by qapi-gen bindings)
> +#[allow(unused_macros)]
> +#[macro_export]
> +#[doc(hidden)]
> +macro_rules! new_ptr {
> +    () => {
> +        #[derive(Copy, Clone)]
> +        pub struct NewPtr<P: Ptr>(pub P);
> +
> +        impl<P: Ptr> Ptr for NewPtr<P> {
> +            #[inline]
> +            fn is_null(&self) -> bool {
> +                self.0.is_null()
> +            }
> +
> +            #[inline]
> +            fn from<X>(ptr: *mut X) -> Self {
> +                NewPtr(P::from(ptr))
> +            }
> +
> +            #[inline]
> +            fn to<X>(self) -> *mut X {
> +                self.0.to()
> +            }
> +        }
> +    };
> +}
> +
> +/// Provides the default pointer type to be used in some container conversions.
> +///
> +/// It's `*mut c_char` for `String`, `*mut ffi::GuestInfo` for `GuestInfo`...
> +pub trait QemuPtrDefault {
> +    type QemuType: Ptr;
> +}
> +
> +impl QemuPtrDefault for String {
> +    type QemuType = *mut c_char;
> +}
> +
> +/// A Stash contains the temporary storage and a pointer into it.
> +///
> +/// The pointer is valid for the lifetime of the `Stash`. As the lifetime of the
> +/// `Stash` returned from `to_qemu_none` is at least the enclosing statement,
> +/// you can avoid explicitly binding the stash in most cases and just take the
> +/// pointer out of it:
> +///
> +/// ```ignore
> +///     pub fn set_device_name(&self, name: &str) {
> +///         unsafe {
> +///             ffi::qemu_device_set_name(self.pointer, name.to_qemu_none().0)
> +///         }
> +///     }
> +/// ```
> +pub struct Stash<'a, P: Copy, T: ?Sized + ToQemuPtr<'a, P>>(
> +    pub P,
> +    pub <T as ToQemuPtr<'a, P>>::Storage,
> +);
> +
> +/// Translate to a pointer.
> +pub trait ToQemuPtr<'a, P: Copy> {
> +    type Storage;
> +
> +    /// The pointer in the `Stash` is only valid for the lifetime of the `Stash`.
> +    fn to_qemu_none(&'a self) -> Stash<'a, P, Self>;
> +
> +    /// Transfer the ownership to the ffi.
> +    fn to_qemu_full(&self) -> P {
> +        unimplemented!();
> +    }
> +}
> +
> +impl<'a, P: Ptr, T: ToQemuPtr<'a, P>> ToQemuPtr<'a, P> for Option<T> {
> +    type Storage = Option<<T as ToQemuPtr<'a, P>>::Storage>;
> +
> +    #[inline]
> +    fn to_qemu_none(&'a self) -> Stash<'a, P, Option<T>> {
> +        self.as_ref()
> +            .map_or(Stash(Ptr::from::<()>(ptr::null_mut()), None), |s| {
> +                let s = s.to_qemu_none();
> +                Stash(s.0, Some(s.1))
> +            })
> +    }
> +
> +    #[inline]
> +    fn to_qemu_full(&self) -> P {
> +        self.as_ref()
> +            .map_or(Ptr::from::<()>(ptr::null_mut()), ToQemuPtr::to_qemu_full)
> +    }
> +}
> +
> +impl<'a, P: Ptr, T: ToQemuPtr<'a, P>> ToQemuPtr<'a, P> for Box<T> {
> +    type Storage = <T as ToQemuPtr<'a, P>>::Storage;
> +
> +    #[inline]
> +    fn to_qemu_none(&'a self) -> Stash<'a, P, Box<T>> {
> +        let s = self.as_ref().to_qemu_none();
> +        Stash(s.0, s.1)
> +    }
> +
> +    #[inline]
> +    fn to_qemu_full(&self) -> P {
> +        ToQemuPtr::to_qemu_full(self.as_ref())
> +    }
> +}
> +
> +impl<'a> ToQemuPtr<'a, *mut c_char> for String {
> +    type Storage = CString;
> +
> +    #[inline]
> +    fn to_qemu_none(&self) -> Stash<'a, *mut c_char, String> {
> +        let tmp = CString::new(&self[..])
> +            .expect("String::ToQemuPtr<*mut c_char>: unexpected '\0' character");
> +        Stash(tmp.as_ptr() as *mut c_char, tmp)
> +    }
> +
> +    #[inline]
> +    fn to_qemu_full(&self) -> *mut c_char {
> +        unsafe { ffi::g_strndup(self.as_ptr() as *const c_char, self.len() as size_t) }
> +    }
> +}
> +
> +/// Translate from a pointer type, without taking ownership.
> +pub trait FromQemuPtrNone<P: Ptr>: Sized {
> +    /// # Safety
> +    ///
> +    /// `ptr` must be a valid pointer. It is not referenced after the call.
> +    unsafe fn from_qemu_none(ptr: P) -> Self;
> +}
> +
> +/// Translate from a pointer type, taking ownership.
> +pub trait FromQemuPtrFull<P: Ptr>: Sized {
> +    /// # Safety
> +    ///
> +    /// `ptr` must be a valid pointer. Ownership is transferred.
> +    unsafe fn from_qemu_full(ptr: P) -> Self;
> +}
> +
> +/// See [`FromQemuPtrNone`](trait.FromQemuPtrNone.html).
> +#[inline]
> +#[allow(clippy::missing_safety_doc)]
> +pub unsafe fn from_qemu_none<P: Ptr, T: FromQemuPtrNone<P>>(ptr: P) -> T {
> +    FromQemuPtrNone::from_qemu_none(ptr)
> +}
> +
> +/// See [`FromQemuPtrFull`](trait.FromQemuPtrFull.html).
> +#[inline]
> +#[allow(clippy::missing_safety_doc)]
> +pub unsafe fn from_qemu_full<P: Ptr, T: FromQemuPtrFull<P>>(ptr: P) -> T {
> +    FromQemuPtrFull::from_qemu_full(ptr)
> +}
> +
> +impl<P: Ptr, T: FromQemuPtrNone<P>> FromQemuPtrNone<P> for Option<T> {
> +    #[inline]
> +    unsafe fn from_qemu_none(ptr: P) -> Option<T> {
> +        if ptr.is_null() {
> +            None
> +        } else {
> +            Some(from_qemu_none(ptr))
> +        }
> +    }
> +}
> +
> +impl<P: Ptr, T: FromQemuPtrFull<P>> FromQemuPtrFull<P> for Option<T> {
> +    #[inline]
> +    unsafe fn from_qemu_full(ptr: P) -> Option<T> {
> +        if ptr.is_null() {
> +            None
> +        } else {
> +            Some(from_qemu_full(ptr))
> +        }
> +    }
> +}
> +
> +impl FromQemuPtrNone<*const c_char> for String {
> +    #[inline]
> +    unsafe fn from_qemu_none(ptr: *const c_char) -> Self {
> +        assert!(!ptr.is_null());
> +        String::from_utf8_lossy(CStr::from_ptr(ptr).to_bytes()).into_owned()
> +    }
> +}
> +
> +impl FromQemuPtrFull<*mut c_char> for String {
> +    #[inline]
> +    unsafe fn from_qemu_full(ptr: *mut c_char) -> Self {
> +        let res = from_qemu_none(ptr as *const _);
> +        ffi::g_free(ptr as *mut _);
> +        res
> +    }
> +}
> +
> +#[doc(hidden)]
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! vec_ffi_wrapper {
> +    ($ffi:ident) => {
> +        #[allow(non_camel_case_types)]
> +        pub struct $ffi(*mut qapi_ffi::$ffi);
> +
> +        impl Drop for $ffi {
> +            fn drop(&mut self) {
> +                let mut list = self.0;
> +                unsafe {
> +                    while !list.is_null() {
> +                        let next = (*list).next;
> +                        Box::from_raw(list);
> +                        list = next;
> +                    }
> +                }
> +            }
> +        }
> +
> +        impl From<NewPtr<*mut qapi_ffi::$ffi>> for *mut qapi_ffi::$ffi {
> +            fn from(p: NewPtr<*mut qapi_ffi::$ffi>) -> Self {
> +                p.0
> +            }
> +        }
> +    };
> +}
> +
> +#[doc(hidden)]
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! impl_vec_scalars_to_qemu {
> +    ($rs:ty, $ffi:ident) => {
> +        impl<'a> ToQemuPtr<'a, NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
> +            type Storage = $ffi;
> +
> +            #[inline]
> +            fn to_qemu_none(&self) -> Stash<NewPtr<*mut qapi_ffi::$ffi>, Self> {
> +                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
> +                for value in self.iter().rev() {
> +                    let b = Box::new(qapi_ffi::$ffi {
> +                        next: list,
> +                        value: *value,
> +                    });
> +                    list = Box::into_raw(b);
> +                }
> +                Stash(NewPtr(list), $ffi(list))
> +            }
> +
> +            #[inline]
> +            fn to_qemu_full(&self) -> NewPtr<*mut qapi_ffi::$ffi> {
> +                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
> +                unsafe {
> +                    for value in self.iter().rev() {
> +                        let l = ffi::g_malloc0(std::mem::size_of::<qapi_ffi::$ffi>())
> +                            as *mut qapi_ffi::$ffi;
> +                        (*l).next = list;
> +                        (*l).value = *value;
> +                        list = l;
> +                    }
> +                }
> +                NewPtr(list)
> +            }
> +        }
> +    };
> +}
> +
> +#[doc(hidden)]
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! impl_vec_scalars_from_qemu {
> +    ($rs:ty, $ffi:ident, $free_ffi:ident) => {
> +        impl FromQemuPtrFull<NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
> +            #[inline]
> +            unsafe fn from_qemu_full(ffi: NewPtr<*mut qapi_ffi::$ffi>) -> Self {
> +                let ret = from_qemu_none(NewPtr(ffi.0 as *const _));
> +                qapi_ffi::$free_ffi(ffi.0);
> +                ret
> +            }
> +        }
> +
> +        impl FromQemuPtrNone<NewPtr<*const qapi_ffi::$ffi>> for Vec<$rs> {
> +            #[inline]
> +            unsafe fn from_qemu_none(ffi: NewPtr<*const qapi_ffi::$ffi>) -> Self {
> +                let mut ret = vec![];
> +                let mut it = ffi.0;
> +                while !it.is_null() {
> +                    let e = &*it;
> +                    ret.push(e.value);
> +                    it = e.next;
> +                }
> +                ret
> +            }
> +        }
> +    };
> +}
> +
> +#[doc(hidden)]
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! impl_vec_to_qemu {
> +    ($rs:ty, $ffi:ident) => {
> +        // impl doesn't use only types from inside the current crate
> +        // impl QemuPtrDefault for Vec<$rs> {
> +        //     type QemuType = NewPtr<*mut qapi_ffi::$ffi>;
> +        // }
> +
> +        impl<'a> ToQemuPtr<'a, NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
> +            type Storage = ($ffi, Vec<Stash<'a, <$rs as QemuPtrDefault>::QemuType, $rs>>);
> +
> +            #[inline]
> +            fn to_qemu_none(&self) -> Stash<NewPtr<*mut qapi_ffi::$ffi>, Self> {
> +                let stash_vec: Vec<_> = self.iter().rev().map(ToQemuPtr::to_qemu_none).collect();
> +                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
> +                for stash in &stash_vec {
> +                    let b = Box::new(qapi_ffi::$ffi {
> +                        next: list,
> +                        value: Ptr::to(stash.0),
> +                    });
> +                    list = Box::into_raw(b);
> +                }
> +                Stash(NewPtr(list), ($ffi(list), stash_vec))
> +            }
> +
> +            #[inline]
> +            fn to_qemu_full(&self) -> NewPtr<*mut qapi_ffi::$ffi> {
> +                let v: Vec<_> = self.iter().rev().map(ToQemuPtr::to_qemu_full).collect();
> +                let mut list: *mut qapi_ffi::$ffi = std::ptr::null_mut();
> +                unsafe {
> +                    for val in v {
> +                        let l = ffi::g_malloc0(std::mem::size_of::<qapi_ffi::$ffi>())
> +                            as *mut qapi_ffi::$ffi;
> +                        (*l).next = list;
> +                        (*l).value = val;
> +                        list = l;
> +                    }
> +                }
> +                NewPtr(list)
> +            }
> +        }
> +    };
> +}
> +
> +#[doc(hidden)]
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! impl_vec_from_qemu {
> +    ($rs:ty, $ffi:ident, $free_ffi:ident) => {
> +        impl FromQemuPtrFull<NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
> +            #[inline]
> +            unsafe fn from_qemu_full(ffi: NewPtr<*mut qapi_ffi::$ffi>) -> Self {
> +                let ret = from_qemu_none(NewPtr(ffi.0 as *const _));
> +                qapi_ffi::$free_ffi(ffi.0);
> +                ret
> +            }
> +        }
> +
> +        impl FromQemuPtrNone<NewPtr<*const qapi_ffi::$ffi>> for Vec<$rs> {
> +            #[inline]
> +            unsafe fn from_qemu_none(ffi: NewPtr<*const qapi_ffi::$ffi>) -> Self {
> +                let mut ret = vec![];
> +                let mut it = ffi.0;
> +                while !it.is_null() {
> +                    let e = &*it;
> +                    ret.push(from_qemu_none(e.value as *const _));
> +                    it = e.next;
> +                }
> +                ret
> +            }
> +        }
> +    };
> +}
> +
> +/// A macro to help the implementation of `Vec<T>` translations.
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! vec_type {
> +    (Vec<$rs:ty>, $ffi:ident, $free_ffi:ident, 0) => {
> +        vec_ffi_wrapper!($ffi);
> +        impl_vec_from_qemu!($rs, $ffi, $free_ffi);
> +        impl_vec_to_qemu!($rs, $ffi);
> +    };
> +    (Vec<$rs:ty>, $ffi:ident, $free_ffi:ident, 1) => {
> +        vec_ffi_wrapper!($ffi);
> +        impl_vec_scalars_from_qemu!($rs, $ffi, $free_ffi);
> +        impl_vec_scalars_to_qemu!($rs, $ffi);
> +    };
> +}
> +
> +/// A macro to implement [`ToQemuPtr`] as boxed scalars
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! impl_to_qemu_scalar_boxed {
> +    ($ty:ty) => {
> +        impl<'a> ToQemuPtr<'a, *mut $ty> for $ty {
> +            type Storage = Box<$ty>;
> +
> +            fn to_qemu_none(&'a self) -> Stash<'a, *mut $ty, Self> {
> +                let mut box_ = Box::new(*self);
> +                Stash(&mut *box_, box_)
> +            }
> +
> +            fn to_qemu_full(&self) -> *mut $ty {
> +                unsafe {
> +                    let ptr = ffi::g_malloc0(std::mem::size_of::<$ty>()) as *mut _;
> +                    *ptr = *self;
> +                    ptr
> +                }
> +            }
> +        }
> +    };
> +}
> +
> +/// A macro to implement [`FromQemuPtrNone`] for scalar pointers.
> +#[allow(unused_macros)]
> +#[macro_export]
> +macro_rules! impl_from_qemu_none_scalar {
> +    ($ty:ty) => {
> +        impl FromQemuPtrNone<*const $ty> for $ty {
> +            unsafe fn from_qemu_none(ptr: *const $ty) -> Self {
> +                *ptr
> +            }
> +        }
> +    };
> +}
> +
> +macro_rules! impl_scalar_boxed {
> +    ($($t:ident)*) => (
> +        $(
> +            impl_to_qemu_scalar_boxed!($t);
> +            impl_from_qemu_none_scalar!($t);
> +        )*
> +    )
> +}
> +
> +// the only built-in used so far, feel free to add more as needed
> +impl_scalar_boxed!(bool i64 f64);
> --
> 2.33.0.113.g6c40894d24
>
>

Re: [RFC v3 12/32] rust: provide a common crate for QEMU

Posted by Marc-André Lureau 3 years, 12 months ago

Hi

On Fri, Sep 10, 2021 at 5:19 AM Alistair Francis <alistair23@gmail.com>
wrote:

> On Tue, Sep 7, 2021 at 10:41 PM <marcandre.lureau@redhat.com> wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > This crates provides common bindings and facilities for QEMU C API
> > shared by various projects.
> >
> > Most importantly, it defines the conversion traits used to convert from
> > C to Rust types. Those traits are largely adapted from glib-rs, since
> > those have proved to be very flexible, and should guide us to bind
> > further QEMU types such as QOM. If glib-rs becomes a dependency, we
> > should consider adopting glib translate traits. For QAPI, we need a
> > smaller subset.
> >
> > Cargo.lock is checked-in, as QEMU produces end-of-chain binaries, and it
> > is desirable to track the exact set of packages that are involved in
> > managed builds.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  Cargo.lock                   |  63 +++++
> >  Cargo.toml                   |   4 +-
> >  rust/common/Cargo.toml       |  11 +
> >  rust/common/src/error.rs     | 113 ++++++++
> >  rust/common/src/ffi.rs       |  93 +++++++
> >  rust/common/src/lib.rs       |  21 ++
> >  rust/common/src/qemu.rs      | 101 ++++++++
> >  rust/common/src/qmp.rs       |   0
> >  rust/common/src/translate.rs | 482 +++++++++++++++++++++++++++++++++++
> >  9 files changed, 887 insertions(+), 1 deletion(-)
> >  create mode 100644 Cargo.lock
> >  create mode 100644 rust/common/Cargo.toml
> >  create mode 100644 rust/common/src/error.rs
> >  create mode 100644 rust/common/src/ffi.rs
> >  create mode 100644 rust/common/src/lib.rs
> >  create mode 100644 rust/common/src/qemu.rs
> >  create mode 100644 rust/common/src/qmp.rs
> >  create mode 100644 rust/common/src/translate.rs
> >
> > diff --git a/Cargo.lock b/Cargo.lock
> > new file mode 100644
> > index 0000000000..8dc2dd9da7
> > --- /dev/null
> > +++ b/Cargo.lock
> > @@ -0,0 +1,63 @@
> > +# This file is automatically @generated by Cargo.
> > +# It is not intended for manual editing.
> > +version = 3
> > +
> > +[[package]]
> > +name = "autocfg"
> > +version = "1.0.1"
> > +source = "registry+https://github.com/rust-lang/crates.io-index"
> > +checksum =
> "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a"
> > +
> > +[[package]]
> > +name = "bitflags"
> > +version = "1.2.1"
> > +source = "registry+https://github.com/rust-lang/crates.io-index"
> > +checksum =
> "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693"
> > +
> > +[[package]]
> > +name = "cc"
> > +version = "1.0.70"
> > +source = "registry+https://github.com/rust-lang/crates.io-index"
> > +checksum =
> "d26a6ce4b6a484fa3edb70f7efa6fc430fd2b87285fe8b84304fd0936faa0dc0"
> > +
> > +[[package]]
> > +name = "cfg-if"
> > +version = "1.0.0"
> > +source = "registry+https://github.com/rust-lang/crates.io-index"
> > +checksum =
> "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
> > +
> > +[[package]]
> > +name = "common"
> > +version = "0.1.0"
> > +dependencies = [
> > + "libc",
> > + "nix",
> > +]
> > +
> > +[[package]]
> > +name = "libc"
> > +version = "0.2.101"
> > +source = "registry+https://github.com/rust-lang/crates.io-index"
> > +checksum =
> "3cb00336871be5ed2c8ed44b60ae9959dc5b9f08539422ed43f09e34ecaeba21"
> > +
> > +[[package]]
> > +name = "memoffset"
> > +version = "0.6.4"
> > +source = "registry+https://github.com/rust-lang/crates.io-index"
> > +checksum =
> "59accc507f1338036a0477ef61afdae33cde60840f4dfe481319ce3ad116ddf9"
> > +dependencies = [
> > + "autocfg",
> > +]
> > +
> > +[[package]]
> > +name = "nix"
> > +version = "0.20.1"
> > +source = "registry+https://github.com/rust-lang/crates.io-index"
> > +checksum =
> "df8e5e343312e7fbeb2a52139114e9e702991ef9c2aea6817ff2440b35647d56"
> > +dependencies = [
> > + "bitflags",
> > + "cc",
> > + "cfg-if",
> > + "libc",
> > + "memoffset",
> > +]
> > diff --git a/Cargo.toml b/Cargo.toml
> > index c4b464ff15..14131eed3c 100644
> > --- a/Cargo.toml
> > +++ b/Cargo.toml
> > @@ -1,2 +1,4 @@
> >  [workspace]
> > -members = []
> > +members = [
> > +  "rust/common",
> > +]
> > diff --git a/rust/common/Cargo.toml b/rust/common/Cargo.toml
> > new file mode 100644
> > index 0000000000..6c240447f3
> > --- /dev/null
> > +++ b/rust/common/Cargo.toml
> > @@ -0,0 +1,11 @@
> > +[package]
> > +name = "common"
> > +version = "0.1.0"
> > +edition = "2018"
> > +publish = false
> > +
> > +[dependencies]
> > +libc = "0.2.92"
> > +
> > +[target."cfg(unix)".dependencies]
> > +nix = "0.20.0"
> > diff --git a/rust/common/src/error.rs b/rust/common/src/error.rs
> > new file mode 100644
> > index 0000000000..f166ac42ea
> > --- /dev/null
> > +++ b/rust/common/src/error.rs
> > @@ -0,0 +1,113 @@
> > +use std::{self, ffi::CString, fmt, io, ptr};
> > +
> > +use crate::translate::*;
>
> It's not uncommon to ban wildcard imports that aren't preludes as it
> can make it confusing to read. Does QEMU have a stance on that type of
> thing?
>

There is no such common rule in Rust afaik. It's based on judgement and
style. If the imported symbols pollute your namespace or not. But yes, in
general, it's better to selectively import what you need, mostly for
readability.


> > +use crate::{ffi, qemu};
> > +
> > +/// Common error type for QEMU and related projects.
> > +#[derive(Debug)]
> > +pub enum Error {
> > +    /// A generic error with file and line location.
> > +    FailedAt(String, &'static str, u32),
> > +    /// An IO error.
> > +    Io(io::Error),
> > +    #[cfg(unix)]
> > +    /// A nix error.
> > +    Nix(nix::Error),
> > +}
> > +
> > +/// Alias for a `Result` with the error type for QEMU.
> > +pub type Result<T> = std::result::Result<T, Error>;
>
> I think this is very confusing. Rust developers expect `Result` to be
> the one from `std::result`, it would be better to call this
> `QEMUResult`
>

It's very common in Rust to redefine Result for your crate error. Users
don't have to import it if they prefer the std::result::Result<T,E>. This
redefinition was probably popularized with the `std::io::Result<T>` type. (
https://doc.rust-lang.org/std/io/type.Result.html)


>
> > +
> > +impl Error {
> > +    fn message(&self) -> String {
> > +        use Error::*;
>
> Do we need this here? Why not put it at the top of the file?
>

It's limited here to avoid enum prefix repetition:
match self {
 Error::FailedAt ..
 Error::Io ..
 Error::Foo ..
}

(It wouldn't be a good idea to import it in the top namespace)


> > +        match self {
> > +            FailedAt(msg, _, _) => msg.into(),
> > +            Io(io) => format!("IO error: {}", io),
> > +            #[cfg(unix)]
> > +            Nix(nix) => format!("Nix error: {}", nix),
> > +        }
> > +    }
> > +
> > +    fn location(&self) -> Option<(&'static str, u32)> {
> > +        use Error::*;
> > +        match self {
> > +            FailedAt(_, file, line) => Some((file, *line)),
> > +            Io(_) => None,
> > +            #[cfg(unix)]
> > +            Nix(_) => None,
> > +        }
> > +    }
> > +}
> > +
> > +impl fmt::Display for Error {
> > +    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
> > +        use Error::*;
> > +        match self {
> > +            FailedAt(msg, file, line) => write!(f, "{} ({}:{})", msg,
> file, line),
> > +            _ => write!(f, "{}", self.message()),
> > +        }
> > +    }
> > +}
> > +
> > +impl From<io::Error> for Error {
> > +    fn from(val: io::Error) -> Self {
> > +        Error::Io(val)
> > +    }
> > +}
> > +
> > +#[cfg(unix)]
> > +impl From<nix::Error> for Error {
> > +    fn from(val: nix::Error) -> Self {
> > +        Error::Nix(val)
> > +    }
> > +}
> > +
> > +impl QemuPtrDefault for Error {
> > +    type QemuType = *mut ffi::Error;
> > +}
> > +
> > +impl<'a> ToQemuPtr<'a, *mut ffi::Error> for Error {
> > +    type Storage = qemu::CError;
> > +
> > +    fn to_qemu_none(&'a self) -> Stash<'a, *mut ffi::Error, Self> {
> > +        let err = self.to_qemu_full();
> > +
> > +        Stash(err, unsafe { from_qemu_full(err) })
> > +    }
> > +
> > +    fn to_qemu_full(&self) -> *mut ffi::Error {
> > +        let cmsg =
> > +            CString::new(self.message()).expect("ToQemuPtr<Error>:
> unexpected '\0' character");
> > +        let mut csrc = CString::new("").unwrap();
> > +        let (src, line) = self.location().map_or((ptr::null(), 0_i32),
> |loc| {
> > +            csrc = CString::new(loc.0).expect("ToQemuPtr<Error>::
> unexpected '\0' character");
> > +            (csrc.as_ptr() as *const libc::c_char, loc.1 as i32)
> > +        });
> > +        let func = ptr::null();
> > +
> > +        let mut err: *mut ffi::Error = ptr::null_mut();
> > +        unsafe {
> > +            ffi::error_setg_internal(
> > +                &mut err as *mut *mut _,
> > +                src,
> > +                line,
> > +                func,
> > +                cmsg.as_ptr() as *const libc::c_char,
> > +            );
> > +            err
> > +        }
> > +    }
> > +}
> > +
> > +/// Convenience macro to build a [`Error::FailedAt`] error.
> > +///
> > +/// Returns a `Result::Err` with the file:line location.
> > +/// (the error can then be converted to a QEMU `ffi::Error`)
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! err {
> > +    ($msg:expr) => {
> > +        Err(Error::FailedAt($msg.into(), file!(), line!()))
> > +    };
> > +}
> > diff --git a/rust/common/src/ffi.rs b/rust/common/src/ffi.rs
> > new file mode 100644
> > index 0000000000..82818d503a
> > --- /dev/null
> > +++ b/rust/common/src/ffi.rs
> > @@ -0,0 +1,93 @@
> > +//! Bindings to the raw low-level C API commonly provided by QEMU
> projects.
> > +//!
> > +//! Manual bindings to C API availabe when linking QEMU projects.
>
> s/availabe/available/g
>
>
yup thanks

> +//! It includes minimal glib allocation functions too, since it's the
> default
> > +//! allocator used by QEMU, and we don't depend on glib-rs crate yet).
> > +//!
> > +//! Higher-level Rust-friendly bindings are provided by different
> modules.
> > +
> > +use libc::{c_char, c_void, size_t};
> > +
> > +extern "C" {
> > +    pub fn g_malloc0(n_bytes: size_t) -> *mut c_void;
> > +    pub fn g_free(ptr: *mut c_void);
> > +    pub fn g_strndup(str: *const c_char, n: size_t) -> *mut c_char;
> > +}
>
> We can get there from the glib/glib_sys crate:
>
> https://gtk-rs.org/gtk-rs-core/stable/latest/docs/glib_sys/fn.g_malloc0.html
>
> If we only plan on using these 3 I think this approach is fine, but
> something to keep in mind if we use more glib functions.
>
>
Yes, I think I mentioned this somewhere. We might need to import glib-sys
or glib-rs depending on what we need to write in Rust. We may actually not
need more than a few FFI functions though, so importing external crates for
that isn't worth it.


> > +
> > +#[repr(C)]
> > +pub struct QObject(c_void);
> > +
> > +impl ::std::fmt::Debug for QObject {
> > +    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
> > +        f.debug_struct(&format!("QObject @ {:?}", self as *const _))
> > +            .finish()
> > +    }
> > +}
> > +
> > +#[repr(C)]
> > +pub struct QNull(c_void);
> > +
> > +impl ::std::fmt::Debug for QNull {
> > +    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
> > +        f.debug_struct(&format!("QNull @ {:?}", self as *const _))
> > +            .finish()
> > +    }
> > +}
> > +
> > +#[repr(C)]
> > +pub struct Error(c_void);
> > +
> > +impl ::std::fmt::Debug for Error {
> > +    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
> > +        f.debug_struct(&format!("Error @ {:?}", self as *const _))
> > +            .finish()
> > +    }
> > +}
> > +
> > +extern "C" {
> > +    pub fn error_setg_internal(
> > +        errp: *mut *mut Error,
> > +        src: *const c_char,
> > +        line: i32,
> > +        func: *const c_char,
> > +        fmt: *const c_char,
> > +        ...
> > +    );
> > +    pub fn error_get_pretty(err: *const Error) -> *const c_char;
> > +    pub fn error_free(err: *mut Error);
> > +}
> > +
> > +/// Wrap a QMP hanlder.
>
> handler
>

thanks!


> > +#[macro_export]
> > +macro_rules! qmp {
> > +    // the basic return value variant
> > +    ($e:expr, $errp:ident, $errval:expr) => {{
> > +        assert!(!$errp.is_null());
> > +        unsafe {
> > +            *$errp = std::ptr::null_mut();
> > +        }
> > +
> > +        match $e {
> > +            Ok(val) => val,
> > +            Err(err) => unsafe {
> > +                *$errp = err.to_qemu_full();
> > +                $errval
> > +            },
> > +        }
> > +    }};
> > +    // the ptr return value variant
> > +    ($e:expr, $errp:ident) => {{
> > +        assert!(!$errp.is_null());
> > +        unsafe {
> > +            *$errp = std::ptr::null_mut();
> > +        }
> > +
> > +        match $e {
> > +            Ok(val) => val.to_qemu_full().into(),
> > +            Err(err) => unsafe {
> > +                *$errp = err.to_qemu_full();
> > +                std::ptr::null_mut()
> > +            },
> > +        }
> > +    }};
> > +}
>
> It would be a good idea to document why this code is safe
>

Hmm, I am not sure that's the question. I assume Rust code is safe :)
However, the FFI borders are full of unsafe {}. It's basically a trust
relationship with the other side. But it's always great to document tricky
unsafe {}, or where panic is unexpected (unwrap()/expect()).. based on
review, judgement, style.. (read also
https://doc.rust-lang.org/book/ch09-03-to-panic-or-not-to-panic.html).

Thanks!

Alistair
>
> > diff --git a/rust/common/src/lib.rs b/rust/common/src/lib.rs
> > new file mode 100644
> > index 0000000000..4de826bc2e
> > --- /dev/null
> > +++ b/rust/common/src/lib.rs
> > @@ -0,0 +1,21 @@
> > +//! Common code for QEMU
> > +//!
> > +//! This crates provides common bindings and facilities for QEMU C API
> shared by
> > +//! various projects. Most importantly, it defines the conversion
> traits used to
> > +//! convert from C to Rust types. Those traits are largely adapted from
> glib-rs,
> > +//! since those have prooven to be very flexible, and should guide us
> to bind
> > +//! further QEMU types such as QOM. If glib-rs becomes a dependency, we
> should
> > +//! consider adopting glib translate traits. For QAPI, we need a
> smaller subset.
> > +
> > +pub use libc;
> > +
> > +mod error;
> > +pub use error::*;
> > +
> > +mod qemu;
> > +pub use qemu::*;
> > +
> > +mod translate;
> > +pub use translate::*;
> > +
> > +pub mod ffi;
> > diff --git a/rust/common/src/qemu.rs b/rust/common/src/qemu.rs
> > new file mode 100644
> > index 0000000000..dd01c6d92d
> > --- /dev/null
> > +++ b/rust/common/src/qemu.rs
> > @@ -0,0 +1,101 @@
> > +use std::{ffi::CStr, ptr, str};
> > +
> > +use crate::{ffi, translate};
> > +use translate::{FromQemuPtrFull, FromQemuPtrNone, QemuPtrDefault,
> Stash, ToQemuPtr};
> > +
> > +/// A type representing an owned C QEMU Error.
> > +pub struct CError(ptr::NonNull<ffi::Error>);
> > +
> > +impl translate::FromQemuPtrFull<*mut ffi::Error> for CError {
> > +    unsafe fn from_qemu_full(ptr: *mut ffi::Error) -> Self {
> > +        assert!(!ptr.is_null());
> > +        Self(ptr::NonNull::new_unchecked(ptr))
> > +    }
> > +}
> > +
> > +impl CError {
> > +    pub fn pretty(&self) -> &str {
> > +        unsafe {
> > +            let pretty = ffi::error_get_pretty(self.0.as_ptr());
> > +            let bytes = CStr::from_ptr(pretty).to_bytes();
> > +            str::from_utf8(bytes)
> > +                .unwrap_or_else(|err|
> str::from_utf8(&bytes[..err.valid_up_to()]).unwrap())
> > +        }
> > +    }
> > +}
> > +
> > +impl Drop for CError {
> > +    fn drop(&mut self) {
> > +        unsafe { ffi::error_free(self.0.as_ptr()) }
> > +    }
> > +}
> > +
> > +/// QObject (JSON object)
> > +#[derive(Clone, Debug)]
> > +pub struct QObject;
> > +
> > +impl QemuPtrDefault for QObject {
> > +    type QemuType = *mut ffi::QObject;
> > +}
> > +
> > +impl FromQemuPtrFull<*mut ffi::QObject> for QObject {
> > +    #[inline]
> > +    unsafe fn from_qemu_full(_ffi: *mut ffi::QObject) -> Self {
> > +        unimplemented!()
> > +    }
> > +}
> > +
> > +impl FromQemuPtrNone<*const ffi::QObject> for QObject {
> > +    #[inline]
> > +    unsafe fn from_qemu_none(_ffi: *const ffi::QObject) -> Self {
> > +        unimplemented!()
> > +    }
> > +}
> > +
> > +impl<'a> ToQemuPtr<'a, *mut ffi::QObject> for QObject {
> > +    type Storage = ();
> > +
> > +    #[inline]
> > +    fn to_qemu_none(&self) -> Stash<'a, *mut ffi::QObject, QObject> {
> > +        unimplemented!()
> > +    }
> > +    #[inline]
> > +    fn to_qemu_full(&self) -> *mut ffi::QObject {
> > +        unimplemented!()
> > +    }
> > +}
> > +
> > +/// QNull (JSON null)
> > +#[derive(Clone, Debug)]
> > +pub struct QNull;
> > +
> > +impl QemuPtrDefault for QNull {
> > +    type QemuType = *mut ffi::QNull;
> > +}
> > +
> > +impl FromQemuPtrFull<*mut ffi::QObject> for QNull {
> > +    #[inline]
> > +    unsafe fn from_qemu_full(_ffi: *mut ffi::QObject) -> Self {
> > +        unimplemented!()
> > +    }
> > +}
> > +
> > +impl FromQemuPtrNone<*const ffi::QObject> for QNull {
> > +    #[inline]
> > +    unsafe fn from_qemu_none(_ffi: *const ffi::QObject) -> Self {
> > +        unimplemented!()
> > +    }
> > +}
> > +
> > +impl<'a> ToQemuPtr<'a, *mut ffi::QNull> for QNull {
> > +    type Storage = ();
> > +
> > +    #[inline]
> > +    fn to_qemu_none(&self) -> Stash<'a, *mut ffi::QNull, QNull> {
> > +        unimplemented!()
> > +    }
> > +    #[inline]
> > +    fn to_qemu_full(&self) -> *mut ffi::QNull {
> > +        unimplemented!()
> > +    }
> > +}
> > diff --git a/rust/common/src/qmp.rs b/rust/common/src/qmp.rs
> > new file mode 100644
> > index 0000000000..e69de29bb2
> > diff --git a/rust/common/src/translate.rs b/rust/common/src/translate.rs
> > new file mode 100644
> > index 0000000000..315e14fa25
> > --- /dev/null
> > +++ b/rust/common/src/translate.rs
> > @@ -0,0 +1,482 @@
> > +// largely adapted from glib-rs
> > +// we don't depend on glib-rs as this brings a lot more code that we
> may not need
> > +// and also because there are issues with the conversion traits for our
> ffi::*mut.
> > +use libc::{c_char, size_t};
> > +use std::ffi::{CStr, CString};
> > +use std::ptr;
> > +
> > +use crate::ffi;
> > +
> > +/// A pointer.
> > +pub trait Ptr: Copy + 'static {
> > +    fn is_null(&self) -> bool;
> > +    fn from<X>(ptr: *mut X) -> Self;
> > +    fn to<X>(self) -> *mut X;
> > +}
> > +
> > +impl<T: 'static> Ptr for *const T {
> > +    #[inline]
> > +    fn is_null(&self) -> bool {
> > +        (*self).is_null()
> > +    }
> > +
> > +    #[inline]
> > +    fn from<X>(ptr: *mut X) -> *const T {
> > +        ptr as *const T
> > +    }
> > +
> > +    #[inline]
> > +    fn to<X>(self) -> *mut X {
> > +        self as *mut X
> > +    }
> > +}
> > +
> > +impl<T: 'static> Ptr for *mut T {
> > +    #[inline]
> > +    fn is_null(&self) -> bool {
> > +        (*self).is_null()
> > +    }
> > +
> > +    #[inline]
> > +    fn from<X>(ptr: *mut X) -> *mut T {
> > +        ptr as *mut T
> > +    }
> > +
> > +    #[inline]
> > +    fn to<X>(self) -> *mut X {
> > +        self as *mut X
> > +    }
> > +}
> > +
> > +/// Macro to declare a `NewPtr` struct.
> > +///
> > +/// A macro to declare a newtype for pointers, to workaround that *T
> are not
> > +/// defined in our binding crates, and allow foreign traits
> implementations.
> > +/// (this is used by qapi-gen bindings)
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +#[doc(hidden)]
> > +macro_rules! new_ptr {
> > +    () => {
> > +        #[derive(Copy, Clone)]
> > +        pub struct NewPtr<P: Ptr>(pub P);
> > +
> > +        impl<P: Ptr> Ptr for NewPtr<P> {
> > +            #[inline]
> > +            fn is_null(&self) -> bool {
> > +                self.0.is_null()
> > +            }
> > +
> > +            #[inline]
> > +            fn from<X>(ptr: *mut X) -> Self {
> > +                NewPtr(P::from(ptr))
> > +            }
> > +
> > +            #[inline]
> > +            fn to<X>(self) -> *mut X {
> > +                self.0.to()
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +/// Provides the default pointer type to be used in some container
> conversions.
> > +///
> > +/// It's `*mut c_char` for `String`, `*mut ffi::GuestInfo` for
> `GuestInfo`...
> > +pub trait QemuPtrDefault {
> > +    type QemuType: Ptr;
> > +}
> > +
> > +impl QemuPtrDefault for String {
> > +    type QemuType = *mut c_char;
> > +}
> > +
> > +/// A Stash contains the temporary storage and a pointer into it.
> > +///
> > +/// The pointer is valid for the lifetime of the `Stash`. As the
> lifetime of the
> > +/// `Stash` returned from `to_qemu_none` is at least the enclosing
> statement,
> > +/// you can avoid explicitly binding the stash in most cases and just
> take the
> > +/// pointer out of it:
> > +///
> > +/// ```ignore
> > +///     pub fn set_device_name(&self, name: &str) {
> > +///         unsafe {
> > +///             ffi::qemu_device_set_name(self.pointer,
> name.to_qemu_none().0)
> > +///         }
> > +///     }
> > +/// ```
> > +pub struct Stash<'a, P: Copy, T: ?Sized + ToQemuPtr<'a, P>>(
> > +    pub P,
> > +    pub <T as ToQemuPtr<'a, P>>::Storage,
> > +);
> > +
> > +/// Translate to a pointer.
> > +pub trait ToQemuPtr<'a, P: Copy> {
> > +    type Storage;
> > +
> > +    /// The pointer in the `Stash` is only valid for the lifetime of
> the `Stash`.
> > +    fn to_qemu_none(&'a self) -> Stash<'a, P, Self>;
> > +
> > +    /// Transfer the ownership to the ffi.
> > +    fn to_qemu_full(&self) -> P {
> > +        unimplemented!();
> > +    }
> > +}
> > +
> > +impl<'a, P: Ptr, T: ToQemuPtr<'a, P>> ToQemuPtr<'a, P> for Option<T> {
> > +    type Storage = Option<<T as ToQemuPtr<'a, P>>::Storage>;
> > +
> > +    #[inline]
> > +    fn to_qemu_none(&'a self) -> Stash<'a, P, Option<T>> {
> > +        self.as_ref()
> > +            .map_or(Stash(Ptr::from::<()>(ptr::null_mut()), None), |s| {
> > +                let s = s.to_qemu_none();
> > +                Stash(s.0, Some(s.1))
> > +            })
> > +    }
> > +
> > +    #[inline]
> > +    fn to_qemu_full(&self) -> P {
> > +        self.as_ref()
> > +            .map_or(Ptr::from::<()>(ptr::null_mut()),
> ToQemuPtr::to_qemu_full)
> > +    }
> > +}
> > +
> > +impl<'a, P: Ptr, T: ToQemuPtr<'a, P>> ToQemuPtr<'a, P> for Box<T> {
> > +    type Storage = <T as ToQemuPtr<'a, P>>::Storage;
> > +
> > +    #[inline]
> > +    fn to_qemu_none(&'a self) -> Stash<'a, P, Box<T>> {
> > +        let s = self.as_ref().to_qemu_none();
> > +        Stash(s.0, s.1)
> > +    }
> > +
> > +    #[inline]
> > +    fn to_qemu_full(&self) -> P {
> > +        ToQemuPtr::to_qemu_full(self.as_ref())
> > +    }
> > +}
> > +
> > +impl<'a> ToQemuPtr<'a, *mut c_char> for String {
> > +    type Storage = CString;
> > +
> > +    #[inline]
> > +    fn to_qemu_none(&self) -> Stash<'a, *mut c_char, String> {
> > +        let tmp = CString::new(&self[..])
> > +            .expect("String::ToQemuPtr<*mut c_char>: unexpected '\0'
> character");
> > +        Stash(tmp.as_ptr() as *mut c_char, tmp)
> > +    }
> > +
> > +    #[inline]
> > +    fn to_qemu_full(&self) -> *mut c_char {
> > +        unsafe { ffi::g_strndup(self.as_ptr() as *const c_char,
> self.len() as size_t) }
> > +    }
> > +}
> > +
> > +/// Translate from a pointer type, without taking ownership.
> > +pub trait FromQemuPtrNone<P: Ptr>: Sized {
> > +    /// # Safety
> > +    ///
> > +    /// `ptr` must be a valid pointer. It is not referenced after the
> call.
> > +    unsafe fn from_qemu_none(ptr: P) -> Self;
> > +}
> > +
> > +/// Translate from a pointer type, taking ownership.
> > +pub trait FromQemuPtrFull<P: Ptr>: Sized {
> > +    /// # Safety
> > +    ///
> > +    /// `ptr` must be a valid pointer. Ownership is transferred.
> > +    unsafe fn from_qemu_full(ptr: P) -> Self;
> > +}
> > +
> > +/// See [`FromQemuPtrNone`](trait.FromQemuPtrNone.html).
> > +#[inline]
> > +#[allow(clippy::missing_safety_doc)]
> > +pub unsafe fn from_qemu_none<P: Ptr, T: FromQemuPtrNone<P>>(ptr: P) ->
> T {
> > +    FromQemuPtrNone::from_qemu_none(ptr)
> > +}
> > +
> > +/// See [`FromQemuPtrFull`](trait.FromQemuPtrFull.html).
> > +#[inline]
> > +#[allow(clippy::missing_safety_doc)]
> > +pub unsafe fn from_qemu_full<P: Ptr, T: FromQemuPtrFull<P>>(ptr: P) ->
> T {
> > +    FromQemuPtrFull::from_qemu_full(ptr)
> > +}
> > +
> > +impl<P: Ptr, T: FromQemuPtrNone<P>> FromQemuPtrNone<P> for Option<T> {
> > +    #[inline]
> > +    unsafe fn from_qemu_none(ptr: P) -> Option<T> {
> > +        if ptr.is_null() {
> > +            None
> > +        } else {
> > +            Some(from_qemu_none(ptr))
> > +        }
> > +    }
> > +}
> > +
> > +impl<P: Ptr, T: FromQemuPtrFull<P>> FromQemuPtrFull<P> for Option<T> {
> > +    #[inline]
> > +    unsafe fn from_qemu_full(ptr: P) -> Option<T> {
> > +        if ptr.is_null() {
> > +            None
> > +        } else {
> > +            Some(from_qemu_full(ptr))
> > +        }
> > +    }
> > +}
> > +
> > +impl FromQemuPtrNone<*const c_char> for String {
> > +    #[inline]
> > +    unsafe fn from_qemu_none(ptr: *const c_char) -> Self {
> > +        assert!(!ptr.is_null());
> > +
> String::from_utf8_lossy(CStr::from_ptr(ptr).to_bytes()).into_owned()
> > +    }
> > +}
> > +
> > +impl FromQemuPtrFull<*mut c_char> for String {
> > +    #[inline]
> > +    unsafe fn from_qemu_full(ptr: *mut c_char) -> Self {
> > +        let res = from_qemu_none(ptr as *const _);
> > +        ffi::g_free(ptr as *mut _);
> > +        res
> > +    }
> > +}
> > +
> > +#[doc(hidden)]
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! vec_ffi_wrapper {
> > +    ($ffi:ident) => {
> > +        #[allow(non_camel_case_types)]
> > +        pub struct $ffi(*mut qapi_ffi::$ffi);
> > +
> > +        impl Drop for $ffi {
> > +            fn drop(&mut self) {
> > +                let mut list = self.0;
> > +                unsafe {
> > +                    while !list.is_null() {
> > +                        let next = (*list).next;
> > +                        Box::from_raw(list);
> > +                        list = next;
> > +                    }
> > +                }
> > +            }
> > +        }
> > +
> > +        impl From<NewPtr<*mut qapi_ffi::$ffi>> for *mut qapi_ffi::$ffi {
> > +            fn from(p: NewPtr<*mut qapi_ffi::$ffi>) -> Self {
> > +                p.0
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +#[doc(hidden)]
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! impl_vec_scalars_to_qemu {
> > +    ($rs:ty, $ffi:ident) => {
> > +        impl<'a> ToQemuPtr<'a, NewPtr<*mut qapi_ffi::$ffi>> for
> Vec<$rs> {
> > +            type Storage = $ffi;
> > +
> > +            #[inline]
> > +            fn to_qemu_none(&self) -> Stash<NewPtr<*mut
> qapi_ffi::$ffi>, Self> {
> > +                let mut list: *mut qapi_ffi::$ffi =
> std::ptr::null_mut();
> > +                for value in self.iter().rev() {
> > +                    let b = Box::new(qapi_ffi::$ffi {
> > +                        next: list,
> > +                        value: *value,
> > +                    });
> > +                    list = Box::into_raw(b);
> > +                }
> > +                Stash(NewPtr(list), $ffi(list))
> > +            }
> > +
> > +            #[inline]
> > +            fn to_qemu_full(&self) -> NewPtr<*mut qapi_ffi::$ffi> {
> > +                let mut list: *mut qapi_ffi::$ffi =
> std::ptr::null_mut();
> > +                unsafe {
> > +                    for value in self.iter().rev() {
> > +                        let l =
> ffi::g_malloc0(std::mem::size_of::<qapi_ffi::$ffi>())
> > +                            as *mut qapi_ffi::$ffi;
> > +                        (*l).next = list;
> > +                        (*l).value = *value;
> > +                        list = l;
> > +                    }
> > +                }
> > +                NewPtr(list)
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +#[doc(hidden)]
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! impl_vec_scalars_from_qemu {
> > +    ($rs:ty, $ffi:ident, $free_ffi:ident) => {
> > +        impl FromQemuPtrFull<NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
> > +            #[inline]
> > +            unsafe fn from_qemu_full(ffi: NewPtr<*mut qapi_ffi::$ffi>)
> -> Self {
> > +                let ret = from_qemu_none(NewPtr(ffi.0 as *const _));
> > +                qapi_ffi::$free_ffi(ffi.0);
> > +                ret
> > +            }
> > +        }
> > +
> > +        impl FromQemuPtrNone<NewPtr<*const qapi_ffi::$ffi>> for
> Vec<$rs> {
> > +            #[inline]
> > +            unsafe fn from_qemu_none(ffi: NewPtr<*const
> qapi_ffi::$ffi>) -> Self {
> > +                let mut ret = vec![];
> > +                let mut it = ffi.0;
> > +                while !it.is_null() {
> > +                    let e = &*it;
> > +                    ret.push(e.value);
> > +                    it = e.next;
> > +                }
> > +                ret
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +#[doc(hidden)]
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! impl_vec_to_qemu {
> > +    ($rs:ty, $ffi:ident) => {
> > +        // impl doesn't use only types from inside the current crate
> > +        // impl QemuPtrDefault for Vec<$rs> {
> > +        //     type QemuType = NewPtr<*mut qapi_ffi::$ffi>;
> > +        // }
> > +
> > +        impl<'a> ToQemuPtr<'a, NewPtr<*mut qapi_ffi::$ffi>> for
> Vec<$rs> {
> > +            type Storage = ($ffi, Vec<Stash<'a, <$rs as
> QemuPtrDefault>::QemuType, $rs>>);
> > +
> > +            #[inline]
> > +            fn to_qemu_none(&self) -> Stash<NewPtr<*mut
> qapi_ffi::$ffi>, Self> {
> > +                let stash_vec: Vec<_> =
> self.iter().rev().map(ToQemuPtr::to_qemu_none).collect();
> > +                let mut list: *mut qapi_ffi::$ffi =
> std::ptr::null_mut();
> > +                for stash in &stash_vec {
> > +                    let b = Box::new(qapi_ffi::$ffi {
> > +                        next: list,
> > +                        value: Ptr::to(stash.0),
> > +                    });
> > +                    list = Box::into_raw(b);
> > +                }
> > +                Stash(NewPtr(list), ($ffi(list), stash_vec))
> > +            }
> > +
> > +            #[inline]
> > +            fn to_qemu_full(&self) -> NewPtr<*mut qapi_ffi::$ffi> {
> > +                let v: Vec<_> =
> self.iter().rev().map(ToQemuPtr::to_qemu_full).collect();
> > +                let mut list: *mut qapi_ffi::$ffi =
> std::ptr::null_mut();
> > +                unsafe {
> > +                    for val in v {
> > +                        let l =
> ffi::g_malloc0(std::mem::size_of::<qapi_ffi::$ffi>())
> > +                            as *mut qapi_ffi::$ffi;
> > +                        (*l).next = list;
> > +                        (*l).value = val;
> > +                        list = l;
> > +                    }
> > +                }
> > +                NewPtr(list)
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +#[doc(hidden)]
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! impl_vec_from_qemu {
> > +    ($rs:ty, $ffi:ident, $free_ffi:ident) => {
> > +        impl FromQemuPtrFull<NewPtr<*mut qapi_ffi::$ffi>> for Vec<$rs> {
> > +            #[inline]
> > +            unsafe fn from_qemu_full(ffi: NewPtr<*mut qapi_ffi::$ffi>)
> -> Self {
> > +                let ret = from_qemu_none(NewPtr(ffi.0 as *const _));
> > +                qapi_ffi::$free_ffi(ffi.0);
> > +                ret
> > +            }
> > +        }
> > +
> > +        impl FromQemuPtrNone<NewPtr<*const qapi_ffi::$ffi>> for
> Vec<$rs> {
> > +            #[inline]
> > +            unsafe fn from_qemu_none(ffi: NewPtr<*const
> qapi_ffi::$ffi>) -> Self {
> > +                let mut ret = vec![];
> > +                let mut it = ffi.0;
> > +                while !it.is_null() {
> > +                    let e = &*it;
> > +                    ret.push(from_qemu_none(e.value as *const _));
> > +                    it = e.next;
> > +                }
> > +                ret
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +/// A macro to help the implementation of `Vec<T>` translations.
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! vec_type {
> > +    (Vec<$rs:ty>, $ffi:ident, $free_ffi:ident, 0) => {
> > +        vec_ffi_wrapper!($ffi);
> > +        impl_vec_from_qemu!($rs, $ffi, $free_ffi);
> > +        impl_vec_to_qemu!($rs, $ffi);
> > +    };
> > +    (Vec<$rs:ty>, $ffi:ident, $free_ffi:ident, 1) => {
> > +        vec_ffi_wrapper!($ffi);
> > +        impl_vec_scalars_from_qemu!($rs, $ffi, $free_ffi);
> > +        impl_vec_scalars_to_qemu!($rs, $ffi);
> > +    };
> > +}
> > +
> > +/// A macro to implement [`ToQemuPtr`] as boxed scalars
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! impl_to_qemu_scalar_boxed {
> > +    ($ty:ty) => {
> > +        impl<'a> ToQemuPtr<'a, *mut $ty> for $ty {
> > +            type Storage = Box<$ty>;
> > +
> > +            fn to_qemu_none(&'a self) -> Stash<'a, *mut $ty, Self> {
> > +                let mut box_ = Box::new(*self);
> > +                Stash(&mut *box_, box_)
> > +            }
> > +
> > +            fn to_qemu_full(&self) -> *mut $ty {
> > +                unsafe {
> > +                    let ptr =
> ffi::g_malloc0(std::mem::size_of::<$ty>()) as *mut _;
> > +                    *ptr = *self;
> > +                    ptr
> > +                }
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +/// A macro to implement [`FromQemuPtrNone`] for scalar pointers.
> > +#[allow(unused_macros)]
> > +#[macro_export]
> > +macro_rules! impl_from_qemu_none_scalar {
> > +    ($ty:ty) => {
> > +        impl FromQemuPtrNone<*const $ty> for $ty {
> > +            unsafe fn from_qemu_none(ptr: *const $ty) -> Self {
> > +                *ptr
> > +            }
> > +        }
> > +    };
> > +}
> > +
> > +macro_rules! impl_scalar_boxed {
> > +    ($($t:ident)*) => (
> > +        $(
> > +            impl_to_qemu_scalar_boxed!($t);
> > +            impl_from_qemu_none_scalar!($t);
> > +        )*
> > +    )
> > +}
> > +
> > +// the only built-in used so far, feel free to add more as needed
> > +impl_scalar_boxed!(bool i64 f64);
> > --
> > 2.33.0.113.g6c40894d24
> >
> >
>
>

Re: [RFC v3 12/32] rust: provide a common crate for QEMU

Posted by Paolo Bonzini 3 years, 11 months ago

On 07/09/21 14:19, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau<marcandre.lureau@redhat.com>
> 
> This crates provides common bindings and facilities for QEMU C API
> shared by various projects.
> 
> Most importantly, it defines the conversion traits used to convert from
> C to Rust types. Those traits are largely adapted from glib-rs, since
> those have proved to be very flexible, and should guide us to bind
> further QEMU types such as QOM. If glib-rs becomes a dependency, we
> should consider adopting glib translate traits. For QAPI, we need a
> smaller subset.
> 
> Cargo.lock is checked-in, as QEMU produces end-of-chain binaries, and it
> is desirable to track the exact set of packages that are involved in
> managed builds.
> 
> Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>

As in my previous review, the main issue I have here is with the 
complexity of this code.

I understand that this has to be manually written, but right now I find 
it really hard to understand what is going on here.  The patch needs to 
be expanded in several parts:

1) generic traits (including implementations for Option/Box)

2) implementation of the generic traits

3) io/nix errors

and these parts should be moved around to the place where they become 
necessary.

Also regarding the code itself:

1) Stash should not be a tuple.  Accesses to it should use standard Rust 
methods, such as borrow()/borrow_mut(), and it should also support 
standard Rust idioms such as map():

pub struct BorrowedMutPointer<'a, P, T: 'a> {
     native: *mut P,
     storage: T,
     _marker: PhantomData<&'a P>,
}

#[allow(dead_code)]
impl<'a, P: Copy, T: 'a> BorrowedMutPointer<'a, P, T> {
     fn as_ptr(&self) -> *const P {
         self.native
     }

     fn as_mut_ptr(&mut self) -> *mut P {
         self.native
     }

     fn map<U: 'a, F: FnOnce(T) -> U>(self, f: F) -> 
BorrowedMutPointer<'a, P, U> {
         BorrowedMutPointer {
             native: self.native,
             storage: f(self.storage),
             _marker: PhantomData,
         }
     }
}

impl<'a, P, T> Borrow<T> for BorrowedMutPointer<'a, P, T> {
     fn borrow(&self) -> &T {
         &self.storage
     }
}

impl<'a, P, T> BorrowMut<T> for BorrowedMutPointer<'a, P, T> {
     fn borrow_mut(&mut self) -> &mut T {
         &mut self.storage
     }
}

2) Does ToQemuPtr need to allow multiple implementations?  Can the type 
parameter in ToQemuPtr<'a, P> be an associated type (for example 
"Native")?  Type parameters really add a lot of complexity.

3) I would rather not have "qemu" in the names.  The Rust parts *are* 
QEMU.  So "foreign" or "c" would be better.

4) full/none is still really confusing to me.  I have finally understood 
that it's because the pair that borrows is from_qemu_full/to_qemu_none, 
and the pair that copies is from_qemu_none/to_qemu_full.  I'd really 
like to use names following the Rust naming conventions.  A possible 
improvement of my proposal from the previous review:

- from_qemu_full -> from_foreign (or from_c, same below)
                     + possibly a dual method into_native or into_rust

- from_qemu_none -> cloned_from_foreign

- to_qemu_none -> as_foreign or as_foreign_mut

- to_qemu_full -> clone_to_foreign

I will see if I have some time to do some of this work.

Paolo

Re: [RFC v3 12/32] rust: provide a common crate for QEMU

Posted by Marc-André Lureau 3 years, 11 months ago

Hi Paolo

On Mon, Sep 13, 2021 at 9:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 07/09/21 14:19, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau<marcandre.lureau@redhat.com>
> >
> > This crates provides common bindings and facilities for QEMU C API
> > shared by various projects.
> >
> > Most importantly, it defines the conversion traits used to convert from
> > C to Rust types. Those traits are largely adapted from glib-rs, since
> > those have proved to be very flexible, and should guide us to bind
> > further QEMU types such as QOM. If glib-rs becomes a dependency, we
> > should consider adopting glib translate traits. For QAPI, we need a
> > smaller subset.
> >
> > Cargo.lock is checked-in, as QEMU produces end-of-chain binaries, and it
> > is desirable to track the exact set of packages that are involved in
> > managed builds.
> >
> > Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
>
> As in my previous review, the main issue I have here is with the
> complexity of this code.
>
> I understand that this has to be manually written, but right now I find
> it really hard to understand what is going on here.  The patch needs to
> be expanded in several parts:
>
> 1) generic traits (including implementations for Option/Box)
>
> 2) implementation of the generic traits
>
> 3) io/nix errors
>
> and these parts should be moved around to the place where they become
> necessary.
>
>
The common crate can be split in many pieces. It is easier to have it as a
single commit during PoC/RFC (the series is already large enough).

Is it really valuable to introduce a new crate/library piece by piece?
Instead, better documentation and tests are more valuable imho. But I will
certainly split it and clean it more as we iterate.

> Also regarding the code itself:
>
> 1) Stash should not be a tuple.  Accesses to it should use standard Rust
> methods, such as borrow()/borrow_mut(), and it should also support
> standard Rust idioms such as map():
>
>

I thought we already discussed that. The stash is an implementation detail
to handle FFI resource release. The only thing the user should care about
the stash is the pointer, which is shortly retrieved with ".0" (cannot be
more succinct).

Ideally, we would like to provide "foo.to_qemu_none()", but to keep the
associated resource allocated, the shortest we can offer is
"foo.to_qemu_none().0". Using "as_ptr()" instead would not only be longer
to type but possibly misleading: there is nothing else you should do with
the stash, no other method should exist.

I don't understand what "map()" would provide here either. The storage part
of the stash is an internal detail (the FFI data), not meant to be
manipulated at all (it would invalidate the stash pointer).

> pub struct BorrowedMutPointer<'a, P, T: 'a> {
>      native: *mut P,
>      storage: T,
>      _marker: PhantomData<&'a P>,
> }
>
> #[allow(dead_code)]
> impl<'a, P: Copy, T: 'a> BorrowedMutPointer<'a, P, T> {
>      fn as_ptr(&self) -> *const P {
>          self.native
>      }
>
>      fn as_mut_ptr(&mut self) -> *mut P {
>          self.native
>      }
>
>      fn map<U: 'a, F: FnOnce(T) -> U>(self, f: F) ->
> BorrowedMutPointer<'a, P, U> {
>          BorrowedMutPointer {
>              native: self.native,
>              storage: f(self.storage),
>              _marker: PhantomData,
>          }
>      }
> }
>
> impl<'a, P, T> Borrow<T> for BorrowedMutPointer<'a, P, T> {
>      fn borrow(&self) -> &T {
>          &self.storage
>      }
> }
>
> impl<'a, P, T> BorrowMut<T> for BorrowedMutPointer<'a, P, T> {
>      fn borrow_mut(&mut self) -> &mut T {
>          &mut self.storage
>      }
> }
>
> 2) Does ToQemuPtr need to allow multiple implementations?  Can the type
> parameter in ToQemuPtr<'a, P> be an associated type (for example
> "Native")?  Type parameters really add a lot of complexity.
>
>

ToQemuPtr is a trait, implemented for the various Rust types that we can
translate to FFI pointers.

For example:

impl<'a> ToQemuPtr<'a, *mut c_char> for String {
    type Storage = CString;
...
}

The pointer type parameter is to return the correct pointer. It is not
necessary to specify it as a user, since Rust infers it from the expected
type.

> 3) I would rather not have "qemu" in the names.  The Rust parts *are*
> QEMU.  So "foreign" or "c" would be better.
>

That's kind of cosmetic.

The main reason for "qemu" in the name is to avoid potential clashes with
other methods and traits.

Also "to_none" could associate in the reader's mind (wrongly) with the
ubiquitous Option::None.

But yes, we could drop "qemu" here.

>
> 4) full/none is still really confusing to me.  I have finally understood
> that it's because the pair that borrows is from_qemu_full/to_qemu_none,
> and the pair that copies is from_qemu_none/to_qemu_full.  I'd really
> like to use names following the Rust naming conventions.  A possible
> improvement of my proposal from the previous review:
>
>

"full" and "none" are inherited from the glib-introspection ownership
transfer annotations.

We discussed and weighed pros and cons last year. I haven't yet found or
agreed on better options. And you can be certain that the glib-rs folks
have already pondered a lot about the naming and design. And, I don't like
to reinvent what others have done (creating various limitations and
incompatibilities). Following the glib traits brings a lot of experience
and could help us reuse glib-rs crates more easily (because they would use
the same design), but also to port and interoperate. Imagine we need to
bind GHashTable<char *, QapiFoo> someday, we will be in a better position
if we have translation traits that follow glib-rs already.

The Rust conventions can be inferred from CString/CStr design and methods,
but they have to be adjusted, because they don't solve the same constraints
(no ownership transfer, and CString/CStr intermediary representations).

They are "CStr::from_ptr(*const)" and "CString::as_ptr(&self) -> *const".

- from_qemu_full -> from_foreign (or from_c, same below)
>                      + possibly a dual method into_native or into_rust
>
>
There is no standard naming for such FFI pointer ownership transfer. Rust
doesn't know how to release FFI resources, in general.

from_raw(*mut) is the closest, but "raw" is reserved for Rust data pointer
types, not FFI (except for slices, probably because the linear layout is
the same as FFI)

We can consider "from_mut_ptr(*mut)", but this doesn't convey the ownership
transfer as explicitly.

> - from_qemu_none -> cloned_from_foreign
>

You are being creative! This is like "CStr::from_ptr", except we go
directly to higher Rust types.

> - to_qemu_none -> as_foreign or as_foreign_mut
>
>
Similarity with "CString::as_ptr", except that it handles the temporary
stash (CString is the internal storage for String), so we use the
"to_qemu_none().0" described earler.

If we follow your considerations it could be "to_stash().as_ptr()", which
is less user friendly, exposes more internal details, etc.

> - to_qemu_full -> clone_to_foreign
>

There is no equivalent in Rust standard. An implementation may want to
store in an existing location, or allocated in different ways etc.

Closest would be to_ptr()

In summary:

from_qemu_full()  vs  from_mut_ptr()
from_qemu_none()      from_ptr()
to_qemu_full()        to_ptr()
to_qemu_none()        to_stash().as_ptr()

I argue there is more consistency and readability by following the glib-rs
traits and method names (among other advantages by staying close to glib-rs
design for the future).

>
> I will see if I have some time to do some of this work.
>
>
thanks

-- 
Marc-André Lureau