Re: [PATCH 01/11] rust: Build separate qemu_api_tools and qemu_api_system

[Paolo Bonzini <pbonzini@redhat.com>]

On 2/11/25 22:43, Kevin Wolf wrote:
> The existing qemu_api library can't be linked into tools because it
> contains a few bindings for things that only exist in the system
> emulator.
> 
> This adds a new "system" feature to the qemu_api crate that enables the
> system emulator parts in it, and build the crate twice: qemu_api_tools
> is the core library that can be linked into tools, and qemu_api_system
> is the full one for the system emulator.

As discussed on IRC, the issue here is ClassInitImpl<>, which has to be 
defined in the same crate for qemu_api::qom and qemu_api::qdev.

Right now, the block layer has no use for QOM, but later it will (for 
secret management, for example), so splitting QOM into a separate crate 
does not work long term.

I'll try to figure out an alternative way to do the class_init bindings.

Paolo

> Unfortunately, since library names have to be unique in meson, this
> means that every user of the library now needs to specify a
> rust_dependency_map to make either qemu_api_tools or qemu_api_system
> show up as the qemu_api crate in Rust.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   rust/wrapper-system.h          | 44 +++++++++++++++++++++
>   rust/wrapper.h                 |  9 -----
>   meson.build                    | 11 +++++-
>   rust/hw/char/pl011/meson.build |  3 +-
>   rust/meson.build               | 11 +++---
>   rust/qemu-api/Cargo.toml       |  1 +
>   rust/qemu-api/build.rs         | 10 ++++-
>   rust/qemu-api/meson.build      | 70 ++++++++++++++++++++++------------
>   rust/qemu-api/src/bindings.rs  | 16 ++++++--
>   rust/qemu-api/src/lib.rs       |  4 ++
>   rust/qemu-api/src/prelude.rs   |  2 +
>   rust/qemu-api/src/zeroable.rs  | 10 +++++
>   12 files changed, 143 insertions(+), 48 deletions(-)
>   create mode 100644 rust/wrapper-system.h
> 
> diff --git a/rust/wrapper-system.h b/rust/wrapper-system.h
> new file mode 100644
> index 0000000000..fc6c571e6d
> --- /dev/null
> +++ b/rust/wrapper-system.h
> @@ -0,0 +1,44 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2024 Linaro Ltd.
> + *
> + * Authors: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +
> +/*
> + * This header file is meant to be used as input to the `bindgen` application
> + * in order to generate C FFI compatible Rust bindings.
> + */
> +
> +/* The system emulator has all of the bindings tools have */
> +#include "wrapper.h"
> +
> +#include "system/system.h"
> +#include "hw/sysbus.h"
> +#include "exec/memory.h"
> +#include "hw/clock.h"
> +#include "hw/qdev-clock.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/qdev-properties-system.h"
> +#include "hw/irq.h"
> +#include "migration/vmstate.h"
> diff --git a/rust/wrapper.h b/rust/wrapper.h
> index a9bc67af0d..41be87adcf 100644
> --- a/rust/wrapper.h
> +++ b/rust/wrapper.h
> @@ -50,15 +50,6 @@ typedef enum memory_order {
>   #include "qemu/osdep.h"
>   #include "qemu/module.h"
>   #include "qemu-io.h"
> -#include "system/system.h"
> -#include "hw/sysbus.h"
> -#include "exec/memory.h"
>   #include "chardev/char-fe.h"
> -#include "hw/clock.h"
> -#include "hw/qdev-clock.h"
> -#include "hw/qdev-properties.h"
> -#include "hw/qdev-properties-system.h"
> -#include "hw/irq.h"
>   #include "qapi/error.h"
> -#include "migration/vmstate.h"
>   #include "chardev/char-serial.h"
> diff --git a/meson.build b/meson.build
> index 18cf9e2913..1f26801b69 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -4094,10 +4094,17 @@ if have_rust
>     # this case you must pass the path to `clang` and `libclang` to your build
>     # command invocation using the environment variables CLANG_PATH and
>     # LIBCLANG_PATH
> -  bindings_rs = rust.bindgen(
> +  bindings_rs_tools = rust.bindgen(
>       input: 'rust/wrapper.h',
> +    output: 'bindings_tools.inc.rs',
> +    include_directories: include_directories('.', 'include'),
> +    bindgen_version: ['>=0.60.0'],
> +    args: bindgen_args,
> +    )
> +  bindings_rs_system = rust.bindgen(
> +    input: 'rust/wrapper-system.h',
>       dependencies: common_ss.all_dependencies(),
> -    output: 'bindings.inc.rs',
> +    output: 'bindings_system.inc.rs',
>       include_directories: include_directories('.', 'include'),
>       bindgen_version: ['>=0.60.0'],
>       args: bindgen_args,
> diff --git a/rust/hw/char/pl011/meson.build b/rust/hw/char/pl011/meson.build
> index 547cca5a96..d2cfede5dc 100644
> --- a/rust/hw/char/pl011/meson.build
> +++ b/rust/hw/char/pl011/meson.build
> @@ -12,9 +12,10 @@ _libpl011_rs = static_library(
>     dependencies: [
>       bilge_dep,
>       bilge_impl_dep,
> -    qemu_api,
> +    qemu_api_system,
>       qemu_api_macros,
>     ],
> +  rust_dependency_map: {'qemu_api_system': 'qemu_api'},
>   )
>   
>   rust_devices_ss.add(when: 'CONFIG_X_PL011_RUST', if_true: [declare_dependency(
> diff --git a/rust/meson.build b/rust/meson.build
> index 91e52b8fb8..50eb23b072 100644
> --- a/rust/meson.build
> +++ b/rust/meson.build
> @@ -9,18 +9,19 @@ if cargo.found()
>     run_target('clippy',
>       command: [config_host['MESON'], 'devenv',
>                 '--workdir', '@CURRENT_SOURCE_DIR@',
> -              cargo, 'clippy', '--tests'],
> -    depends: bindings_rs)
> +              cargo, 'clippy', '--tests', '--features', 'system'],
> +    depends: bindings_rs_system)
>   
>     run_target('rustfmt',
>       command: [config_host['MESON'], 'devenv',
>                 '--workdir', '@CURRENT_SOURCE_DIR@',
>                 cargo, 'fmt'],
> -    depends: bindings_rs)
> +    depends: bindings_rs_system)
>   
>     run_target('rustdoc',
>       command: [config_host['MESON'], 'devenv',
>                 '--workdir', '@CURRENT_SOURCE_DIR@',
> -              cargo, 'doc', '--no-deps', '--document-private-items'],
> -    depends: bindings_rs)
> +              cargo, 'doc', '--no-deps', '--document-private-items',
> +              '--features', 'system'],
> +    depends: bindings_rs_system)
>   endif
> diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
> index a51dd14285..ed7b60bc80 100644
> --- a/rust/qemu-api/Cargo.toml
> +++ b/rust/qemu-api/Cargo.toml
> @@ -24,6 +24,7 @@ version_check = "~0.9"
>   default = ["debug_cell"]
>   allocator = []
>   debug_cell = []
> +system= []
>   
>   [lints]
>   workspace = true
> diff --git a/rust/qemu-api/build.rs b/rust/qemu-api/build.rs
> index 471e6c633d..b14f9d4e4a 100644
> --- a/rust/qemu-api/build.rs
> +++ b/rust/qemu-api/build.rs
> @@ -16,7 +16,13 @@ fn main() -> Result<()> {
>       let path = env::var("MESON_BUILD_ROOT")
>           .unwrap_or_else(|_| format!("{}/src", env!("CARGO_MANIFEST_DIR")));
>   
> -    let file = format!("{}/bindings.inc.rs", path);
> +    let basename = if cfg!(feature = "system") {
> +        "bindings_system.inc.rs"
> +    } else {
> +        "bindings_tools.inc.rs"
> +    };
> +
> +    let file = format!("{}/{}", path, basename);
>       let file = Path::new(&file);
>       if !Path::new(&file).exists() {
>           panic!(concat!(
> @@ -31,7 +37,7 @@ fn main() -> Result<()> {
>       }
>   
>       let out_dir = env::var("OUT_DIR").unwrap();
> -    let dest_path = format!("{}/bindings.inc.rs", out_dir);
> +    let dest_path = format!("{}/{}", out_dir, basename);
>       let dest_path = Path::new(&dest_path);
>       if dest_path.symlink_metadata().is_ok() {
>           remove_file(dest_path)?;
> diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> index 60944a657d..acac384936 100644
> --- a/rust/qemu-api/meson.build
> +++ b/rust/qemu-api/meson.build
> @@ -10,39 +10,58 @@ if get_option('debug_mutex')
>     _qemu_api_cfg += ['--cfg', 'feature="debug_cell"']
>   endif
>   
> -_qemu_api_rs = static_library(
> -  'qemu_api',
> +sources_core = [
> +  'src/lib.rs',
> +  'src/assertions.rs',
> +  'src/bindings.rs',
> +  'src/bitops.rs',
> +  'src/callbacks.rs',
> +  'src/cell.rs',
> +  'src/c_str.rs',
> +  'src/module.rs',
> +  'src/offset_of.rs',
> +  'src/prelude.rs',
> +  'src/qom.rs',
> +  'src/zeroable.rs',
> +]
> +sources_system = sources_core + [
> +  'src/irq.rs',
> +  'src/qdev.rs',
> +  'src/sysbus.rs',
> +  'src/vmstate.rs',
> +]
> +
> +
> +_qemu_api_tools_rs = static_library(
> +  'qemu_api_tools',
>     structured_sources(
> -    [
> -      'src/lib.rs',
> -      'src/assertions.rs',
> -      'src/bindings.rs',
> -      'src/bitops.rs',
> -      'src/callbacks.rs',
> -      'src/cell.rs',
> -      'src/c_str.rs',
> -      'src/irq.rs',
> -      'src/module.rs',
> -      'src/offset_of.rs',
> -      'src/prelude.rs',
> -      'src/qdev.rs',
> -      'src/qom.rs',
> -      'src/sysbus.rs',
> -      'src/vmstate.rs',
> -      'src/zeroable.rs',
> -    ],
> -    {'.' : bindings_rs},
> +    sources_core,
> +    {'.' : bindings_rs_tools},
>     ),
>     override_options: ['rust_std=2021', 'build.rust_std=2021'],
>     rust_abi: 'rust',
>     rust_args: _qemu_api_cfg,
>   )
> +_qemu_api_system_rs = static_library(
> +  'qemu_api_system',
> +  structured_sources(
> +    sources_system,
> +    {'.' : bindings_rs_system},
> +  ),
> +  override_options: ['rust_std=2021', 'build.rust_std=2021'],
> +  rust_abi: 'rust',
> +  rust_args: _qemu_api_cfg + ['--cfg', 'feature="system"'],
> +)
>   
> -rust.test('rust-qemu-api-tests', _qemu_api_rs,
> +rust.test('rust-qemu-api-tests', _qemu_api_system_rs,
>             suite: ['unit', 'rust'])
>   
> -qemu_api = declare_dependency(
> -  link_with: _qemu_api_rs,
> +qemu_api_tools = declare_dependency(
> +  link_with: _qemu_api_tools_rs,
> +  dependencies: qemu_api_macros,
> +)
> +qemu_api_system = declare_dependency(
> +  link_with: _qemu_api_system_rs,
>     dependencies: qemu_api_macros,
>   )
>   
> @@ -59,7 +78,8 @@ test('rust-qemu-api-integration',
>           override_options: ['rust_std=2021', 'build.rust_std=2021'],
>           rust_args: ['--test'],
>           install: false,
> -        dependencies: [qemu_api, qemu_api_macros],
> +        dependencies: [qemu_api_system, qemu_api_macros],
> +        rust_dependency_map: {'qemu_api_system': 'qemu_api'},
>           link_whole: [rust_qemu_api_objs, libqemuutil]),
>       args: [
>           '--test', '--test-threads', '1',
> diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
> index 8a9b821bb9..2bf6c13a32 100644
> --- a/rust/qemu-api/src/bindings.rs
> +++ b/rust/qemu-api/src/bindings.rs
> @@ -15,15 +15,23 @@
>       clippy::missing_safety_doc
>   )]
>   
> -#[cfg(MESON)]
> -include!("bindings.inc.rs");
> +#[cfg(all(MESON, not(feature="system")))]
> +include!("bindings_tools.inc.rs");
> +#[cfg(all(MESON, feature="system"))]
> +include!("bindings_system.inc.rs");
>   
> -#[cfg(not(MESON))]
> -include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
> +#[cfg(all(not(MESON), not(feature="system")))]
> +include!(concat!(env!("OUT_DIR"), "/bindings_tools.inc.rs"));
> +#[cfg(all(not(MESON), feature="system"))]
> +include!(concat!(env!("OUT_DIR"), "/bindings_system.inc.rs"));
>   
>   unsafe impl Send for Property {}
>   unsafe impl Sync for Property {}
>   unsafe impl Sync for TypeInfo {}
> +
> +#[cfg(feature="system")]
>   unsafe impl Sync for VMStateDescription {}
> +#[cfg(feature="system")]
>   unsafe impl Sync for VMStateField {}
> +#[cfg(feature="system")]
>   unsafe impl Sync for VMStateInfo {}
> diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
> index 3cf9371cff..3c6c154f3d 100644
> --- a/rust/qemu-api/src/lib.rs
> +++ b/rust/qemu-api/src/lib.rs
> @@ -18,12 +18,16 @@
>   pub mod c_str;
>   pub mod callbacks;
>   pub mod cell;
> +#[cfg(feature = "system")]
>   pub mod irq;
>   pub mod module;
>   pub mod offset_of;
> +#[cfg(feature = "system")]
>   pub mod qdev;
>   pub mod qom;
> +#[cfg(feature = "system")]
>   pub mod sysbus;
> +#[cfg(feature = "system")]
>   pub mod vmstate;
>   pub mod zeroable;
>   
> diff --git a/rust/qemu-api/src/prelude.rs b/rust/qemu-api/src/prelude.rs
> index 2dc86e19b2..1b8d7d319e 100644
> --- a/rust/qemu-api/src/prelude.rs
> +++ b/rust/qemu-api/src/prelude.rs
> @@ -17,6 +17,8 @@
>   
>   pub use crate::qom_isa;
>   
> +#[cfg(feature="system")]
>   pub use crate::sysbus::SysBusDeviceMethods;
>   
> +#[cfg(feature="system")]
>   pub use crate::vmstate::VMState;
> diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
> index 7b04947cb6..b454e9e05e 100644
> --- a/rust/qemu-api/src/zeroable.rs
> +++ b/rust/qemu-api/src/zeroable.rs
> @@ -56,6 +56,7 @@ pub unsafe trait Zeroable: Default {
>   /// ## Differences with `core::mem::zeroed`
>   ///
>   /// `const_zero` zeroes padding bits, while `core::mem::zeroed` doesn't
> +#[allow(unused)]
>   macro_rules! const_zero {
>       // This macro to produce a type-generic zero constant is taken from the
>       // const_zero crate (v0.1.1):
> @@ -77,6 +78,7 @@ union TypeAsBytes {
>   }
>   
>   /// A wrapper to implement the `Zeroable` trait through the `const_zero` macro.
> +#[allow(unused)]
>   macro_rules! impl_zeroable {
>       ($type:ty) => {
>           unsafe impl Zeroable for $type {
> @@ -86,6 +88,7 @@ unsafe impl Zeroable for $type {
>   }
>   
>   // bindgen does not derive Default here
> +#[cfg(feature = "system")]
>   #[allow(clippy::derivable_impls)]
>   impl Default for crate::bindings::VMStateFlags {
>       fn default() -> Self {
> @@ -93,10 +96,17 @@ fn default() -> Self {
>       }
>   }
>   
> +#[cfg(feature = "system")]
>   impl_zeroable!(crate::bindings::Property__bindgen_ty_1);
> +#[cfg(feature = "system")]
>   impl_zeroable!(crate::bindings::Property);
> +#[cfg(feature = "system")]
>   impl_zeroable!(crate::bindings::VMStateFlags);
> +#[cfg(feature = "system")]
>   impl_zeroable!(crate::bindings::VMStateField);
> +#[cfg(feature = "system")]
>   impl_zeroable!(crate::bindings::VMStateDescription);
> +#[cfg(feature = "system")]
>   impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_1);
> +#[cfg(feature = "system")]
>   impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_2);