* [PATCH 07/14] rust: qemu-api: add tests for Error bindings
2025-05-30 8:02 [PATCH v2 00/14] rust: bindings for Error Paolo Bonzini
@ 2025-05-30 8:02 ` Paolo Bonzini
0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-05-30 8:02 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-rust
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/error.rs | 104 +++++++++++++++++++++++++++++++++++++
1 file changed, 104 insertions(+)
diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
index 0bdd413a0a2..a91ce6fefaf 100644
--- a/rust/qemu-api/src/error.rs
+++ b/rust/qemu-api/src/error.rs
@@ -297,3 +297,107 @@ unsafe fn cloned_from_foreign(c_error: *const bindings::Error) -> Self {
}
}
}
+
+#[cfg(test)]
+mod tests {
+ use std::ffi::CStr;
+
+ use anyhow::anyhow;
+ use foreign::OwnedPointer;
+
+ use super::*;
+ use crate::{assert_match, bindings};
+
+ #[track_caller]
+ fn error_for_test(msg: &CStr) -> OwnedPointer<Error> {
+ // SAFETY: all arguments are controlled by this function
+ let location = panic::Location::caller();
+ unsafe {
+ let err: *mut c_void = libc::malloc(std::mem::size_of::<bindings::Error>());
+ let err: &mut bindings::Error = &mut *err.cast();
+ *err = bindings::Error {
+ msg: msg.clone_to_foreign_ptr(),
+ err_class: bindings::ERROR_CLASS_GENERIC_ERROR,
+ src_len: location.file().len() as c_int,
+ src: location.file().as_ptr().cast::<c_char>(),
+ line: location.line() as c_int,
+ func: ptr::null_mut(),
+ hint: ptr::null_mut(),
+ };
+ OwnedPointer::new(err)
+ }
+ }
+
+ unsafe fn error_get_pretty<'a>(local_err: *mut bindings::Error) -> &'a CStr {
+ unsafe { CStr::from_ptr(bindings::error_get_pretty(local_err)) }
+ }
+
+ #[test]
+ #[allow(deprecated)]
+ fn test_description() {
+ use std::error::Error;
+
+ assert_eq!(super::Error::from("msg").description(), "msg");
+ assert_eq!(super::Error::from("msg".to_owned()).description(), "msg");
+ }
+
+ #[test]
+ fn test_display() {
+ assert_eq!(&*format!("{}", Error::from("msg")), "msg");
+ assert_eq!(&*format!("{}", Error::from("msg".to_owned())), "msg");
+ assert_eq!(&*format!("{}", Error::from(anyhow!("msg"))), "msg");
+
+ assert_eq!(
+ &*format!("{}", Error::with_error("msg", anyhow!("cause"))),
+ "msg: cause"
+ );
+ }
+
+ #[test]
+ fn test_bool_or_propagate() {
+ unsafe {
+ let mut local_err: *mut bindings::Error = ptr::null_mut();
+
+ assert!(Error::bool_or_propagate(Ok(()), &mut local_err));
+ assert_eq!(local_err, ptr::null_mut());
+
+ let my_err = Error::from("msg");
+ assert!(!Error::bool_or_propagate(Err(my_err), &mut local_err));
+ assert_ne!(local_err, ptr::null_mut());
+ assert_eq!(error_get_pretty(local_err), c"msg");
+ bindings::error_free(local_err);
+ }
+ }
+
+ #[test]
+ fn test_ptr_or_propagate() {
+ unsafe {
+ let mut local_err: *mut bindings::Error = ptr::null_mut();
+
+ let ret = Error::ptr_or_propagate(Ok("abc".to_owned()), &mut local_err);
+ assert_eq!(String::from_foreign(ret), "abc");
+ assert_eq!(local_err, ptr::null_mut());
+
+ let my_err = Error::from("msg");
+ assert_eq!(
+ Error::ptr_or_propagate(Err::<String, _>(my_err), &mut local_err),
+ ptr::null_mut()
+ );
+ assert_ne!(local_err, ptr::null_mut());
+ assert_eq!(error_get_pretty(local_err), c"msg");
+ bindings::error_free(local_err);
+ }
+ }
+
+ #[test]
+ fn test_err_or_unit() {
+ unsafe {
+ let result = Error::err_or_unit(ptr::null_mut());
+ assert_match!(result, Ok(()));
+
+ let err = error_for_test(c"msg");
+ let err = Error::err_or_unit(err.into_inner()).unwrap_err();
+ assert_eq!(&*format!("{err}"), "msg");
+ }
+ }
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 00/14] rust: bindings for Error
@ 2025-06-05 10:15 Paolo Bonzini
2025-06-05 10:15 ` Paolo Bonzini via
` (13 more replies)
0 siblings, 14 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
As explained for v1, the impetus for this series is to remove BqlCell<>
from HPETState::num_timers. However, it's also an important step for QAPI:
error propagation is pretty central for example to QMP, and the series
is also a first example of two-way conversion between C and native-Rust
structs (i.e. not using bindgen-generated structs or their opaque wrappers).
The differences from v2 and v3 are:
* documentation
* correctly using error_propagate()
* panicking on errors with no message and no cause
Based-on: <20250603214523.131185-1-pbonzini@redhat.com>
Paolo Bonzini (13):
subprojects: add the anyhow crate
subprojects: add the foreign crate
util/error: expose Error definition to Rust code
util/error: allow non-NUL-terminated err->src
util/error: make func optional
rust: qemu-api: add bindings to Error
rust: qemu-api: add tests for Error bindings
rust: qdev: support returning errors from realize
rust/hpet: change type of num_timers to usize
hpet: adjust VMState for consistency with Rust version
hpet: return errors from realize if properties are incorrect
rust/hpet: return errors from realize if properties are incorrect
docs: update Rust module status
Zhao Liu (1):
rust/hpet: Drop BqlCell wrapper for num_timers
docs/devel/rust.rst | 7 +-
include/qapi/error-internal.h | 35 ++
rust/wrapper.h | 1 +
hw/timer/hpet.c | 21 +-
util/error.c | 20 +-
rust/Cargo.lock | 17 +
rust/Cargo.toml | 1 +
rust/hw/char/pl011/src/device.rs | 5 +-
rust/hw/timer/hpet/src/device.rs | 62 ++-
rust/hw/timer/hpet/src/fw_cfg.rs | 7 +-
rust/meson.build | 4 +
rust/qemu-api/Cargo.toml | 2 +
rust/qemu-api/meson.build | 3 +-
rust/qemu-api/src/error.rs | 416 ++++++++++++++++++
rust/qemu-api/src/lib.rs | 3 +
rust/qemu-api/src/qdev.rs | 10 +-
scripts/archive-source.sh | 5 +-
scripts/make-release | 5 +-
subprojects/.gitignore | 2 +
subprojects/anyhow-1-rs.wrap | 7 +
subprojects/foreign-0.3-rs.wrap | 7 +
.../packagefiles/anyhow-1.0-rs/meson.build | 33 ++
.../packagefiles/foreign-0.3-rs/meson.build | 26 ++
23 files changed, 628 insertions(+), 71 deletions(-)
create mode 100644 include/qapi/error-internal.h
create mode 100644 rust/qemu-api/src/error.rs
create mode 100644 subprojects/anyhow-1-rs.wrap
create mode 100644 subprojects/foreign-0.3-rs.wrap
create mode 100644 subprojects/packagefiles/anyhow-1.0-rs/meson.build
create mode 100644 subprojects/packagefiles/foreign-0.3-rs/meson.build
--
2.49.0
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 01/14] subprojects: add the anyhow crate
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini via
2025-06-05 10:15 ` [PATCH 02/14] subprojects: add the foreign crate Paolo Bonzini
` (12 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
This is a standard replacement for Box<dyn Error> which is more efficient (it only
occcupies one word) and provides a backtrace of the error. This could be plumbed
into &error_abort in the future.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/meson.build | 2 ++
rust/qemu-api/meson.build | 2 +-
scripts/archive-source.sh | 2 +-
scripts/make-release | 2 +-
subprojects/.gitignore | 1 +
subprojects/anyhow-1-rs.wrap | 7 ++++
.../packagefiles/anyhow-1.0-rs/meson.build | 33 +++++++++++++++++++
7 files changed, 46 insertions(+), 3 deletions(-)
create mode 100644 subprojects/anyhow-1-rs.wrap
create mode 100644 subprojects/packagefiles/anyhow-1.0-rs/meson.build
diff --git a/rust/meson.build b/rust/meson.build
index b1b3315be97..f752a064649 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -1,7 +1,9 @@
+subproject('anyhow-1-rs', required: true)
subproject('bilge-0.2-rs', required: true)
subproject('bilge-impl-0.2-rs', required: true)
subproject('libc-0.2-rs', required: true)
+anyhow_rs = dependency('anyhow-1-rs')
bilge_rs = dependency('bilge-0.2-rs')
bilge_impl_rs = dependency('bilge-impl-0.2-rs')
libc_rs = dependency('libc-0.2-rs')
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index b532281e8c0..da70720e4c1 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -35,7 +35,7 @@ _qemu_api_rs = static_library(
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
rust_args: _qemu_api_cfg,
- dependencies: [libc_rs, qemu_api_macros, qemuutil_rs,
+ dependencies: [anyhow_rs, libc_rs, qemu_api_macros, qemuutil_rs,
qom, hwcore, chardev, migration],
)
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index e461c1531ed..816062fee94 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -27,7 +27,7 @@ sub_file="${sub_tdir}/submodule.tar"
# in their checkout, because the build environment is completely
# different to the host OS.
subprojects="keycodemapdb libvfio-user berkeley-softfloat-3
- berkeley-testfloat-3 arbitrary-int-1-rs bilge-0.2-rs
+ berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/scripts/make-release b/scripts/make-release
index 8c3594a1a47..ea65bdcc0cf 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -40,7 +40,7 @@ fi
# Only include wraps that are invoked with subproject()
SUBPROJECTS="libvfio-user keycodemapdb berkeley-softfloat-3
- berkeley-testfloat-3 arbitrary-int-1-rs bilge-0.2-rs
+ berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index d12d34618cc..b9ae507b85a 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -6,6 +6,7 @@
/keycodemapdb
/libvfio-user
/slirp
+/anyhow-1.0.98
/arbitrary-int-1.2.7
/bilge-0.2.0
/bilge-impl-0.2.0
diff --git a/subprojects/anyhow-1-rs.wrap b/subprojects/anyhow-1-rs.wrap
new file mode 100644
index 00000000000..a69a3645b49
--- /dev/null
+++ b/subprojects/anyhow-1-rs.wrap
@@ -0,0 +1,7 @@
+[wrap-file]
+directory = anyhow-1.0.98
+source_url = https://crates.io/api/v1/crates/anyhow/1.0.98/download
+source_filename = anyhow-1.0.98.tar.gz
+source_hash = e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487
+#method = cargo
+patch_directory = anyhow-1-rs
diff --git a/subprojects/packagefiles/anyhow-1.0-rs/meson.build b/subprojects/packagefiles/anyhow-1.0-rs/meson.build
new file mode 100644
index 00000000000..348bab98b9f
--- /dev/null
+++ b/subprojects/packagefiles/anyhow-1.0-rs/meson.build
@@ -0,0 +1,33 @@
+project('anyhow-1-rs', 'rust',
+ meson_version: '>=1.5.0',
+ version: '1.0.98',
+ license: 'MIT OR Apache-2.0',
+ default_options: [])
+
+rustc = meson.get_compiler('rust')
+
+rust_args = ['--cap-lints', 'allow']
+rust_args += ['--cfg', 'feature="std"']
+if rustc.version().version_compare('<1.65.0')
+ error('rustc version ' + rustc.version() + ' is unsupported. Please upgrade to at least 1.65.0')
+endif
+rust_args += [ '--cfg', 'std_backtrace' ] # >= 1.65.0
+if rustc.version().version_compare('<1.81.0')
+ rust_args += [ '--cfg', 'anyhow_no_core_error' ]
+endif
+
+_anyhow_rs = static_library(
+ 'anyhow',
+ files('src/lib.rs'),
+ gnu_symbol_visibility: 'hidden',
+ override_options: ['rust_std=2018', 'build.rust_std=2018'],
+ rust_abi: 'rust',
+ rust_args: rust_args,
+ dependencies: [],
+)
+
+anyhow_dep = declare_dependency(
+ link_with: _anyhow_rs,
+)
+
+meson.override_dependency('anyhow-1-rs', anyhow_dep)
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 01/14] subprojects: add the anyhow crate
@ 2025-06-05 10:15 ` Paolo Bonzini via
0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini via @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
This is a standard replacement for Box<dyn Error> which is more efficient (it only
occcupies one word) and provides a backtrace of the error. This could be plumbed
into &error_abort in the future.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/meson.build | 2 ++
rust/qemu-api/meson.build | 2 +-
scripts/archive-source.sh | 2 +-
scripts/make-release | 2 +-
subprojects/.gitignore | 1 +
subprojects/anyhow-1-rs.wrap | 7 ++++
.../packagefiles/anyhow-1.0-rs/meson.build | 33 +++++++++++++++++++
7 files changed, 46 insertions(+), 3 deletions(-)
create mode 100644 subprojects/anyhow-1-rs.wrap
create mode 100644 subprojects/packagefiles/anyhow-1.0-rs/meson.build
diff --git a/rust/meson.build b/rust/meson.build
index b1b3315be97..f752a064649 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -1,7 +1,9 @@
+subproject('anyhow-1-rs', required: true)
subproject('bilge-0.2-rs', required: true)
subproject('bilge-impl-0.2-rs', required: true)
subproject('libc-0.2-rs', required: true)
+anyhow_rs = dependency('anyhow-1-rs')
bilge_rs = dependency('bilge-0.2-rs')
bilge_impl_rs = dependency('bilge-impl-0.2-rs')
libc_rs = dependency('libc-0.2-rs')
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index b532281e8c0..da70720e4c1 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -35,7 +35,7 @@ _qemu_api_rs = static_library(
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
rust_args: _qemu_api_cfg,
- dependencies: [libc_rs, qemu_api_macros, qemuutil_rs,
+ dependencies: [anyhow_rs, libc_rs, qemu_api_macros, qemuutil_rs,
qom, hwcore, chardev, migration],
)
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index e461c1531ed..816062fee94 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -27,7 +27,7 @@ sub_file="${sub_tdir}/submodule.tar"
# in their checkout, because the build environment is completely
# different to the host OS.
subprojects="keycodemapdb libvfio-user berkeley-softfloat-3
- berkeley-testfloat-3 arbitrary-int-1-rs bilge-0.2-rs
+ berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/scripts/make-release b/scripts/make-release
index 8c3594a1a47..ea65bdcc0cf 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -40,7 +40,7 @@ fi
# Only include wraps that are invoked with subproject()
SUBPROJECTS="libvfio-user keycodemapdb berkeley-softfloat-3
- berkeley-testfloat-3 arbitrary-int-1-rs bilge-0.2-rs
+ berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index d12d34618cc..b9ae507b85a 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -6,6 +6,7 @@
/keycodemapdb
/libvfio-user
/slirp
+/anyhow-1.0.98
/arbitrary-int-1.2.7
/bilge-0.2.0
/bilge-impl-0.2.0
diff --git a/subprojects/anyhow-1-rs.wrap b/subprojects/anyhow-1-rs.wrap
new file mode 100644
index 00000000000..a69a3645b49
--- /dev/null
+++ b/subprojects/anyhow-1-rs.wrap
@@ -0,0 +1,7 @@
+[wrap-file]
+directory = anyhow-1.0.98
+source_url = https://crates.io/api/v1/crates/anyhow/1.0.98/download
+source_filename = anyhow-1.0.98.tar.gz
+source_hash = e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487
+#method = cargo
+patch_directory = anyhow-1-rs
diff --git a/subprojects/packagefiles/anyhow-1.0-rs/meson.build b/subprojects/packagefiles/anyhow-1.0-rs/meson.build
new file mode 100644
index 00000000000..348bab98b9f
--- /dev/null
+++ b/subprojects/packagefiles/anyhow-1.0-rs/meson.build
@@ -0,0 +1,33 @@
+project('anyhow-1-rs', 'rust',
+ meson_version: '>=1.5.0',
+ version: '1.0.98',
+ license: 'MIT OR Apache-2.0',
+ default_options: [])
+
+rustc = meson.get_compiler('rust')
+
+rust_args = ['--cap-lints', 'allow']
+rust_args += ['--cfg', 'feature="std"']
+if rustc.version().version_compare('<1.65.0')
+ error('rustc version ' + rustc.version() + ' is unsupported. Please upgrade to at least 1.65.0')
+endif
+rust_args += [ '--cfg', 'std_backtrace' ] # >= 1.65.0
+if rustc.version().version_compare('<1.81.0')
+ rust_args += [ '--cfg', 'anyhow_no_core_error' ]
+endif
+
+_anyhow_rs = static_library(
+ 'anyhow',
+ files('src/lib.rs'),
+ gnu_symbol_visibility: 'hidden',
+ override_options: ['rust_std=2018', 'build.rust_std=2018'],
+ rust_abi: 'rust',
+ rust_args: rust_args,
+ dependencies: [],
+)
+
+anyhow_dep = declare_dependency(
+ link_with: _anyhow_rs,
+)
+
+meson.override_dependency('anyhow-1-rs', anyhow_dep)
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 02/14] subprojects: add the foreign crate
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
2025-06-05 10:15 ` Paolo Bonzini via
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 10:15 ` [PATCH 03/14] util/error: expose Error definition to Rust code Paolo Bonzini
` (11 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
This is a cleaned up and separated version of the patches at
https://lore.kernel.org/all/20240701145853.1394967-4-pbonzini@redhat.com/
https://lore.kernel.org/all/20240701145853.1394967-5-pbonzini@redhat.com/
Its first user will be the Error bindings; for example a QEMU Error ** can be
converted to a Rust Option using
unsafe { Option::<Error>::from_foreign(c_error) }
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/meson.build | 2 ++
rust/qemu-api/meson.build | 2 +-
scripts/archive-source.sh | 3 ++-
scripts/make-release | 3 ++-
subprojects/.gitignore | 1 +
subprojects/foreign-0.3-rs.wrap | 7 +++++
.../packagefiles/foreign-0.3-rs/meson.build | 26 +++++++++++++++++++
7 files changed, 41 insertions(+), 3 deletions(-)
create mode 100644 subprojects/foreign-0.3-rs.wrap
create mode 100644 subprojects/packagefiles/foreign-0.3-rs/meson.build
diff --git a/rust/meson.build b/rust/meson.build
index f752a064649..99ae7956cd0 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -1,11 +1,13 @@
subproject('anyhow-1-rs', required: true)
subproject('bilge-0.2-rs', required: true)
subproject('bilge-impl-0.2-rs', required: true)
+subproject('foreign-0.3-rs', required: true)
subproject('libc-0.2-rs', required: true)
anyhow_rs = dependency('anyhow-1-rs')
bilge_rs = dependency('bilge-0.2-rs')
bilge_impl_rs = dependency('bilge-impl-0.2-rs')
+foreign_rs = dependency('foreign-0.3-rs')
libc_rs = dependency('libc-0.2-rs')
subproject('proc-macro2-1-rs', required: true)
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index da70720e4c1..2f0f3b2aae1 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -35,7 +35,7 @@ _qemu_api_rs = static_library(
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
rust_args: _qemu_api_cfg,
- dependencies: [anyhow_rs, libc_rs, qemu_api_macros, qemuutil_rs,
+ dependencies: [anyhow_rs, foreign_rs, libc_rs, qemu_api_macros, qemuutil_rs,
qom, hwcore, chardev, migration],
)
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 816062fee94..035828c532e 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -28,7 +28,8 @@ sub_file="${sub_tdir}/submodule.tar"
# different to the host OS.
subprojects="keycodemapdb libvfio-user berkeley-softfloat-3
berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
- bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
+ bilge-impl-0.2-rs either-1-rs foreign-0.3-rs itertools-0.11-rs
+ libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
sub_deinit=""
diff --git a/scripts/make-release b/scripts/make-release
index ea65bdcc0cf..4509a9fabf5 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -41,7 +41,8 @@ fi
# Only include wraps that are invoked with subproject()
SUBPROJECTS="libvfio-user keycodemapdb berkeley-softfloat-3
berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
- bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
+ bilge-impl-0.2-rs either-1-rs foreign-0.3-rs itertools-0.11-rs
+ libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index b9ae507b85a..f4281934ce1 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -11,6 +11,7 @@
/bilge-0.2.0
/bilge-impl-0.2.0
/either-1.12.0
+/foreign-0.3.1
/itertools-0.11.0
/libc-0.2.162
/proc-macro-error-1.0.4
diff --git a/subprojects/foreign-0.3-rs.wrap b/subprojects/foreign-0.3-rs.wrap
new file mode 100644
index 00000000000..0d218ec2c25
--- /dev/null
+++ b/subprojects/foreign-0.3-rs.wrap
@@ -0,0 +1,7 @@
+[wrap-file]
+directory = foreign-0.3.1
+source_url = https://crates.io/api/v1/crates/foreign/0.3.1/download
+source_filename = foreign-0.3.1.tar.gz
+source_hash = 17ca1b5be8c9d320daf386f1809c7acc0cb09accbae795c2001953fa50585846
+#method = cargo
+patch_directory = foreign-0.3-rs
diff --git a/subprojects/packagefiles/foreign-0.3-rs/meson.build b/subprojects/packagefiles/foreign-0.3-rs/meson.build
new file mode 100644
index 00000000000..0901c02c527
--- /dev/null
+++ b/subprojects/packagefiles/foreign-0.3-rs/meson.build
@@ -0,0 +1,26 @@
+project('foreign-0.3-rs', 'rust',
+ meson_version: '>=1.5.0',
+ version: '0.2.0',
+ license: 'MIT OR Apache-2.0',
+ default_options: [])
+
+subproject('libc-0.2-rs', required: true)
+libc_rs = dependency('libc-0.2-rs')
+
+_foreign_rs = static_library(
+ 'foreign',
+ files('src/lib.rs'),
+ gnu_symbol_visibility: 'hidden',
+ override_options: ['rust_std=2021', 'build.rust_std=2021'],
+ rust_abi: 'rust',
+ rust_args: [
+ '--cap-lints', 'allow',
+ ],
+ dependencies: [libc_rs],
+)
+
+foreign_dep = declare_dependency(
+ link_with: _foreign_rs,
+)
+
+meson.override_dependency('foreign-0.3-rs', foreign_dep)
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 03/14] util/error: expose Error definition to Rust code
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
2025-06-05 10:15 ` Paolo Bonzini via
2025-06-05 10:15 ` [PATCH 02/14] subprojects: add the foreign crate Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 13:31 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 04/14] util/error: allow non-NUL-terminated err->src Paolo Bonzini
` (10 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
This is used to preserve the file and line in a roundtrip from
C Error to Rust and back to C.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qapi/error-internal.h | 26 ++++++++++++++++++++++++++
rust/wrapper.h | 1 +
util/error.c | 10 +---------
3 files changed, 28 insertions(+), 9 deletions(-)
create mode 100644 include/qapi/error-internal.h
diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
new file mode 100644
index 00000000000..d5c3904adec
--- /dev/null
+++ b/include/qapi/error-internal.h
@@ -0,0 +1,26 @@
+/*
+ * QEMU Error Objects - struct definition
+ *
+ * Copyright IBM, Corp. 2011
+ * Copyright (C) 2011-2015 Red Hat, Inc.
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ * Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+
+#ifndef QAPI_ERROR_INTERNAL_H
+
+struct Error
+{
+ char *msg;
+ ErrorClass err_class;
+ const char *src, *func;
+ int line;
+ GString *hint;
+};
+
+#endif
diff --git a/rust/wrapper.h b/rust/wrapper.h
index beddd9aab2f..6060d3ba1ab 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -60,6 +60,7 @@ typedef enum memory_order {
#include "hw/qdev-properties-system.h"
#include "hw/irq.h"
#include "qapi/error.h"
+#include "qapi/error-internal.h"
#include "migration/vmstate.h"
#include "chardev/char-serial.h"
#include "exec/memattrs.h"
diff --git a/util/error.c b/util/error.c
index 673011b89e9..e5bcb7c0225 100644
--- a/util/error.c
+++ b/util/error.c
@@ -15,15 +15,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
-
-struct Error
-{
- char *msg;
- ErrorClass err_class;
- const char *src, *func;
- int line;
- GString *hint;
-};
+#include "qapi/error-internal.h"
Error *error_abort;
Error *error_fatal;
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 04/14] util/error: allow non-NUL-terminated err->src
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (2 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 03/14] util/error: expose Error definition to Rust code Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 11:54 ` Markus Armbruster
2025-06-05 13:32 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 05/14] util/error: make func optional Paolo Bonzini
` (9 subsequent siblings)
13 siblings, 2 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
Rust makes the current file available as a statically-allocated string,
but without a NUL terminator. Allow this by storing an optional maximum
length in the Error.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qapi/error-internal.h | 9 ++++++++-
util/error.c | 5 +++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
index d5c3904adec..1ec3ceb40f0 100644
--- a/include/qapi/error-internal.h
+++ b/include/qapi/error-internal.h
@@ -18,7 +18,14 @@ struct Error
{
char *msg;
ErrorClass err_class;
- const char *src, *func;
+ const char *func;
+
+ /*
+ * src might be NUL-terminated or not. If it is, src_len is negative.
+ * If it is not, src_len is the length.
+ */
+ const char *src;
+ int src_len;
int line;
GString *hint;
};
diff --git a/util/error.c b/util/error.c
index e5bcb7c0225..3449ecc0b92 100644
--- a/util/error.c
+++ b/util/error.c
@@ -24,8 +24,8 @@ Error *error_warn;
static void error_handle(Error **errp, Error *err)
{
if (errp == &error_abort) {
- fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
- err->func, err->src, err->line);
+ fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
+ err->func, err->src_len, err->src, err->line);
error_report("%s", error_get_pretty(err));
if (err->hint) {
error_printf("%s", err->hint->str);
@@ -67,6 +67,7 @@ static void error_setv(Error **errp,
g_free(msg);
}
err->err_class = err_class;
+ err->src_len = -1;
err->src = src;
err->line = line;
err->func = func;
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 05/14] util/error: make func optional
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (3 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 04/14] util/error: allow non-NUL-terminated err->src Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 11:57 ` Markus Armbruster
2025-06-05 10:15 ` [PATCH 06/14] rust: qemu-api: add bindings to Error Paolo Bonzini
` (8 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
The function name is not available in Rust, so make it optional.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qapi/error-internal.h | 2 ++
util/error.c | 9 +++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
index 1ec3ceb40f0..ff18a2086d2 100644
--- a/include/qapi/error-internal.h
+++ b/include/qapi/error-internal.h
@@ -18,6 +18,8 @@ struct Error
{
char *msg;
ErrorClass err_class;
+
+ /* Used for error_abort only, may be NULL. */
const char *func;
/*
diff --git a/util/error.c b/util/error.c
index 3449ecc0b92..daea2142f30 100644
--- a/util/error.c
+++ b/util/error.c
@@ -24,8 +24,13 @@ Error *error_warn;
static void error_handle(Error **errp, Error *err)
{
if (errp == &error_abort) {
- fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
- err->func, err->src_len, err->src, err->line);
+ if (err->func) {
+ fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
+ err->func, err->src_len, err->src, err->line);
+ } else {
+ fprintf(stderr, "Unexpected error at %.*s:%d:\n",
+ err->src_len, err->src, err->line);
+ }
error_report("%s", error_get_pretty(err));
if (err->hint) {
error_printf("%s", err->hint->str);
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 06/14] rust: qemu-api: add bindings to Error
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (4 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 05/14] util/error: make func optional Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 12:06 ` Markus Armbruster
2025-06-05 13:45 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 07/14] rust: qemu-api: add tests for Error bindings Paolo Bonzini
` (7 subsequent siblings)
13 siblings, 2 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
Provide an implementation of std::error::Error that bridges the Rust
anyhow::Error and std::panic::Location types with QEMU's Error*.
It also has several utility methods, analogous to error_propagate(),
that convert a Result into a return value + Error** pair. One important
difference is that these propagation methods *panic* if *errp is NULL,
unlike error_propagate() which eats subsequent errors[1]. The reason
for this is that in C you have an error_set*() call at the site where
the error is created, and calls to error_propagate() are relatively rare.
In Rust instead, even though these functions do "propagate" a
qemu_api::Error into a C Error**, there is no error_setg() anywhere that
could check for non-NULL errp and call abort(). error_propagate()'s
behavior of ignoring subsequent errors is generally considered weird,
and there would be a bigger risk of triggering it from Rust code.
[1] This is actually a violation of the preconditions of error_propagate(),
so it should not happen. But you never know...
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 5 +
rust/Cargo.lock | 17 ++
rust/Cargo.toml | 1 +
rust/qemu-api/Cargo.toml | 2 +
rust/qemu-api/meson.build | 1 +
rust/qemu-api/src/error.rs | 312 +++++++++++++++++++++++++++++++++++++
rust/qemu-api/src/lib.rs | 3 +
7 files changed, 341 insertions(+)
create mode 100644 rust/qemu-api/src/error.rs
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 34d9c7945b7..98803d5e8dd 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -96,6 +96,11 @@ are missing:
architecture (VMState). Right now, VMState lacks type safety because
it is hard to place the ``VMStateField`` definitions in traits.
+* NUL-terminated file names with ``#[track_caller]`` are scheduled for
+ inclusion as `#![feature(location_file_nul)]`, but it will be a while
+ before QEMU can use them. For now, there is special code in
+ ``util/error.c`` to support non-NUL-terminated file names.
+
* associated const equality would be nice to have for some users of
``callbacks::FnCall``, but is still experimental. ``ASSERT_IS_SOME``
replaces it.
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index bccfe855a70..b785c718f31 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -2,6 +2,12 @@
# It is not intended for manual editing.
version = 3
+[[package]]
+name = "anyhow"
+version = "1.0.98"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487"
+
[[package]]
name = "arbitrary-int"
version = "1.2.7"
@@ -44,6 +50,15 @@ version = "1.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
+[[package]]
+name = "foreign"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "17ca1b5be8c9d320daf386f1809c7acc0cb09accbae795c2001953fa50585846"
+dependencies = [
+ "libc",
+]
+
[[package]]
name = "hpet"
version = "0.1.0"
@@ -114,6 +129,8 @@ dependencies = [
name = "qemu_api"
version = "0.1.0"
dependencies = [
+ "anyhow",
+ "foreign",
"libc",
"qemu_api_macros",
]
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index fd4c2fbf84b..0868e1b4268 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -67,6 +67,7 @@ missing_safety_doc = "deny"
mut_mut = "deny"
needless_bitwise_bool = "deny"
needless_pass_by_ref_mut = "deny"
+needless_update = "deny"
no_effect_underscore_binding = "deny"
option_option = "deny"
or_fun_call = "deny"
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index c96cf50e7a1..db7000dee44 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -15,7 +15,9 @@ rust-version.workspace = true
[dependencies]
qemu_api_macros = { path = "../qemu-api-macros" }
+anyhow = "~1.0"
libc = "0.2.162"
+foreign = "~0.3.1"
[features]
default = ["debug_cell"]
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 2f0f3b2aae1..cac8595a148 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -19,6 +19,7 @@ _qemu_api_rs = static_library(
'src/cell.rs',
'src/chardev.rs',
'src/errno.rs',
+ 'src/error.rs',
'src/irq.rs',
'src/memory.rs',
'src/module.rs',
diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
new file mode 100644
index 00000000000..80157f6ea1b
--- /dev/null
+++ b/rust/qemu-api/src/error.rs
@@ -0,0 +1,312 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Error propagation for QEMU Rust code
+//!
+//! This module contains [`Error`], the bridge between Rust errors and
+//! [`Result`](std::result::Result)s and QEMU's C [`Error`](bindings::Error)
+//! struct.
+//!
+//! For FFI code, [`Error`] provides functions to simplify conversion between
+//! the Rust ([`Result<>`](std::result::Result)) and C (`Error**`) conventions:
+//!
+//! * [`ok_or_propagate`](crate::Error::ok_or_propagate),
+//! [`bool_or_propagate`](crate::Error::bool_or_propagate),
+//! [`ptr_or_propagate`](crate::Error::ptr_or_propagate) can be used to build
+//! a C return value while also propagating an error condition
+//!
+//! * [`err_or_else`](crate::Error::err_or_else) and
+//! [`err_or_unit`](crate::Error::err_or_unit) can be used to build a `Result`
+//!
+//! This module is most commonly used at the boundary between C and Rust code;
+//! other code will usually access it through the
+//! [`qemu_api::Result`](crate::Result) type alias, and will use the
+//! [`std::error::Error`] interface to let C errors participate in Rust's error
+//! handling functionality.
+//!
+//! Rust code can also create use this module to create an error object that
+//! will be passed up to C code, though in most cases this will be done
+//! transparently through the `?` operator. Errors can be constructed from a
+//! simple error string, from an [`anyhow::Error`] to pass any other Rust error
+//! type up to C code, or from a combination of the two.
+//!
+//! The third case, corresponding to [`Error::with_error`], is the only one that
+//! requires mentioning [`qemu_api::Error`](crate::Error) explicitly. Similar
+//! to how QEMU's C code handles errno values, the string and the
+//! `anyhow::Error` object will be concatenated with `:` as the separator.
+
+use std::{
+ borrow::Cow,
+ ffi::{c_char, c_int, c_void, CStr},
+ fmt::{self, Display},
+ panic, ptr,
+};
+
+use foreign::{prelude::*, OwnedPointer};
+
+use crate::bindings;
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+#[derive(Debug)]
+pub struct Error {
+ msg: Option<Cow<'static, str>>,
+ /// Appends the print string of the error to the msg if not None
+ cause: Option<anyhow::Error>,
+ file: &'static str,
+ line: u32,
+}
+
+impl std::error::Error for Error {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ self.cause.as_ref().map(AsRef::as_ref)
+ }
+
+ #[allow(deprecated)]
+ fn description(&self) -> &str {
+ self.msg
+ .as_deref()
+ .or_else(|| self.cause.as_deref().map(std::error::Error::description))
+ .expect("no message nor cause?")
+ }
+}
+
+impl Display for Error {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ let mut prefix = "";
+ if let Some(ref msg) = self.msg {
+ write!(f, "{msg}")?;
+ prefix = ": ";
+ }
+ if let Some(ref cause) = self.cause {
+ write!(f, "{prefix}{cause}")?;
+ } else if prefix.is_empty() {
+ panic!("no message nor cause?");
+ }
+ Ok(())
+ }
+}
+
+impl From<String> for Error {
+ #[track_caller]
+ fn from(msg: String) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: Some(Cow::Owned(msg)),
+ cause: None,
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+}
+
+impl From<&'static str> for Error {
+ #[track_caller]
+ fn from(msg: &'static str) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: Some(Cow::Borrowed(msg)),
+ cause: None,
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+}
+
+impl From<anyhow::Error> for Error {
+ #[track_caller]
+ fn from(error: anyhow::Error) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: None,
+ cause: Some(error),
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+}
+
+impl Error {
+ /// Create a new error, prepending `msg` to the
+ /// description of `cause`
+ #[track_caller]
+ pub fn with_error(msg: impl Into<Cow<'static, str>>, cause: impl Into<anyhow::Error>) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: Some(msg.into()),
+ cause: Some(cause.into()),
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+
+ /// Consume a result, returning `false` if it is an error and
+ /// `true` if it is successful. The error is propagated into
+ /// `errp` like the C API `error_propagate` would do.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`;
+ /// typically it is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ pub unsafe fn bool_or_propagate(result: Result<()>, errp: *mut *mut bindings::Error) -> bool {
+ // SAFETY: caller guarantees errp is valid
+ unsafe { Self::ok_or_propagate(result, errp) }.is_some()
+ }
+
+ /// Consume a result, returning a `NULL` pointer if it is an error and
+ /// a C representation of the contents if it is successful. This is
+ /// similar to the C API `error_propagate`, but it panics if `*errp`
+ /// is not `NULL`.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`;
+ /// typically it is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ ///
+ /// See [`propagate`](Error::propagate) for more information.
+ #[must_use]
+ pub unsafe fn ptr_or_propagate<T: CloneToForeign>(
+ result: Result<T>,
+ errp: *mut *mut bindings::Error,
+ ) -> *mut T::Foreign {
+ // SAFETY: caller guarantees errp is valid
+ unsafe { Self::ok_or_propagate(result, errp) }.clone_to_foreign_ptr()
+ }
+
+ /// Consume a result in the same way as `self.ok()`, but also propagate
+ /// a possible error into `errp`. This is similar to the C API
+ /// `error_propagate`, but it panics if `*errp` is not `NULL`.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`;
+ /// typically it is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ ///
+ /// See [`propagate`](Error::propagate) for more information.
+ pub unsafe fn ok_or_propagate<T>(
+ result: Result<T>,
+ errp: *mut *mut bindings::Error,
+ ) -> Option<T> {
+ result.map_err(|err| unsafe { err.propagate(errp) }).ok()
+ }
+
+ /// Equivalent of the C function `error_propagate`. Fill `*errp`
+ /// with the information container in `self` if `errp` is not NULL;
+ /// then consume it.
+ ///
+ /// This is similar to the C API `error_propagate`, but it panics if
+ /// `*errp` is not `NULL`.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`; it can be
+ /// `NULL` or it can point to any of:
+ /// * `error_abort`
+ /// * `error_fatal`
+ /// * a local variable of (C) type `Error *`
+ ///
+ /// Typically `errp` is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ pub unsafe fn propagate(self, errp: *mut *mut bindings::Error) {
+ if errp.is_null() {
+ return;
+ }
+
+ // SAFETY: caller guarantees that errp and *errp are valid
+ unsafe {
+ assert_eq!(*errp, ptr::null_mut());
+ bindings::error_propagate(errp, self.clone_to_foreign_ptr());
+ }
+ }
+
+ /// Convert a C `Error*` into a Rust `Result`, using
+ /// `Ok(())` if `c_error` is NULL. Free the `Error*`.
+ ///
+ /// # Safety
+ ///
+ /// `c_error` must be `NULL` or valid; typically it was initialized
+ /// with `ptr::null_mut()` and passed by reference to a C function.
+ pub unsafe fn err_or_unit(c_error: *mut bindings::Error) -> Result<()> {
+ // SAFETY: caller guarantees c_error is valid
+ unsafe { Self::err_or_else(c_error, || ()) }
+ }
+
+ /// Convert a C `Error*` into a Rust `Result`, calling `f()` to
+ /// obtain an `Ok` value if `c_error` is NULL. Free the `Error*`.
+ ///
+ /// # Safety
+ ///
+ /// `c_error` must be `NULL` or point to a valid C [`struct
+ /// Error`](bindings::Error); typically it was initialized with
+ /// `ptr::null_mut()` and passed by reference to a C function.
+ pub unsafe fn err_or_else<T, F: FnOnce() -> T>(
+ c_error: *mut bindings::Error,
+ f: F,
+ ) -> Result<T> {
+ // SAFETY: caller guarantees c_error is valid
+ let err = unsafe { Option::<Self>::from_foreign(c_error) };
+ match err {
+ None => Ok(f()),
+ Some(err) => Err(err),
+ }
+ }
+}
+
+impl FreeForeign for Error {
+ type Foreign = bindings::Error;
+
+ unsafe fn free_foreign(p: *mut bindings::Error) {
+ // SAFETY: caller guarantees p is valid
+ unsafe {
+ bindings::error_free(p);
+ }
+ }
+}
+
+impl CloneToForeign for Error {
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ // SAFETY: all arguments are controlled by this function
+ unsafe {
+ let err: *mut c_void = libc::malloc(std::mem::size_of::<bindings::Error>());
+ let err: &mut bindings::Error = &mut *err.cast();
+ *err = bindings::Error {
+ msg: format!("{self}").clone_to_foreign_ptr(),
+ err_class: bindings::ERROR_CLASS_GENERIC_ERROR,
+ src_len: self.file.len() as c_int,
+ src: self.file.as_ptr().cast::<c_char>(),
+ line: self.line as c_int,
+ func: ptr::null_mut(),
+ hint: ptr::null_mut(),
+ };
+ OwnedPointer::new(err)
+ }
+ }
+}
+
+impl FromForeign for Error {
+ unsafe fn cloned_from_foreign(c_error: *const bindings::Error) -> Self {
+ // SAFETY: caller guarantees c_error is valid
+ unsafe {
+ let error = &*c_error;
+ let file = if error.src_len < 0 {
+ // NUL-terminated
+ CStr::from_ptr(error.src).to_str()
+ } else {
+ // Can become str::from_utf8 with Rust 1.87.0
+ std::str::from_utf8(std::slice::from_raw_parts(
+ &*error.src.cast::<u8>(),
+ error.src_len as usize,
+ ))
+ };
+
+ Error {
+ msg: FromForeign::cloned_from_foreign(error.msg),
+ cause: None,
+ file: file.unwrap(),
+ line: error.line as u32,
+ }
+ }
+ }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 234a94e3c29..93902fc94bc 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -19,6 +19,7 @@
pub mod cell;
pub mod chardev;
pub mod errno;
+pub mod error;
pub mod irq;
pub mod memory;
pub mod module;
@@ -34,6 +35,8 @@
ffi::c_void,
};
+pub use error::{Error, Result};
+
#[cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)]
extern "C" {
fn g_aligned_alloc0(
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 07/14] rust: qemu-api: add tests for Error bindings
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (5 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 06/14] rust: qemu-api: add bindings to Error Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 13:53 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 08/14] rust: qdev: support returning errors from realize Paolo Bonzini
` (6 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/error.rs | 104 +++++++++++++++++++++++++++++++++++++
1 file changed, 104 insertions(+)
diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
index 80157f6ea1b..e114fc4178b 100644
--- a/rust/qemu-api/src/error.rs
+++ b/rust/qemu-api/src/error.rs
@@ -310,3 +310,107 @@ unsafe fn cloned_from_foreign(c_error: *const bindings::Error) -> Self {
}
}
}
+
+#[cfg(test)]
+mod tests {
+ use std::ffi::CStr;
+
+ use anyhow::anyhow;
+ use foreign::OwnedPointer;
+
+ use super::*;
+ use crate::{assert_match, bindings};
+
+ #[track_caller]
+ fn error_for_test(msg: &CStr) -> OwnedPointer<Error> {
+ // SAFETY: all arguments are controlled by this function
+ let location = panic::Location::caller();
+ unsafe {
+ let err: *mut c_void = libc::malloc(std::mem::size_of::<bindings::Error>());
+ let err: &mut bindings::Error = &mut *err.cast();
+ *err = bindings::Error {
+ msg: msg.clone_to_foreign_ptr(),
+ err_class: bindings::ERROR_CLASS_GENERIC_ERROR,
+ src_len: location.file().len() as c_int,
+ src: location.file().as_ptr().cast::<c_char>(),
+ line: location.line() as c_int,
+ func: ptr::null_mut(),
+ hint: ptr::null_mut(),
+ };
+ OwnedPointer::new(err)
+ }
+ }
+
+ unsafe fn error_get_pretty<'a>(local_err: *mut bindings::Error) -> &'a CStr {
+ unsafe { CStr::from_ptr(bindings::error_get_pretty(local_err)) }
+ }
+
+ #[test]
+ #[allow(deprecated)]
+ fn test_description() {
+ use std::error::Error;
+
+ assert_eq!(super::Error::from("msg").description(), "msg");
+ assert_eq!(super::Error::from("msg".to_owned()).description(), "msg");
+ }
+
+ #[test]
+ fn test_display() {
+ assert_eq!(&*format!("{}", Error::from("msg")), "msg");
+ assert_eq!(&*format!("{}", Error::from("msg".to_owned())), "msg");
+ assert_eq!(&*format!("{}", Error::from(anyhow!("msg"))), "msg");
+
+ assert_eq!(
+ &*format!("{}", Error::with_error("msg", anyhow!("cause"))),
+ "msg: cause"
+ );
+ }
+
+ #[test]
+ fn test_bool_or_propagate() {
+ unsafe {
+ let mut local_err: *mut bindings::Error = ptr::null_mut();
+
+ assert!(Error::bool_or_propagate(Ok(()), &mut local_err));
+ assert_eq!(local_err, ptr::null_mut());
+
+ let my_err = Error::from("msg");
+ assert!(!Error::bool_or_propagate(Err(my_err), &mut local_err));
+ assert_ne!(local_err, ptr::null_mut());
+ assert_eq!(error_get_pretty(local_err), c"msg");
+ bindings::error_free(local_err);
+ }
+ }
+
+ #[test]
+ fn test_ptr_or_propagate() {
+ unsafe {
+ let mut local_err: *mut bindings::Error = ptr::null_mut();
+
+ let ret = Error::ptr_or_propagate(Ok("abc".to_owned()), &mut local_err);
+ assert_eq!(String::from_foreign(ret), "abc");
+ assert_eq!(local_err, ptr::null_mut());
+
+ let my_err = Error::from("msg");
+ assert_eq!(
+ Error::ptr_or_propagate(Err::<String, _>(my_err), &mut local_err),
+ ptr::null_mut()
+ );
+ assert_ne!(local_err, ptr::null_mut());
+ assert_eq!(error_get_pretty(local_err), c"msg");
+ bindings::error_free(local_err);
+ }
+ }
+
+ #[test]
+ fn test_err_or_unit() {
+ unsafe {
+ let result = Error::err_or_unit(ptr::null_mut());
+ assert_match!(result, Ok(()));
+
+ let err = error_for_test(c"msg");
+ let err = Error::err_or_unit(err.into_inner()).unwrap_err();
+ assert_eq!(&*format!("{err}"), "msg");
+ }
+ }
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 08/14] rust: qdev: support returning errors from realize
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (6 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 07/14] rust: qemu-api: add tests for Error bindings Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 10:15 ` [PATCH 09/14] rust/hpet: change type of num_timers to usize Paolo Bonzini
` (5 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 5 +++--
rust/hw/timer/hpet/src/device.rs | 5 +++--
rust/qemu-api/src/qdev.rs | 10 ++++++----
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 0501fa5be9c..be8387f6f2d 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -175,7 +175,7 @@ fn properties() -> &'static [Property] {
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&device_class::VMSTATE_PL011)
}
- const REALIZE: Option<fn(&Self)> = Some(Self::realize);
+ const REALIZE: Option<fn(&Self) -> qemu_api::Result<()>> = Some(Self::realize);
}
impl ResettablePhasesImpl for PL011State {
@@ -619,9 +619,10 @@ fn event(&self, event: Event) {
}
}
- fn realize(&self) {
+ fn realize(&self) -> qemu_api::Result<()> {
self.char_backend
.enable_handlers(self, Self::can_receive, Self::receive, Self::event);
+ Ok(())
}
fn reset_hold(&self, _type: ResetType) {
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index e3ba62b2875..68c82b09b60 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -724,7 +724,7 @@ fn post_init(&self) {
}
}
- fn realize(&self) {
+ fn realize(&self) -> qemu_api::Result<()> {
if self.int_route_cap == 0 {
// TODO: Add error binding: warn_report()
println!("Hpet's hpet-intcap property not initialized");
@@ -751,6 +751,7 @@ fn realize(&self) {
self.init_gpio_in(2, HPETState::handle_legacy_irq);
self.init_gpio_out(from_ref(&self.pit_enabled));
+ Ok(())
}
fn reset_hold(&self, _type: ResetType) {
@@ -1042,7 +1043,7 @@ fn vmsd() -> Option<&'static VMStateDescription> {
Some(&VMSTATE_HPET)
}
- const REALIZE: Option<fn(&Self)> = Some(Self::realize);
+ const REALIZE: Option<fn(&Self) -> qemu_api::Result<()>> = Some(Self::realize);
}
impl ResettablePhasesImpl for HPETState {
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 1279d7a58d5..81052097071 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -12,10 +12,11 @@
pub use bindings::{ClockEvent, DeviceClass, Property, ResetType};
use crate::{
- bindings::{self, qdev_init_gpio_in, qdev_init_gpio_out, Error, ResettableClass},
+ bindings::{self, qdev_init_gpio_in, qdev_init_gpio_out, ResettableClass},
callbacks::FnCall,
cell::{bql_locked, Opaque},
chardev::Chardev,
+ error::{Error, Result},
irq::InterruptSource,
prelude::*,
qom::{ObjectClass, ObjectImpl, Owned},
@@ -108,7 +109,7 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
///
/// If not `None`, the parent class's `realize` method is overridden
/// with the function pointed to by `REALIZE`.
- const REALIZE: Option<fn(&Self)> = None;
+ const REALIZE: Option<fn(&Self) -> Result<()>> = None;
/// An array providing the properties that the user can set on the
/// device. Not a `const` because referencing statics in constants
@@ -134,10 +135,11 @@ fn vmsd() -> Option<&'static VMStateDescription> {
/// readable/writeable from one thread at any time.
unsafe extern "C" fn rust_realize_fn<T: DeviceImpl>(
dev: *mut bindings::DeviceState,
- _errp: *mut *mut Error,
+ errp: *mut *mut bindings::Error,
) {
let state = NonNull::new(dev).unwrap().cast::<T>();
- T::REALIZE.unwrap()(unsafe { state.as_ref() });
+ let result = T::REALIZE.unwrap()(unsafe { state.as_ref() });
+ unsafe { Error::ok_or_propagate(result, errp); }
}
unsafe impl InterfaceType for ResettableClass {
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 09/14] rust/hpet: change type of num_timers to usize
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (7 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 08/14] rust: qdev: support returning errors from realize Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 10:15 ` [PATCH 10/14] hpet: adjust VMState for consistency with Rust version Paolo Bonzini
` (4 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
Remove the need to convert after every read of the BqlCell. Because the
vmstate uses a u8 as the size of the VARRAY, this requires switching
the VARRAY to use num_timers_save; which in turn requires ensuring that
the num_timers_save is always there. For simplicity do this by
removing support for version 1, which QEMU has not been producing for
~15 years.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/device.rs | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index 68c82b09b60..a957de1e767 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -12,7 +12,7 @@
use qemu_api::{
bindings::{
address_space_memory, address_space_stl_le, qdev_prop_bit, qdev_prop_bool,
- qdev_prop_uint32, qdev_prop_uint8,
+ qdev_prop_uint32, qdev_prop_usize,
},
cell::{BqlCell, BqlRefCell},
irq::InterruptSource,
@@ -36,9 +36,9 @@
const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
/// Minimum recommended hardware implementation.
-const HPET_MIN_TIMERS: u8 = 3;
+const HPET_MIN_TIMERS: usize = 3;
/// Maximum timers in each timer block.
-const HPET_MAX_TIMERS: u8 = 32;
+const HPET_MAX_TIMERS: usize = 32;
/// Flags that HPETState.flags supports.
const HPET_FLAG_MSI_SUPPORT_SHIFT: usize = 0;
@@ -561,8 +561,8 @@ pub struct HPETState {
/// HPET timer array managed by this timer block.
#[doc(alias = "timer")]
- timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS as usize],
- num_timers: BqlCell<u8>,
+ timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
+ num_timers: BqlCell<usize>,
num_timers_save: BqlCell<u8>,
/// Instance id (HPET timer block ID).
@@ -572,7 +572,7 @@ pub struct HPETState {
impl HPETState {
// Get num_timers with `usize` type, which is useful to play with array index.
fn get_num_timers(&self) -> usize {
- self.num_timers.get().into()
+ self.num_timers.get()
}
const fn has_msi_flag(&self) -> bool {
@@ -854,7 +854,7 @@ fn pre_save(&self) -> i32 {
* also added to the migration stream. Check that it matches the value
* that was configured.
*/
- self.num_timers_save.set(self.num_timers.get());
+ self.num_timers_save.set(self.num_timers.get() as u8);
0
}
@@ -884,7 +884,7 @@ fn is_offset_needed(&self) -> bool {
}
fn validate_num_timers(&self, _version_id: u8) -> bool {
- self.num_timers.get() == self.num_timers_save.get()
+ self.num_timers.get() == self.num_timers_save.get().into()
}
}
@@ -911,7 +911,7 @@ impl ObjectImpl for HPETState {
c"timers",
HPETState,
num_timers,
- unsafe { &qdev_prop_uint8 },
+ unsafe { &qdev_prop_usize },
u8,
default = HPET_MIN_TIMERS
),
@@ -1016,16 +1016,16 @@ impl ObjectImpl for HPETState {
static VMSTATE_HPET: VMStateDescription = VMStateDescription {
name: c"hpet".as_ptr(),
version_id: 2,
- minimum_version_id: 1,
+ minimum_version_id: 2,
pre_save: Some(hpet_pre_save),
post_load: Some(hpet_post_load),
fields: vmstate_fields! {
vmstate_of!(HPETState, config),
vmstate_of!(HPETState, int_status),
vmstate_of!(HPETState, counter),
- vmstate_of!(HPETState, num_timers_save).with_version_id(2),
+ vmstate_of!(HPETState, num_timers_save),
vmstate_validate!(HPETState, VALIDATE_TIMERS_NAME, HPETState::validate_num_timers),
- vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
+ vmstate_struct!(HPETState, timers[0 .. num_timers_save], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
},
subsections: vmstate_subsections! {
VMSTATE_HPET_RTC_IRQ_LEVEL,
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 10/14] hpet: adjust VMState for consistency with Rust version
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (8 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 09/14] rust/hpet: change type of num_timers to usize Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 10:15 ` [PATCH 11/14] hpet: return errors from realize if properties are incorrect Paolo Bonzini
` (3 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
No functional change intended.
Suggested-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/timer/hpet.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 0fd1337a156..9db027cf76f 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -328,16 +328,16 @@ static const VMStateDescription vmstate_hpet_timer = {
static const VMStateDescription vmstate_hpet = {
.name = "hpet",
.version_id = 2,
- .minimum_version_id = 1,
+ .minimum_version_id = 2,
.pre_save = hpet_pre_save,
.post_load = hpet_post_load,
.fields = (const VMStateField[]) {
VMSTATE_UINT64(config, HPETState),
VMSTATE_UINT64(isr, HPETState),
VMSTATE_UINT64(hpet_counter, HPETState),
- VMSTATE_UINT8_V(num_timers_save, HPETState, 2),
+ VMSTATE_UINT8(num_timers_save, HPETState),
VMSTATE_VALIDATE("num_timers must match", hpet_validate_num_timers),
- VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
+ VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers_save, 0,
vmstate_hpet_timer, HPETTimer),
VMSTATE_END_OF_LIST()
},
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 11/14] hpet: return errors from realize if properties are incorrect
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (9 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 10/14] hpet: adjust VMState for consistency with Rust version Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 10:15 ` [PATCH 12/14] rust/hpet: " Paolo Bonzini
` (2 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
Do not silently adjust num_timers, and fail if intcap is 0.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/timer/hpet.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 9db027cf76f..cb48cc151f1 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -691,8 +691,14 @@ static void hpet_realize(DeviceState *dev, Error **errp)
int i;
HPETTimer *timer;
+ if (s->num_timers < HPET_MIN_TIMERS || s->num_timers > HPET_MAX_TIMERS) {
+ error_setg(errp, "hpet.num_timers must be between %d and %d",
+ HPET_MIN_TIMERS, HPET_MAX_TIMERS);
+ return;
+ }
if (!s->intcap) {
- warn_report("Hpet's intcap not initialized");
+ error_setg(errp, "hpet.hpet-intcap not initialized");
+ return;
}
if (hpet_fw_cfg.count == UINT8_MAX) {
/* first instance */
@@ -700,7 +706,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
}
if (hpet_fw_cfg.count == 8) {
- error_setg(errp, "Only 8 instances of HPET is allowed");
+ error_setg(errp, "Only 8 instances of HPET are allowed");
return;
}
@@ -710,11 +716,6 @@ static void hpet_realize(DeviceState *dev, Error **errp)
sysbus_init_irq(sbd, &s->irqs[i]);
}
- if (s->num_timers < HPET_MIN_TIMERS) {
- s->num_timers = HPET_MIN_TIMERS;
- } else if (s->num_timers > HPET_MAX_TIMERS) {
- s->num_timers = HPET_MAX_TIMERS;
- }
for (i = 0; i < HPET_MAX_TIMERS; i++) {
timer = &s->timer[i];
timer->qemu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, hpet_timer, timer);
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 12/14] rust/hpet: return errors from realize if properties are incorrect
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (10 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 11/14] hpet: return errors from realize if properties are incorrect Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 10:15 ` [PATCH 13/14] rust/hpet: Drop BqlCell wrapper for num_timers Paolo Bonzini
2025-06-05 10:15 ` [PATCH 14/14] docs: update Rust module status Paolo Bonzini
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
Match the code in hpet.c; this also allows removing the
BqlCell from the num_timers field.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/device.rs | 16 +++++++---------
rust/hw/timer/hpet/src/fw_cfg.rs | 7 +++----
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index a957de1e767..cd439f90b7e 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -725,18 +725,16 @@ fn post_init(&self) {
}
fn realize(&self) -> qemu_api::Result<()> {
+ if self.num_timers.get() < HPET_MIN_TIMERS || self.num_timers.get() > HPET_MAX_TIMERS {
+ Err(format!(
+ "hpet.num_timers must be between {HPET_MIN_TIMERS} and {HPET_MAX_TIMERS}"
+ ))?;
+ }
if self.int_route_cap == 0 {
- // TODO: Add error binding: warn_report()
- println!("Hpet's hpet-intcap property not initialized");
+ Err("hpet.hpet-intcap property not initialized")?;
}
- self.hpet_id.set(HPETFwConfig::assign_hpet_id());
-
- if self.num_timers.get() < HPET_MIN_TIMERS {
- self.num_timers.set(HPET_MIN_TIMERS);
- } else if self.num_timers.get() > HPET_MAX_TIMERS {
- self.num_timers.set(HPET_MAX_TIMERS);
- }
+ self.hpet_id.set(HPETFwConfig::assign_hpet_id()?);
self.init_timer();
// 64-bit General Capabilities and ID Register; LegacyReplacementRoute.
diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
index 6c10316104c..619d662ee1e 100644
--- a/rust/hw/timer/hpet/src/fw_cfg.rs
+++ b/rust/hw/timer/hpet/src/fw_cfg.rs
@@ -36,7 +36,7 @@ unsafe impl Zeroable for HPETFwConfig {}
};
impl HPETFwConfig {
- pub(crate) fn assign_hpet_id() -> usize {
+ pub(crate) fn assign_hpet_id() -> Result<usize, &'static str> {
assert!(bql_locked());
// SAFETY: all accesses go through these methods, which guarantee
// that the accesses are protected by the BQL.
@@ -48,13 +48,12 @@ pub(crate) fn assign_hpet_id() -> usize {
}
if fw_cfg.count == 8 {
- // TODO: Add error binding: error_setg()
- panic!("Only 8 instances of HPET is allowed");
+ Err("Only 8 instances of HPET are allowed")?;
}
let id: usize = fw_cfg.count.into();
fw_cfg.count += 1;
- id
+ Ok(id)
}
pub(crate) fn update_hpet_cfg(hpet_id: usize, timer_block_id: u32, address: u64) {
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 13/14] rust/hpet: Drop BqlCell wrapper for num_timers
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (11 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 12/14] rust/hpet: " Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
2025-06-05 10:15 ` [PATCH 14/14] docs: update Rust module status Paolo Bonzini
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
From: Zhao Liu <zhao1.liu@intel.com>
Now that the num_timers field is initialized as a property, someone may
change its default value using qdev_prop_set_uint8(), but the value is
fixed after the Rust code sees it first. Since there is no need to modify
it after realize(), it is not to be necessary to have a BqlCell wrapper.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250520152750.2542612-4-zhao1.liu@intel.com
[Remove .into() as well. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/device.rs | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index cd439f90b7e..735b2fbef79 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -562,7 +562,7 @@ pub struct HPETState {
/// HPET timer array managed by this timer block.
#[doc(alias = "timer")]
timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
- num_timers: BqlCell<usize>,
+ num_timers: usize,
num_timers_save: BqlCell<u8>,
/// Instance id (HPET timer block ID).
@@ -570,11 +570,6 @@ pub struct HPETState {
}
impl HPETState {
- // Get num_timers with `usize` type, which is useful to play with array index.
- fn get_num_timers(&self) -> usize {
- self.num_timers.get()
- }
-
const fn has_msi_flag(&self) -> bool {
self.flags & (1 << HPET_FLAG_MSI_SUPPORT_SHIFT) != 0
}
@@ -636,7 +631,7 @@ fn set_cfg_reg(&self, shift: u32, len: u32, val: u64) {
self.hpet_offset
.set(ticks_to_ns(self.counter.get()) - CLOCK_VIRTUAL.get_ns());
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
let mut t = timer.borrow_mut();
if t.is_int_enabled() && t.is_int_active() {
@@ -648,7 +643,7 @@ fn set_cfg_reg(&self, shift: u32, len: u32, val: u64) {
// Halt main counter and disable interrupt generation.
self.counter.set(self.get_ticks());
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
timer.borrow_mut().del_timer();
}
}
@@ -671,7 +666,7 @@ fn set_int_status_reg(&self, shift: u32, _len: u32, val: u64) {
let new_val = val << shift;
let cleared = new_val & self.int_status.get();
- for (index, timer) in self.timers.iter().take(self.get_num_timers()).enumerate() {
+ for (index, timer) in self.timers.iter().take(self.num_timers).enumerate() {
if cleared & (1 << index) != 0 {
timer.borrow_mut().update_irq(false);
}
@@ -725,7 +720,7 @@ fn post_init(&self) {
}
fn realize(&self) -> qemu_api::Result<()> {
- if self.num_timers.get() < HPET_MIN_TIMERS || self.num_timers.get() > HPET_MAX_TIMERS {
+ if self.num_timers < HPET_MIN_TIMERS || self.num_timers > HPET_MAX_TIMERS {
Err(format!(
"hpet.num_timers must be between {HPET_MIN_TIMERS} and {HPET_MAX_TIMERS}"
))?;
@@ -743,7 +738,7 @@ fn realize(&self) -> qemu_api::Result<()> {
1 << HPET_CAP_COUNT_SIZE_CAP_SHIFT |
1 << HPET_CAP_LEG_RT_CAP_SHIFT |
HPET_CAP_VENDER_ID_VALUE << HPET_CAP_VENDER_ID_SHIFT |
- ((self.get_num_timers() - 1) as u64) << HPET_CAP_NUM_TIM_SHIFT | // indicate the last timer
+ ((self.num_timers - 1) as u64) << HPET_CAP_NUM_TIM_SHIFT | // indicate the last timer
(HPET_CLK_PERIOD * FS_PER_NS) << HPET_CAP_CNT_CLK_PERIOD_SHIFT, // 10 ns
);
@@ -753,7 +748,7 @@ fn realize(&self) -> qemu_api::Result<()> {
}
fn reset_hold(&self, _type: ResetType) {
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
timer.borrow_mut().reset();
}
@@ -781,7 +776,7 @@ fn decode(&self, mut addr: hwaddr, size: u32) -> HPETAddrDecode {
GlobalRegister::try_from(addr).map(HPETRegister::Global)
} else {
let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
- if timer_id <= self.get_num_timers() {
+ if timer_id < self.num_timers {
// TODO: Add trace point - trace_hpet_ram_[read|write]_timer_id(timer_id)
TimerRegister::try_from(addr & 0x18)
.map(|reg| HPETRegister::Timer(&self.timers[timer_id], reg))
@@ -852,12 +847,12 @@ fn pre_save(&self) -> i32 {
* also added to the migration stream. Check that it matches the value
* that was configured.
*/
- self.num_timers_save.set(self.num_timers.get() as u8);
+ self.num_timers_save.set(self.num_timers as u8);
0
}
fn post_load(&self, _version_id: u8) -> i32 {
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
let mut t = timer.borrow_mut();
t.cmp64 = t.calculate_cmp64(t.get_state().counter.get(), t.cmp);
@@ -882,7 +877,7 @@ fn is_offset_needed(&self) -> bool {
}
fn validate_num_timers(&self, _version_id: u8) -> bool {
- self.num_timers.get() == self.num_timers_save.get().into()
+ self.num_timers == self.num_timers_save.get().into()
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 14/14] docs: update Rust module status
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
` (12 preceding siblings ...)
2025-06-05 10:15 ` [PATCH 13/14] rust/hpet: Drop BqlCell wrapper for num_timers Paolo Bonzini
@ 2025-06-05 10:15 ` Paolo Bonzini
13 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2025-06-05 10:15 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, zhao1.liu, qemu-rust
error is new; offset_of is gone.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 98803d5e8dd..67acd7d2867 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -160,10 +160,10 @@ module status
``callbacks`` complete
``cell`` stable
``errno`` complete
+``error`` stable
``irq`` complete
``memory`` stable
``module`` complete
-``offset_of`` stable
``qdev`` stable
``qom`` stable
``sysbus`` stable
--
2.49.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 04/14] util/error: allow non-NUL-terminated err->src
2025-06-05 10:15 ` [PATCH 04/14] util/error: allow non-NUL-terminated err->src Paolo Bonzini
@ 2025-06-05 11:54 ` Markus Armbruster
2025-06-05 11:57 ` Markus Armbruster
2025-06-05 13:32 ` Zhao Liu
1 sibling, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2025-06-05 11:54 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, armbru, zhao1.liu, qemu-rust
Paolo Bonzini <pbonzini@redhat.com> writes:
> Rust makes the current file available as a statically-allocated string,
> but without a NUL terminator. Allow this by storing an optional maximum
> length in the Error.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qapi/error-internal.h | 9 ++++++++-
> util/error.c | 5 +++--
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
> index d5c3904adec..1ec3ceb40f0 100644
> --- a/include/qapi/error-internal.h
> +++ b/include/qapi/error-internal.h
> @@ -18,7 +18,14 @@ struct Error
> {
> char *msg;
> ErrorClass err_class;
> - const char *src, *func;
> + const char *func;
> +
> + /*
> + * src might be NUL-terminated or not. If it is, src_len is negative.
> + * If it is not, src_len is the length.
> + */
I habitually prefix identifiers with @ in comments, like this:
/*
* @src might be NUL-terminated or not. If it is, @src_len is
* negative. If it is not, @src_len is the length.
*/
Can really help readability when identifiers are also common English
words. Not much of a difference here. Still nice for consistency with
error.h.
> + const char *src;
> + int src_len;
> int line;
> GString *hint;
> };
> diff --git a/util/error.c b/util/error.c
> index e5bcb7c0225..3449ecc0b92 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -24,8 +24,8 @@ Error *error_warn;
> static void error_handle(Error **errp, Error *err)
> {
> if (errp == &error_abort) {
> - fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
> - err->func, err->src, err->line);
> + fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
> + err->func, err->src_len, err->src, err->line);
> error_report("%s", error_get_pretty(err));
> if (err->hint) {
> error_printf("%s", err->hint->str);
> @@ -67,6 +67,7 @@ static void error_setv(Error **errp,
> g_free(msg);
> }
> err->err_class = err_class;
> + err->src_len = -1;
> err->src = src;
> err->line = line;
> err->func = func;
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 04/14] util/error: allow non-NUL-terminated err->src
2025-06-05 11:54 ` Markus Armbruster
@ 2025-06-05 11:57 ` Markus Armbruster
0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2025-06-05 11:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, zhao1.liu, qemu-rust
Markus Armbruster <armbru@redhat.com> writes:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> Rust makes the current file available as a statically-allocated string,
>> but without a NUL terminator. Allow this by storing an optional maximum
>> length in the Error.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> include/qapi/error-internal.h | 9 ++++++++-
>> util/error.c | 5 +++--
>> 2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
>> index d5c3904adec..1ec3ceb40f0 100644
>> --- a/include/qapi/error-internal.h
>> +++ b/include/qapi/error-internal.h
>> @@ -18,7 +18,14 @@ struct Error
>> {
>> char *msg;
>> ErrorClass err_class;
>> - const char *src, *func;
>> + const char *func;
>> +
>> + /*
>> + * src might be NUL-terminated or not. If it is, src_len is negative.
>> + * If it is not, src_len is the length.
>> + */
>
> I habitually prefix identifiers with @ in comments, like this:
>
> /*
> * @src might be NUL-terminated or not. If it is, @src_len is
> * negative. If it is not, @src_len is the length.
> */
>
> Can really help readability when identifiers are also common English
> words. Not much of a difference here. Still nice for consistency with
> error.h.
>
>> + const char *src;
>> + int src_len;
>> int line;
>> GString *hint;
>> };
>> diff --git a/util/error.c b/util/error.c
>> index e5bcb7c0225..3449ecc0b92 100644
>> --- a/util/error.c
>> +++ b/util/error.c
>> @@ -24,8 +24,8 @@ Error *error_warn;
>> static void error_handle(Error **errp, Error *err)
>> {
>> if (errp == &error_abort) {
>> - fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
>> - err->func, err->src, err->line);
>> + fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
>> + err->func, err->src_len, err->src, err->line);
>> error_report("%s", error_get_pretty(err));
>> if (err->hint) {
>> error_printf("%s", err->hint->str);
>> @@ -67,6 +67,7 @@ static void error_setv(Error **errp,
>> g_free(msg);
>> }
>> err->err_class = err_class;
>> + err->src_len = -1;
>> err->src = src;
>> err->line = line;
>> err->func = func;
Almost forgot:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 05/14] util/error: make func optional
2025-06-05 10:15 ` [PATCH 05/14] util/error: make func optional Paolo Bonzini
@ 2025-06-05 11:57 ` Markus Armbruster
0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2025-06-05 11:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, armbru, zhao1.liu, qemu-rust
Paolo Bonzini <pbonzini@redhat.com> writes:
> The function name is not available in Rust, so make it optional.
>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/14] rust: qemu-api: add bindings to Error
2025-06-05 10:15 ` [PATCH 06/14] rust: qemu-api: add bindings to Error Paolo Bonzini
@ 2025-06-05 12:06 ` Markus Armbruster
2025-06-05 13:45 ` Zhao Liu
1 sibling, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2025-06-05 12:06 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, zhao1.liu, qemu-rust
Paolo Bonzini <pbonzini@redhat.com> writes:
> Provide an implementation of std::error::Error that bridges the Rust
> anyhow::Error and std::panic::Location types with QEMU's Error*.
>
> It also has several utility methods, analogous to error_propagate(),
> that convert a Result into a return value + Error** pair. One important
> difference is that these propagation methods *panic* if *errp is NULL,
> unlike error_propagate() which eats subsequent errors[1]. The reason
> for this is that in C you have an error_set*() call at the site where
> the error is created, and calls to error_propagate() are relatively rare.
>
> In Rust instead, even though these functions do "propagate" a
> qemu_api::Error into a C Error**, there is no error_setg() anywhere that
> could check for non-NULL errp and call abort(). error_propagate()'s
> behavior of ignoring subsequent errors is generally considered weird,
> and there would be a bigger risk of triggering it from Rust code.
>
> [1] This is actually a violation of the preconditions of error_propagate(),
> so it should not happen. But you never know...
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[...]
> diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
> new file mode 100644
> index 00000000000..80157f6ea1b
> --- /dev/null
> +++ b/rust/qemu-api/src/error.rs
[...]
> + /// Equivalent of the C function `error_propagate`. Fill `*errp`
> + /// with the information container in `self` if `errp` is not NULL;
> + /// then consume it.
> + ///
> + /// This is similar to the C API `error_propagate`, but it panics if
> + /// `*errp` is not `NULL`.
> + ///
> + /// # Safety
> + ///
> + /// `errp` must be a valid argument to `error_propagate`; it can be
> + /// `NULL` or it can point to any of:
> + /// * `error_abort`
> + /// * `error_fatal`
> + /// * a local variable of (C) type `Error *`
This local variable must contain NULL.
> + ///
> + /// Typically `errp` is received from C code and need not be
> + /// checked further at the Rust↔C boundary.
> + pub unsafe fn propagate(self, errp: *mut *mut bindings::Error) {
> + if errp.is_null() {
> + return;
> + }
> +
> + // SAFETY: caller guarantees that errp and *errp are valid
> + unsafe {
> + assert_eq!(*errp, ptr::null_mut());
> + bindings::error_propagate(errp, self.clone_to_foreign_ptr());
> + }
> + }
[...]
With the comment tightened:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
The commit message and comment improvements are lovely!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 03/14] util/error: expose Error definition to Rust code
2025-06-05 10:15 ` [PATCH 03/14] util/error: expose Error definition to Rust code Paolo Bonzini
@ 2025-06-05 13:31 ` Zhao Liu
0 siblings, 0 replies; 25+ messages in thread
From: Zhao Liu @ 2025-06-05 13:31 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, armbru, qemu-rust
On Thu, Jun 05, 2025 at 12:15:32PM +0200, Paolo Bonzini wrote:
> Date: Thu, 5 Jun 2025 12:15:32 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 03/14] util/error: expose Error definition to Rust code
> X-Mailer: git-send-email 2.49.0
>
> This is used to preserve the file and line in a roundtrip from
> C Error to Rust and back to C.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qapi/error-internal.h | 26 ++++++++++++++++++++++++++
> rust/wrapper.h | 1 +
> util/error.c | 10 +---------
> 3 files changed, 28 insertions(+), 9 deletions(-)
> create mode 100644 include/qapi/error-internal.h
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 04/14] util/error: allow non-NUL-terminated err->src
2025-06-05 10:15 ` [PATCH 04/14] util/error: allow non-NUL-terminated err->src Paolo Bonzini
2025-06-05 11:54 ` Markus Armbruster
@ 2025-06-05 13:32 ` Zhao Liu
1 sibling, 0 replies; 25+ messages in thread
From: Zhao Liu @ 2025-06-05 13:32 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, armbru, qemu-rust
On Thu, Jun 05, 2025 at 12:15:33PM +0200, Paolo Bonzini wrote:
> Date: Thu, 5 Jun 2025 12:15:33 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 04/14] util/error: allow non-NUL-terminated err->src
> X-Mailer: git-send-email 2.49.0
>
> Rust makes the current file available as a statically-allocated string,
> but without a NUL terminator. Allow this by storing an optional maximum
> length in the Error.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qapi/error-internal.h | 9 ++++++++-
> util/error.c | 5 +++--
> 2 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 06/14] rust: qemu-api: add bindings to Error
2025-06-05 10:15 ` [PATCH 06/14] rust: qemu-api: add bindings to Error Paolo Bonzini
2025-06-05 12:06 ` Markus Armbruster
@ 2025-06-05 13:45 ` Zhao Liu
1 sibling, 0 replies; 25+ messages in thread
From: Zhao Liu @ 2025-06-05 13:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, armbru, qemu-rust
On Thu, Jun 05, 2025 at 12:15:35PM +0200, Paolo Bonzini wrote:
> Date: Thu, 5 Jun 2025 12:15:35 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 06/14] rust: qemu-api: add bindings to Error
> X-Mailer: git-send-email 2.49.0
>
> Provide an implementation of std::error::Error that bridges the Rust
> anyhow::Error and std::panic::Location types with QEMU's Error*.
>
> It also has several utility methods, analogous to error_propagate(),
> that convert a Result into a return value + Error** pair. One important
> difference is that these propagation methods *panic* if *errp is NULL,
nit: *panic* if *errp is not NULL
> unlike error_propagate() which eats subsequent errors[1]. The reason
> for this is that in C you have an error_set*() call at the site where
> the error is created, and calls to error_propagate() are relatively rare.
>
> In Rust instead, even though these functions do "propagate" a
> qemu_api::Error into a C Error**, there is no error_setg() anywhere that
> could check for non-NULL errp and call abort(). error_propagate()'s
> behavior of ignoring subsequent errors is generally considered weird,
> and there would be a bigger risk of triggering it from Rust code.
>
> [1] This is actually a violation of the preconditions of error_propagate(),
> so it should not happen. But you never know...
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> docs/devel/rust.rst | 5 +
> rust/Cargo.lock | 17 ++
> rust/Cargo.toml | 1 +
> rust/qemu-api/Cargo.toml | 2 +
> rust/qemu-api/meson.build | 1 +
> rust/qemu-api/src/error.rs | 312 +++++++++++++++++++++++++++++++++++++
> rust/qemu-api/src/lib.rs | 3 +
> 7 files changed, 341 insertions(+)
> create mode 100644 rust/qemu-api/src/error.rs
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 07/14] rust: qemu-api: add tests for Error bindings
2025-06-05 10:15 ` [PATCH 07/14] rust: qemu-api: add tests for Error bindings Paolo Bonzini
@ 2025-06-05 13:53 ` Zhao Liu
0 siblings, 0 replies; 25+ messages in thread
From: Zhao Liu @ 2025-06-05 13:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, armbru, qemu-rust
On Thu, Jun 05, 2025 at 12:15:36PM +0200, Paolo Bonzini wrote:
> Date: Thu, 5 Jun 2025 12:15:36 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 07/14] rust: qemu-api: add tests for Error bindings
> X-Mailer: git-send-email 2.49.0
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> rust/qemu-api/src/error.rs | 104 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 104 insertions(+)
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2025-06-05 13:32 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-05 10:15 [PATCH v3 00/14] rust: bindings for Error Paolo Bonzini
2025-06-05 10:15 ` [PATCH 01/14] subprojects: add the anyhow crate Paolo Bonzini
2025-06-05 10:15 ` Paolo Bonzini via
2025-06-05 10:15 ` [PATCH 02/14] subprojects: add the foreign crate Paolo Bonzini
2025-06-05 10:15 ` [PATCH 03/14] util/error: expose Error definition to Rust code Paolo Bonzini
2025-06-05 13:31 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 04/14] util/error: allow non-NUL-terminated err->src Paolo Bonzini
2025-06-05 11:54 ` Markus Armbruster
2025-06-05 11:57 ` Markus Armbruster
2025-06-05 13:32 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 05/14] util/error: make func optional Paolo Bonzini
2025-06-05 11:57 ` Markus Armbruster
2025-06-05 10:15 ` [PATCH 06/14] rust: qemu-api: add bindings to Error Paolo Bonzini
2025-06-05 12:06 ` Markus Armbruster
2025-06-05 13:45 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 07/14] rust: qemu-api: add tests for Error bindings Paolo Bonzini
2025-06-05 13:53 ` Zhao Liu
2025-06-05 10:15 ` [PATCH 08/14] rust: qdev: support returning errors from realize Paolo Bonzini
2025-06-05 10:15 ` [PATCH 09/14] rust/hpet: change type of num_timers to usize Paolo Bonzini
2025-06-05 10:15 ` [PATCH 10/14] hpet: adjust VMState for consistency with Rust version Paolo Bonzini
2025-06-05 10:15 ` [PATCH 11/14] hpet: return errors from realize if properties are incorrect Paolo Bonzini
2025-06-05 10:15 ` [PATCH 12/14] rust/hpet: " Paolo Bonzini
2025-06-05 10:15 ` [PATCH 13/14] rust/hpet: Drop BqlCell wrapper for num_timers Paolo Bonzini
2025-06-05 10:15 ` [PATCH 14/14] docs: update Rust module status Paolo Bonzini
-- strict thread matches above, loose matches on Subject: below --
2025-05-30 8:02 [PATCH v2 00/14] rust: bindings for Error Paolo Bonzini
2025-05-30 8:02 ` [PATCH 07/14] rust: qemu-api: add tests for Error bindings Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.