[PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API

Bernhard Beschow posted 2 patches 5 months, 1 week ago
Maintainers: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
There is a newer version of this series
[PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Bernhard Beschow 5 months, 1 week ago
A 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  | 76 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 81 insertions(+)
 create mode 100644 rust/qemu-api/src/log.rs

diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 47e9677fcb..dc8c44109e 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -162,6 +162,7 @@ module           status
 ``errno``        complete
 ``error``        stable
 ``irq``          complete
+``log``          proof of concept
 ``memory``       stable
 ``module``       complete
 ``qdev``         stable
diff --git a/rust/wrapper.h b/rust/wrapper.h
index 6060d3ba1a..15a1b19847 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 cac8595a14..33caee3c4f 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -21,6 +21,7 @@ _qemu_api_rs = static_library(
       'src/errno.rs',
       'src/error.rs',
       'src/irq.rs',
+      'src/log.rs',
       'src/memory.rs',
       'src/module.rs',
       'src/prelude.rs',
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 93902fc94b..e20be35460 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -21,6 +21,7 @@
 pub mod errno;
 pub mod error;
 pub mod irq;
+pub mod log;
 pub mod memory;
 pub mod module;
 pub mod qdev;
diff --git a/rust/qemu-api/src/log.rs b/rust/qemu-api/src/log.rs
new file mode 100644
index 0000000000..9e3c61b8b7
--- /dev/null
+++ b/rust/qemu-api/src/log.rs
@@ -0,0 +1,76 @@
+// Copyright 2025 Bernhard Beschow <shentey@gmail.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#[repr(u32)]
+/// Represents specific error categories within QEMU's logging system.
+///
+/// The `Log` enum provides a Rust abstraction for logging errors, corresponding
+/// to a subset of the error categories defined in the C implementation.
+pub enum Log {
+    /// Log an invalid access caused by the guest.
+    /// Corresponds to `LOG_GUEST_ERROR` in the C implementation.
+    GuestError = crate::bindings::LOG_GUEST_ERROR,
+
+    /// Log guest access of unimplemented functionality.
+    /// Corresponds to `LOG_UNIMP` in the C implementation.
+    Unimp = crate::bindings::LOG_UNIMP,
+}
+
+/// A macro to log messages conditionally based on a provided mask.
+///
+/// The `log_mask` macro checks whether the given mask matches the current log
+/// level and, if so, formats and logs the message. It is the Rust counterpart
+/// of the qemu_log_mask() macro in the C implementation.
+///
+/// # Parameters
+///
+/// - `$mask`: A log level mask. This should be a variant of the `Log` enum.
+/// - `$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;
+/// use qemu_api::log_mask;
+///
+/// let error_address = 0xbad;
+/// log_mask!(
+///     Log::GuestError,
+///     "Address 0x{error_address:x} out of range\n"
+/// );
+/// ```
+///
+/// It is also possible to use printf-style formatting, as well as having a
+/// trailing `,`:
+///
+/// ```
+/// use qemu_api::log::Log;
+/// use qemu_api::log_mask;
+///
+/// let error_address = 0xbad;
+/// log_mask!(
+///     Log::GuestError,
+///     "Address 0x{:x} out of range\n",
+///     error_address,
+/// );
+/// ```
+#[macro_export]
+macro_rules! log_mask {
+    ($mask:expr, $fmt:tt $($args:tt)*) => {{
+        // Type assertion to enforce type `Log` for $mask
+        let _: Log = $mask;
+
+        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());
+            }
+        }
+    }};
+}
-- 
2.49.0
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Manos Pitsidianakis 5 months, 1 week ago
On Tue, Jun 10, 2025 at 11:21 PM Bernhard Beschow <shentey@gmail.com> wrote:
>
> A 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  | 76 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 81 insertions(+)
>  create mode 100644 rust/qemu-api/src/log.rs
>
> diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
> index 47e9677fcb..dc8c44109e 100644
> --- a/docs/devel/rust.rst
> +++ b/docs/devel/rust.rst
> @@ -162,6 +162,7 @@ module           status
>  ``errno``        complete
>  ``error``        stable
>  ``irq``          complete
> +``log``          proof of concept
>  ``memory``       stable
>  ``module``       complete
>  ``qdev``         stable
> diff --git a/rust/wrapper.h b/rust/wrapper.h
> index 6060d3ba1a..15a1b19847 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 cac8595a14..33caee3c4f 100644
> --- a/rust/qemu-api/meson.build
> +++ b/rust/qemu-api/meson.build
> @@ -21,6 +21,7 @@ _qemu_api_rs = static_library(
>        'src/errno.rs',
>        'src/error.rs',
>        'src/irq.rs',
> +      'src/log.rs',
>        'src/memory.rs',
>        'src/module.rs',
>        'src/prelude.rs',
> diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
> index 93902fc94b..e20be35460 100644
> --- a/rust/qemu-api/src/lib.rs
> +++ b/rust/qemu-api/src/lib.rs
> @@ -21,6 +21,7 @@
>  pub mod errno;
>  pub mod error;
>  pub mod irq;
> +pub mod log;
>  pub mod memory;
>  pub mod module;
>  pub mod qdev;
> diff --git a/rust/qemu-api/src/log.rs b/rust/qemu-api/src/log.rs
> new file mode 100644
> index 0000000000..9e3c61b8b7
> --- /dev/null
> +++ b/rust/qemu-api/src/log.rs
> @@ -0,0 +1,76 @@
> +// Copyright 2025 Bernhard Beschow <shentey@gmail.com>
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#[repr(u32)]
> +/// Represents specific error categories within QEMU's logging system.
> +///
> +/// The `Log` enum provides a Rust abstraction for logging errors, corresponding
> +/// to a subset of the error categories defined in the C implementation.
> +pub enum Log {
> +    /// Log an invalid access caused by the guest.
> +    /// Corresponds to `LOG_GUEST_ERROR` in the C implementation.
> +    GuestError = crate::bindings::LOG_GUEST_ERROR,
> +
> +    /// Log guest access of unimplemented functionality.
> +    /// Corresponds to `LOG_UNIMP` in the C implementation.
> +    Unimp = crate::bindings::LOG_UNIMP,
> +}
> +
> +/// A macro to log messages conditionally based on a provided mask.
> +///
> +/// The `log_mask` macro checks whether the given mask matches the current log
> +/// level and, if so, formats and logs the message. It is the Rust counterpart
> +/// of the qemu_log_mask() macro in the C implementation.
> +///
> +/// # Parameters
> +///
> +/// - `$mask`: A log level mask. This should be a variant of the `Log` enum.
> +/// - `$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;
> +/// use qemu_api::log_mask;
> +///
> +/// let error_address = 0xbad;
> +/// log_mask!(
> +///     Log::GuestError,
> +///     "Address 0x{error_address:x} out of range\n"
> +/// );
> +/// ```
> +///
> +/// It is also possible to use printf-style formatting, as well as having a
> +/// trailing `,`:
> +///
> +/// ```
> +/// use qemu_api::log::Log;
> +/// use qemu_api::log_mask;
> +///
> +/// let error_address = 0xbad;
> +/// log_mask!(
> +///     Log::GuestError,
> +///     "Address 0x{:x} out of range\n",
> +///     error_address,
> +/// );
> +/// ```
> +#[macro_export]
> +macro_rules! log_mask {
> +    ($mask:expr, $fmt:tt $($args:tt)*) => {{
> +        // Type assertion to enforce type `Log` for $mask
> +        let _: Log = $mask;
> +
> +        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());
> +            }
> +        }
> +    }};
> +}
> --
> 2.49.0
>

