[RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[Paolo Bonzini]

This includes a few fixes to the Rust build system machinery, and
removes constructs that were added or stabilized after version 1.63.0:

- "let else" (by patching bilge-impl, patch 4; stable in 1.65.0)

- std::sync::OnceLock (patch 6; stable in 1.70.0)

- core::ffi (patch 7; stable in 1.64.0)

- c"" literals (patch 9; stable in 1.77.0)

- offset_of! (patch 10; stable in 1.77.0)

- MaybeUninit::zeroed() (patch 11; stable in 1.75.0 in const context)

It also replicates the configuration checks normally done by
proc-macro2's build.rs into our Meson-based build rules, so that
the crate can be made to work with an older version of rustc.

As a small bonus, patch 11 removes some uses of unsafe, so that patch
probably won't just be simply reverted even once we can assume version
1.75.0 of the language.  And as another small bonus this series introduces
the first use of Rust unit tests.

On top of this, the required version of bindgen is still too new
for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.

This is an RFC for two reasons.  First, because it would be a valid
decision to delay enabling of Rust until at least some of these
features are available in all supported distros.  Another possibility
could be to accept Rust 1.64.0 but require installing a newer bindgen
(0.66.x for example) on those two distros with an older release.  Second,
because the series is missing the CI updates to actually ensure that
these minimum versions keep working.

The main purpose is to show the kind of hacks that would be needed
to support older toolchains.  The fixes (for example patches
1/2/3/6/8/11/13/14) can be easily extracted independent of the outcome
of the discussion, and/or while the CI updates are made.

Thanks,

Paolo

Paolo Bonzini (16):
  meson: import rust module into a global variable
  meson: remove repeated search for rust_root_crate.sh
  rust: pass rustc_args when building all crates
  rust: patch bilge-impl to allow compilation with 1.63.0
  rust: fix cfgs of proc-macro2 for 1.63.0
  rust: do not use OnceLock for properties
  rust: use std::os::raw instead of core::ffi
  rust: build tests for the qemu_api crate
  rust: introduce a c_str macro
  rust: introduce alternative implementation of offset_of!
  rust: do not use MaybeUninit::zeroed()
  rust: allow version 1.63.0 of rustc
  rust: do not use TYPE_CHARDEV unnecessarily
  rust: do not use --no-size_t-is-usize
  rust: do not use --generate-cstr
  rust: allow older version of bindgen

 meson.build                                   |  42 ++++---
 .gitattributes                                |   2 +
 rust/hw/char/pl011/src/device.rs              | 117 ++++++++++--------
 rust/hw/char/pl011/src/device_class.rs        |  13 +-
 rust/hw/char/pl011/src/lib.rs                 |   4 +-
 rust/hw/char/pl011/src/memory_ops.rs          |  21 ++--
 rust/qemu-api-macros/meson.build              |   2 +-
 rust/qemu-api/meson.build                     |  17 ++-
 rust/qemu-api/src/c_str.rs                    |  52 ++++++++
 rust/qemu-api/src/definitions.rs              |  10 +-
 rust/qemu-api/src/device_class.rs             |  49 +++++---
 rust/qemu-api/src/lib.rs                      |  14 ++-
 rust/qemu-api/src/offset_of.rs                | 106 ++++++++++++++++
 rust/qemu-api/src/tests.rs                    |  21 ++--
 rust/qemu-api/src/zeroable.rs                 |  75 +++++++++++
 subprojects/bilge-impl-0.2-rs.wrap            |   1 +
 .../packagefiles/bilge-impl-1.63.0.patch      |  45 +++++++
 .../packagefiles/proc-macro2-1-rs/meson.build |   4 +-
 18 files changed, 469 insertions(+), 126 deletions(-)
 create mode 100644 rust/qemu-api/src/c_str.rs
 create mode 100644 rust/qemu-api/src/offset_of.rs
 create mode 100644 rust/qemu-api/src/zeroable.rs
 create mode 100644 subprojects/packagefiles/bilge-impl-1.63.0.patch

-- 
2.46.2

[PATCH 01/16] meson: import rust module into a global variable

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                      | 1 +
 rust/qemu-api-macros/meson.build | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index c85f964bed4..c2e736d2051 100644
--- a/meson.build
+++ b/meson.build
@@ -15,6 +15,7 @@ meson.add_postconf_script(find_program('scripts/symlink-install-tree.py'))
 
 not_found = dependency('', required: false)
 keyval = import('keyval')
+rust = import('rust')
 ss = import('sourceset')
 fs = import('fs')
 
diff --git a/rust/qemu-api-macros/meson.build b/rust/qemu-api-macros/meson.build
index 517b9a4d2d5..24325dea5c2 100644
--- a/rust/qemu-api-macros/meson.build
+++ b/rust/qemu-api-macros/meson.build
@@ -2,7 +2,7 @@ quote_dep = dependency('quote-1-rs', native: true)
 syn_dep = dependency('syn-2-rs', native: true)
 proc_macro2_dep = dependency('proc-macro2-1-rs', native: true)
 
-_qemu_api_macros_rs = import('rust').proc_macro(
+_qemu_api_macros_rs = rust.proc_macro(
   'qemu_api_macros',
   files('src/lib.rs'),
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
-- 
2.46.2

Re: [PATCH 01/16] meson: import rust module into a global variable

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:19PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:19 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 01/16] meson: import rust module into a global variable
> X-Mailer: git-send-email 2.46.2
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                      | 1 +
>  rust/qemu-api-macros/meson.build | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 

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

[PATCH 02/16] meson: remove repeated search for rust_root_crate.sh

[Paolo Bonzini]

Avoid repeated lines of the form

Program scripts/rust/rust_root_crate.sh found: YES (/home/pbonzini/work/upstream/qemu/scripts/rust/rust_root_crate.sh)

in the meson logs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index c2e736d2051..37f94ab32aa 100644
--- a/meson.build
+++ b/meson.build
@@ -3977,6 +3977,7 @@ endif
 
 
 feature_to_c = find_program('scripts/feature_to_c.py')
+rust_root_crate = find_program('scripts/rust/rust_root_crate.sh')
 
 if host_os == 'darwin'
   entitlement = find_program('scripts/entitlement.sh')
@@ -4078,7 +4079,7 @@ foreach target : target_dirs
     if crates.length() > 0
       rlib_rs = custom_target('rust_' + target.underscorify() + '.rs',
                               output: 'rust_' + target.underscorify() + '.rs',
-                              command: [find_program('scripts/rust/rust_root_crate.sh')] + crates,
+                              command: [rust_root_crate, crates],
                               capture: true,
                               build_by_default: true,
                               build_always_stale: true)
-- 
2.46.2

Re: [PATCH 02/16] meson: remove repeated search for rust_root_crate.sh

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Avoid repeated lines of the form
>
> Program scripts/rust/rust_root_crate.sh found: YES (/home/pbonzini/work/upstream/qemu/scripts/rust/rust_root_crate.sh)
>
> in the meson logs.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Junjie Mao <junjie.mao@hotmail.com>

Thanks for cleaning this up!

--
Best Regards
Junjie Mao

Re: [PATCH 02/16] meson: remove repeated search for rust_root_crate.sh

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:20PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:20 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 02/16] meson: remove repeated search for rust_root_crate.sh
> X-Mailer: git-send-email 2.46.2
> 
> Avoid repeated lines of the form
> 
> Program scripts/rust/rust_root_crate.sh found: YES (/home/pbonzini/work/upstream/qemu/scripts/rust/rust_root_crate.sh)
> 
> in the meson logs.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

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

[PATCH 03/16] rust: pass rustc_args when building all crates

[Paolo Bonzini]

rustc_args is needed to smooth the difference in warnings between the various
versions of rustc.  Always include those arguments.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                       | 18 +++++++++++-------
 rust/qemu-api/meson.build         |  2 +-
 rust/qemu-api/src/device_class.rs | 10 ++++++----
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/meson.build b/meson.build
index 37f94ab32aa..2545185014e 100644
--- a/meson.build
+++ b/meson.build
@@ -3317,6 +3317,17 @@ endif
 
 genh += configure_file(output: 'config-host.h', configuration: config_host_data)
 
+if have_rust and have_system
+  rustc_args = run_command(
+    find_program('scripts/rust/rustc_args.py'),
+    '--config-headers', meson.project_build_root() / 'config-host.h',
+    capture : true,
+    check: true).stdout().strip().split()
+  rustc_args += ['-D', 'unsafe_op_in_unsafe_fn']
+  add_project_arguments(rustc_args, native: false, language: 'rust')
+  add_project_arguments(rustc_args, native: true, language: 'rust')
+endif
+
 hxtool = find_program('scripts/hxtool')
 shaderinclude = find_program('scripts/shaderinclude.py')
 qapi_gen = find_program('scripts/qapi-gen.py')
@@ -3909,12 +3920,6 @@ common_all = static_library('common',
                             dependencies: common_ss.all_dependencies())
 
 if have_rust and have_system
-  rustc_args = run_command(
-    find_program('scripts/rust/rustc_args.py'),
-    '--config-headers', meson.project_build_root() / 'config-host.h',
-    capture : true,
-    check: true).stdout().strip().split()
-  rustc_args += ['-D', 'unsafe_op_in_unsafe_fn']
   bindgen_args = [
     '--disable-header-comment',
     '--raw-line', '// @generated',
@@ -4087,7 +4092,6 @@ foreach target : target_dirs
                             rlib_rs,
                             dependencies: target_rust.dependencies(),
                             override_options: ['rust_std=2021', 'build.rust_std=2021'],
-                            rust_args: rustc_args,
                             rust_abi: 'c')
       arch_deps += declare_dependency(link_whole: [rlib])
     endif
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index c72d34b607d..42ea815fa5a 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -10,7 +10,7 @@ _qemu_api_rs = static_library(
   ),
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
-  rust_args: rustc_args + [
+  rust_args: [
     '--cfg', 'MESON',
     # '--cfg', 'feature="allocator"',
   ],
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index 1ea95beb78d..b6b68cf9ce2 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -16,10 +16,12 @@ macro_rules! device_class_init {
         ) {
             let mut dc =
                 ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap();
-            dc.as_mut().realize = $realize_fn;
-            dc.as_mut().vmsd = &$vmsd;
-            $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn);
-            $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr());
+            unsafe {
+                dc.as_mut().realize = $realize_fn;
+                dc.as_mut().vmsd = &$vmsd;
+                $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn);
+                $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr());
+            }
         }
     };
 }
-- 
2.46.2

Re: [PATCH 03/16] rust: pass rustc_args when building all crates

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:21PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:21 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 03/16] rust: pass rustc_args when building all crates
> X-Mailer: git-send-email 2.46.2
> 
> rustc_args is needed to smooth the difference in warnings between the various
> versions of rustc.  Always include those arguments.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                       | 18 +++++++++++-------
>  rust/qemu-api/meson.build         |  2 +-
>  rust/qemu-api/src/device_class.rs | 10 ++++++----
>  3 files changed, 18 insertions(+), 12 deletions(-)

LGTM (with one trivial comment inline)

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

> diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
> index 1ea95beb78d..b6b68cf9ce2 100644
> --- a/rust/qemu-api/src/device_class.rs
> +++ b/rust/qemu-api/src/device_class.rs
> @@ -16,10 +16,12 @@ macro_rules! device_class_init {
>          ) {
>              let mut dc =
>                  ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap();
> -            dc.as_mut().realize = $realize_fn;
> -            dc.as_mut().vmsd = &$vmsd;
> -            $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn);
> -            $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr());
> +            unsafe {
> +                dc.as_mut().realize = $realize_fn;
> +                dc.as_mut().vmsd = &$vmsd;
> +                $crate::bindings::device_class_set_legacy_reset(dc.as_mut(), $legacy_reset_fn);
> +                $crate::bindings::device_class_set_props(dc.as_mut(), $props.as_mut_ptr());
> +            }

The issue exists because the unsafe_op_in_unsafe_fn is allowed in
rust/qemu-api/src/lib.rs. So should we wrap the bindings in a separate
lib (similar to the rust/bindings in the Linux kernel)?

This way, the special lint settings can be applied only to the binding
files, while the default lint checks can cover the other user
development code.

In addition, another thing that confuses me is why bindgen still
generates code that does not follow the unsafe_op_in_unsafe_fn
requirement. It seems that bindgen has supported unsafe_op_in_unsafe_fn
since v0.62 [1, 2], but binding code we generated still violates
unsafe_op_in_unsafe_fn. Is this a bug of bindgen?

[1]: https://github.com/rust-lang/rust-bindgen/pull/2266 
[2]: https://github.com/rust-lang/rust-bindgen/blob/main/CHANGELOG.md#changed-12

Regards,
Zhao

Re: [PATCH 03/16] rust: pass rustc_args when building all crates

[Paolo Bonzini]

On Mon, Oct 21, 2024 at 8:16 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> unsafe_op_in_unsafe_fn is allowed in
> rust/qemu-api/src/lib.rs. So should we wrap the bindings in a separate
> lib (similar to the rust/bindings in the Linux kernel)?
>
> This way, the special lint settings can be applied only to the binding
> files, while the default lint checks can cover the other user
> development code.
>
> In addition, another thing that confuses me is why bindgen still
> generates code that does not follow the unsafe_op_in_unsafe_fn
> requirement. It seems that bindgen has supported unsafe_op_in_unsafe_fn
> since v0.62 [1, 2], but binding code we generated still violates
> unsafe_op_in_unsafe_fn. Is this a bug of bindgen?

The plan is to support older versions of bindgen (0.60.x) as long as
Debian has them. One possibility to fix this is, as you said, to use a
completely separate crate. Another is to add #![allow()] to just the
bindings module, for example by changing bindgen.rs to

#![allow(...)]
include!("bindgen.rs.inc")

This is related to the fact that we don't have yet a good way to run
"clippy", because "cargo clippy" needs the bindgen.rs file. So we
should probably look at these issues at once.

Paolo

Re: [PATCH 03/16] rust: pass rustc_args when building all crates

[Zhao Liu]

