[PATCH] rust: log: implement io::Write

[PATCH] rust: log: implement io::Write

[Paolo Bonzini]

This makes it possible to lock the log file; it also makes log_mask_ln!
not allocate memory when logging a constant string.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/log.h       |  2 +
 util/log.c               | 12 ++++++
 rust/qemu-api/src/log.rs | 87 ++++++++++++++++++++++++++++++++++++----
 3 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/include/qemu/log.h b/include/qemu/log.h
index 60da703e670..aae72985f0d 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -84,6 +84,8 @@ typedef struct QEMULogItem {
 
 extern const QEMULogItem qemu_log_items[];
 
+ssize_t rust_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);
+
 bool qemu_set_log(int log_flags, Error **errp);
 bool qemu_set_log_filename(const char *filename, Error **errp);
 bool qemu_set_log_filename_flags(const char *name, int flags, Error **errp);
diff --git a/util/log.c b/util/log.c
index b87d399e4cb..58d24de48a0 100644
--- a/util/log.c
+++ b/util/log.c
@@ -558,3 +558,15 @@ void qemu_print_log_usage(FILE *f)
     fprintf(f, "\nUse \"-d trace:help\" to get a list of trace events.\n\n");
 #endif
 }
+
+#ifdef CONFIG_HAVE_RUST
+ssize_t rust_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream)
+{
+    /*
+     * Same as fwrite, but return -errno because Rust libc does not provide
+     * portable access to errno. :(
+     */
+    int ret = fwrite(ptr, size, nmemb, stream);
+    return ret < 0 ? -errno : 0;
+}
+#endif
diff --git a/rust/qemu-api/src/log.rs b/rust/qemu-api/src/log.rs
index d6c3d6c1b63..ef7acd32520 100644
--- a/rust/qemu-api/src/log.rs
+++ b/rust/qemu-api/src/log.rs
@@ -3,6 +3,10 @@
 
 //! Bindings for QEMU's logging infrastructure
 
+use std::{io, ptr::NonNull};
+
+use crate::{bindings, errno};
+
 #[repr(u32)]
 /// Represents specific error categories within QEMU's logging system.
 ///
@@ -11,11 +15,79 @@
 pub enum Log {
     /// Log invalid access caused by the guest.
     /// Corresponds to `LOG_GUEST_ERROR` in the C implementation.
-    GuestError = crate::bindings::LOG_GUEST_ERROR,
+    GuestError = bindings::LOG_GUEST_ERROR,
 
     /// Log guest access of unimplemented functionality.
     /// Corresponds to `LOG_UNIMP` in the C implementation.
-    Unimp = crate::bindings::LOG_UNIMP,
+    Unimp = bindings::LOG_UNIMP,
+}
+
+/// A RAII guard for QEMU's logging infrastructure.  Creating the guard
+/// locks the log file, and dropping it (letting it go out of scope) unlocks
+/// the file.
+///
+/// As long as the guard lives, it can be written to using [`std::io::Write`].
+///
+/// The locking is recursive, therefore owning a guard does not prevent
+/// using [`log_mask_ln!()`](crate::log_mask_ln).
+pub struct LogGuard(NonNull<bindings::FILE>);
+
+impl LogGuard {
+    /// Return a RAII guard that writes to QEMU's logging infrastructure.
+    /// The log file is locked while the guard exists, ensuring that there
+    /// is no tearing of the messages.
+    ///
+    /// Return `None` if the log file is closed and could not be opened.
+    /// Do *not* use `unwrap()` on the result; failure can be handled simply
+    /// by not logging anything.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use qemu_api::log::LogGuard;
+    /// # use std::io::Write;
+    /// if let Some(mut log) = LogGuard::new() {
+    ///     writeln!(log, "test");
+    /// }
+    /// ```
+    pub fn new() -> Option<Self> {
+        let f = unsafe { bindings::qemu_log_trylock() }.cast();
+        NonNull::new(f).map(Self)
+    }
+
+    /// Writes a formatted string into the log, returning any error encountered.
+    ///
+    /// This method is primarily used by the [`log_mask_ln!()`](crate::log_mask_ln)
+    /// macro, and it is rare for it to be called explicitly.
+    pub fn log_fmt(args: std::fmt::Arguments) -> io::Result<()> {
+        if let Some(mut log) = Self::new() {
+            use io::Write;
+            log.write_fmt(args)?;
+        }
+        Ok(())
+    }
+}
+
+impl io::Write for LogGuard {
+    fn write(&mut self, bytes: &[u8]) -> io::Result<usize> {
+        let ret = unsafe {
+            bindings::rust_fwrite(bytes.as_ptr().cast(), 1, bytes.len(), self.0.as_ptr())
+        };
+        errno::into_io_result(ret)
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        // Do nothing, dropping the guard takes care of flushing
+        Ok(())
+    }
+}
+
+impl Drop for LogGuard {
+    fn drop(&mut self) {
+        unsafe {
+            bindings::qemu_log_unlock(self.0.as_ptr());
+        }
+    }
 }
 
 /// A macro to log messages conditionally based on a provided mask.