Maybe we could take this chance to remove the requirement for trailing
newline? Not urgent, and also something we could change afterwards
anyway. We could also introduce log_mask_ln! macro but now I'm just
bikeshedding.

Besides that, I think it'd be useful to have the macro re-exported in
rust/qemu-api/src/prelude.rs as well. Please add it for the next
version.

-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Paolo Bonzini 5 months, 1 week ago
On Wed, Jun 11, 2025 at 12:57 PM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:

> Maybe we could take this chance to remove the requirement for trailing
> newline? Not urgent, and also something we could change afterwards
> anyway. We could also introduce log_mask_ln! macro but now I'm just
> bikeshedding.

Good idea; there is no "formatln!" but I think you could use concat instead.

If that doesn't work for whatever reason we can indeed add it later. I
had the idea of a struct that wraps the logging functions
qemu_log_trylock() and qemu_log_unlock() and implements io::Write; at
which point, implementing log_mask_ln! (or _nl! following the unstable
format_args_nl!) with writeln! would be trivial.

> Besides that, I think it'd be useful to have the macro re-exported in
> rust/qemu-api/src/prelude.rs as well. Please add it for the next
> version.

Yes, I agree.

Paolo
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Manos Pitsidianakis 5 months, 1 week ago
On Wed, Jun 11, 2025 at 2:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Wed, Jun 11, 2025 at 12:57 PM Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
>
> > Maybe we could take this chance to remove the requirement for trailing
> > newline? Not urgent, and also something we could change afterwards
> > anyway. We could also introduce log_mask_ln! macro but now I'm just
> > bikeshedding.
>
> Good idea; there is no "formatln!" but I think you could use concat instead.