On Mon, Oct 21, 2024 at 03:38:06PM +0200, Paolo Bonzini wrote:
> Date: Mon, 21 Oct 2024 15:38:06 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 03/16] rust: pass rustc_args when building all crates
> 
> On Mon, Oct 21, 2024 at 8:16 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> > unsafe_op_in_unsafe_fn is allowed in
> > rust/qemu-api/src/lib.rs. So should we wrap the bindings in a separate
> > lib (similar to the rust/bindings in the Linux kernel)?
> >
> > This way, the special lint settings can be applied only to the binding
> > files, while the default lint checks can cover the other user
> > development code.
> >
> > In addition, another thing that confuses me is why bindgen still
> > generates code that does not follow the unsafe_op_in_unsafe_fn
> > requirement. It seems that bindgen has supported unsafe_op_in_unsafe_fn
> > since v0.62 [1, 2], but binding code we generated still violates
> > unsafe_op_in_unsafe_fn. Is this a bug of bindgen?
> 
> The plan is to support older versions of bindgen (0.60.x) as long as
> Debian has them. One possibility to fix this is, as you said, to use a
> completely separate crate. Another is to add #![allow()] to just the
> bindings module, for example by changing bindgen.rs to
> 
> #![allow(...)]
> include!("bindgen.rs.inc")
> 
> This is related to the fact that we don't have yet a good way to run
> "clippy", because "cargo clippy" needs the bindgen.rs file. So we
> should probably look at these issues at once.

Thank you. I agree, it's a better way.

Regards,
Zhao

Re: [PATCH 03/16] rust: pass rustc_args when building all crates

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On Mon, Oct 21, 2024 at 8:16 AM Zhao Liu <zhao1.liu@intel.com> wrote:
>> unsafe_op_in_unsafe_fn is allowed in
>> rust/qemu-api/src/lib.rs. So should we wrap the bindings in a separate
>> lib (similar to the rust/bindings in the Linux kernel)?
>>
>> This way, the special lint settings can be applied only to the binding
>> files, while the default lint checks can cover the other user
>> development code.
>>
>> In addition, another thing that confuses me is why bindgen still
>> generates code that does not follow the unsafe_op_in_unsafe_fn
>> requirement. It seems that bindgen has supported unsafe_op_in_unsafe_fn
>> since v0.62 [1, 2], but binding code we generated still violates
>> unsafe_op_in_unsafe_fn. Is this a bug of bindgen?
>
> The plan is to support older versions of bindgen (0.60.x) as long as
> Debian has them. One possibility to fix this is, as you said, to use a
> completely separate crate. Another is to add #![allow()] to just the
> bindings module, for example by changing bindgen.rs to
>
> #![allow(...)]
> include!("bindgen.rs.inc")
>
> This is related to the fact that we don't have yet a good way to run
> "clippy", because "cargo clippy" needs the bindgen.rs file. So we
> should probably look at these issues at once.
>
> Paolo

Since meson 0.6.0 clippy-driver can be used as a wrapper of rustc. So we
can run clippy by:

   mkdir build.clippy && cd build.clippy
   RUSTC=clippy-driver ../configure --enable-rust ...
   ninja librust_x86_64_softmmu.a

--
Best Regards
Junjie Mao

Re: [PATCH 03/16] rust: pass rustc_args when building all crates

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

Il mar 22 ott 2024, 04:35 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > On Mon, Oct 21, 2024 at 8:16 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> >> unsafe_op_in_unsafe_fn is allowed in
> >> rust/qemu-api/src/lib.rs. So should we wrap the bindings in a separate
> >> lib (similar to the rust/bindings in the Linux kernel)?
> >>
> >> This way, the special lint settings can be applied only to the binding
> >> files, while the default lint checks can cover the other user
> >> development code.
> >>
> >> In addition, another thing that confuses me is why bindgen still
> >> generates code that does not follow the unsafe_op_in_unsafe_fn
> >> requirement. It seems that bindgen has supported unsafe_op_in_unsafe_fn
> >> since v0.62 [1, 2], but binding code we generated still violates
> >> unsafe_op_in_unsafe_fn. Is this a bug of bindgen?
> >
> > The plan is to support older versions of bindgen (0.60.x) as long as
> > Debian has them. One possibility to fix this is, as you said, to use a
> > completely separate crate. Another is to add #![allow()] to just the
> > bindings module, for example by changing bindgen.rs to
> >
> > #![allow(...)]
> > include!("bindgen.rs.inc")
> >
> > This is related to the fact that we don't have yet a good way to run
> > "clippy", because "cargo clippy" needs the bindgen.rs file. So we
> > should probably look at these issues at once.
> >
> > Paolo
>
> Since meson 0.6.0 clippy-driver can be used as a wrapper of rustc. So we
> can run clippy by:
>
>    mkdir build.clippy && cd build.clippy
>    RUSTC=clippy-driver ../configure --enable-rust ...
>    ninja librust_x86_64_softmmu.a
>

True but it's less handy to have a separately-configured tree instead of
just "make clippy". Also the same is true of rustfmt and rustdoc (which
ideally would be part of the build so that doctests are also run by make
check-unit). So the question of how to emulate these other cargo tools is
open.

Paolo


> --
> Best Regards
> Junjie Mao
>
>

