* [PATCH v3 0/2] rust: add module to convert between success/-errno and io::Result conventions
@ 2025-02-20 11:36 Paolo Bonzini
2025-02-20 11:36 ` [PATCH v3 1/2] rust: subprojects: add libc crate Paolo Bonzini
2025-02-20 11:36 ` [PATCH v3 2/2] rust: add module to convert between success/-errno and io::Result Paolo Bonzini
0 siblings, 2 replies; 5+ messages in thread
From: Paolo Bonzini @ 2025-02-20 11:36 UTC (permalink / raw)
To: qemu-devel; +Cc: zhao1.liu, kwolf
Wrap access to errno, for use in the block layer and character device
bindings.
This first version of errno.rs focuses on io::Result. Kevin
suggested allowing conversion from io::Error to negative errno.
For now, I'd rather avoid that to encourage separation between code
that can propagate errors simply with "?", and code at the Rust/C
boundary that needs errno::into_neg_errno(). Since Rust code rarely
passes or returns Error objects that aren't part of a Result, it is
plausible that the same is true of QEMU.
However, if this turns out to be wrong, then it can be changed.
Paolo
Supersedes: <20250212093958.3703269-1-pbonzini@redhat.com>
v2->v3:
- add libc to subprojects/.gitignore
- change subject to reflect the focus to be Result rather than Error
- fix docs
v1->v2:
- use the libc crate
- provide separate From<Errno> implementation for io::ErrorKind
- hide GetErrno trait inside a submodule
- add into_neg_errno() and corresponding tests
- add more doctests
- make Errno field public, so that Errno can have other From<...>
implementations (e.g. going from *mut Error to Errno).
Paolo Bonzini (2):
rust: subprojects: add libc crate
rust: add module to convert between success/-errno and io::Result
docs/devel/rust.rst | 1 +
rust/Cargo.lock | 7 +
rust/qemu-api/Cargo.toml | 1 +
rust/qemu-api/meson.build | 4 +
rust/qemu-api/src/assertions.rs | 28 ++
rust/qemu-api/src/errno.rs | 343 ++++++++++++++++++
rust/qemu-api/src/lib.rs | 1 +
rust/qemu-api/src/prelude.rs | 2 +
scripts/archive-source.sh | 2 +-
scripts/make-release | 2 +-
subprojects/.gitignore | 1 +
subprojects/libc-0.2-rs.wrap | 7 +
.../packagefiles/libc-0.2-rs/meson.build | 37 ++
13 files changed, 434 insertions(+), 2 deletions(-)
create mode 100644 rust/qemu-api/src/errno.rs
create mode 100644 subprojects/libc-0.2-rs.wrap
create mode 100644 subprojects/packagefiles/libc-0.2-rs/meson.build
--
2.48.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] rust: subprojects: add libc crate
2025-02-20 11:36 [PATCH v3 0/2] rust: add module to convert between success/-errno and io::Result conventions Paolo Bonzini
@ 2025-02-20 11:36 ` Paolo Bonzini
2025-02-20 11:36 ` [PATCH v3 2/2] rust: add module to convert between success/-errno and io::Result Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2025-02-20 11:36 UTC (permalink / raw)
To: qemu-devel; +Cc: zhao1.liu, kwolf
This allows access to errno values.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/Cargo.lock | 7 ++++
rust/qemu-api/Cargo.toml | 1 +
scripts/archive-source.sh | 2 +-
scripts/make-release | 2 +-
subprojects/.gitignore | 1 +
subprojects/libc-0.2-rs.wrap | 7 ++++
.../packagefiles/libc-0.2-rs/meson.build | 37 +++++++++++++++++++
7 files changed, 55 insertions(+), 2 deletions(-)
create mode 100644 subprojects/libc-0.2-rs.wrap
create mode 100644 subprojects/packagefiles/libc-0.2-rs/meson.build
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 79e142723b8..2ebf0a11ea4 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -54,6 +54,12 @@ dependencies = [
"either",
]
+[[package]]
+name = "libc"
+version = "0.2.162"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "18d287de67fe55fd7e1581fe933d965a5a9477b38e949cfa9f8574ef01506398"
+
[[package]]
name = "pl011"
version = "0.1.0"
@@ -100,6 +106,7 @@ dependencies = [
name = "qemu_api"
version = "0.1.0"
dependencies = [
+ "libc",
"qemu_api_macros",
"version_check",
]
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index a51dd142852..57747bc9341 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -16,6 +16,7 @@ rust-version = "1.63.0"
[dependencies]
qemu_api_macros = { path = "../qemu-api-macros" }
+libc = "0.2.162"
[build-dependencies]
version_check = "~0.9"
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 30677c3ec90..e461c1531ed 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -28,7 +28,7 @@ sub_file="${sub_tdir}/submodule.tar"
# different to the host OS.
subprojects="keycodemapdb libvfio-user berkeley-softfloat-3
berkeley-testfloat-3 arbitrary-int-1-rs bilge-0.2-rs
- bilge-impl-0.2-rs either-1-rs itertools-0.11-rs proc-macro2-1-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"
sub_deinit=""
diff --git a/scripts/make-release b/scripts/make-release
index 1b89b3423a8..8c3594a1a47 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -41,7 +41,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
- bilge-impl-0.2-rs either-1-rs itertools-0.11-rs proc-macro2-1-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 50f173f90db..d12d34618cc 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -11,6 +11,7 @@
/bilge-impl-0.2.0
/either-1.12.0
/itertools-0.11.0
+/libc-0.2.162
/proc-macro-error-1.0.4
/proc-macro-error-attr-1.0.4
/proc-macro2-1.0.84
diff --git a/subprojects/libc-0.2-rs.wrap b/subprojects/libc-0.2-rs.wrap
new file mode 100644
index 00000000000..bbe08f87883
--- /dev/null
+++ b/subprojects/libc-0.2-rs.wrap
@@ -0,0 +1,7 @@
+[wrap-file]
+directory = libc-0.2.162
+source_url = https://crates.io/api/v1/crates/libc/0.2.162/download
+source_filename = libc-0.2.162.tar.gz
+source_hash = 18d287de67fe55fd7e1581fe933d965a5a9477b38e949cfa9f8574ef01506398
+#method = cargo
+patch_directory = libc-0.2-rs
diff --git a/subprojects/packagefiles/libc-0.2-rs/meson.build b/subprojects/packagefiles/libc-0.2-rs/meson.build
new file mode 100644
index 00000000000..ac4f80dba98
--- /dev/null
+++ b/subprojects/packagefiles/libc-0.2-rs/meson.build
@@ -0,0 +1,37 @@
+project('libc-0.2-rs', 'rust',
+ meson_version: '>=1.5.0',
+ version: '0.2.162',
+ license: 'MIT OR Apache-2.0',
+ default_options: [])
+
+_libc_rs = static_library(
+ 'libc',
+ files('src/lib.rs'),
+ gnu_symbol_visibility: 'hidden',
+ override_options: ['rust_std=2015', 'build.rust_std=2015'],
+ rust_abi: 'rust',
+ rust_args: [
+ '--cap-lints', 'allow',
+ '--cfg', 'freebsd11',
+ '--cfg', 'libc_priv_mod_use',
+ '--cfg', 'libc_union',
+ '--cfg', 'libc_const_size_of',
+ '--cfg', 'libc_align',
+ '--cfg', 'libc_int128',
+ '--cfg', 'libc_core_cvoid',
+ '--cfg', 'libc_packedN',
+ '--cfg', 'libc_cfg_target_vendor',
+ '--cfg', 'libc_non_exhaustive',
+ '--cfg', 'libc_long_array',
+ '--cfg', 'libc_ptr_addr_of',
+ '--cfg', 'libc_underscore_const_names',
+ '--cfg', 'libc_const_extern_fn',
+ ],
+ dependencies: [],
+)
+
+libc_dep = declare_dependency(
+ link_with: _libc_rs,
+)
+
+meson.override_dependency('libc-0.2-rs', libc_dep)
--
2.48.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] rust: add module to convert between success/-errno and io::Result
2025-02-20 11:36 [PATCH v3 0/2] rust: add module to convert between success/-errno and io::Result conventions Paolo Bonzini
2025-02-20 11:36 ` [PATCH v3 1/2] rust: subprojects: add libc crate Paolo Bonzini
@ 2025-02-20 11:36 ` Paolo Bonzini
2025-02-21 11:01 ` Zhao Liu
1 sibling, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2025-02-20 11:36 UTC (permalink / raw)
To: qemu-devel; +Cc: zhao1.liu, kwolf
It is a common convention in QEMU to return a positive value in case of
success, and a negated errno value in case of error. Unfortunately,
using errno portably in Rust is a bit complicated; on Unix the errno
values are supported natively by io::Error, but on Windows they are not;
so, use the libc crate.
This is a set of utility functions that are used by both chardev and
block layer bindings.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 1 +
rust/qemu-api/meson.build | 4 +
rust/qemu-api/src/assertions.rs | 28 +++
rust/qemu-api/src/errno.rs | 343 ++++++++++++++++++++++++++++++++
rust/qemu-api/src/lib.rs | 1 +
rust/qemu-api/src/prelude.rs | 2 +
6 files changed, 379 insertions(+)
create mode 100644 rust/qemu-api/src/errno.rs
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 90958e5a306..c75dccdbb7c 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -179,6 +179,7 @@ module status
``callbacks`` complete
``cell`` stable
``c_str`` complete
+``errno`` complete
``irq`` complete
``memory`` stable
``module`` complete
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 2e9c1078b9b..bcf1cf780f3 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -2,6 +2,8 @@ _qemu_api_cfg = run_command(rustc_args,
'--config-headers', config_host_h, '--features', files('Cargo.toml'),
capture: true, check: true).stdout().strip().splitlines()
+libc_dep = dependency('libc-0.2-rs')
+
# _qemu_api_cfg += ['--cfg', 'feature="allocator"']
if rustc.version().version_compare('>=1.77.0')
_qemu_api_cfg += ['--cfg', 'has_offset_of']
@@ -22,6 +24,7 @@ _qemu_api_rs = static_library(
'src/cell.rs',
'src/chardev.rs',
'src/c_str.rs',
+ 'src/errno.rs',
'src/irq.rs',
'src/memory.rs',
'src/module.rs',
@@ -39,6 +42,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_dep,
)
rust.test('rust-qemu-api-tests', _qemu_api_rs,
diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs
index fa1a18de6fe..104dec39774 100644
--- a/rust/qemu-api/src/assertions.rs
+++ b/rust/qemu-api/src/assertions.rs
@@ -92,3 +92,31 @@ fn types_must_be_equal<T, U>(_: T)
};
};
}
+
+/// Assert that an expression matches a pattern. This can also be
+/// useful to compare enums that do not implement `Eq`.
+///
+/// # Examples
+///
+/// ```
+/// # use qemu_api::assert_match;
+/// // JoinHandle does not implement `Eq`, therefore the result
+/// // does not either.
+/// let result: Result<std::thread::JoinHandle<()>, u32> = Err(42);
+/// assert_match!(result, Err(42));
+/// ```
+#[macro_export]
+macro_rules! assert_match {
+ ($a:expr, $b:pat) => {
+ assert!(
+ match $a {
+ $b => true,
+ _ => false,
+ },
+ "{} = {:?} does not match {}",
+ stringify!($a),
+ $a,
+ stringify!($b)
+ );
+ };
+}
diff --git a/rust/qemu-api/src/errno.rs b/rust/qemu-api/src/errno.rs
new file mode 100644
index 00000000000..bfd972df951
--- /dev/null
+++ b/rust/qemu-api/src/errno.rs
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Utility functions to convert `errno` to and from
+//! [`io::Error`]/[`io::Result`]
+//!
+//! QEMU C functions often have a "positive success/negative `errno`" calling
+//! convention. This module provides functions to portably convert an integer
+//! into an [`io::Result`] and back.
+
+use std::{convert::TryFrom, io, io::ErrorKind};
+
+/// An `errno` value that can be converted into an [`io::Error`]
+pub struct Errno(pub u16);
+
+// On Unix, from_raw_os_error takes an errno value and OS errors
+// are printed using strerror. On Windows however it takes a
+// GetLastError() value; therefore we need to convert errno values
+// into io::Error by hand. This is the same mapping that the
+// standard library uses to retrieve the kind of OS errors
+// (`std::sys::pal::unix::decode_error_kind`).
+impl From<Errno> for ErrorKind {
+ fn from(value: Errno) -> ErrorKind {
+ let Errno(errno) = value;
+ match i32::from(errno) {
+ libc::EPERM | libc::EACCES => ErrorKind::PermissionDenied,
+ libc::ENOENT => ErrorKind::NotFound,
+ libc::EINTR => ErrorKind::Interrupted,
+ x if x == libc::EAGAIN || x == libc::EWOULDBLOCK => ErrorKind::WouldBlock,
+ libc::ENOMEM => ErrorKind::OutOfMemory,
+ libc::EEXIST => ErrorKind::AlreadyExists,
+ libc::EINVAL => ErrorKind::InvalidInput,
+ libc::EPIPE => ErrorKind::BrokenPipe,
+ libc::EADDRINUSE => ErrorKind::AddrInUse,
+ libc::EADDRNOTAVAIL => ErrorKind::AddrNotAvailable,
+ libc::ECONNABORTED => ErrorKind::ConnectionAborted,
+ libc::ECONNREFUSED => ErrorKind::ConnectionRefused,
+ libc::ECONNRESET => ErrorKind::ConnectionReset,
+ libc::ENOTCONN => ErrorKind::NotConnected,
+ libc::ENOTSUP => ErrorKind::Unsupported,
+ libc::ETIMEDOUT => ErrorKind::TimedOut,
+ _ => ErrorKind::Other,
+ }
+ }
+}
+
+// This is used on Windows for all io::Errors, but also on Unix if the
+// io::Error does not have a raw OS error. This is the reversed
+// mapping of the above.
+impl From<io::ErrorKind> for Errno {
+ fn from(value: io::ErrorKind) -> Errno {
+ let errno = match value {
+ // can be both EPERM or EACCES :( pick one
+ ErrorKind::PermissionDenied => libc::EPERM,
+ ErrorKind::NotFound => libc::ENOENT,
+ ErrorKind::Interrupted => libc::EINTR,
+ ErrorKind::WouldBlock => libc::EAGAIN,
+ ErrorKind::OutOfMemory => libc::ENOMEM,
+ ErrorKind::AlreadyExists => libc::EEXIST,
+ ErrorKind::InvalidInput => libc::EINVAL,
+ ErrorKind::BrokenPipe => libc::EPIPE,
+ ErrorKind::AddrInUse => libc::EADDRINUSE,
+ ErrorKind::AddrNotAvailable => libc::EADDRNOTAVAIL,
+ ErrorKind::ConnectionAborted => libc::ECONNABORTED,
+ ErrorKind::ConnectionRefused => libc::ECONNREFUSED,
+ ErrorKind::ConnectionReset => libc::ECONNRESET,
+ ErrorKind::NotConnected => libc::ENOTCONN,
+ ErrorKind::Unsupported => libc::ENOTSUP,
+ ErrorKind::TimedOut => libc::ETIMEDOUT,
+ _ => libc::EIO,
+ };
+ Errno(errno as u16)
+ }
+}
+
+impl From<Errno> for io::Error {
+ #[cfg(unix)]
+ fn from(value: Errno) -> io::Error {
+ let Errno(errno) = value;
+ io::Error::from_raw_os_error(errno.into())
+ }
+
+ #[cfg(windows)]
+ fn from(value: Errno) -> io::Error {
+ let error_kind: ErrorKind = value.into();
+ error_kind.into()
+ }
+}
+
+impl From<io::Error> for Errno {
+ fn from(value: io::Error) -> Errno {
+ if cfg!(unix) {
+ if let Some(errno) = value.raw_os_error() {
+ return Errno(u16::try_from(errno).unwrap());
+ }
+ }
+ value.kind().into()
+ }
+}
+
+/// Internal traits; used to enable [`into_io_result`] and [`into_neg_errno`]
+/// for the "right" set of types.
+mod traits {
+ use super::Errno;
+
+ /// A signed type that can be converted into an
+ /// [`io::Result`](std::io::Result)
+ pub trait GetErrno {
+ /// Unsigned variant of `Self`, used as the type for the `Ok` case.
+ type Out;
+
+ /// Return `Ok(self)` if positive, `Err(Errno(-self))` if negative
+ fn into_errno_result(self) -> Result<Self::Out, Errno>;
+ }
+
+ /// A type that can be taken out of an [`io::Result`](std::io::Result) and
+ /// converted into "positive success/negative `errno`" convention.
+ pub trait MergeErrno {
+ /// Signed variant of `Self`, used as the return type of
+ /// [`into_neg_errno`](super::into_neg_errno).
+ type Out: From<u16> + std::ops::Neg<Output = Self::Out>;
+
+ /// Return `self`, asserting that it is in range
+ fn map_ok(self) -> Self::Out;
+ }
+
+ macro_rules! get_errno {
+ ($t:ty, $out:ty) => {
+ impl GetErrno for $t {
+ type Out = $out;
+ fn into_errno_result(self) -> Result<Self::Out, Errno> {
+ match self {
+ 0.. => Ok(self as $out),
+ -65535..=-1 => Err(Errno(-self as u16)),
+ _ => panic!("{self} is not a negative errno"),
+ }
+ }
+ }
+ };
+ }
+
+ get_errno!(i32, u32);
+ get_errno!(i64, u64);
+ get_errno!(isize, usize);
+
+ macro_rules! merge_errno {
+ ($t:ty, $out:ty) => {
+ impl MergeErrno for $t {
+ type Out = $out;
+ fn map_ok(self) -> Self::Out {
+ self.try_into().unwrap()
+ }
+ }
+ };
+ }
+
+ merge_errno!(u8, i32);
+ merge_errno!(u16, i32);
+ merge_errno!(u32, i32);
+ merge_errno!(u64, i64);
+
+ impl MergeErrno for () {
+ type Out = i32;
+ fn map_ok(self) -> i32 {
+ 0
+ }
+ }
+}
+
+use traits::{GetErrno, MergeErrno};
+
+/// Convert an integer value into a [`io::Result`].
+///
+/// Positive values are turned into an `Ok` result; negative values
+/// are interpreted as negated `errno` and turned into an `Err`.
+///
+/// ```
+/// # use qemu_api::errno::into_io_result;
+/// # use std::io::ErrorKind;
+/// let ok = into_io_result(1i32).unwrap();
+/// assert_eq!(ok, 1u32);
+///
+/// let err = into_io_result(-1i32).unwrap_err(); // -EPERM
+/// assert_eq!(err.kind(), ErrorKind::PermissionDenied);
+/// ```
+///
+/// # Panics
+///
+/// Since the result is an unsigned integer, negative values must
+/// be close to 0; values that are too far away are considered
+/// likely overflows and will panic:
+///
+/// ```should_panic
+/// # use qemu_api::errno::into_io_result;
+/// # #[allow(dead_code)]
+/// let err = into_io_result(-0x1234_5678i32); // panic
+/// ```
+pub fn into_io_result<T: GetErrno>(value: T) -> io::Result<T::Out> {
+ value.into_errno_result().map_err(Into::into)
+}
+
+/// Convert a [`Result`] into an integer value, using negative `errno`
+/// values to report errors.
+///
+/// ```
+/// # use qemu_api::errno::into_neg_errno;
+/// # use std::io::{self, ErrorKind};
+/// let ok: io::Result<()> = Ok(());
+/// assert_eq!(into_neg_errno(ok), 0);
+///
+/// let err: io::Result<()> = Err(ErrorKind::InvalidInput.into());
+/// assert_eq!(into_neg_errno(err), -22); // -EINVAL
+/// ```
+///
+/// Since this module also provides the ability to convert [`io::Error`]
+/// to an `errno` value, [`io::Result`] is the most commonly used type
+/// for the argument of this function:
+///
+/// # Panics
+///
+/// Since the result is a signed integer, integer `Ok` values must remain
+/// positive:
+///
+/// ```should_panic
+/// # use qemu_api::errno::into_neg_errno;
+/// # use std::io;
+/// let err: io::Result<u32> = Ok(0x8899_AABB);
+/// into_neg_errno(err) // panic
+/// # ;
+/// ```
+pub fn into_neg_errno<T: MergeErrno, E: Into<Errno>>(value: Result<T, E>) -> T::Out {
+ match value {
+ Ok(x) => x.map_ok(),
+ Err(err) => -T::Out::from(err.into().0),
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use std::io::ErrorKind;
+
+ use super::*;
+ use crate::assert_match;
+
+ #[test]
+ pub fn test_from_u8() {
+ let ok: io::Result<_> = Ok(42u8);
+ assert_eq!(into_neg_errno(ok), 42);
+
+ let err: io::Result<u8> = Err(io::ErrorKind::PermissionDenied.into());
+ assert_eq!(into_neg_errno(err), -1);
+
+ if cfg!(unix) {
+ let os_err: io::Result<u8> = Err(io::Error::from_raw_os_error(10));
+ assert_eq!(into_neg_errno(os_err), -10);
+ }
+ }
+
+ #[test]
+ pub fn test_from_u16() {
+ let ok: io::Result<_> = Ok(1234u16);
+ assert_eq!(into_neg_errno(ok), 1234);
+
+ let err: io::Result<u16> = Err(io::ErrorKind::PermissionDenied.into());
+ assert_eq!(into_neg_errno(err), -1);
+
+ if cfg!(unix) {
+ let os_err: io::Result<u16> = Err(io::Error::from_raw_os_error(10));
+ assert_eq!(into_neg_errno(os_err), -10);
+ }
+ }
+
+ #[test]
+ pub fn test_i32() {
+ assert_match!(into_io_result(1234i32), Ok(1234));
+
+ let err = into_io_result(-1i32).unwrap_err();
+ #[cfg(unix)]
+ assert_match!(err.raw_os_error(), Some(1));
+ assert_match!(err.kind(), ErrorKind::PermissionDenied);
+ }
+
+ #[test]
+ pub fn test_from_u32() {
+ let ok: io::Result<_> = Ok(1234u32);
+ assert_eq!(into_neg_errno(ok), 1234);
+
+ let err: io::Result<u32> = Err(io::ErrorKind::PermissionDenied.into());
+ assert_eq!(into_neg_errno(err), -1);
+
+ if cfg!(unix) {
+ let os_err: io::Result<u32> = Err(io::Error::from_raw_os_error(10));
+ assert_eq!(into_neg_errno(os_err), -10);
+ }
+ }
+
+ #[test]
+ pub fn test_i64() {
+ assert_match!(into_io_result(1234i64), Ok(1234));
+
+ let err = into_io_result(-22i64).unwrap_err();
+ #[cfg(unix)]
+ assert_match!(err.raw_os_error(), Some(22));
+ assert_match!(err.kind(), ErrorKind::InvalidInput);
+ }
+
+ #[test]
+ pub fn test_from_u64() {
+ let ok: io::Result<_> = Ok(1234u64);
+ assert_eq!(into_neg_errno(ok), 1234);
+
+ let err: io::Result<u64> = Err(io::ErrorKind::InvalidInput.into());
+ assert_eq!(into_neg_errno(err), -22);
+
+ if cfg!(unix) {
+ let os_err: io::Result<u64> = Err(io::Error::from_raw_os_error(6));
+ assert_eq!(into_neg_errno(os_err), -6);
+ }
+ }
+
+ #[test]
+ pub fn test_isize() {
+ assert_match!(into_io_result(1234isize), Ok(1234));
+
+ let err = into_io_result(-4isize).unwrap_err();
+ #[cfg(unix)]
+ assert_match!(err.raw_os_error(), Some(4));
+ assert_match!(err.kind(), ErrorKind::Interrupted);
+ }
+
+ #[test]
+ pub fn test_from_unit() {
+ let ok: io::Result<_> = Ok(());
+ assert_eq!(into_neg_errno(ok), 0);
+
+ let err: io::Result<()> = Err(io::ErrorKind::OutOfMemory.into());
+ assert_eq!(into_neg_errno(err), -12);
+
+ if cfg!(unix) {
+ let os_err: io::Result<()> = Err(io::Error::from_raw_os_error(2));
+ assert_eq!(into_neg_errno(os_err), -2);
+ }
+ }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index ed1a8f9a2b4..05f38b51d30 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -19,6 +19,7 @@
pub mod callbacks;
pub mod cell;
pub mod chardev;
+pub mod errno;
pub mod irq;
pub mod memory;
pub mod module;
diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
index fbf0ee23e0b..634acf37a85 100644
--- a/rust/qemu-api/src/prelude.rs
+++ b/rust/qemu-api/src/prelude.rs
@@ -9,6 +9,8 @@
pub use crate::cell::BqlCell;
pub use crate::cell::BqlRefCell;
+pub use crate::errno;
+
pub use crate::qdev::DeviceMethods;
pub use crate::qom::InterfaceType;
--
2.48.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] rust: add module to convert between success/-errno and io::Result
2025-02-20 11:36 ` [PATCH v3 2/2] rust: add module to convert between success/-errno and io::Result Paolo Bonzini
@ 2025-02-21 11:01 ` Zhao Liu
2025-02-25 13:21 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Zhao Liu @ 2025-02-21 11:01 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, kwolf
Hi Paolo,
> It is a common convention in QEMU to return a positive value in case of
> success, and a negated errno value in case of error. Unfortunately,
> using errno portably in Rust is a bit complicated; on Unix the errno
> values are supported natively by io::Error, but on Windows they are not;
> so, use the libc crate.
I'm a bit confused. The doc of error.h just said the negative value for
failure:
• integer-valued functions return non-negative / negative.
Why do we need to using libc's -errno for Windows as well?
Converting `io::Error::last_os_error().raw_os_error().unwrap()` to a
negative value seems compatible with Windows, except it returns Windows
error codes.
> This is a set of utility functions that are used by both chardev and
> block layer bindings.
...
> +// On Unix, from_raw_os_error takes an errno value and OS errors
> +// are printed using strerror. On Windows however it takes a
> +// GetLastError() value; therefore we need to convert errno values
> +// into io::Error by hand. This is the same mapping that the
> +// standard library uses to retrieve the kind of OS errors
> +// (`std::sys::pal::unix::decode_error_kind`).
> +impl From<Errno> for ErrorKind {
> + fn from(value: Errno) -> ErrorKind {
What about `use ErrorKind::*;` to oimt the following "ErrorKind::"
prefix?
> + let Errno(errno) = value;
> + match i32::from(errno) {
Maybe `match i32::from(errno.0)` ?
> + libc::EPERM | libc::EACCES => ErrorKind::PermissionDenied,
> + libc::ENOENT => ErrorKind::NotFound,
> + libc::EINTR => ErrorKind::Interrupted,
> + x if x == libc::EAGAIN || x == libc::EWOULDBLOCK => ErrorKind::WouldBlock,
> + libc::ENOMEM => ErrorKind::OutOfMemory,
> + libc::EEXIST => ErrorKind::AlreadyExists,
> + libc::EINVAL => ErrorKind::InvalidInput,
> + libc::EPIPE => ErrorKind::BrokenPipe,
> + libc::EADDRINUSE => ErrorKind::AddrInUse,
> + libc::EADDRNOTAVAIL => ErrorKind::AddrNotAvailable,
> + libc::ECONNABORTED => ErrorKind::ConnectionAborted,
> + libc::ECONNREFUSED => ErrorKind::ConnectionRefused,
> + libc::ECONNRESET => ErrorKind::ConnectionReset,
> + libc::ENOTCONN => ErrorKind::NotConnected,
> + libc::ENOTSUP => ErrorKind::Unsupported,
> + libc::ETIMEDOUT => ErrorKind::TimedOut,
> + _ => ErrorKind::Other,
Are these errno cases specifically selected? It seems to have fewer than
`decode_error_kind` lists. Why not support all the cases `decode_error_kind`
mentions? Or do we need to try to cover as many errno cases as possible
from rust/kernel/error.rs?
> + }
> + }
> +}
> +
> +// This is used on Windows for all io::Errors, but also on Unix if the
> +// io::Error does not have a raw OS error. This is the reversed
> +// mapping of the above.
Maybe:
This is the "almost" reversed (except the default case) mapping
?
> +impl From<io::ErrorKind> for Errno {
> + fn from(value: io::ErrorKind) -> Errno {
`use ErrorKind::*;` could save some words, too.
> + let errno = match value {
> + // can be both EPERM or EACCES :( pick one
> + ErrorKind::PermissionDenied => libc::EPERM,
> + ErrorKind::NotFound => libc::ENOENT,
> + ErrorKind::Interrupted => libc::EINTR,
> + ErrorKind::WouldBlock => libc::EAGAIN,
> + ErrorKind::OutOfMemory => libc::ENOMEM,
> + ErrorKind::AlreadyExists => libc::EEXIST,
> + ErrorKind::InvalidInput => libc::EINVAL,
> + ErrorKind::BrokenPipe => libc::EPIPE,
> + ErrorKind::AddrInUse => libc::EADDRINUSE,
> + ErrorKind::AddrNotAvailable => libc::EADDRNOTAVAIL,
> + ErrorKind::ConnectionAborted => libc::ECONNABORTED,
> + ErrorKind::ConnectionRefused => libc::ECONNREFUSED,
> + ErrorKind::ConnectionReset => libc::ECONNRESET,
> + ErrorKind::NotConnected => libc::ENOTCONN,
> + ErrorKind::Unsupported => libc::ENOTSUP,
> + ErrorKind::TimedOut => libc::ETIMEDOUT,
> + _ => libc::EIO,
> + };
> + Errno(errno as u16)
> + }
> +}
> +
> +impl From<Errno> for io::Error {
> + #[cfg(unix)]
> + fn from(value: Errno) -> io::Error {
> + let Errno(errno) = value;
> + io::Error::from_raw_os_error(errno.into())
Maybe `io::Error::from_raw_os_error(value.0.into())`?
> + }
> +
> + #[cfg(windows)]
> + fn from(value: Errno) -> io::Error {
> + let error_kind: ErrorKind = value.into();
> + error_kind.into()
Even this works:
fn from(value: Errno) -> io::Error {
- let error_kind: ErrorKind = value.into();
- error_kind.into()
+ value.into()
However, it's less readability, so I still prefer your current codes.
:-)
Thanks,
Zhao
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] rust: add module to convert between success/-errno and io::Result
2025-02-21 11:01 ` Zhao Liu
@ 2025-02-25 13:21 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2025-02-25 13:21 UTC (permalink / raw)
To: Zhao Liu; +Cc: qemu-devel, Wolf, Kevin
[-- Attachment #1: Type: text/plain, Size: 3522 bytes --]
Il ven 21 feb 2025, 11:41 Zhao Liu <zhao1.liu@intel.com> ha scritto:
> Hi Paolo,
>
> > It is a common convention in QEMU to return a positive value in case of
> > success, and a negated errno value in case of error. Unfortunately,
> > using errno portably in Rust is a bit complicated; on Unix the errno
> > values are supported natively by io::Error, but on Windows they are not;
> > so, use the libc crate.
>
> I'm a bit confused. The doc of error.h just said the negative value for
> failure:
>
> • integer-valued functions return non-negative / negative.
>
> Why do we need to using libc's -errno for Windows as well?
>
error.h doesn't explicitly mention errno because there are functions that
return -1 on all errors, but errno is a common convention for negative
return values too.
Converting `io::Error::last_os_error().raw_os_error().unwrap()` to a
> negative value seems compatible with Windows, except it returns Windows
> error codes.
>
... but that's not what C functions expect. They might pass it to set
error_setg_errno for example.
> +// On Unix, from_raw_os_error takes an errno value and OS errors
> > +// are printed using strerror. On Windows however it takes a
> > +// GetLastError() value; therefore we need to convert errno values
> > +// into io::Error by hand. This is the same mapping that the
> > +// standard library uses to retrieve the kind of OS errors
> > +// (`std::sys::pal::unix::decode_error_kind`).
> > +impl From<Errno> for ErrorKind {
> > + fn from(value: Errno) -> ErrorKind {
>
> What about `use ErrorKind::*;` to omit the following "ErrorKind::"
> prefix?
>
Yeah, that's possible.
> > + let Errno(errno) = value;
> > + match i32::from(errno) {
>
> Maybe `match i32::from(errno.0)` ?
Well, I don't like .0 too much...
> + libc::EPERM | libc::EACCES => ErrorKind::PermissionDenied,
> > + libc::ENOENT => ErrorKind::NotFound,
> > + libc::EINTR => ErrorKind::Interrupted,
> > + x if x == libc::EAGAIN || x == libc::EWOULDBLOCK =>
> ErrorKind::WouldBlock,
> > + libc::ENOMEM => ErrorKind::OutOfMemory,
> > + libc::EEXIST => ErrorKind::AlreadyExists,
> > + libc::EINVAL => ErrorKind::InvalidInput,
> > + libc::EPIPE => ErrorKind::BrokenPipe,
> > + libc::EADDRINUSE => ErrorKind::AddrInUse,
> > + libc::EADDRNOTAVAIL => ErrorKind::AddrNotAvailable,
> > + libc::ECONNABORTED => ErrorKind::ConnectionAborted,
> > + libc::ECONNREFUSED => ErrorKind::ConnectionRefused,
> > + libc::ECONNRESET => ErrorKind::ConnectionReset,
> > + libc::ENOTCONN => ErrorKind::NotConnected,
> > + libc::ENOTSUP => ErrorKind::Unsupported,
> > + libc::ETIMEDOUT => ErrorKind::TimedOut,
> > + _ => ErrorKind::Other,
>
> Are these errno cases specifically selected? It seems to have fewer than
> `decode_error_kind` lists. Why not support all the cases
> `decode_error_kind`
> mentions?
Not all of them are in all Rust versions; see
https://stdrs.dev/nightly/x86_64-unknown-linux-gnu/src/std/io/error.rs.html#179-395
.
> +// This is used on Windows for all io::Errors, but also on Unix if the
> > +// io::Error does not have a raw OS error. This is the reversed
> > +// mapping of the above.
>
> Maybe:
>
> This is the "almost" reversed (except the default case) mapping
>
True, thanks.
Paolo
[-- Attachment #2: Type: text/html, Size: 5834 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-25 13:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-20 11:36 [PATCH v3 0/2] rust: add module to convert between success/-errno and io::Result conventions Paolo Bonzini
2025-02-20 11:36 ` [PATCH v3 1/2] rust: subprojects: add libc crate Paolo Bonzini
2025-02-20 11:36 ` [PATCH v3 2/2] rust: add module to convert between success/-errno and io::Result Paolo Bonzini
2025-02-21 11:01 ` Zhao Liu
2025-02-25 13:21 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).