I think `let formatted_string = format!("{}\n", format_args!($fmt
$($args)*));` might be sufficient, but I haven't checked it myself

-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Bernhard Beschow 5 months ago

Am 11. Juni 2025 12:17:57 UTC schrieb Manos Pitsidianakis <manos.pitsidianakis@linaro.org>:
>On Wed, Jun 11, 2025 at 2:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On Wed, Jun 11, 2025 at 12:57 PM Manos Pitsidianakis
>> <manos.pitsidianakis@linaro.org> wrote:
>>
>> > Maybe we could take this chance to remove the requirement for trailing
>> > newline? Not urgent, and also something we could change afterwards
>> > anyway. We could also introduce log_mask_ln! macro but now I'm just
>> > bikeshedding.
>>
>> Good idea; there is no "formatln!" but I think you could use concat instead.
>
>I think `let formatted_string = format!("{}\n", format_args!($fmt
>$($args)*));` might be sufficient, but I haven't checked it myself
>

So the idea is to have a log_mask_ln! macro instead, since there isn't really a point for a macro that doesn't add `\n` at the end. Correct?
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Paolo Bonzini 5 months ago
Il gio 12 giu 2025, 09:37 Bernhard Beschow <shentey@gmail.com> ha scritto:

> So the idea is to have a log_mask_ln! macro instead, since there isn't
> really a point for a macro that doesn't add `\n` at the end. Correct?
>

Yes, or both.

Paolo

>
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Bernhard Beschow 5 months ago

Am 11. Juni 2025 12:17:57 UTC schrieb Manos Pitsidianakis <manos.pitsidianakis@linaro.org>:
>On Wed, Jun 11, 2025 at 2:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On Wed, Jun 11, 2025 at 12:57 PM Manos Pitsidianakis
>> <manos.pitsidianakis@linaro.org> wrote:
>>
>> > Maybe we could take this chance to remove the requirement for trailing
>> > newline? Not urgent, and also something we could change afterwards
>> > anyway. We could also introduce log_mask_ln! macro but now I'm just
>> > bikeshedding.
>>
>> Good idea; there is no "formatln!" but I think you could use concat instead.
>
>I think `let formatted_string = format!("{}\n", format_args!($fmt
>$($args)*));` might be sufficient, but I haven't checked it myself
>

So the idea is to have a log_mask_ln! macro instead, since there isn't really a point for a macro that doesn't add `\n` at the end. Correct?
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Paolo Bonzini 5 months, 1 week ago
On 6/10/25 22:21, Bernhard Beschow wrote:
> +/// A macro to log messages conditionally based on a provided mask.
> +///
> +/// The `log_mask` macro checks whether the given mask matches the current log
> +/// level and, if so, formats and logs the message. It is the Rust counterpart
> +/// of the qemu_log_mask() macro in the C implementation.

Clippy complains that it wants `` around the function name.

Paolo
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Bernhard Beschow 5 months, 1 week ago

Am 11. Juni 2025 07:56:23 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>On 6/10/25 22:21, Bernhard Beschow wrote:
>> +/// A macro to log messages conditionally based on a provided mask.
>> +///
>> +/// The `log_mask` macro checks whether the given mask matches the current log
>> +/// level and, if so, formats and logs the message. It is the Rust counterpart
>> +/// of the qemu_log_mask() macro in the C implementation.
>
>Clippy complains that it wants `` around the function name.

Will fix in v3.

>
>Paolo
>
Re: [PATCH v2 1/2] rust/qemu-api: Add initial logging support based on C API
Posted by Paolo Bonzini 5 months, 1 week ago
Il mer 11 giu 2025, 12:21 Bernhard Beschow <shentey@gmail.com> ha scritto:

> Am 11. Juni 2025 07:56:23 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
> >Clippy complains that it wants `` around the function name.
>
> Will fix in v3.
>

While at it, please run it through rustfmt and add a "//!" comment at the
top, such as "Bindings for QEMU's logging infrastructure".

You can build the documentation with "ninja rustdoc" so you can check the
result of the doc comments.

Thanks,

Paolo


> >
> >Paolo
> >
>
>