@@ -24,6 +96,8 @@ pub enum Log {
 /// 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.
 ///
+/// Errors from writing to the log are ignored.
+///
 /// # Parameters
 ///
 /// - `$mask`: A log level mask. This should be a variant of the `Log` enum.
@@ -62,12 +136,9 @@ macro_rules! log_mask_ln {
         if unsafe {
             (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
         } {
-            let formatted_string = format!("{}\n", format_args!($fmt $($args)*));
-            let c_string = std::ffi::CString::new(formatted_string).unwrap();
-
-            unsafe {
-                ::qemu_api::bindings::qemu_log(c_string.as_ptr());
-            }
+            #[allow(unused_must_use)]
+            ::qemu_api::log::LogGuard::log_fmt(
+                format_args!("{}\n", format_args!($fmt $($args)*)));
         }
     }};
 }
-- 
2.49.0

Re: [PATCH] rust: log: implement io::Write

[Zhao Liu]

> +/// A RAII guard for QEMU's logging infrastructure.  Creating the guard
> +/// locks the log file, and dropping it (letting it go out of scope) unlocks
> +/// the file.
> +///
> +/// As long as the guard lives, it can be written to using [`std::io::Write`].
> +///
> +/// The locking is recursive, therefore owning a guard does not prevent
> +/// using [`log_mask_ln!()`](crate::log_mask_ln).
> +pub struct LogGuard(NonNull<bindings::FILE>);

Nice! I happen to be implementing a similar RAII for FlatView:

pub struct FlatViewRefGuard(NonNull<FlatView>);

> +impl LogGuard {

...

> +    /// Writes a formatted string into the log, returning any error encountered.
> +    ///
> +    /// This method is primarily used by the [`log_mask_ln!()`](crate::log_mask_ln)
> +    /// macro, and it is rare for it to be called explicitly.
> +    pub fn log_fmt(args: std::fmt::Arguments) -> io::Result<()> {
> +        if let Some(mut log) = Self::new() {
> +            use io::Write;

I think it's better to place this "use" at the beginning of the file.

> +            log.write_fmt(args)?;
> +        }
> +        Ok(())
> +    }
> +}

Overall, LGTM,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH] rust: log: implement io::Write

[Zhao Liu]

