| Message ID | 20250330205857.1615-2-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Initial logging support for Rust | expand |
On 3/30/25 22:58, Bernhard Beschow wrote: > A qemu_log_mask!() macro is provided which expects similar arguments as the C > version. However, the formatting works as one would expect from Rust. > > To maximize code reuse the macro is just a thin wrapper around qemu_log(). > Also, just the bare minimum of logging masks is provided which should suffice > for the current use case of Rust in QEMU. It's probably better to use an enum for this. One possibility is also to change the #defines to a C enum, and see which enum translation of the several allowed by bindgen is best. Also, while this is good for now, later on we probably want to reimplement logging at a lower level via the std::fmt::Write trait. But that's just for efficiency and your macro is indeed good enough to define what the API would look like. Right now I have a project for GSoC that will look at that, and the student can look into it later on. This means answering the following two questions: - the mapping the LOG_* constants into Rust - whether to keep the "qemu" prefix for the API (personal opinion: no) - whether it makes sense to add more macros such as log_guest_error! or log_unimp! for the most common LOG_* values > +#[macro_export] > +macro_rules! qemu_log_mask { > + ($mask:expr, $fmt:expr $(, $args:expr)*) => {{ Looking at https://doc.rust-lang.org/std/macro.write.html they just use $($arg:tt)* for what is passed to format_args! (or in your case format!), so we can do the same here too. The main advantage is that it allows giving a trailing comma to qemu_log_mask!. Paolo > + if unsafe { > + (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0 > + } { > + let formatted_string = format!($fmt, $($args),*); > + let c_string = std::ffi::CString::new(formatted_string).unwrap(); > + > + unsafe { > + ::qemu_api::bindings::qemu_log(c_string.as_ptr()); > + } > + } > + }}; > +}
Am 31. März 2025 09:53:41 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>: >On 3/30/25 22:58, Bernhard Beschow wrote: >> A qemu_log_mask!() macro is provided which expects similar arguments as the C >> version. However, the formatting works as one would expect from Rust. >> >> To maximize code reuse the macro is just a thin wrapper around qemu_log(). >> Also, just the bare minimum of logging masks is provided which should suffice >> for the current use case of Rust in QEMU. > >It's probably better to use an enum for this. One possibility is also to change the #defines to a C enum, and see which enum translation of the several allowed by bindgen is best. Currently the #defines contain some holes for "private" mask bits. Turning these into an enum without exposing all publicly, and changing the type of qemu_loglevel for consistency, would result in undefined behavior. Or do you suggest to convert just the public #defines into an enum to expose them to Rust, and keep the rest of the C API including the type of qemu_loglevel as is? There are surely several tradeoffs and/or cleanups possible here, but that's way beyond for what I wanted to achieve -- which is closing a gap between C and Rust. My main goal is just to get my feet wet with Rust. > >Also, while this is good for now, later on we probably want to reimplement logging at a lower level via the std::fmt::Write trait. But that's just for efficiency and your macro is indeed good enough to define what the API would look like. Can we live with an easy solution then for now? As you suggest below, further abstractions like log_guest_error! can be built on top which further insulates client code from implementation details such as the representation of the mask bits. > Right now I have a project for GSoC that will look at that, and the student can look into it later on. Whoops, I didn't intend to spoil the project. > >This means answering the following two questions: > >- the mapping the LOG_* constants into Rust > >- whether to keep the "qemu" prefix for the API (personal opinion: no) > >- whether it makes sense to add more macros such as log_guest_error! or log_unimp! for the most common LOG_* values > >> +#[macro_export] >> +macro_rules! qemu_log_mask { >> + ($mask:expr, $fmt:expr $(, $args:expr)*) => {{ > >Looking at https://doc.rust-lang.org/std/macro.write.html they just use $($arg:tt)* for what is passed to format_args! (or in your case format!), so we can do the same here too. The main advantage is that it allows giving a trailing comma to qemu_log_mask!. Easy to fix. Before proceeding I'd like to see a solution for the topic above. Best regards, Bernhard > >Paolo > >> + if unsafe { >> + (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0 >> + } { >> + let formatted_string = format!($fmt, $($args),*); >> + let c_string = std::ffi::CString::new(formatted_string).unwrap(); >> + >> + unsafe { >> + ::qemu_api::bindings::qemu_log(c_string.as_ptr()); >> + } >> + } >> + }}; >> +} >
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst index 88bdec1eb2..bc50216dc6 100644 --- a/docs/devel/rust.rst +++ b/docs/devel/rust.rst @@ -187,6 +187,7 @@ module status ``c_str`` complete ``errno`` complete ``irq`` complete +``log`` proof of concept ``memory`` stable ``module`` complete ``offset_of`` stable diff --git a/rust/wrapper.h b/rust/wrapper.h index d4fec54657..9518face53 100644 --- a/rust/wrapper.h +++ b/rust/wrapper.h @@ -48,6 +48,8 @@ typedef enum memory_order { #endif /* __CLANG_STDATOMIC_H */ #include "qemu/osdep.h" +#include "qemu/log.h" +#include "qemu/log-for-trace.h" #include "qemu/module.h" #include "qemu-io.h" #include "system/system.h" diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build index 858685ddd4..112f6735a9 100644 --- a/rust/qemu-api/meson.build +++ b/rust/qemu-api/meson.build @@ -26,6 +26,7 @@ _qemu_api_rs = static_library( 'src/c_str.rs', 'src/errno.rs', 'src/irq.rs', + 'src/log.rs', 'src/memory.rs', 'src/module.rs', 'src/offset_of.rs', diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs index 05f38b51d3..b54989a243 100644 --- a/rust/qemu-api/src/lib.rs +++ b/rust/qemu-api/src/lib.rs @@ -21,6 +21,7 @@ pub mod chardev; pub mod errno; pub mod irq; +pub mod log; pub mod memory; pub mod module; pub mod offset_of; diff --git a/rust/qemu-api/src/log.rs b/rust/qemu-api/src/log.rs new file mode 100644 index 0000000000..3103a86efa --- /dev/null +++ b/rust/qemu-api/src/log.rs @@ -0,0 +1,55 @@ +// Copyright 2025 Bernhard Beschow <shentey@gmail.com> +// SPDX-License-Identifier: GPL-2.0-or-later + +pub use crate::bindings::{LOG_GUEST_ERROR, LOG_INVALID_MEM, LOG_UNIMP}; + +/// A macro to log messages conditionally based on a provided mask. +/// +/// The `qemu_log_mask` macro checks whether the given mask matches the +/// current log level and, if so, formats and logs the message. +/// +/// # Parameters +/// +/// - `$mask`: The log mask to check. This should be an integer value +/// indicating the log level mask. +/// - `$fmt`: A format string following the syntax and rules of the `format!` +/// macro. It specifies the structure of the log message. +/// - `$args`: Optional arguments to be interpolated into the format string. +/// +/// # Example +/// +/// ``` +/// use qemu_api::log::LOG_GUEST_ERROR; +/// +/// qemu_log_mask!( +/// LOG_GUEST_ERROR, +/// "Address 0x{error_address:x} out of range\n" +/// ); +/// ``` +/// +/// It is also possible to use printf style: +/// +/// ``` +/// use qemu_api::log::LOG_GUEST_ERROR; +/// +/// qemu_log_mask!( +/// LOG_GUEST_ERROR, +/// "Address 0x{:x} out of range\n", +/// error_address +/// ); +/// ``` +#[macro_export] +macro_rules! qemu_log_mask { + ($mask:expr, $fmt:expr $(, $args:expr)*) => {{ + if unsafe { + (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0 + } { + let formatted_string = format!($fmt, $($args),*); + let c_string = std::ffi::CString::new(formatted_string).unwrap(); + + unsafe { + ::qemu_api::bindings::qemu_log(c_string.as_ptr()); + } + } + }}; +}
A qemu_log_mask!() macro is provided which expects similar arguments as the C version. However, the formatting works as one would expect from Rust. To maximize code reuse the macro is just a thin wrapper around qemu_log(). Also, just the bare minimum of logging masks is provided which should suffice for the current use case of Rust in QEMU. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- docs/devel/rust.rst | 1 + rust/wrapper.h | 2 ++ rust/qemu-api/meson.build | 1 + rust/qemu-api/src/lib.rs | 1 + rust/qemu-api/src/log.rs | 55 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+) create mode 100644 rust/qemu-api/src/log.rs