[-- Attachment #2: Type: text/html, Size: 3336 bytes --]

[PATCH 04/16] rust: patch bilge-impl to allow compilation with 1.63.0

[Paolo Bonzini]

Apply a patch that removes "let ... else" constructs, replacing them with
"if let ... else" or "let ... = match ...".  "let ... else" was stabilized in
Rust 1.65.0.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitattributes                                |  2 +
 subprojects/bilge-impl-0.2-rs.wrap            |  1 +
 .../packagefiles/bilge-impl-1.63.0.patch      | 45 +++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 subprojects/packagefiles/bilge-impl-1.63.0.patch

diff --git a/.gitattributes b/.gitattributes
index 6dc6383d3d1..9ce7a19581a 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -5,3 +5,5 @@
 *.rs            diff=rust
 *.rs.inc        diff=rust
 Cargo.lock      diff=toml merge=binary
+
+*.patch         -text -whitespace
diff --git a/subprojects/bilge-impl-0.2-rs.wrap b/subprojects/bilge-impl-0.2-rs.wrap
index eefb10c36c2..b24c34a9043 100644
--- a/subprojects/bilge-impl-0.2-rs.wrap
+++ b/subprojects/bilge-impl-0.2-rs.wrap
@@ -5,3 +5,4 @@ source_filename = bilge-impl-0.2.0.tar.gz
 source_hash = feb11e002038ad243af39c2068c8a72bcf147acf05025dcdb916fcc000adb2d8
 #method = cargo
 patch_directory = bilge-impl-0.2-rs
+diff_files = bilge-impl-1.63.0.patch
diff --git a/subprojects/packagefiles/bilge-impl-1.63.0.patch b/subprojects/packagefiles/bilge-impl-1.63.0.patch
new file mode 100644
index 00000000000..987428a6d65
--- /dev/null
+++ b/subprojects/packagefiles/bilge-impl-1.63.0.patch
@@ -0,0 +1,45 @@
+--- a/src/shared/discriminant_assigner.rs
++++ b/src/shared/discriminant_assigner.rs
+@@ -26,20 +26,20 @@
+         let discriminant_expr = &discriminant.1;
+         let variant_name = &variant.ident;
+ 
+-        let Expr::Lit(ExprLit { lit: Lit::Int(int), .. }) = discriminant_expr else {
++        if let Expr::Lit(ExprLit { lit: Lit::Int(int), .. }) = discriminant_expr {
++            let discriminant_value: u128 = int.base10_parse().unwrap_or_else(unreachable);
++            if discriminant_value > self.max_value() {
++                abort!(variant, "Value of variant exceeds the given number of bits")
++            }
++
++            Some(discriminant_value)
++        } else {
+             abort!(
+                 discriminant_expr,
+                 "variant `{}` is not a number", variant_name;
+                 help = "only literal integers currently supported"
+             )
+-        };
+-
+-        let discriminant_value: u128 = int.base10_parse().unwrap_or_else(unreachable);
+-        if discriminant_value > self.max_value() {
+-            abort!(variant, "Value of variant exceeds the given number of bits")
+         }
+-
+-        Some(discriminant_value)
+     }
+ 
+     fn assign(&mut self, variant: &Variant) -> u128 {
+--- a/src/shared/fallback.rs
++++ b/src/shared/fallback.rs
+@@ -22,8 +22,9 @@
+             }
+             Unnamed(fields) => {
+                 let variant_fields = fields.unnamed.iter();
+-                let Ok(fallback_value) = variant_fields.exactly_one() else {
+-                    abort!(variant, "fallback variant must have exactly one field"; help = "use only one field or change to a unit variant")
++                let fallback_value = match variant_fields.exactly_one() {
++                    Ok(ok) => ok,
++                    _ => abort!(variant, "fallback variant must have exactly one field"; help = "use only one field or change to a unit variant")
+                 };
+ 
+                 if !is_last_variant {
-- 
2.46.2

Re: [PATCH 04/16] rust: patch bilge-impl to allow compilation with 1.63.0

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:22PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:22 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 04/16] rust: patch bilge-impl to allow compilation with
>  1.63.0
> X-Mailer: git-send-email 2.46.2
> 
> Apply a patch that removes "let ... else" constructs, replacing them with
> "if let ... else" or "let ... = match ...".  "let ... else" was stabilized in
> Rust 1.65.0.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  .gitattributes                                |  2 +
>  subprojects/bilge-impl-0.2-rs.wrap            |  1 +
>  .../packagefiles/bilge-impl-1.63.0.patch      | 45 +++++++++++++++++++
>  3 files changed, 48 insertions(+)
>  create mode 100644 subprojects/packagefiles/bilge-impl-1.63.0.patch
> 

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

[PATCH 05/16] rust: fix cfgs of proc-macro2 for 1.63.0

[Paolo Bonzini]

Replay the configuration that would be computed by build.rs when compiling
on a 1.63.0 compiler.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 subprojects/packagefiles/proc-macro2-1-rs/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/subprojects/packagefiles/proc-macro2-1-rs/meson.build b/subprojects/packagefiles/proc-macro2-1-rs/meson.build
index 818ec59336b..8e601b50ccc 100644
--- a/subprojects/packagefiles/proc-macro2-1-rs/meson.build
+++ b/subprojects/packagefiles/proc-macro2-1-rs/meson.build
@@ -15,7 +15,9 @@ _proc_macro2_rs = static_library(
   rust_abi: 'rust',
   rust_args: [
     '--cfg', 'feature="proc-macro"',
-    '--cfg', 'span_locations',
+    '--cfg', 'no_literal_byte_character',
+    '--cfg', 'no_literal_c_string',
+    '--cfg', 'no_source_text',
     '--cfg', 'wrap_proc_macro',
   ],
   dependencies: [
-- 
2.46.2

Re: [PATCH 05/16] rust: fix cfgs of proc-macro2 for 1.63.0

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:23PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:23 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 05/16] rust: fix cfgs of proc-macro2 for 1.63.0
> X-Mailer: git-send-email 2.46.2
> 
> Replay the configuration that would be computed by build.rs when compiling
> on a 1.63.0 compiler.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  subprojects/packagefiles/proc-macro2-1-rs/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

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

[PATCH 06/16] rust: do not use OnceLock for properties

[Paolo Bonzini]

Properties are initialized lazily but always accessed within the big QEMU lock.

There is no need to have a OnceLock around them, and also OnceCell/OnceLock
were only stabilized in 1.70.0; so remove it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/src/device_class.rs | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index b6b68cf9ce2..87892b50c63 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -2,8 +2,6 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use std::sync::OnceLock;
-
 use crate::bindings::Property;
 
 #[macro_export]
@@ -73,12 +71,15 @@ macro_rules! define_property {
 }
 
 #[repr(C)]
-pub struct Properties<const N: usize>(pub OnceLock<[Property; N]>, pub fn() -> [Property; N]);
+pub struct Properties<const N: usize>(pub Option<[Property; N]>, pub fn() -> [Property; N]);
 
 impl<const N: usize> Properties<N> {
     pub fn as_mut_ptr(&mut self) -> *mut Property {
-        _ = self.0.get_or_init(self.1);
-        self.0.get_mut().unwrap().as_mut_ptr()
+        match self.0 {
+            None => { self.0 = Some(self.1()); },
+            Some(_) => {},
+        }
+        self.0.as_mut().unwrap().as_mut_ptr()
     }
 }
 
@@ -104,7 +105,7 @@ const fn _calc_prop_len() -> usize {
         }
 
         #[no_mangle]
-        pub static mut $ident: $crate::device_class::Properties<PROP_LEN> = $crate::device_class::Properties(::std::sync::OnceLock::new(), _make_properties);
+        pub static mut $ident: $crate::device_class::Properties<PROP_LEN> = $crate::device_class::Properties(None, _make_properties);
     };
 }
 
-- 
2.46.2

Re: [PATCH 06/16] rust: do not use OnceLock for properties

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:24PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:24 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 06/16] rust: do not use OnceLock for properties
> X-Mailer: git-send-email 2.46.2
> 
> Properties are initialized lazily but always accessed within the big QEMU lock.
> 
> There is no need to have a OnceLock around them, and also OnceCell/OnceLock
> were only stabilized in 1.70.0; so remove it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/device_class.rs | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

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

[PATCH 07/16] rust: use std::os::raw instead of core::ffi

[Paolo Bonzini]

core::ffi::c_* types were introduced in Rust 1.64.0.  Use the older types
in std::os::raw, which are now aliases of the types in core::ffi.  There is
no need to compile QEMU as no_std, so this is acceptable as long as we support
a version of Debian with Rust 1.63.0.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                          |  3 +--
 rust/hw/char/pl011/src/device.rs     | 20 +++++++++++---------
 rust/hw/char/pl011/src/lib.rs        |  2 +-
 rust/hw/char/pl011/src/memory_ops.rs | 10 ++++++----
 rust/qemu-api/src/definitions.rs     |  4 +++-
 rust/qemu-api/src/device_class.rs    |  8 ++++----
 rust/qemu-api/src/lib.rs             |  8 +++++---
 7 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/meson.build b/meson.build
index 2545185014e..175b8d82228 100644
--- a/meson.build
+++ b/meson.build
@@ -3923,14 +3923,13 @@ if have_rust and have_system
   bindgen_args = [
     '--disable-header-comment',
     '--raw-line', '// @generated',
-    '--ctypes-prefix', 'core::ffi',
+    '--ctypes-prefix', 'std::os::raw',
     '--formatter', 'rustfmt',
     '--generate-block',
     '--generate-cstr',
     '--impl-debug',
     '--merge-extern-blocks',
     '--no-doc-comments',
-    '--use-core',
     '--with-derive-default',
     '--no-size_t-is-usize',
     '--no-layout-tests',
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index c7193b41bee..cd4c01c2336 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -2,9 +2,11 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use core::{
-    ffi::{c_int, c_uchar, c_uint, c_void, CStr},
-    ptr::{addr_of, addr_of_mut, NonNull},
+use core::ptr::{addr_of, addr_of_mut, NonNull};
+
+use std::{
+    ffi::CStr,
+    os::raw::{c_int, c_uchar, c_uint, c_void}
 };
 
 use qemu_api::{
@@ -89,10 +91,10 @@ pub struct PL011Class {
 
 impl qemu_api::definitions::Class for PL011Class {
     const CLASS_INIT: Option<
-        unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void),
+        unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
     > = Some(crate::device_class::pl011_class_init);
     const CLASS_BASE_INIT: Option<
-        unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut core::ffi::c_void),
+        unsafe extern "C" fn(klass: *mut ObjectClass, data: *mut c_void),
     > = None;
 }
 
@@ -151,7 +153,7 @@ pub unsafe fn init(&mut self) {
     pub fn read(
         &mut self,
         offset: hwaddr,
-        _size: core::ffi::c_uint,
+        _size: c_uint,
     ) -> std::ops::ControlFlow<u64, u64> {
         use RegisterOffset::*;
 
@@ -532,9 +534,9 @@ pub fn update(&self) {
 /// The buffer and size arguments must also be valid.
 #[no_mangle]
 pub unsafe extern "C" fn pl011_receive(
-    opaque: *mut core::ffi::c_void,
+    opaque: *mut c_void,
     buf: *const u8,
-    size: core::ffi::c_int,
+    size: c_int,
 ) {
     unsafe {
         debug_assert!(!opaque.is_null());
@@ -555,7 +557,7 @@ pub fn update(&self) {
 /// the same size as [`PL011State`]. We also expect the device is
 /// readable/writeable from one thread at any time.
 #[no_mangle]
-pub unsafe extern "C" fn pl011_event(opaque: *mut core::ffi::c_void, event: QEMUChrEvent) {
+pub unsafe extern "C" fn pl011_event(opaque: *mut c_void, event: QEMUChrEvent) {
     unsafe {
         debug_assert!(!opaque.is_null());
         let mut state = NonNull::new_unchecked(opaque.cast::<PL011State>());
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index 2939ee50c99..2b157868b0f 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -45,7 +45,7 @@
 pub mod device_class;
 pub mod memory_ops;
 
-pub const TYPE_PL011: &::core::ffi::CStr = c"pl011";
+pub const TYPE_PL011: &::std::ffi::CStr = c"pl011";
 
 /// Offset of each register from the base memory address of the device.
 ///
diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs
index 8d066ebf6d0..2c664fd45ed 100644
--- a/rust/hw/char/pl011/src/memory_ops.rs
+++ b/rust/hw/char/pl011/src/memory_ops.rs
@@ -4,6 +4,8 @@
 
 use core::{mem::MaybeUninit, ptr::NonNull};
 
+use std::os::raw::{c_uint, c_void};
+
 use qemu_api::bindings::*;
 
 use crate::device::PL011State;
@@ -24,9 +26,9 @@
 
 #[no_mangle]
 unsafe extern "C" fn pl011_read(
-    opaque: *mut core::ffi::c_void,
+    opaque: *mut c_void,
     addr: hwaddr,
-    size: core::ffi::c_uint,
+    size: c_uint,
 ) -> u64 {
     assert!(!opaque.is_null());
     let mut state = unsafe { NonNull::new_unchecked(opaque.cast::<PL011State>()) };
@@ -46,10 +48,10 @@
 
 #[no_mangle]
 unsafe extern "C" fn pl011_write(
-    opaque: *mut core::ffi::c_void,
+    opaque: *mut c_void,
     addr: hwaddr,
     data: u64,
-    _size: core::ffi::c_uint,
+    _size: c_uint,
 ) {
     unsafe {
         assert!(!opaque.is_null());
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index 60bd3f8aaa6..aa7cdd69c99 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -4,7 +4,9 @@
 
 //! Definitions required by QEMU when registering a device.
 
-use ::core::ffi::{c_void, CStr};
+use std::ffi::CStr;
+
+use std::os::raw::c_void;
 
 use crate::bindings::{Object, ObjectClass, TypeInfo};
 
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index 87892b50c63..871063d4a92 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -10,7 +10,7 @@ macro_rules! device_class_init {
         #[no_mangle]
         pub unsafe extern "C" fn $func(
             klass: *mut $crate::bindings::ObjectClass,
-            _: *mut ::core::ffi::c_void,
+            _: *mut ::std::os::raw::c_void,
         ) {
             let mut dc =
                 ::core::ptr::NonNull::new(klass.cast::<$crate::bindings::DeviceClass>()).unwrap();
@@ -30,7 +30,7 @@ macro_rules! define_property {
         $crate::bindings::Property {
             name: {
                 #[used]
-                static _TEMP: &::core::ffi::CStr = $name;
+                static _TEMP: &::std::ffi::CStr = $name;
                 _TEMP.as_ptr()
             },
             info: $prop,
@@ -51,7 +51,7 @@ macro_rules! define_property {
         $crate::bindings::Property {
             name: {
                 #[used]
-                static _TEMP: &::core::ffi::CStr = $name;
+                static _TEMP: &::std::ffi::CStr = $name;
                 _TEMP.as_ptr()
             },
             info: $prop,
@@ -121,7 +121,7 @@ macro_rules! vm_state_description {
         pub static $name: $crate::bindings::VMStateDescription = $crate::bindings::VMStateDescription {
             $(name: {
                 #[used]
-                static VMSTATE_NAME: &::core::ffi::CStr = $vname;
+                static VMSTATE_NAME: &::std::ffi::CStr = $vname;
                 $vname.as_ptr()
             },)*
             unmigratable: true,
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index e72fb4b4bb1..c2f60ac4727 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -35,6 +35,8 @@ unsafe impl Sync for bindings::VMStateDescription {}
 
 use std::alloc::{GlobalAlloc, Layout};
 
+use std::os::raw::c_void;
+
 #[cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)]
 extern "C" {
     fn g_aligned_alloc0(
@@ -47,8 +49,8 @@ fn g_aligned_alloc0(
 
 #[cfg(not(HAVE_GLIB_WITH_ALIGNED_ALLOC))]
 extern "C" {
-    fn qemu_memalign(alignment: usize, size: usize) -> *mut ::core::ffi::c_void;
-    fn qemu_vfree(ptr: *mut ::core::ffi::c_void);
+    fn qemu_memalign(alignment: usize, size: usize) -> *mut c_void;
+    fn qemu_vfree(ptr: *mut c_void);
 }
 
 extern "C" {
@@ -113,7 +115,7 @@ fn default() -> Self {
 }
 
 // Sanity check.
-const _: [(); 8] = [(); ::core::mem::size_of::<*mut ::core::ffi::c_void>()];
+const _: [(); 8] = [(); ::core::mem::size_of::<*mut c_void>()];
 
 unsafe impl GlobalAlloc for QemuAllocator {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
-- 
2.46.2

Re: [PATCH 07/16] rust: use std::os::raw instead of core::ffi

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:25PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:25 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 07/16] rust: use std::os::raw instead of core::ffi
> X-Mailer: git-send-email 2.46.2
> 
> core::ffi::c_* types were introduced in Rust 1.64.0.  Use the older types
> in std::os::raw, which are now aliases of the types in core::ffi.  There is
> no need to compile QEMU as no_std, so this is acceptable as long as we support
> a version of Debian with Rust 1.63.0.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                          |  3 +--
>  rust/hw/char/pl011/src/device.rs     | 20 +++++++++++---------
>  rust/hw/char/pl011/src/lib.rs        |  2 +-
>  rust/hw/char/pl011/src/memory_ops.rs | 10 ++++++----
>  rust/qemu-api/src/definitions.rs     |  4 +++-
>  rust/qemu-api/src/device_class.rs    |  8 ++++----
>  rust/qemu-api/src/lib.rs             |  8 +++++---
>  7 files changed, 31 insertions(+), 24 deletions(-)
> 

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

[PATCH 08/16] rust: build tests for the qemu_api crate

[Paolo Bonzini]

Fix some bitrot in tests.rs, and allow the unit tests to be run via
"meson test".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/qemu-api/meson.build  | 3 +++
 rust/qemu-api/src/tests.rs | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 42ea815fa5a..436e2f1e836 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -5,6 +5,7 @@ _qemu_api_rs = static_library(
       'src/lib.rs',
       'src/definitions.rs',
       'src/device_class.rs',
+      'src/tests.rs',
     ],
     {'.' : bindings_rs},
   ),
@@ -19,6 +20,8 @@ _qemu_api_rs = static_library(
   ],
 )
 
+rust.test('rust-qemu-api-tests', _qemu_api_rs)
+
 qemu_api = declare_dependency(
   link_with: _qemu_api_rs,
 )
diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
index df54edbd4e2..f0cd4d5d716 100644
--- a/rust/qemu-api/src/tests.rs
+++ b/rust/qemu-api/src/tests.rs
@@ -43,7 +43,7 @@ pub struct DummyState {
         dummy_class_init,
         props => DUMMY_PROPERTIES,
         realize_fn => None,
-        reset_fn => None,
+        legacy_reset_fn => None,
         vmsd => VMSTATE,
     }
 }
-- 
2.46.2

Re: [PATCH 08/16] rust: build tests for the qemu_api crate

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:26PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:26 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 08/16] rust: build tests for the qemu_api crate
> X-Mailer: git-send-email 2.46.2
> 
> Fix some bitrot in tests.rs, and allow the unit tests to be run via
> "meson test".
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/meson.build  | 3 +++
>  rust/qemu-api/src/tests.rs | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> index 42ea815fa5a..436e2f1e836 100644
> --- a/rust/qemu-api/meson.build
> +++ b/rust/qemu-api/meson.build
> @@ -5,6 +5,7 @@ _qemu_api_rs = static_library(
>        'src/lib.rs',
>        'src/definitions.rs',
>        'src/device_class.rs',
> +      'src/tests.rs',
>      ],
>      {'.' : bindings_rs},
>    ),
> @@ -19,6 +20,8 @@ _qemu_api_rs = static_library(
>    ],
>  )
>  
> +rust.test('rust-qemu-api-tests', _qemu_api_rs)
> +

It seems the change will bring a warning for "./configure --enable-rust":

WARNING: Unknown keyword argument(s) in target rust-qemu-api-tests: rust_abi, prelink, pic.

 

Re: [PATCH 08/16] rust: build tests for the qemu_api crate

[Paolo Bonzini]

On Mon, Oct 21, 2024 at 10:50 AM Zhao Liu <zhao1.liu@intel.com> wrote:
> > +rust.test('rust-qemu-api-tests', _qemu_api_rs)
>
> It seems the change will bring a warning for "./configure --enable-rust":
>
> WARNING: Unknown keyword argument(s) in target rust-qemu-api-tests: rust_abi, prelink, pic.

This is a Meson bug, I'll try to fix it.

Paolo

Re: [PATCH 08/16] rust: build tests for the qemu_api crate

[Zhao Liu]

On Tue, Oct 15, 2024 at 03:17:26PM +0200, Paolo Bonzini wrote:
> Date: Tue, 15 Oct 2024 15:17:26 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 08/16] rust: build tests for the qemu_api crate
> X-Mailer: git-send-email 2.46.2
> 
> Fix some bitrot in tests.rs, and allow the unit tests to be run via
> "meson test".
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/meson.build  | 3 +++
>  rust/qemu-api/src/tests.rs | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 

Codes look good to me,

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

Re: [PATCH 08/16] rust: build tests for the qemu_api crate

[Paolo Bonzini]

On Mon, Oct 21, 2024 at 4:35 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> On Tue, Oct 15, 2024 at 03:17:26PM +0200, Paolo Bonzini wrote:
> > Date: Tue, 15 Oct 2024 15:17:26 +0200
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: [PATCH 08/16] rust: build tests for the qemu_api crate
> > X-Mailer: git-send-email 2.46.2
> >
> > Fix some bitrot in tests.rs, and allow the unit tests to be run via
> > "meson test".
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  rust/qemu-api/meson.build  | 3 +++
> >  rust/qemu-api/src/tests.rs | 2 +-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
>
> Codes look good to me,
>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Note that, in the extracted series (to which I am moving your
Reviewed-by tags, so no need to go through it again), I'm changing
this to an integration test and making it actually create the object
it defines.

Paolo

Re: [PATCH 08/16] rust: build tests for the qemu_api crate

[Zhao Liu]

On Mon, Oct 21, 2024 at 04:36:24PM +0200, Paolo Bonzini wrote:
> Date: Mon, 21 Oct 2024 16:36:24 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 08/16] rust: build tests for the qemu_api crate
> 
> On Mon, Oct 21, 2024 at 4:35 PM Zhao Liu <zhao1.liu@intel.com> wrote:
> >
> > On Tue, Oct 15, 2024 at 03:17:26PM +0200, Paolo Bonzini wrote:
> > > Date: Tue, 15 Oct 2024 15:17:26 +0200
> > > From: Paolo Bonzini <pbonzini@redhat.com>
> > > Subject: [PATCH 08/16] rust: build tests for the qemu_api crate
> > > X-Mailer: git-send-email 2.46.2
> > >
> > > Fix some bitrot in tests.rs, and allow the unit tests to be run via
> > > "meson test".
> > >
> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > >  rust/qemu-api/meson.build  | 3 +++
> > >  rust/qemu-api/src/tests.rs | 2 +-
> > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > >
> >
> > Codes look good to me,
> >
> > Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> 
> Note that, in the extracted series (to which I am moving your
> Reviewed-by tags, so no need to go through it again), I'm changing
> this to an integration test and making it actually create the object
> it defines.

Thank you! I mixed up the versions; I will move over to there.

[PATCH 09/16] rust: introduce a c_str macro

[Paolo Bonzini]

This allows CStr constants to be defined easily on Rust 1.63.0, while
checking that there are no embedded NULs.  c"" literals were only
stabilized in Rust 1.77.0.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs       |  3 +-
 rust/hw/char/pl011/src/device_class.rs | 10 +++--
 rust/hw/char/pl011/src/lib.rs          |  4 +-
 rust/qemu-api/meson.build              |  1 +
 rust/qemu-api/src/c_str.rs             | 52 ++++++++++++++++++++++++++
 rust/qemu-api/src/lib.rs               |  1 +
 rust/qemu-api/src/tests.rs             |  8 ++--
 7 files changed, 70 insertions(+), 9 deletions(-)
 create mode 100644 rust/qemu-api/src/c_str.rs

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index cd4c01c2336..55d933ee5e9 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -11,6 +11,7 @@
 
 use qemu_api::{
     bindings::{self, *},
+    c_str,
     definitions::ObjectImpl,
 };
 
@@ -99,7 +100,7 @@ impl qemu_api::definitions::Class for PL011Class {
 }
 
 #[used]
-pub static CLK_NAME: &CStr = c"clk";
+pub static CLK_NAME: &CStr = c_str!("clk");
 
 impl PL011State {
     /// Initializes a pre-allocated, unitialized instance of `PL011State`.
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index b7ab31af02d..a3d1b1e929a 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -4,7 +4,11 @@
 
 use core::ptr::NonNull;
 
-use qemu_api::{bindings::*, definitions::ObjectImpl};
+use qemu_api::{
+    bindings::*,
+    c_str,
+    definitions::ObjectImpl
+};
 
 use crate::device::PL011State;
 
@@ -18,14 +22,14 @@
 qemu_api::declare_properties! {
     PL011_PROPERTIES,
     qemu_api::define_property!(
-        c"chardev",
+        c_str!("chardev"),
         PL011State,
         char_backend,
         unsafe { &qdev_prop_chr },
         CharBackend
     ),
     qemu_api::define_property!(
-        c"migrate-clk",
+        c_str!("migrate-clk"),
         PL011State,
         migrate_clock,
         unsafe { &qdev_prop_bool },
diff --git a/rust/hw/char/pl011/src/lib.rs b/rust/hw/char/pl011/src/lib.rs
index 2b157868b0f..0a598e5629d 100644
--- a/rust/hw/char/pl011/src/lib.rs
+++ b/rust/hw/char/pl011/src/lib.rs
@@ -41,11 +41,13 @@
 extern crate bilge_impl;
 extern crate qemu_api;
 
+use qemu_api::c_str;
+
 pub mod device;
 pub mod device_class;
 pub mod memory_ops;
 
-pub const TYPE_PL011: &::std::ffi::CStr = c"pl011";
+pub const TYPE_PL011: &::std::ffi::CStr = c_str!("pl011");
 
 /// Offset of each register from the base memory address of the device.
 ///
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 436e2f1e836..b55931c6490 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -3,6 +3,7 @@ _qemu_api_rs = static_library(
   structured_sources(
     [
       'src/lib.rs',
+      'src/c_str.rs',
       'src/definitions.rs',
       'src/device_class.rs',
       'src/tests.rs',
diff --git a/rust/qemu-api/src/c_str.rs b/rust/qemu-api/src/c_str.rs
new file mode 100644
index 00000000000..0286dade306
--- /dev/null
+++ b/rust/qemu-api/src/c_str.rs
@@ -0,0 +1,52 @@
+// Copyright 2024 Red Hat, Inc.
+// Author(s): Paolo Bonzini <pbonzini@redhat.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#[macro_export]
+/// Given a string constant _without_ embedded or trailing NULs, return
+/// a CStr.
+///
+/// Needed for compatibility with Rust <1.77.
+macro_rules! c_str {
+    ($str:expr) => {{
+        const STRING: &str = concat!($str, "\0");
+        const BYTES: &[u8] = STRING.as_bytes();
+
+        // "for" is not allowed in const context... oh well,
+        // everybody loves some lisp.  This could be turned into
+        // a procedural macro if this is a problem; alternatively
+        // Rust 1.72 makes CStr::from_bytes_with_nul a const function.
+        const fn f(b: &[u8], i: usize) {
+            if i == BYTES.len() - 1 {}
+            else if BYTES[i] == 0 {
+                panic!("c_str argument contains NUL")
+            } else {
+                f(b, i + 1)
+            }
+        }
+        f(BYTES, 0);
+
+        // SAFETY: absence of NULs apart from the final byte was checked above
+        unsafe { std::ffi::CStr::from_bytes_with_nul_unchecked(BYTES) }
+    }};
+}
+
+#[cfg(test)]
+mod tests {
+    use std::ffi::CStr;
+    use crate::c_str;
+
+    #[test]
+    fn test_cstr_macro() {
+        let good = c_str!("🦀");
+        let good_bytes = b"\xf0\x9f\xa6\x80\0";
+        assert_eq!(good.to_bytes_with_nul(), good_bytes);
+    }
+
+    #[test]
+    fn test_cstr_macro_const() {
+        const GOOD: &CStr = c_str!("🦀");
+        const GOOD_BYTES: &[u8] = b"\xf0\x9f\xa6\x80\0";
+        assert_eq!(GOOD.to_bytes_with_nul(), GOOD_BYTES);
+    }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index c2f60ac4727..9b2483fbfa3 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -27,6 +27,7 @@ unsafe impl Sync for bindings::Property {}
 unsafe impl Sync for bindings::TypeInfo {}
 unsafe impl Sync for bindings::VMStateDescription {}
 
+pub mod c_str;
 pub mod definitions;
 pub mod device_class;
 
diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
index f0cd4d5d716..d34b8d24187 100644
--- a/rust/qemu-api/src/tests.rs
+++ b/rust/qemu-api/src/tests.rs
@@ -3,7 +3,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 use crate::{
-    bindings::*, declare_properties, define_property, device_class_init, vm_state_description,
+    bindings::*, c_str, declare_properties, define_property, device_class_init, vm_state_description,
 };
 
 #[test]
@@ -11,7 +11,7 @@ fn test_device_decl_macros() {
     // Test that macros can compile.
     vm_state_description! {
         VMSTATE,
-        name: c"name",
+        name: c_str!("name"),
         unmigratable: true,
     }
 
@@ -24,14 +24,14 @@ pub struct DummyState {
     declare_properties! {
         DUMMY_PROPERTIES,
             define_property!(
-                c"chardev",
+                c_str!("chardev"),
                 DummyState,
                 char_backend,
                 unsafe { &qdev_prop_chr },
                 CharBackend
             ),
             define_property!(
-                c"migrate-clk",
+                c_str!("migrate-clk"),
                 DummyState,
                 migrate_clock,
                 unsafe { &qdev_prop_bool },
-- 
2.46.2

[PATCH 10/16] rust: introduce alternative implementation of offset_of!

[Paolo Bonzini]

offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
that was found on the Rust forums, and whose author agreed to license as
MIT for use in QEMU.

The alternative allows only one level of field access, but apart
from this can be used just by replacing core::mem::offset_of! with
qemu_api::offset_of!.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs  |  91 ++++++++++++-------------
 rust/qemu-api/meson.build         |  12 ++--
 rust/qemu-api/src/device_class.rs |   8 +--
 rust/qemu-api/src/lib.rs          |   4 ++
 rust/qemu-api/src/offset_of.rs    | 106 ++++++++++++++++++++++++++++++
 rust/qemu-api/src/tests.rs        |  11 ++--
 6 files changed, 176 insertions(+), 56 deletions(-)
 create mode 100644 rust/qemu-api/src/offset_of.rs

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 55d933ee5e9..f331a13b5f1 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -13,6 +13,7 @@
     bindings::{self, *},
     c_str,
     definitions::ObjectImpl,
+    with_offsets,
 };
 
 use crate::{
@@ -28,50 +29,52 @@
 /// QEMU sourced constant.
 pub const PL011_FIFO_DEPTH: usize = 16_usize;
 
-#[repr(C)]
-#[derive(Debug, qemu_api_macros::Object)]
-/// PL011 Device Model in QEMU
-pub struct PL011State {
-    pub parent_obj: SysBusDevice,
-    pub iomem: MemoryRegion,
-    #[doc(alias = "fr")]
-    pub flags: registers::Flags,
-    #[doc(alias = "lcr")]
-    pub line_control: registers::LineControl,
-    #[doc(alias = "rsr")]
-    pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
-    #[doc(alias = "cr")]
-    pub control: registers::Control,
-    pub dmacr: u32,
-    pub int_enabled: u32,
-    pub int_level: u32,
-    pub read_fifo: [u32; PL011_FIFO_DEPTH],
-    pub ilpr: u32,
-    pub ibrd: u32,
-    pub fbrd: u32,
-    pub ifl: u32,
-    pub read_pos: usize,
-    pub read_count: usize,
-    pub read_trigger: usize,
-    #[doc(alias = "chr")]
-    pub char_backend: CharBackend,
-    /// QEMU interrupts
-    ///
-    /// ```text
-    ///  * sysbus MMIO region 0: device registers
-    ///  * sysbus IRQ 0: `UARTINTR` (combined interrupt line)
-    ///  * sysbus IRQ 1: `UARTRXINTR` (receive FIFO interrupt line)
-    ///  * sysbus IRQ 2: `UARTTXINTR` (transmit FIFO interrupt line)
-    ///  * sysbus IRQ 3: `UARTRTINTR` (receive timeout interrupt line)
-    ///  * sysbus IRQ 4: `UARTMSINTR` (momem status interrupt line)
-    ///  * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
-    /// ```
-    #[doc(alias = "irq")]
-    pub interrupts: [qemu_irq; 6usize],
-    #[doc(alias = "clk")]
-    pub clock: NonNull<Clock>,
-    #[doc(alias = "migrate_clk")]
-    pub migrate_clock: bool,
+with_offsets! {
+    #[repr(C)]
+    #[derive(Debug, qemu_api_macros::Object)]
+    /// PL011 Device Model in QEMU
+    pub struct PL011State {
+        pub parent_obj: SysBusDevice,
+        pub iomem: MemoryRegion,
+        #[doc(alias = "fr")]
+        pub flags: registers::Flags,
+        #[doc(alias = "lcr")]
+        pub line_control: registers::LineControl,
+        #[doc(alias = "rsr")]
+        pub receive_status_error_clear: registers::ReceiveStatusErrorClear,
+        #[doc(alias = "cr")]
+        pub control: registers::Control,
+        pub dmacr: u32,
+        pub int_enabled: u32,
+        pub int_level: u32,
+        pub read_fifo: [u32; PL011_FIFO_DEPTH],
+        pub ilpr: u32,
+        pub ibrd: u32,
+        pub fbrd: u32,
+        pub ifl: u32,
+        pub read_pos: usize,
+        pub read_count: usize,
+        pub read_trigger: usize,
+        #[doc(alias = "chr")]
+        pub char_backend: CharBackend,
+        /// QEMU interrupts
+        ///
+        /// ```text
+        ///  * sysbus MMIO region 0: device registers
+        ///  * sysbus IRQ 0: `UARTINTR` (combined interrupt line)
+        ///  * sysbus IRQ 1: `UARTRXINTR` (receive FIFO interrupt line)
+        ///  * sysbus IRQ 2: `UARTTXINTR` (transmit FIFO interrupt line)
+        ///  * sysbus IRQ 3: `UARTRTINTR` (receive timeout interrupt line)
+        ///  * sysbus IRQ 4: `UARTMSINTR` (momem status interrupt line)
+        ///  * sysbus IRQ 5: `UARTEINTR` (error interrupt line)
+        /// ```
+        #[doc(alias = "irq")]
+        pub interrupts: [qemu_irq; 6usize],
+        #[doc(alias = "clk")]
+        pub clock: NonNull<Clock>,
+        #[doc(alias = "migrate_clk")]
+        pub migrate_clock: bool,
+    }
 }
 
 impl ObjectImpl for PL011State {
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index b55931c6490..57f813fc8f9 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -1,3 +1,9 @@
+_qemu_api_cfg = ['--cfg', 'MESON']
+# _qemu_api_cfg += ['--cfg', 'feature="allocator"']
+if rustc.version().version_compare('>=1.77.0')
+  _qemu_api_cfg += ['--cfg', 'has_offset_of']
+endif
+
 _qemu_api_rs = static_library(
   'qemu_api',
   structured_sources(
@@ -6,16 +12,14 @@ _qemu_api_rs = static_library(
       'src/c_str.rs',
       'src/definitions.rs',
       'src/device_class.rs',
+      'src/offset_of.rs',
       'src/tests.rs',
     ],
     {'.' : bindings_rs},
   ),
   override_options: ['rust_std=2021', 'build.rust_std=2021'],
   rust_abi: 'rust',
-  rust_args: [
-    '--cfg', 'MESON',
-    # '--cfg', 'feature="allocator"',
-  ],
+  rust_args: _qemu_api_cfg,
   dependencies: [
     qemu_api_macros,
   ],
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index 871063d4a92..d4fa544df39 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -26,7 +26,7 @@ macro_rules! device_class_init {
 
 #[macro_export]
 macro_rules! define_property {
-    ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr, default = $defval:expr$(,)*) => {
+    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr, default = $defval:expr$(,)*) => {
         $crate::bindings::Property {
             name: {
                 #[used]
@@ -34,7 +34,7 @@ macro_rules! define_property {
                 _TEMP.as_ptr()
             },
             info: $prop,
-            offset: ::core::mem::offset_of!($state, $field)
+            offset: $crate::offset_of!($state, $field)
                 .try_into()
                 .expect("Could not fit offset value to type"),
             bitnr: 0,
@@ -47,7 +47,7 @@ macro_rules! define_property {
             link_type: ::core::ptr::null(),
         }
     };
-    ($name:expr, $state:ty, $field:expr, $prop:expr, $type:expr$(,)*) => {
+    ($name:expr, $state:ty, $field:ident, $prop:expr, $type:expr$(,)*) => {
         $crate::bindings::Property {
             name: {
                 #[used]
@@ -55,7 +55,7 @@ macro_rules! define_property {
                 _TEMP.as_ptr()
             },
             info: $prop,
-            offset: ::core::mem::offset_of!($state, $field)
+            offset: $crate::offset_of!($state, $field)
                 .try_into()
                 .expect("Could not fit offset value to type"),
             bitnr: 0,
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 9b2483fbfa3..082f1addb10 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -30,6 +30,7 @@ unsafe impl Sync for bindings::VMStateDescription {}
 pub mod c_str;
 pub mod definitions;
 pub mod device_class;
+pub mod offset_of;
 
 #[cfg(test)]
 mod tests;
@@ -167,3 +168,6 @@ unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
         }
     }
 }
+
+#[cfg(has_offset_of)]
+pub use std::mem::offset_of;
diff --git a/rust/qemu-api/src/offset_of.rs b/rust/qemu-api/src/offset_of.rs
new file mode 100644
index 00000000000..4e1de373674
--- /dev/null
+++ b/rust/qemu-api/src/offset_of.rs
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: MIT
+
+/// This macro provides the same functionality as `core::mem::offset_of`,
+/// except that only one level of field access is supported.  The declaration
+/// of the struct must be wrapped with `with_offsets! { }`.
+///
+/// It is needed because `offset_of!` was only stabilized in Rust 1.77.
+#[cfg(not(has_offset_of))]
+#[macro_export]
+macro_rules! offset_of {
+    ($Container:ty, $field:ident) => {
+        <$Container>::offset_to.$field
+    };
+}
+
+/// A wrapper for struct declarations, that allows using `offset_of!` in
+/// versions of Rust prior to 1.77
+#[macro_export]
+macro_rules! with_offsets {
+    // source: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=10a22a9b8393abd7b541d8fc844bc0df
+    // used under MIT license with permission of Yandros aka Daniel Henry-Mantilla
+    (
+        #[repr(C)]
+        $(#[$struct_meta:meta])*
+        $struct_vis:vis
+        struct $StructName:ident {
+            $(
+                $(#[$field_meta:meta])*
+                $field_vis:vis
+                $field_name:ident : $field_ty:ty
+            ),*
+            $(,)?
+        }
+    ) => (
+        #[repr(C)]
+        $(#[$struct_meta])*
+        $struct_vis
+        struct $StructName {
+            $(
+                $(#[$field_meta])*
+                $field_vis
+                $field_name : $field_ty ,
+            )*
+        }
+
+        #[cfg(not(has_offset_of))]
+        #[allow(nonstandard_style)]
+        const _: () = {
+            pub
+            struct StructOffsets {
+                $(
+                    $field_vis
+                    $field_name: usize,
+                )*
+            }
+            struct Helper;
+            impl $StructName {
+                pub
+                const offset_to: StructOffsets = StructOffsets {
+                    $(
+                        $field_name: Helper::$field_name,
+                    )*
+                };
+            }
+            const END_OF_PREV_FIELD: usize = 0;
+            $crate::with_offsets! {
+                @names [ $($field_name)* ]
+                @tys [ $($field_ty ,)*]
+            }
+        };
+    );
+
+    (
+        @names []
+        @tys []
+    ) => ();
+
+    (
+        @names [$field_name:ident $($other_names:tt)*]
+        @tys [$field_ty:ty , $($other_tys:tt)*]
+    ) => (
+        impl Helper {
+            const $field_name: usize = {
+                let align =
+                    std::mem::align_of::<$field_ty>()
+                ;
+                let trail =
+                    END_OF_PREV_FIELD % align
+                ;
+                0   + END_OF_PREV_FIELD
+                    + (align - trail)
+                        * [1, 0][(trail == 0) as usize]
+            };
+        }
+        const _: () = {
+            const END_OF_PREV_FIELD: usize =
+                Helper::$field_name +
+                std::mem::size_of::<$field_ty>()
+            ;
+            $crate::with_offsets! {
+                @names [$($other_names)*]
+                @tys [$($other_tys)*]
+            }
+        };
+    );
+}
diff --git a/rust/qemu-api/src/tests.rs b/rust/qemu-api/src/tests.rs
index d34b8d24187..b582f27baa1 100644
--- a/rust/qemu-api/src/tests.rs
+++ b/rust/qemu-api/src/tests.rs
@@ -4,6 +4,7 @@
 
 use crate::{
     bindings::*, c_str, declare_properties, define_property, device_class_init, vm_state_description,
+    with_offsets,
 };
 
 #[test]
@@ -15,10 +16,12 @@ fn test_device_decl_macros() {
         unmigratable: true,
     }
 
-    #[repr(C)]
-    pub struct DummyState {
-        pub char_backend: CharBackend,
-        pub migrate_clock: bool,
+    with_offsets! {
+        #[repr(C)]
+        pub struct DummyState {
+            pub char_backend: CharBackend,
+            pub migrate_clock: bool,
+        }
     }
 
     declare_properties! {
-- 
2.46.2

Re: [PATCH 10/16] rust: introduce alternative implementation of offset_of!

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
> that was found on the Rust forums, and whose author agreed to license as
> MIT for use in QEMU.
>
> The alternative allows only one level of field access, but apart
> from this can be used just by replacing core::mem::offset_of! with
> qemu_api::offset_of!.

How about a macro like this (which essentially comes from memoffset
crate [1])? It has the same use as core::mem::offset_of! (except the
same single-level limitation) and does not need wrapping structures with
with_offsets!.

macro_rules! offset_of {
    ($parent:ty, $field:tt) => {{
	let uninit = std::mem::MaybeUninit::<$parent>::uninit();
	let base = uninit.as_ptr();
	// SAFETY:
	//
	// MaybeUninit<$parent> has the same size and alignment as $parent, so
	// projection to $field is in bound.
	//
	// addr_of! does not create intermediate references to the uninitialized
	// memory, thus no UB is involved.
	let field = unsafe { std::ptr::addr_of!((*base).$field) };
	// SAFETY:
	//
	// Both base and field point to the MaybeUninit<$parent> and are casted
	// to u8 for calculating their distance.
	unsafe { field.cast::<u8>().offset_from(base.cast::<u8>()) as usize }
    }};
}

[1] https://docs.rs/memoffset/latest/memoffset/

--
Best Regards
Junjie Mao

Re: [PATCH 10/16] rust: introduce alternative implementation of offset_of!

[Paolo Bonzini]

On Thu, Oct 17, 2024 at 7:35 AM Junjie Mao <junjie.mao@hotmail.com> wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> > offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
> > that was found on the Rust forums, and whose author agreed to license as
> > MIT for use in QEMU.
> >
> > The alternative allows only one level of field access, but apart
> > from this can be used just by replacing core::mem::offset_of! with
> > qemu_api::offset_of!.
>
> How about a macro like this (which essentially comes from memoffset
> crate [1])? It has the same use as core::mem::offset_of! (except the
> same single-level limitation) and does not need wrapping structures with
> with_offsets!.

Unfortunately offset_from is not available in const context, and
offset_of! is needed to fill in the Property and VMStateDescription
arrays.

That said, I noticed now that declare_properties! does not declare the
resulting array as const, so that would be possible. But if
declare_properties could use a non-mut static, that would be better.

Paolo

> macro_rules! offset_of {
>     ($parent:ty, $field:tt) => {{
>         let uninit = std::mem::MaybeUninit::<$parent>::uninit();
>         let base = uninit.as_ptr();
>         // SAFETY:
>         //
>         // MaybeUninit<$parent> has the same size and alignment as $parent, so
>         // projection to $field is in bound.
>         //
>         // addr_of! does not create intermediate references to the uninitialized
>         // memory, thus no UB is involved.
>         let field = unsafe { std::ptr::addr_of!((*base).$field) };
>         // SAFETY:
>         //
>         // Both base and field point to the MaybeUninit<$parent> and are casted
>         // to u8 for calculating their distance.
>         unsafe { field.cast::<u8>().offset_from(base.cast::<u8>()) as usize }
>     }};
> }
>
> [1] https://docs.rs/memoffset/latest/memoffset/
>
> --
> Best Regards
> Junjie Mao
>

Re: [PATCH 10/16] rust: introduce alternative implementation of offset_of!

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On Thu, Oct 17, 2024 at 7:35 AM Junjie Mao <junjie.mao@hotmail.com> wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> > offset_of! was stabilized in Rust 1.77.0.  Use an alternative implemenation
>> > that was found on the Rust forums, and whose author agreed to license as
>> > MIT for use in QEMU.
>> >
>> > The alternative allows only one level of field access, but apart
>> > from this can be used just by replacing core::mem::offset_of! with
>> > qemu_api::offset_of!.
>>
>> How about a macro like this (which essentially comes from memoffset
>> crate [1])? It has the same use as core::mem::offset_of! (except the
>> same single-level limitation) and does not need wrapping structures with
>> with_offsets!.
>
> Unfortunately offset_from is not available in const context, and
> offset_of! is needed to fill in the Property and VMStateDescription
> arrays.
>
> That said, I noticed now that declare_properties! does not declare the
> resulting array as const, so that would be possible. But if
> declare_properties could use a non-mut static, that would be better.

Agree.

Then how about converting with_offsets! to a derive attribute
(e.g. #[derive(offsets)])? The current approach introduces one more
level of indentation. When we later upgrade the minimal supported
version of Rust and switch to std::mem::offset_of!, we'll need a large
diff to adjust the indentation which may be annoying to rebase upon. An
attribute seems easier to manage.

I can help draft the macro early next week if you think that is valuable.

Junjie Mao

>
> Paolo
>
>> macro_rules! offset_of {
>>     ($parent:ty, $field:tt) => {{
>>         let uninit = std::mem::MaybeUninit::<$parent>::uninit();
>>         let base = uninit.as_ptr();
>>         // SAFETY:
>>         //
>>         // MaybeUninit<$parent> has the same size and alignment as $parent, so
>>         // projection to $field is in bound.
>>         //
>>         // addr_of! does not create intermediate references to the uninitialized
>>         // memory, thus no UB is involved.
>>         let field = unsafe { std::ptr::addr_of!((*base).$field) };
>>         // SAFETY:
>>         //
>>         // Both base and field point to the MaybeUninit<$parent> and are casted
>>         // to u8 for calculating their distance.
>>         unsafe { field.cast::<u8>().offset_from(base.cast::<u8>()) as usize }
>>     }};
>> }
>>
>> [1] https://docs.rs/memoffset/latest/memoffset/

Re: [PATCH 10/16] rust: introduce alternative implementation of offset_of!

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2857 bytes --]

Il ven 18 ott 2024, 05:16 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

>
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > On Thu, Oct 17, 2024 at 7:35 AM Junjie Mao <junjie.mao@hotmail.com>
> wrote:
> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >> > offset_of! was stabilized in Rust 1.77.0.  Use an alternative
> implemenation
> >> > that was found on the Rust forums, and whose author agreed to license
> as
> >> > MIT for use in QEMU.
> >> >
> >> > The alternative allows only one level of field access, but apart
> >> > from this can be used just by replacing core::mem::offset_of! with
> >> > qemu_api::offset_of!.
> >>
> >> How about a macro like this (which essentially comes from memoffset
> >> crate [1])? It has the same use as core::mem::offset_of! (except the
> >> same single-level limitation) and does not need wrapping structures with
> >> with_offsets!.
> >
> > Unfortunately offset_from is not available in const context, and
> > offset_of! is needed to fill in the Property and VMStateDescription
> > arrays.
> >
> > That said, I noticed now that declare_properties! does not declare the
> > resulting array as const, so that would be possible. But if
> > declare_properties could use a non-mut static, that would be better.
>
> Agree.
>
> Then how about converting with_offsets! to a derive attribute
> (e.g. #[derive(offsets)])? The current approach introduces one more
> level of indentation. When we later upgrade the minimal supported
> version of Rust and switch to std::mem::offset_of!, we'll need a large
> diff to adjust the indentation which may be annoying to rebase upon. An
> attribute seems easier to manage.
>

Ok, using quote! to generate the with_offsets! {} call should be easy.

Paolo


> I can help draft the macro early next week if you think that is valuable.
>
> Junjie Mao
>
> >
> > Paolo
> >
> >> macro_rules! offset_of {
> >>     ($parent:ty, $field:tt) => {{
> >>         let uninit = std::mem::MaybeUninit::<$parent>::uninit();
> >>         let base = uninit.as_ptr();
> >>         // SAFETY:
> >>         //
> >>         // MaybeUninit<$parent> has the same size and alignment as
> $parent, so
> >>         // projection to $field is in bound.
> >>         //
> >>         // addr_of! does not create intermediate references to the
> uninitialized
> >>         // memory, thus no UB is involved.
> >>         let field = unsafe { std::ptr::addr_of!((*base).$field) };
> >>         // SAFETY:
> >>         //
> >>         // Both base and field point to the MaybeUninit<$parent> and
> are casted
> >>         // to u8 for calculating their distance.
> >>         unsafe { field.cast::<u8>().offset_from(base.cast::<u8>()) as
> usize }
> >>     }};
> >> }
> >>
> >> [1] https://docs.rs/memoffset/latest/memoffset/
>
>

[-- Attachment #2: Type: text/html, Size: 4289 bytes --]

[PATCH 11/16] rust: do not use MaybeUninit::zeroed()

[Paolo Bonzini]

MaybeUninit::zeroed() is handy, but it introduces unsafe (and has a pretty heavy
syntax in general) and anyway it is not available as a "const" function until
Rust 1.75.0.

Introduce a trait that provides the same functionality while staying within
safe Rust.  In the future we may want to add automatic derivation and
implementation of the trait, once we can assume Rust 1.75.0; for now the
implementation is manual.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device_class.rs |  5 +-
 rust/hw/char/pl011/src/memory_ops.rs   | 11 ++--
 rust/qemu-api/meson.build              |  1 +
 rust/qemu-api/src/device_class.rs      |  4 +-
 rust/qemu-api/src/lib.rs               |  1 +
 rust/qemu-api/src/zeroable.rs          | 75 ++++++++++++++++++++++++++
 6 files changed, 89 insertions(+), 8 deletions(-)
 create mode 100644 rust/qemu-api/src/zeroable.rs

diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index a3d1b1e929a..37bbf6d36cc 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -7,7 +7,8 @@
 use qemu_api::{
     bindings::*,
     c_str,
-    definitions::ObjectImpl
+    definitions::ObjectImpl,
+    zeroable::Zeroable,
 };
 
 use crate::device::PL011State;
@@ -16,7 +17,7 @@
 pub static VMSTATE_PL011: VMStateDescription = VMStateDescription {
     name: PL011State::TYPE_INFO.name,
     unmigratable: true,
-    ..unsafe { ::core::mem::MaybeUninit::<VMStateDescription>::zeroed().assume_init() }
+    ..Zeroable::ZERO
 };
 
 qemu_api::declare_properties! {
diff --git a/rust/hw/char/pl011/src/memory_ops.rs b/rust/hw/char/pl011/src/memory_ops.rs
index 2c664fd45ed..88d17ec2e3a 100644
--- a/rust/hw/char/pl011/src/memory_ops.rs
+++ b/rust/hw/char/pl011/src/memory_ops.rs
@@ -2,11 +2,14 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use core::{mem::MaybeUninit, ptr::NonNull};
+use core::ptr::NonNull;
 
 use std::os::raw::{c_uint, c_void};
 
-use qemu_api::bindings::*;
+use qemu_api::{
+    bindings::*,
+    zeroable::Zeroable
+};
 
 use crate::device::PL011State;
 
@@ -16,11 +19,11 @@
     read_with_attrs: None,
     write_with_attrs: None,
     endianness: device_endian::DEVICE_NATIVE_ENDIAN,
-    valid: unsafe { MaybeUninit::<MemoryRegionOps__bindgen_ty_1>::zeroed().assume_init() },
+    valid: Zeroable::ZERO,
     impl_: MemoryRegionOps__bindgen_ty_2 {
         min_access_size: 4,
         max_access_size: 4,
-        ..unsafe { MaybeUninit::<MemoryRegionOps__bindgen_ty_2>::zeroed().assume_init() }
+        ..Zeroable::ZERO
     },
 };
 
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 57f813fc8f9..547fc5caa3a 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -14,6 +14,7 @@ _qemu_api_rs = static_library(
       'src/device_class.rs',
       'src/offset_of.rs',
       'src/tests.rs',
+      'src/zeroable.rs',
     ],
     {'.' : bindings_rs},
   ),
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index d4fa544df39..d2535125c48 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -100,7 +100,7 @@ const fn _calc_prop_len() -> usize {
         fn _make_properties() -> [$crate::bindings::Property; PROP_LEN] {
             [
                 $($prop),*,
-                    unsafe { ::core::mem::MaybeUninit::<$crate::bindings::Property>::zeroed().assume_init() },
+                $crate::zeroable::Zeroable::ZERO,
             ]
         }
 
@@ -125,7 +125,7 @@ macro_rules! vm_state_description {
                 $vname.as_ptr()
             },)*
             unmigratable: true,
-            ..unsafe { ::core::mem::MaybeUninit::<$crate::bindings::VMStateDescription>::zeroed().assume_init() }
+            ..$crate::zeroable::Zeroable::ZERO
         };
     }
 }
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 082f1addb10..50951d20e14 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -31,6 +31,7 @@ unsafe impl Sync for bindings::VMStateDescription {}
 pub mod definitions;
 pub mod device_class;
 pub mod offset_of;
+pub mod zeroable;
 
 #[cfg(test)]
 mod tests;
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
new file mode 100644
index 00000000000..faa93d1fb33
--- /dev/null
+++ b/rust/qemu-api/src/zeroable.rs
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::ptr;
+
+/// This trait provides a replacement for core::mem::zeroed() that can be
+/// used as a `const fn` prior to Rust 1.75.0.  As an added bonus it removes
+/// usage of `unsafe` blocks.
+///
+/// Unlike other Zeroable traits found in other crates (e.g.
+/// [`pinned_init`](https://docs.rs/pinned-init/latest/pinned_init/trait.Zeroable.html))
+/// this is a safe trait because the value `ZERO` constant has to be written by
+/// hand.  The `pinned_init` crate instead makes the trait unsafe, but it
+/// provides a `#[derive(Zeroable)]` macro to define it with compile-time
+/// safety checks. Once we can assume Rust 1.75.0 is available, we could
+/// switch to their idea, and use `core::mem::zeroed()` to provide a blanked
+/// implementation of the `ZERO` constant.
+pub trait Zeroable: Default {
+    const ZERO: Self;
+}
+
+impl Zeroable for crate::bindings::Property__bindgen_ty_1 {
+    const ZERO: Self = Self { i: 0 };
+}
+
+impl Zeroable for crate::bindings::Property {
+    const ZERO: Self = Self {
+        name: ptr::null(),
+        info: ptr::null(),
+        offset: 0,
+        bitnr: 0,
+        bitmask: 0,
+        set_default: false,
+        defval: Zeroable::ZERO,
+        arrayoffset: 0,
+        arrayinfo: ptr::null(),
+        arrayfieldsize: 0,
+        link_type: ptr::null(),
+    };
+}
+
+impl Zeroable for crate::bindings::VMStateDescription {
+    const ZERO: Self = Self {
+        name: ptr::null(),
+        unmigratable: false,
+        early_setup: false,
+        version_id: 0,
+        minimum_version_id: 0,
+        priority: crate::bindings::MigrationPriority::MIG_PRI_DEFAULT,
+        pre_load: None,
+        post_load: None,
+        pre_save: None,
+        post_save: None,
+        needed: None,
+        dev_unplug_pending: None,
+        fields: ptr::null(),
+        subsections: ptr::null(),
+    };
+}
+
+impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_1 {
+    const ZERO: Self = Self {
+        min_access_size: 0,
+        max_access_size: 0,
+        unaligned: false,
+        accepts: None,
+    };
+}
+
+impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_2 {
+    const ZERO: Self = Self {
+        min_access_size: 0,
+        max_access_size: 0,
+        unaligned: false,
+    };
+}
-- 
2.46.2

[PATCH 12/16] rust: allow version 1.63.0 of rustc

[Paolo Bonzini]

All constructs introduced by newer versions of Rust have been removed.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index 175b8d82228..0e279d245b4 100644
--- a/meson.build
+++ b/meson.build
@@ -76,11 +76,11 @@ if not get_option('rust').disabled() and add_languages('rust', required: get_opt
     and add_languages('rust', required: get_option('rust'), native: true)
   rustc = meson.get_compiler('rust')
   have_rust = true
-  if rustc.version().version_compare('<1.80.0')
+  if rustc.version().version_compare('<1.63.0')
     if get_option('rust').enabled()
-      error('rustc version ' + rustc.version() + ' is unsupported: Please upgrade to at least 1.80.0')
+      error('rustc version ' + rustc.version() + ' is unsupported: Please upgrade to at least 1.63.0')
     else
-      warning('rustc version ' + rustc.version() + ' is unsupported: Disabling Rust compilation. Please upgrade to at least 1.80.0 to use Rust.')
+      warning('rustc version ' + rustc.version() + ' is unsupported: Disabling Rust compilation. Please upgrade to at least 1.63.0 to use Rust.')
       have_rust = false
     endif
   endif
-- 
2.46.2

Re: [PATCH 12/16] rust: allow version 1.63.0 of rustc

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> All constructs introduced by newer versions of Rust have been removed.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 175b8d82228..0e279d245b4 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -76,11 +76,11 @@ if not get_option('rust').disabled() and add_languages('rust', required: get_opt
>      and add_languages('rust', required: get_option('rust'), native: true)
>    rustc = meson.get_compiler('rust')
>    have_rust = true
> -  if rustc.version().version_compare('<1.80.0')
> +  if rustc.version().version_compare('<1.63.0')

In my Ubuntu 22.04 environment (rustc 1.76.0 and bindgen 0.59.1 from
apt) the feature `proc_macro_byte_character` is not yet stablized but
used in proc-macro2. Downgrading proc-macro2 to 1.0.79 [1] and syn to
2.0.58 fixes that issue for me.

[1] https://github.com/drmingdrmer/openraft/commit/d496b6db4c6128d33f0f211165c08a7925cf20f7

Here're my changes:

diff --git a/subprojects/proc-macro2-1-rs.wrap b/subprojects/proc-macro2-1-rs.wrap
index 7053e2c013..d5713b45d7 100644
--- a/subprojects/proc-macro2-1-rs.wrap
+++ b/subprojects/proc-macro2-1-rs.wrap
@@ -1,7 +1,7 @@
 [wrap-file]
-directory = proc-macro2-1.0.84
-source_url = https://crates.io/api/v1/crates/proc-macro2/1.0.84/download
-source_filename = proc-macro2-1.0.84.0.tar.gz
-source_hash = ec96c6a92621310b51366f1e28d05ef11489516e93be030060e5fc12024a49d6
+directory = proc-macro2-1.0.79
+source_url = https://crates.io/api/v1/crates/proc-macro2/1.0.79/download
+source_filename = proc-macro2-1.0.79.0.tar.gz
+source_hash = e835ff2298f5721608eb1a980ecaee1aef2c132bf95ecc026a11b7bf3c01c02e
 #method = cargo
 patch_directory = proc-macro2-1-rs
diff --git a/subprojects/syn-2-rs.wrap b/subprojects/syn-2-rs.wrap
index 13ffdac3c3..9d413a0c57 100644
--- a/subprojects/syn-2-rs.wrap
+++ b/subprojects/syn-2-rs.wrap
@@ -1,7 +1,7 @@
 [wrap-file]
-directory = syn-2.0.66
-source_url = https://crates.io/api/v1/crates/syn/2.0.66/download
-source_filename = syn-2.0.66.0.tar.gz
-source_hash = c42f3f41a2de00b01c0aaad383c5a45241efc8b2d1eda5661812fda5f3cdcff5
+directory = syn-2.0.58
+source_url = https://crates.io/api/v1/crates/syn/2.0.58/download
+source_filename = syn-2.0.58.0.tar.gz
+source_hash = 44cfb93f38070beee36b3fef7d4f5a16f27751d94b187b666a5cc5e9b0d30687
 #method = cargo
 patch_directory = syn-2-rs

--
Best Regards
Junjie Mao

>      if get_option('rust').enabled()
> -      error('rustc version ' + rustc.version() + ' is unsupported: Please upgrade to at least 1.80.0')
> +      error('rustc version ' + rustc.version() + ' is unsupported: Please upgrade to at least 1.63.0')
>      else
> -      warning('rustc version ' + rustc.version() + ' is unsupported: Disabling Rust compilation. Please upgrade to at least 1.80.0 to use Rust.')
> +      warning('rustc version ' + rustc.version() + ' is unsupported: Disabling Rust compilation. Please upgrade to at least 1.63.0 to use Rust.')
>        have_rust = false
>      endif
>    endif

Re: [PATCH 12/16] rust: allow version 1.63.0 of rustc

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2838 bytes --]

Il mer 16 ott 2024, 08:10 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

> In my Ubuntu 22.04 environment (rustc 1.76.0 and bindgen 0.59.1 from
> apt) the feature `proc_macro_byte_character` is not yet stablized but
> used in proc-macro2. Downgrading proc-macro2 to 1.0.79 [1] and syn to
> 2.0.58 fixes that issue for me.
>

This is handled by patch 5. Try "meson subprojects update --reset".

Paolo

[1]
> https://github.com/drmingdrmer/openraft/commit/d496b6db4c6128d33f0f211165c08a7925cf20f7
>
> Here're my changes:
>
> diff --git a/subprojects/proc-macro2-1-rs.wrap
> b/subprojects/proc-macro2-1-rs.wrap
> index 7053e2c013..d5713b45d7 100644
> --- a/subprojects/proc-macro2-1-rs.wrap
> +++ b/subprojects/proc-macro2-1-rs.wrap
> @@ -1,7 +1,7 @@
>  [wrap-file]
> -directory = proc-macro2-1.0.84
> -source_url = https://crates.io/api/v1/crates/proc-macro2/1.0.84/download
> -source_filename
> <https://crates.io/api/v1/crates/proc-macro2/1.0.84/download-source_filename>
> = proc-macro2-1.0.84.0.tar.gz
> -source_hash =
> ec96c6a92621310b51366f1e28d05ef11489516e93be030060e5fc12024a49d6
> +directory = proc-macro2-1.0.79
> +source_url = https://crates.io/api/v1/crates/proc-macro2/1.0.79/download
> +source_filename = proc-macro2-1.0.79.0.tar.gz
> +source_hash =
> e835ff2298f5721608eb1a980ecaee1aef2c132bf95ecc026a11b7bf3c01c02e
>  #method = cargo
>  patch_directory = proc-macro2-1-rs
> diff --git a/subprojects/syn-2-rs.wrap b/subprojects/syn-2-rs.wrap
> index 13ffdac3c3..9d413a0c57 100644
> --- a/subprojects/syn-2-rs.wrap
> +++ b/subprojects/syn-2-rs.wrap
> @@ -1,7 +1,7 @@
>  [wrap-file]
> -directory = syn-2.0.66
> -source_url = https://crates.io/api/v1/crates/syn/2.0.66/download
> -source_filename
> <https://crates.io/api/v1/crates/syn/2.0.66/download-source_filename> =
> syn-2.0.66.0.tar.gz
> -source_hash =
> c42f3f41a2de00b01c0aaad383c5a45241efc8b2d1eda5661812fda5f3cdcff5
> +directory = syn-2.0.58
> +source_url = https://crates.io/api/v1/crates/syn/2.0.58/download
> +source_filename = syn-2.0.58.0.tar.gz
> +source_hash =
> 44cfb93f38070beee36b3fef7d4f5a16f27751d94b187b666a5cc5e9b0d30687
>  #method = cargo
>  patch_directory = syn-2-rs
>
> --
> Best Regards
> Junjie Mao
>
> >      if get_option('rust').enabled()
> > -      error('rustc version ' + rustc.version() + ' is unsupported:
> Please upgrade to at least 1.80.0')
> > +      error('rustc version ' + rustc.version() + ' is unsupported:
> Please upgrade to at least 1.63.0')
> >      else
> > -      warning('rustc version ' + rustc.version() + ' is unsupported:
> Disabling Rust compilation. Please upgrade to at least 1.80.0 to use Rust.')
> > +      warning('rustc version ' + rustc.version() + ' is unsupported:
> Disabling Rust compilation. Please upgrade to at least 1.63.0 to use Rust.')
> >        have_rust = false
> >      endif
> >    endif
>
>

[-- Attachment #2: Type: text/html, Size: 4214 bytes --]

Re: [PATCH 12/16] rust: allow version 1.63.0 of rustc

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il mer 16 ott 2024, 08:10 Junjie Mao <junjie.mao@hotmail.com> ha scritto:
>
>  In my Ubuntu 22.04 environment (rustc 1.76.0 and bindgen 0.59.1 from
>  apt) the feature `proc_macro_byte_character` is not yet stablized but
>  used in proc-macro2. Downgrading proc-macro2 to 1.0.79 [1] and syn to
>  2.0.58 fixes that issue for me.
>
> This is handled by patch 5. Try "meson subprojects update --reset".
>

Yes, that works. Thanks for the info!

--
Best Regards
Junjie Mao

Re: [PATCH 12/16] rust: allow version 1.63.0 of rustc

[Junjie Mao]

Junjie Mao <junjie.mao@hotmail.com> writes:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> Il mer 16 ott 2024, 08:10 Junjie Mao <junjie.mao@hotmail.com> ha scritto:
>>
>>  In my Ubuntu 22.04 environment (rustc 1.76.0 and bindgen 0.59.1 from
>>  apt) the feature `proc_macro_byte_character` is not yet stablized but
>>  used in proc-macro2. Downgrading proc-macro2 to 1.0.79 [1] and syn to
>>  2.0.58 fixes that issue for me.
>>
>> This is handled by patch 5. Try "meson subprojects update --reset".
>>
>
> Yes, that works. Thanks for the info!

After cleaning everything to build from scratch, I met another issue:

  bilge-0.2-rs| Downloading bilge-impl-0.2-rs source from https://crates.io/api/v1/crates/bilge-impl/0.2.0/download
  Download size: 24524
  Downloading: ..........
  bilge-0.2-rs| Applying diff file "bilge-impl-1.63.0.patch"
  bilge-0.2-rs| patching file src/shared/discriminant_assigner.rs
  bilge-0.2-rs| Hunk #1 FAILED at 26 (different line endings).
  bilge-0.2-rs| 1 out of 1 hunk FAILED -- saving rejects to file src/shared/discriminant_assigner.rs.rej
  bilge-0.2-rs| patching file src/shared/fallback.rs
  bilge-0.2-rs| Hunk #1 FAILED at 22 (different line endings).
  bilge-0.2-rs| 1 out of 1 hunk FAILED -- saving rejects to file src/shared/fallback.rs.rej

  ../subprojects/bilge-0.2.0/meson.build:9:0: ERROR: Failed to apply diff file "bilge-impl-1.63.0.patch"

It turns out that the sources in bilge-impl have CRLF line endings and
`patch` does not ignore that even with the `-l` option:

  ~/Projects/qemu/subprojects$ tar xf packagecache/bilge-impl-0.2.0.tar.gz
  ~/Projects/qemu/subprojects$ cd bilge-impl-0.2.0/
  ~/Projects/qemu/subprojects/bilge-impl-0.2.0$ patch -l -f -p1 -i ../packagefiles/bilge-impl-1.63.0.patch
  patching file src/shared/discriminant_assigner.rs
  Hunk #1 FAILED at 26 (different line endings).
  1 out of 1 hunk FAILED -- saving rejects to file src/shared/discriminant_assigner.rs.rej
  patching file src/shared/fallback.rs
  Hunk #1 FAILED at 22 (different line endings).
  1 out of 1 hunk FAILED -- saving rejects to file src/shared/fallback.rs.rej
  ~/Projects/qemu/subprojects/bilge-impl-0.2.0$ file src/shared/discriminant_assigner.rs
  src/shared/discriminant_assigner.rs: ASCII text, with CRLF line terminators

Meson uses patch with the command above as the first preference to apply
diffs. That command is not yet customizable.

--
Best Regards
Junjie Mao

Re: [PATCH 12/16] rust: allow version 1.63.0 of rustc

[Paolo Bonzini]

On Fri, Oct 18, 2024 at 5:00 AM Junjie Mao <junjie.mao@hotmail.com> wrote:
>
>
> Junjie Mao <junjie.mao@hotmail.com> writes:
>
> > Paolo Bonzini <pbonzini@redhat.com> writes:
> >
> >> Il mer 16 ott 2024, 08:10 Junjie Mao <junjie.mao@hotmail.com> ha scritto:
> >>
> >>  In my Ubuntu 22.04 environment (rustc 1.76.0 and bindgen 0.59.1 from
> >>  apt) the feature `proc_macro_byte_character` is not yet stablized but
> >>  used in proc-macro2. Downgrading proc-macro2 to 1.0.79 [1] and syn to
> >>  2.0.58 fixes that issue for me.
> >>
> >> This is handled by patch 5. Try "meson subprojects update --reset".
> >>
> >
> > Yes, that works. Thanks for the info!
>
> After cleaning everything to build from scratch, I met another issue:
>
>   bilge-0.2-rs| Downloading bilge-impl-0.2-rs source from https://crates.io/api/v1/crates/bilge-impl/0.2.0/download
>   Download size: 24524
>   Downloading: ..........
>   bilge-0.2-rs| Applying diff file "bilge-impl-1.63.0.patch"
>   bilge-0.2-rs| patching file src/shared/discriminant_assigner.rs
>   bilge-0.2-rs| Hunk #1 FAILED at 26 (different line endings).
>   bilge-0.2-rs| 1 out of 1 hunk FAILED -- saving rejects to file src/shared/discriminant_assigner.rs.rej
>   bilge-0.2-rs| patching file src/shared/fallback.rs
>   bilge-0.2-rs| Hunk #1 FAILED at 22 (different line endings).
>   bilge-0.2-rs| 1 out of 1 hunk FAILED -- saving rejects to file src/shared/fallback.rs.rej
>
>   ../subprojects/bilge-0.2.0/meson.build:9:0: ERROR: Failed to apply diff file "bilge-impl-1.63.0.patch"
>
> It turns out that the sources in bilge-impl have CRLF line endings and
> `patch` does not ignore that even with the `-l` option:

Thanks, we need a .gitattributes file to ensure that line endings are
kept unmodified on checkout.

Paolo

[PATCH 13/16] rust: do not use TYPE_CHARDEV unnecessarily

[Paolo Bonzini]

In the invocation of qdev_prop_set_chr(), "chardev" is the name of a
property rather than a type and has to match the name of the property
in device_class.rs.  Do not use TYPE_CHARDEV here, just like in the C
version of pl011_create.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rust/hw/char/pl011/src/device.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index f331a13b5f1..ca67d452e7e 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -582,7 +582,7 @@ pub fn update(&self) {
         let dev: *mut DeviceState = qdev_new(PL011State::TYPE_INFO.name);
         let sysbus: *mut SysBusDevice = dev.cast::<SysBusDevice>();
 
-        qdev_prop_set_chr(dev, bindings::TYPE_CHARDEV.as_ptr(), chr);
+        qdev_prop_set_chr(dev, c_str!("chardev").as_ptr(), chr);
         sysbus_realize_and_unref(sysbus, addr_of!(error_fatal) as *mut *mut Error);
         sysbus_mmio_map(sysbus, 0, addr);
         sysbus_connect_irq(sysbus, 0, irq);
-- 
2.46.2

[PATCH 14/16] rust: do not use --no-size_t-is-usize

[Paolo Bonzini]

This not necessary, and adds an extra cast since size_of and align_of
already return the right type.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                      | 1 -
 rust/qemu-api/src/definitions.rs | 6 +++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 0e279d245b4..3b35fcb3feb 100644
--- a/meson.build
+++ b/meson.build
@@ -3931,7 +3931,6 @@ if have_rust and have_system
     '--merge-extern-blocks',
     '--no-doc-comments',
     '--with-derive-default',
-    '--no-size_t-is-usize',
     '--no-layout-tests',
     '--no-prepend-enum-name',
     '--allowlist-file', meson.project_source_root() + '/include/.*',
diff --git a/rust/qemu-api/src/definitions.rs b/rust/qemu-api/src/definitions.rs
index aa7cdd69c99..4eaf2a91fab 100644
--- a/rust/qemu-api/src/definitions.rs
+++ b/rust/qemu-api/src/definitions.rs
@@ -83,13 +83,13 @@ macro_rules! type_info {
             } else {
                 ::core::ptr::null_mut()
             },
-            instance_size: ::core::mem::size_of::<$t>() as $crate::bindings::size_t,
-            instance_align: ::core::mem::align_of::<$t>() as $crate::bindings::size_t,
+            instance_size: ::core::mem::size_of::<$t>(),
+            instance_align: ::core::mem::align_of::<$t>(),
             instance_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_INIT,
             instance_post_init: <$t as $crate::definitions::ObjectImpl>::INSTANCE_POST_INIT,
             instance_finalize: <$t as $crate::definitions::ObjectImpl>::INSTANCE_FINALIZE,
             abstract_: <$t as $crate::definitions::ObjectImpl>::ABSTRACT,
-            class_size:  ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>() as $crate::bindings::size_t,
+            class_size:  ::core::mem::size_of::<<$t as $crate::definitions::ObjectImpl>::Class>(),
             class_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_INIT,
             class_base_init: <<$t as $crate::definitions::ObjectImpl>::Class as $crate::definitions::Class>::CLASS_BASE_INIT,
             class_data: ::core::ptr::null_mut(),
-- 
2.46.2

[PATCH 15/16] rust: do not use --generate-cstr

[Paolo Bonzini]

--generate-cstr is a good idea and generally the right thing to do,
but it is not available in Debian 12 and Ubuntu 22.04.  Work around
the absence.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                       | 4 +++-
 rust/hw/char/pl011/src/device.rs  | 1 +
 rust/qemu-api/src/device_class.rs | 8 +++++++-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 3b35fcb3feb..e08f226ee8a 100644
--- a/meson.build
+++ b/meson.build
@@ -3920,13 +3920,15 @@ common_all = static_library('common',
                             dependencies: common_ss.all_dependencies())
 
 if have_rust and have_system
+  # We would like to use --generate-cstr, but it is only available
+  # starting with bindgen 0.66.0.  The oldest supported versions
+  # are in Ubuntu 22.04 (0.59.1) and Debian 12 (0.60.1).
   bindgen_args = [
     '--disable-header-comment',
     '--raw-line', '// @generated',
     '--ctypes-prefix', 'std::os::raw',
     '--formatter', 'rustfmt',
     '--generate-block',
-    '--generate-cstr',
     '--impl-debug',
     '--merge-extern-blocks',
     '--no-doc-comments',
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index ca67d452e7e..ec82a2619f2 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -13,6 +13,7 @@
     bindings::{self, *},
     c_str,
     definitions::ObjectImpl,
+    device_class::TYPE_SYS_BUS_DEVICE,
     with_offsets,
 };
 
diff --git a/rust/qemu-api/src/device_class.rs b/rust/qemu-api/src/device_class.rs
index d2535125c48..f4eec2f748a 100644
--- a/rust/qemu-api/src/device_class.rs
+++ b/rust/qemu-api/src/device_class.rs
@@ -2,7 +2,9 @@
 // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-use crate::bindings::Property;
+use std::ffi::CStr;
+
+use crate::bindings::{self, Property};
 
 #[macro_export]
 macro_rules! device_class_init {
@@ -129,3 +131,7 @@ macro_rules! vm_state_description {
         };
     }
 }
+
+// workaround until we can use --generate-cstr in bindgen.
+pub const TYPE_SYS_BUS_DEVICE: &CStr =
+    unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_SYS_BUS_DEVICE) };
-- 
2.46.2

[PATCH 16/16] rust: allow older version of bindgen

[Paolo Bonzini]

Cope with the old version that is provided in Debian 12 and Ubuntu 22.04.
--size_t-is-usize is needed on bindgen <0.61.0 (Debian 12, Ubuntu 22.04),
and it was removed in bindgen 0.65.0, so check for it in meson.build.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index e08f226ee8a..2a292935a94 100644
--- a/meson.build
+++ b/meson.build
@@ -74,6 +74,7 @@ endif
 have_rust = false
 if not get_option('rust').disabled() and add_languages('rust', required: get_option('rust'), native: false) \
     and add_languages('rust', required: get_option('rust'), native: true)
+  bindgen = find_program('bindgen')
   rustc = meson.get_compiler('rust')
   have_rust = true
   if rustc.version().version_compare('<1.63.0')
@@ -3939,6 +3940,9 @@ if have_rust and have_system
     '--allowlist-file', meson.project_source_root() + '/.*',
     '--allowlist-file', meson.project_build_root() + '/.*'
     ]
+  if bindgen.version().version_compare('<0.65.0')
+    bindgen_args += ['--size_t-is-usize']
+  endif
   c_enums = [
     'DeviceCategory',
     'GpioPolarity',
@@ -3974,7 +3978,7 @@ if have_rust and have_system
     dependencies: common_ss.all_dependencies(),
     output: 'bindings.rs',
     include_directories: include_directories('.', 'include'),
-    bindgen_version: ['>=0.69.4'],
+    bindgen_version: ['>=0.59.1'],
     args: bindgen_args,
     )
   subdir('rust')
-- 
2.46.2

Re: [PATCH 16/16] rust: allow older version of bindgen

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Cope with the old version that is provided in Debian 12 and Ubuntu 22.04.
> --size_t-is-usize is needed on bindgen <0.61.0 (Debian 12, Ubuntu 22.04),
> and it was removed in bindgen 0.65.0, so check for it in meson.build.

The bindgen 0.59.1 installed from Ubuntu 22.04 apt source does not
support the following args:

    '--formatter', 'rustfmt',
    '--merge-extern-blocks',
    '--allowlist-file', meson.project_source_root() + '/include/.*',
    '--allowlist-file', meson.project_source_root() + '/.*',
    '--allowlist-file', meson.project_build_root() + '/.*'

The first two args are cosmetic and should not hurt if removed (but I
need to double check).

Removing the allowlist-file, however, causes IPPORT_RESERVED to be
generated twice using different types and thus break the
build. Allowlists for bindgen 0.59.1 can only be specified as regex on
function, type or var. I don't find (yet) an equivalent way of
--allowlist-file. A dirty trick is `--blocklist-item IPPORT_RESERVED`,
which works but is so ad-hoc.

--
Best Regards
Junjie Mao

Re: [PATCH 16/16] rust: allow older version of bindgen

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]

Il mer 16 ott 2024, 08:29 Junjie Mao <junjie.mao@hotmail.com> ha scritto:

>
> The bindgen 0.59.1 installed from Ubuntu 22.04 apt source does not
> support the following args:
>
>     '--formatter', 'rustfmt',
>     '--merge-extern-blocks',
>     '--allowlist-file', meson.project_source_root() + '/include/.*',
>     '--allowlist-file', meson.project_source_root() + '/.*',
>     '--allowlist-file', meson.project_build_root() + '/.*'
>
>
Ouch. --allowlist-file was added in 0.60, so Debian has it. I think we
should ask Canonical if they can update bindgen in addition to rustc.

Paolo

The first two args are cosmetic and should not hurt if removed (but I
> need to double check).
>
> Removing the allowlist-file, however, causes IPPORT_RESERVED to be
> generated twice using different types and thus break the
> build. Allowlists for bindgen 0.59.1 can only be specified as regex on
> function, type or var. I don't find (yet) an equivalent way of
> --allowlist-file. A dirty trick is `--blocklist-item IPPORT_RESERVED`,
> which works but is so ad-hoc.
>
> --
> Best Regards
> Junjie Mao
>
>

[-- Attachment #2: Type: text/html, Size: 1863 bytes --]

Re: [RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[Kevin Wolf]

Am 15.10.2024 um 15:17 hat Paolo Bonzini geschrieben:
> This includes a few fixes to the Rust build system machinery, and
> removes constructs that were added or stabilized after version 1.63.0:

Most of this series looks harmless in the sense that we need to write
some workaround code in a single place and can forget about it. So
that's good.

> - "let else" (by patching bilge-impl, patch 4; stable in 1.65.0)

This one affects all of the code we'll write and the replacement is a
bit unwieldy. It might be the most annoying one from the list.

> - std::sync::OnceLock (patch 6; stable in 1.70.0)
> 
> - core::ffi (patch 7; stable in 1.64.0)
> 
> - c"" literals (patch 9; stable in 1.77.0)

This one will be fairly widespread, too, but only a minor inconvenience.

> - offset_of! (patch 10; stable in 1.77.0)

Requiring structs to be wrapped in with_offsets! has potential to become
quite annoying, too, but it seems we already have a solution for this
with the proc macro.

> - MaybeUninit::zeroed() (patch 11; stable in 1.75.0 in const context)
> 
> It also replicates the configuration checks normally done by
> proc-macro2's build.rs into our Meson-based build rules, so that
> the crate can be made to work with an older version of rustc.
> 
> As a small bonus, patch 11 removes some uses of unsafe, so that patch
> probably won't just be simply reverted even once we can assume version
> 1.75.0 of the language.  And as another small bonus this series introduces
> the first use of Rust unit tests.
> 
> On top of this, the required version of bindgen is still too new
> for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
> 
> This is an RFC for two reasons.  First, because it would be a valid
> decision to delay enabling of Rust until at least some of these
> features are available in all supported distros.  Another possibility
> could be to accept Rust 1.64.0 but require installing a newer bindgen
> (0.66.x for example) on those two distros with an older release.  Second,
> because the series is missing the CI updates to actually ensure that
> these minimum versions keep working.
> 
> The main purpose is to show the kind of hacks that would be needed
> to support older toolchains.  The fixes (for example patches
> 1/2/3/6/8/11/13/14) can be easily extracted independent of the outcome
> of the discussion, and/or while the CI updates are made.

It would probably make sense to just go ahead and apply the fixes. They
seem a lot more obvious than the question which toolchains we want to
support.

Kevin

Re: [RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[Daniel P. Berrangé]

On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
> On top of this, the required version of bindgen is still too new
> for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
> 
> This is an RFC for two reasons.  First, because it would be a valid
> decision to delay enabling of Rust until at least some of these
> features are available in all supported distros.

Lets say we maximise our back compatibility today, and have to
carry some sub-optimal code patterns.

1, 2, 3, 4 years down the lines, we can gradually eliminate
those undesired code patterns / workarounds, as older distros
naturally age-out of our matrix.  After 4 years our entire
matrix will have cycled, so we're not needing to carry this
debt for very long (4 years is not long in the context of a
project like QEMU which has been going several decades)

IOW, we're deciding between

 * creating a bit of rust technical debt in the immediate
   term, in order to enable rust by default sooner

vs

 * avoiding Rust technical debt, but delaying ability to
   enable rust by default.

We could consider all C code to be technical debt though,
and if we don't have Rust by default we'll continue  adding
yet more C code. IOW, option is just moving the debt from
Rust back to C, which is arguably worse on balance.

Personally I tend towards quicker adoption of Rust, despite
the need for short term workarounds, as they'll disappear
again reasonably quickly.

>                                                  Another possibility
> could be to accept Rust 1.64.0 but require installing a newer bindgen
> (0.66.x for example) on those two distros with an older release.

How difficult is it to get newer 'bindgen' installed on these
platforms ? The audience here is not so much distros trying to
package new QEMU, as that's ony relevant for new distro, but
rather it is end usrs/contributors building QEMU for themslves.

Can it be done automagically in the same way we "do the right thing"
with the 3rd party crates we depend on, or is bindgen special in
some way that makes it more inconvenient for users ?

>                                                                   Second,
> because the series is missing the CI updates to actually ensure that
> these minimum versions keep working.

On the last point, see

  https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg02688.html

with that series, it should be just a matter of adding '--enable-rust'
in a few key jobs.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[Paolo Bonzini]

On 10/18/24 15:31, Daniel P. Berrangé wrote:
> On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
>> On top of this, the required version of bindgen is still too new
>> for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
>>
>> This is an RFC for two reasons.  First, because it would be a valid
>> decision to delay enabling of Rust until at least some of these
>> features are available in all supported distros.
> 
> Lets say we maximise our back compatibility today, and have to
> carry some sub-optimal code patterns.
> 
> 1, 2, 3, 4 years down the lines, we can gradually eliminate
> those undesired code patterns / workarounds, as older distros
> naturally age-out of our matrix.  After 4 years our entire
> matrix will have cycled, so we're not needing to carry this
> debt for very long (4 years is not long in the context of a
> project like QEMU which has been going several decades)

I agree, for what it's worth.

> Personally I tend towards quicker adoption of Rust, despite
> the need for short term workarounds, as they'll disappear
> again reasonably quickly.

Yes, especially since (as Kevin pointed out) most of the workarounds are 
okay in terms of maintainability.  If the worst is "if let", and it only 
occurs in a dependency, we're in a good place overall.

>>                                                   Another possibility
>> could be to accept Rust 1.64.0 but require installing a newer bindgen
>> (0.66.x for example) on those two distros with an older release.
> 
> How difficult is it to get newer 'bindgen' installed on these
> platforms ? The audience here is not so much distros trying to
> package new QEMU, as that's ony relevant for new distro, but
> rather it is end usrs/contributors building QEMU for themslves.

Very simple - "cargo install bindgen-cli", as already seen in the 
fedora-rust-nightly container's Dockerfile (note: building QEMU does 
_not_ need cargo).  In fact we could in fact do it via libvirt-ci, and 
it's quite possible that MacOS or some BSDs will need it.

Personally I'd be okay with allowing Debian 12 but not Ubuntu 22.04, for 
various reasons:

- Ubuntu 22.04 has a new rustc and an old bindgen---so it's really just 
laziness.

- any workarounds for Debian 12 would last shorter, and anyway

- Debian 12 has the really important feature (--allowlist-file), whereas 
the lack of --generate-cstr is only annoying.

> Can it be done automagically in the same way we "do the right thing"
> with the 3rd party crates we depend on, or is bindgen special in
> some way that makes it more inconvenient for users ?

bindgen is special in that it has a metric ton of indirect dependencies, 
which we'd all have to write a meson.build for (by hand). :/

> On the last point, see
> 
>    https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg02688.html
> 
> with that series, it should be just a matter of adding '--enable-rust'
> in a few key jobs.

Indeed, thanks.

Paolo

Re: [RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[Kevin Wolf]

Am 18.10.2024 um 17:43 hat Paolo Bonzini geschrieben:
> On 10/18/24 15:31, Daniel P. Berrangé wrote:
> > On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
> > > On top of this, the required version of bindgen is still too new
> > > for Debian 12 and Ubuntu 22.04.  This is fixed by the last four patches.
> > > 
> > > This is an RFC for two reasons.  First, because it would be a valid
> > > decision to delay enabling of Rust until at least some of these
> > > features are available in all supported distros.
> > 
> > Lets say we maximise our back compatibility today, and have to
> > carry some sub-optimal code patterns.
> > 
> > 1, 2, 3, 4 years down the lines, we can gradually eliminate
> > those undesired code patterns / workarounds, as older distros
> > naturally age-out of our matrix.  After 4 years our entire
> > matrix will have cycled, so we're not needing to carry this
> > debt for very long (4 years is not long in the context of a
> > project like QEMU which has been going several decades)
> 
> I agree, for what it's worth.
> 
> > Personally I tend towards quicker adoption of Rust, despite
> > the need for short term workarounds, as they'll disappear
> > again reasonably quickly.
> 
> Yes, especially since (as Kevin pointed out) most of the workarounds are
> okay in terms of maintainability.  If the worst is "if let", and it only
> occurs in a dependency, we're in a good place overall.

s/if let/let else/

"only occurs in a dependency" is probably not the right argument while
we haven't really started writing our own Rust code. If it were
available, we would probably use it in new code. But even without that,
the conclusion is the same, of course: This doesn't prevent implementing
anything and is far from being a show stopper.

I'm in favour of anything that lets up keep the phase of duplication as
short as possible if it doesn't severly limit what we can do. And I
don't see anything in this series that would do that.

> > >                                                   Another possibility
> > > could be to accept Rust 1.64.0 but require installing a newer bindgen
> > > (0.66.x for example) on those two distros with an older release.
> > 
> > How difficult is it to get newer 'bindgen' installed on these
> > platforms ? The audience here is not so much distros trying to
> > package new QEMU, as that's ony relevant for new distro, but
> > rather it is end usrs/contributors building QEMU for themslves.
> 
> Very simple - "cargo install bindgen-cli", as already seen in the
> fedora-rust-nightly container's Dockerfile (note: building QEMU does _not_
> need cargo).  In fact we could in fact do it via libvirt-ci, and it's quite
> possible that MacOS or some BSDs will need it.
> 
> Personally I'd be okay with allowing Debian 12 but not Ubuntu 22.04, for
> various reasons:
> 
> - Ubuntu 22.04 has a new rustc and an old bindgen---so it's really just
> laziness.
> 
> - any workarounds for Debian 12 would last shorter, and anyway
> 
> - Debian 12 has the really important feature (--allowlist-file), whereas the
> lack of --generate-cstr is only annoying.
> 
> > Can it be done automagically in the same way we "do the right thing"
> > with the 3rd party crates we depend on, or is bindgen special in
> > some way that makes it more inconvenient for users ?
> 
> bindgen is special in that it has a metric ton of indirect dependencies,
> which we'd all have to write a meson.build for (by hand). :/

If configure errors out with a message "Please run 'cargo install
bindgen-cli'" and that is really enough to make it build, I think that
should be enough to be counted as supporting the distro.

Kevin

Re: [RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[Peter Maydell]

On Fri, 18 Oct 2024 at 16:43, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 10/18/24 15:31, Daniel P. Berrangé wrote:
> > On Tue, Oct 15, 2024 at 03:17:18PM +0200, Paolo Bonzini wrote:
> >>                                                   Another possibility
> >> could be to accept Rust 1.64.0 but require installing a newer bindgen
> >> (0.66.x for example) on those two distros with an older release.
> >
> > How difficult is it to get newer 'bindgen' installed on these
> > platforms ? The audience here is not so much distros trying to
> > package new QEMU, as that's ony relevant for new distro, but
> > rather it is end usrs/contributors building QEMU for themslves.
>
> Very simple - "cargo install bindgen-cli", as already seen in the
> fedora-rust-nightly container's Dockerfile (note: building QEMU does
> _not_ need cargo).  In fact we could in fact do it via libvirt-ci, and
> it's quite possible that MacOS or some BSDs will need it.

Why doesn't 'rustup update' do this automatically? My Ubuntu 22.04
system I'm using 'rustup' to provide the rust toolchain,
and 'rustup update' updates rustc, cargo, clippy, etc, so
why isn't it also providing and updating bindgen?
('bindgen' for me is ~/.cargo/bin/bindgen, so not the system one.)
My expectation here was that "rustup update" would keep
the whole toolchain up-to-date...

thanks
-- PMM

Re: [RFC PATCH 00/16] rust: allow older versions of rustc and bindgen

[Paolo Bonzini]

On 10/21/24 16:15, Peter Maydell wrote:
>> Very simple - "cargo install bindgen-cli", as already seen in the
>> fedora-rust-nightly container's Dockerfile (note: building QEMU does
>> _not_ need cargo).  In fact we could in fact do it via libvirt-ci, and
>> it's quite possible that MacOS or some BSDs will need it.
> 
> Why doesn't 'rustup update' do this automatically? My Ubuntu 22.04
> system I'm using 'rustup' to provide the rust toolchain,
> and 'rustup update' updates rustc, cargo, clippy, etc, so
> why isn't it also providing and updating bindgen?
> ('bindgen' for me is ~/.cargo/bin/bindgen, so not the system one.)
> My expectation here was that "rustup update" would keep
> the whole toolchain up-to-date...

I'd agree with you, but bindgen is not part of the toolchain. :/  In the 
Cargo way of doing things, bindgen is specified in build-dependencies 
and it's rebuilt together with the rest of the library that uses it.

This is as wasteful as it sounds; I guess it makes it easier for bindgen 
authors to "move fast and break things"?  Even in the 3-years span of 
bindgen versions supported by QEMU we have the case where 
--size_t-is-usize is needed on older versions and was removed only a 
handful of versions after it became the default.

Hopefully, this becomes less problematic as Linux forces the tools to 
mature (my hope is that, in general, Linux will force the Rust team to 
give more consideration to non-Cargo mixed-language projects).

Paolo