>  /// A macro to log messages conditionally based on a provided mask.
> @@ -24,6 +96,8 @@ pub enum Log {
>  /// 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.
>  ///
> +/// Errors from writing to the log are ignored.
> +///
>  /// # Parameters
>  ///
>  /// - `$mask`: A log level mask. This should be a variant of the `Log` enum.
> @@ -62,12 +136,9 @@ macro_rules! log_mask_ln {
>          if unsafe {
>              (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
>          } {
> -            let formatted_string = format!("{}\n", format_args!($fmt $($args)*));
> -            let c_string = std::ffi::CString::new(formatted_string).unwrap();
> -
> -            unsafe {
> -                ::qemu_api::bindings::qemu_log(c_string.as_ptr());
> -            }
> +            #[allow(unused_must_use)]

I found this doesn't work :-( :

error: unused `Result` that must be used
   --> ../rust/hw/char/pl011/src/device.rs:281:21
    |
281 |                     log_mask_ln!(Log::Unimp, "pl011: DMA not implemented");
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: this `Result` may be an `Err` variant, which should be handled
    = note: `-D unused-must-use` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(unused_must_use)]`
    = note: this error originates in the macro `log_mask_ln` (in Nightly builds, run with -Z macro-backtrace for more info)

I understand meson sets `-D warings` so that `allow` can't work...

What about just ignoring the return value? Afterall pl011 doesn't care
the returned value.

@@ -136,8 +137,7 @@ macro_rules! log_mask_ln {
         if unsafe {
             (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
         } {
-            #[allow(unused_must_use)]
-            ::qemu_api::log::LogGuard::log_fmt(
+            let _ = ::qemu_api::log::LogGuard::log_fmt(
                 format_args!("{}\n", format_args!($fmt $($args)*)));
         }
     }};

Thanks,
Zhao

> +            ::qemu_api::log::LogGuard::log_fmt(
> +                format_args!("{}\n", format_args!($fmt $($args)*)));
>          }
>      }};
>  }
> -- 
> 2.49.0
> 
> 

Re: [PATCH] rust: log: implement io::Write

[Manos Pitsidianakis]

On Tue, Jun 24, 2025 at 10:00 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> >  /// A macro to log messages conditionally based on a provided mask.
> > @@ -24,6 +96,8 @@ pub enum Log {
> >  /// 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.
> >  ///
> > +/// Errors from writing to the log are ignored.
> > +///
> >  /// # Parameters
> >  ///
> >  /// - `$mask`: A log level mask. This should be a variant of the `Log` enum.
> > @@ -62,12 +136,9 @@ macro_rules! log_mask_ln {
> >          if unsafe {
> >              (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
> >          } {
> > -            let formatted_string = format!("{}\n", format_args!($fmt $($args)*));
> > -            let c_string = std::ffi::CString::new(formatted_string).unwrap();
> > -
> > -            unsafe {
> > -                ::qemu_api::bindings::qemu_log(c_string.as_ptr());
> > -            }
> > +            #[allow(unused_must_use)]
>
> I found this doesn't work :-( :
>
> error: unused `Result` that must be used
>    --> ../rust/hw/char/pl011/src/device.rs:281:21
>     |
> 281 |                     log_mask_ln!(Log::Unimp, "pl011: DMA not implemented");
>     |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>     |
>     = note: this `Result` may be an `Err` variant, which should be handled
>     = note: `-D unused-must-use` implied by `-D warnings`
>     = help: to override `-D warnings` add `#[allow(unused_must_use)]`
>     = note: this error originates in the macro `log_mask_ln` (in Nightly builds, run with -Z macro-backtrace for more info)
>
> I understand meson sets `-D warings` so that `allow` can't work...
>
> What about just ignoring the return value? Afterall pl011 doesn't care
> the returned value.
>
> @@ -136,8 +137,7 @@ macro_rules! log_mask_ln {
>          if unsafe {
>              (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
>          } {
> -            #[allow(unused_must_use)]
> -            ::qemu_api::log::LogGuard::log_fmt(
> +            let _ = ::qemu_api::log::LogGuard::log_fmt(
>                  format_args!("{}\n", format_args!($fmt $($args)*)));
>          }
>      }};


Just `_ = ::qemu_api::log::LogGuard::log_fmt(...);` is sufficient, no
`let` needed.

Paolo: I haven't tested it to see the warning Zhao did (busy
lately...), but LGTM, as well. With that fixed,

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

Re: [PATCH] rust: log: implement io::Write

[Zhao Liu]

> > @@ -136,8 +137,7 @@ macro_rules! log_mask_ln {
> >          if unsafe {
> >              (::qemu_api::bindings::qemu_loglevel & ($mask as std::os::raw::c_int)) != 0
> >          } {
> > -            #[allow(unused_must_use)]
> > -            ::qemu_api::log::LogGuard::log_fmt(
> > +            let _ = ::qemu_api::log::LogGuard::log_fmt(
> >                  format_args!("{}\n", format_args!($fmt $($args)*)));
> >          }
> >      }};
> 
> 
> Just `_ = ::qemu_api::log::LogGuard::log_fmt(...);` is sufficient, no
> `let` needed.

Good catch! Thank you Manos.

Regards,
Zhao