* [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro
@ 2025-05-22 8:12 Manos Pitsidianakis
2025-05-22 11:17 ` Zhao Liu
2025-05-23 13:05 ` Paolo Bonzini
0 siblings, 2 replies; 5+ messages in thread
From: Manos Pitsidianakis @ 2025-05-22 8:12 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-rust, Alex Bennée, Paolo Bonzini, Zhao Liu,
Manos Pitsidianakis
Add derive macro for declaring qdev properties directly above the field
definitions. To do this, we split DeviceImpl::properties method on a
separate trait so we can implement only that part in the derive macro
expansion (we cannot partially implement the DeviceImpl trait).
Adding a `property` attribute above the field declaration will generate
a `qemu_api::bindings::Property` array member in the device's property
list.
As a proof of concept, I added a typed alias for booleans,
`bool_property` that allows you to skip specifying qdev_prop_bool.
This is unnecessary though, because once we have the
const_refs_to_static feature we can introduce a QdevProp trait that
returns a reference to a type's qdev_prop_* global variable. We cannot
do this now because for our minimum Rust version we cannot refer to
statics from const values.
It'd look like this:
pub trait QDevProp {
const VALUE: &'static bindings::PropertyInfo;
}
impl QDevProp for bool {
const VALUE: &'static bindings::PropertyInfo = unsafe {
&bindings::qdev_prop_bool };
}
impl QDevProp for u64 {
const VALUE: &'static bindings::PropertyInfo = unsafe {
&bindings::qdev_prop_uint64 };
}
// etc.. for all basic types
So, this patch is not for merging yet, I will wait until we upgrade the
Rust version and re-spin it with a proper trait-based implementation (and
also split it into individual steps/patches).
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
rust/hw/char/pl011/src/device.rs | 13 +--
rust/hw/char/pl011/src/device_class.rs | 26 +-----
rust/hw/timer/hpet/src/hpet.rs | 4 +-
rust/qemu-api-macros/src/lib.rs | 157 ++++++++++++++++++++++++++++++++-
rust/qemu-api/src/qdev.rs | 22 +++--
rust/qemu-api/tests/tests.rs | 9 +-
6 files changed, 187 insertions(+), 44 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index bde3be65c5b0d9dbb117407734d93fff577ddecf..e22f5421dc5d9cd5c6fa8b11ab746e5c254bdb37 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -10,7 +10,10 @@
irq::{IRQState, InterruptSource},
memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
prelude::*,
- qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
+ qdev::{
+ Clock, ClockEvent, DeviceImpl, DevicePropertiesImpl, DeviceState, ResetType,
+ ResettablePhasesImpl,
+ },
qom::{ObjectImpl, Owned, ParentField},
static_assert,
sysbus::{SysBusDevice, SysBusDeviceImpl},
@@ -98,12 +101,13 @@ pub struct PL011Registers {
}
#[repr(C)]
-#[derive(qemu_api_macros::Object)]
+#[derive(qemu_api_macros::Object, qemu_api_macros::DeviceProperties)]
/// PL011 Device Model in QEMU
pub struct PL011State {
pub parent_obj: ParentField<SysBusDevice>,
pub iomem: MemoryRegion,
#[doc(alias = "chr")]
+ #[property(name = c"chardev", qdev_prop = qemu_api::bindings::qdev_prop_chr)]
pub char_backend: CharBackend,
pub regs: BqlRefCell<PL011Registers>,
/// QEMU interrupts
@@ -122,6 +126,7 @@ pub struct PL011State {
#[doc(alias = "clk")]
pub clock: Owned<Clock>,
#[doc(alias = "migrate_clk")]
+ #[bool_property(name = c"migrate-clk", default = true)]
pub migrate_clock: bool,
}
@@ -169,9 +174,6 @@ impl ObjectImpl for PL011State {
}
impl DeviceImpl for PL011State {
- fn properties() -> &'static [Property] {
- &device_class::PL011_PROPERTIES
- }
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&device_class::VMSTATE_PL011)
}
@@ -703,6 +705,7 @@ impl PL011Impl for PL011Luminary {
const DEVICE_ID: DeviceId = DeviceId(&[0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]);
}
+impl DevicePropertiesImpl for PL011Luminary {}
impl DeviceImpl for PL011Luminary {}
impl ResettablePhasesImpl for PL011Luminary {}
impl SysBusDeviceImpl for PL011Luminary {}
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index d328d846323f6080a9573053767e51481eb32941..83d70d7d82aac4a3252a0b4cb24af705b01d3635 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -8,11 +8,8 @@
};
use qemu_api::{
- bindings::{qdev_prop_bool, qdev_prop_chr},
- prelude::*,
- vmstate::VMStateDescription,
- vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_unused,
- zeroable::Zeroable,
+ prelude::*, vmstate::VMStateDescription, vmstate_clock, vmstate_fields, vmstate_of,
+ vmstate_struct, vmstate_subsections, vmstate_unused, zeroable::Zeroable,
};
use crate::device::{PL011Registers, PL011State};
@@ -82,22 +79,3 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, version_id: c_int) -> c_int {
},
..Zeroable::ZERO
};
-
-qemu_api::declare_properties! {
- PL011_PROPERTIES,
- qemu_api::define_property!(
- c"chardev",
- PL011State,
- char_backend,
- unsafe { &qdev_prop_chr },
- CharBackend
- ),
- qemu_api::define_property!(
- c"migrate-clk",
- PL011State,
- migrate_clock,
- unsafe { &qdev_prop_bool },
- bool,
- default = true
- ),
-}
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 779681d650998291f138e8cc61807612b8597961..21ebcfc9f22f8f5463812db218a1dc2039eda75b 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -1033,11 +1033,13 @@ impl ObjectImpl for HPETState {
..Zeroable::ZERO
};
-impl DeviceImpl for HPETState {
+impl qemu_api::qdev::DevicePropertiesImpl for HPETState {
fn properties() -> &'static [Property] {
&HPET_PROPERTIES
}
+}
+impl DeviceImpl for HPETState {
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&VMSTATE_HPET)
}
diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index f97449bb304b575c7d8c3272f287a81a9f8c9131..c5b746198d183d214526c8f0132b23d375e2d27b 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -3,10 +3,11 @@
// SPDX-License-Identifier: GPL-2.0-or-later
use proc_macro::TokenStream;
-use quote::quote;
+use quote::{quote, quote_spanned, ToTokens};
use syn::{
- parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned, token::Comma, Data,
- DeriveInput, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token, Variant,
+ parse::Parse, parse_macro_input, parse_quote, punctuated::Punctuated, spanned::Spanned,
+ token::Comma, Data, DeriveInput, Field, Fields, FieldsUnnamed, Ident, Meta, Path, Token,
+ Variant,
};
mod utils;
@@ -143,6 +144,156 @@ pub const fn raw_get(slot: *mut Self) -> *mut <Self as crate::cell::Wrapper>::Wr
})
}
+#[derive(Debug)]
+struct DeviceProperty {
+ name: Option<syn::LitCStr>,
+ qdev_prop: Option<syn::Path>,
+ assert_type: Option<proc_macro2::TokenStream>,
+ bitnr: Option<syn::Expr>,
+ defval: Option<syn::Expr>,
+}
+
+impl Parse for DeviceProperty {
+ fn parse(input: syn::parse::ParseStream) -> syn::Result<Self> {
+ let _: syn::Token![#] = input.parse()?;
+ let bracketed;
+ _ = syn::bracketed!(bracketed in input);
+ let attribute = bracketed.parse::<syn::Ident>()?.to_string();
+ let (assert_type, qdev_prop) = match attribute.as_str() {
+ "property" => (None, None),
+ "bool_property" => (
+ Some(quote! { bool }),
+ Some(syn::parse2(
+ quote! { ::qemu_api::bindings::qdev_prop_bool },
+ )?),
+ ),
+ other => unreachable!("Got unexpected DeviceProperty attribute `{}`", other),
+ };
+ let mut retval = Self {
+ name: None,
+ qdev_prop,
+ assert_type,
+ bitnr: None,
+ defval: None,
+ };
+ let content;
+ _ = syn::parenthesized!(content in bracketed);
+ while !content.is_empty() {
+ let value: syn::Ident = content.parse()?;
+ if value == "name" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.name.is_some() {
+ panic!("`name` can only be used at most once");
+ }
+ retval.name = Some(content.parse()?);
+ } else if value == "qdev_prop" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.assert_type.is_some() {
+ // qdev_prop will be Some(_), but we want to print a helpful error message
+ // explaining why you should use #[property(...)] instead of saying "you
+ // defined qdev_prop twice".
+ panic!("Use `property` attribute instead of `{attribute}` if you want to override `qdev_prop` value.");
+ }
+ if retval.qdev_prop.is_some() {
+ panic!("`qdev_prop` can only be used at most once");
+ }
+ retval.qdev_prop = Some(content.parse()?);
+ } else if value == "bitnr" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.bitnr.is_some() {
+ panic!("`bitnr` can only be used at most once");
+ }
+ retval.bitnr = Some(content.parse()?);
+ } else if value == "default" {
+ let _: syn::Token![=] = content.parse()?;
+ if retval.defval.is_some() {
+ panic!("`default` can only be used at most once");
+ }
+ retval.defval = Some(content.parse()?);
+ } else {
+ panic!("unrecognized field `{value}`");
+ }
+
+ if !content.is_empty() {
+ let _: syn::Token![,] = content.parse()?;
+ }
+ }
+ Ok(retval)
+ }
+}
+
+#[proc_macro_derive(DeviceProperties, attributes(property, bool_property))]
+pub fn derive_device_properties(input: TokenStream) -> TokenStream {
+ let span = proc_macro::Span::call_site();
+ let input = parse_macro_input!(input as DeriveInput);
+ let properties: Vec<(syn::Field, proc_macro2::Span, DeviceProperty)> = match input.data {
+ syn::Data::Struct(syn::DataStruct {
+ fields: syn::Fields::Named(fields),
+ ..
+ }) => fields
+ .named
+ .iter()
+ .flat_map(|f| {
+ f.attrs
+ .iter()
+ .filter(|a| a.path().is_ident("property") || a.path().is_ident("bool_property"))
+ .map(|a| {
+ (
+ f.clone(),
+ f.span(),
+ syn::parse(a.to_token_stream().into())
+ .expect("could not parse property attr"),
+ )
+ })
+ })
+ .collect::<Vec<_>>(),
+ _other => unreachable!(),
+ };
+ let name = &input.ident;
+
+ let mut assertions = vec![];
+ let mut properties_expanded = vec![];
+ let zero = syn::Expr::Verbatim(quote! { 0 });
+ for (field, field_span, prop) in properties {
+ let prop_name = prop.name.as_ref().unwrap();
+ let field_name = field.ident.as_ref().unwrap();
+ let qdev_prop = prop.qdev_prop.as_ref().unwrap();
+ let bitnr = prop.bitnr.as_ref().unwrap_or(&zero);
+ let set_default = prop.defval.is_some();
+ let defval = prop.defval.as_ref().unwrap_or(&zero);
+ if let Some(assert_type) = prop.assert_type {
+ assertions.push(quote_spanned! {field_span=>
+ ::qemu_api::assert_field_type! ( #name, #field_name, #assert_type );
+ });
+ }
+ properties_expanded.push(quote_spanned! {field_span=>
+ ::qemu_api::bindings::Property {
+ // use associated function syntax for type checking
+ name: ::std::ffi::CStr::as_ptr(#prop_name),
+ info: unsafe { &#qdev_prop },
+ offset: ::core::mem::offset_of!(#name, #field_name) as isize,
+ bitnr: #bitnr,
+ set_default: #set_default,
+ defval: ::qemu_api::bindings::Property__bindgen_ty_1 { u: #defval as u64 },
+ ..::qemu_api::zeroable::Zeroable::ZERO
+ }
+ });
+ }
+ let properties_expanded = quote_spanned! {span.into()=>
+ #(#assertions)*
+
+ impl ::qemu_api::qdev::DevicePropertiesImpl for #name {
+ fn properties() -> &'static [::qemu_api::bindings::Property] {
+ static PROPERTIES: &'static [::qemu_api::bindings::Property] = &[#(#properties_expanded),*];
+
+ PROPERTIES
+ }
+ }
+ };
+
+ TokenStream::from(properties_expanded)
+}
+
#[proc_macro_derive(Wrapper)]
pub fn derive_opaque(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 1279d7a58d50e1bf6c8d2e6f00d7229bbb19e003..2fd8b2750ffabcaa1065558d38a700e35fbc9136 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -100,8 +100,19 @@ pub trait ResettablePhasesImpl {
T::EXIT.unwrap()(unsafe { state.as_ref() }, typ);
}
+pub trait DevicePropertiesImpl {
+ /// An array providing the properties that the user can set on the
+ /// device. Not a `const` because referencing statics in constants
+ /// is unstable until Rust 1.83.0.
+ fn properties() -> &'static [Property] {
+ &[]
+ }
+}
+
/// Trait providing the contents of [`DeviceClass`].
-pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
+pub trait DeviceImpl:
+ ObjectImpl + ResettablePhasesImpl + DevicePropertiesImpl + IsA<DeviceState>
+{
/// _Realization_ is the second stage of device creation. It contains
/// all operations that depend on device properties and can fail (note:
/// this is not yet supported for Rust devices).
@@ -110,13 +121,6 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
/// with the function pointed to by `REALIZE`.
const REALIZE: Option<fn(&Self)> = None;
- /// An array providing the properties that the user can set on the
- /// device. Not a `const` because referencing statics in constants
- /// is unstable until Rust 1.83.0.
- fn properties() -> &'static [Property] {
- &[]
- }
-
/// A `VMStateDescription` providing the migration format for the device
/// Not a `const` because referencing statics in constants is unstable
/// until Rust 1.83.0.
@@ -171,7 +175,7 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
self.vmsd = vmsd;
}
- let prop = <T as DeviceImpl>::properties();
+ let prop = <T as DevicePropertiesImpl>::properties();
if !prop.is_empty() {
unsafe {
bindings::device_class_set_props_n(self, prop.as_ptr(), prop.len());
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index a658a49fcfdda8fa4b9d139c10afb6ff3243790b..e8eadfd6e9add385ffc97de015b84aae825c18ee 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -9,7 +9,7 @@
cell::{self, BqlCell},
declare_properties, define_property,
prelude::*,
- qdev::{DeviceImpl, DeviceState, Property, ResettablePhasesImpl},
+ qdev::{DeviceImpl, DevicePropertiesImpl, DeviceState, Property, ResettablePhasesImpl},
qom::{ObjectImpl, ParentField},
sysbus::SysBusDevice,
vmstate::VMStateDescription,
@@ -68,10 +68,13 @@ impl ObjectImpl for DummyState {
impl ResettablePhasesImpl for DummyState {}
-impl DeviceImpl for DummyState {
+impl DevicePropertiesImpl for DummyState {
fn properties() -> &'static [Property] {
&DUMMY_PROPERTIES
}
+}
+
+impl DeviceImpl for DummyState {
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&VMSTATE)
}
@@ -85,6 +88,8 @@ pub struct DummyChildState {
qom_isa!(DummyChildState: Object, DeviceState, DummyState);
+impl DevicePropertiesImpl for DummyChildState {}
+
pub struct DummyChildClass {
parent_class: <DummyState as ObjectType>::Class,
}
---
base-commit: 2af4a82ab2cce3412ffc92cd4c96bd870e33bc8e
change-id: 20250522-rust-qdev-properties-728e8f6a468e
--
γαῖα πυρί μιχθήτω
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro
2025-05-22 8:12 [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro Manos Pitsidianakis
@ 2025-05-22 11:17 ` Zhao Liu
2025-05-23 13:05 ` Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: Zhao Liu @ 2025-05-22 11:17 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: qemu-devel, qemu-rust, Alex Bennée, Paolo Bonzini
On Thu, May 22, 2025 at 11:12:30AM +0300, Manos Pitsidianakis wrote:
> Date: Thu, 22 May 2025 11:12:30 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro
> X-Mailer: b4 0.14.2
>
> Add derive macro for declaring qdev properties directly above the field
> definitions. To do this, we split DeviceImpl::properties method on a
> separate trait so we can implement only that part in the derive macro
> expansion (we cannot partially implement the DeviceImpl trait).
>
> Adding a `property` attribute above the field declaration will generate
> a `qemu_api::bindings::Property` array member in the device's property
> list.
>
> As a proof of concept, I added a typed alias for booleans,
> `bool_property` that allows you to skip specifying qdev_prop_bool.
>
> This is unnecessary though, because once we have the
> const_refs_to_static feature we can introduce a QdevProp trait that
> returns a reference to a type's qdev_prop_* global variable. We cannot
> do this now because for our minimum Rust version we cannot refer to
> statics from const values.
>
> It'd look like this:
>
> pub trait QDevProp {
> const VALUE: &'static bindings::PropertyInfo;
> }
>
> impl QDevProp for bool {
> const VALUE: &'static bindings::PropertyInfo = unsafe {
> &bindings::qdev_prop_bool };
> }
>
> impl QDevProp for u64 {
> const VALUE: &'static bindings::PropertyInfo = unsafe {
> &bindings::qdev_prop_uint64 };
> }
>
> // etc.. for all basic types
Good idea! We can have something like current impl_vmstate_scalar.
> So, this patch is not for merging yet, I will wait until we upgrade the
> Rust version and re-spin it with a proper trait-based implementation (and
> also split it into individual steps/patches).
I think this can be aligned with Paolo's optimization for vmstate [*]
[*]: https://lore.kernel.org/qemu-devel/20250505100854.73936-5-pbonzini@redhat.com/
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
> rust/hw/char/pl011/src/device.rs | 13 +--
> rust/hw/char/pl011/src/device_class.rs | 26 +-----
> rust/hw/timer/hpet/src/hpet.rs | 4 +-
> rust/qemu-api-macros/src/lib.rs | 157 ++++++++++++++++++++++++++++++++-
> rust/qemu-api/src/qdev.rs | 22 +++--
> rust/qemu-api/tests/tests.rs | 9 +-
> 6 files changed, 187 insertions(+), 44 deletions(-)
...
> #[repr(C)]
> -#[derive(qemu_api_macros::Object)]
> +#[derive(qemu_api_macros::Object, qemu_api_macros::DeviceProperties)]
> /// PL011 Device Model in QEMU
> pub struct PL011State {
> pub parent_obj: ParentField<SysBusDevice>,
> pub iomem: MemoryRegion,
> #[doc(alias = "chr")]
> + #[property(name = c"chardev", qdev_prop = qemu_api::bindings::qdev_prop_chr)]
This macro pattern looks good!
As you mentioned above, with QDevProp, this could be nicer so that user
wouldn't have to bind type and PropertyInfo manually.
> pub char_backend: CharBackend,
> pub regs: BqlRefCell<PL011Registers>,
> /// QEMU interrupts
> @@ -122,6 +126,7 @@ pub struct PL011State {
> #[doc(alias = "clk")]
> pub clock: Owned<Clock>,
> #[doc(alias = "migrate_clk")]
> + #[bool_property(name = c"migrate-clk", default = true)]
> pub migrate_clock: bool,
> }
>
> @@ -169,9 +174,6 @@ impl ObjectImpl for PL011State {
> }
>
> impl DeviceImpl for PL011State {
> - fn properties() -> &'static [Property] {
> - &device_class::PL011_PROPERTIES
> - }
> fn vmsd() -> Option<&'static VMStateDescription> {
> Some(&device_class::VMSTATE_PL011)
> }
> @@ -703,6 +705,7 @@ impl PL011Impl for PL011Luminary {
> const DEVICE_ID: DeviceId = DeviceId(&[0x11, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1]);
> }
>
> +impl DevicePropertiesImpl for PL011Luminary {}
> impl DeviceImpl for PL011Luminary {}
> impl ResettablePhasesImpl for PL011Luminary {}
> impl SysBusDeviceImpl for PL011Luminary {}
...
> diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
> index 1279d7a58d50e1bf6c8d2e6f00d7229bbb19e003..2fd8b2750ffabcaa1065558d38a700e35fbc9136 100644
> --- a/rust/qemu-api/src/qdev.rs
> +++ b/rust/qemu-api/src/qdev.rs
> @@ -100,8 +100,19 @@ pub trait ResettablePhasesImpl {
> T::EXIT.unwrap()(unsafe { state.as_ref() }, typ);
> }
>
> +pub trait DevicePropertiesImpl {
> + /// An array providing the properties that the user can set on the
> + /// device. Not a `const` because referencing statics in constants
> + /// is unstable until Rust 1.83.0.
> + fn properties() -> &'static [Property] {
> + &[]
> + }
> +}
> +
> /// Trait providing the contents of [`DeviceClass`].
> -pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
> +pub trait DeviceImpl:
> + ObjectImpl + ResettablePhasesImpl + DevicePropertiesImpl + IsA<DeviceState>
> +{
DeviceImpl has a new bound: DevicePropertiesImpl, which means that
devices that don't need properties also need to implement the empty
DevicePropertiesImpl.
We now have ResettablePhasesImpl that needs to do this, but I'm not sure
if we should continue to add more examples?
> /// _Realization_ is the second stage of device creation. It contains
> /// all operations that depend on device properties and can fail (note:
> /// this is not yet supported for Rust devices).
> @@ -110,13 +121,6 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
> /// with the function pointed to by `REALIZE`.
> const REALIZE: Option<fn(&Self)> = None;
>
> - /// An array providing the properties that the user can set on the
> - /// device. Not a `const` because referencing statics in constants
> - /// is unstable until Rust 1.83.0.
> - fn properties() -> &'static [Property] {
> - &[]
> - }
> -
This default empty Property array looks more convenient than this:
impl DevicePropertiesImpl for DummyChildState {}
However, if there's no way to avoid adding DevicePropertiesImpl, the
cost of an empty trait could be acceptable relative to the benefit of
this more friendly macro, I think!
Thanks,
Zhao
> /// A `VMStateDescription` providing the migration format for the device
> /// Not a `const` because referencing statics in constants is unstable
> /// until Rust 1.83.0.
> @@ -171,7 +175,7 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
> if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
> self.vmsd = vmsd;
> }
> - let prop = <T as DeviceImpl>::properties();
> + let prop = <T as DevicePropertiesImpl>::properties();
> if !prop.is_empty() {
> unsafe {
> bindings::device_class_set_props_n(self, prop.as_ptr(), prop.len());
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro
2025-05-22 8:12 [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro Manos Pitsidianakis
2025-05-22 11:17 ` Zhao Liu
@ 2025-05-23 13:05 ` Paolo Bonzini
2025-05-28 10:49 ` Manos Pitsidianakis
1 sibling, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2025-05-23 13:05 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, qemu-rust, Alex Bennée, Zhao Liu
[-- Attachment #1: Type: text/plain, Size: 8287 bytes --]
Il gio 22 mag 2025, 10:12 Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> ha scritto:
> This is unnecessary though, because once we have the
> const_refs_to_static feature we can introduce a QdevProp trait that
> returns a reference to a type's qdev_prop_* global variable. We cannot
> do this now because for our minimum Rust version we cannot refer to
> statics from const values.
>
Why don't you already write it assuming const_refs_to_static? If needed we
can add a hack to make the VALUE an enum, similar to what is already in
vmstate.rs.
So, this patch is not for merging yet, I will wait until we upgrade the
> Rust version and re-spin it with a proper trait-based implementation (and
> also split it into individual steps/patches).
>
It's not too large, overall.
#[repr(C)]
> -#[derive(qemu_api_macros::Object)]
> +#[derive(qemu_api_macros::Object, qemu_api_macros::DeviceProperties)]
>
Is there more to be derived that is useful for Devices? Maybe the macro can
be DeviceState or Device.
+ #[property(name = c"chardev", qdev_prop =
> qemu_api::bindings::qdev_prop_chr)]
>
Can you change the name to be a "normal" string and change it to a C
literal in the macro?
diff --git a/rust/hw/char/pl011/src/device_class.rs
> b/rust/hw/char/pl011/src/device_class.rs
> index
> d328d846323f6080a9573053767e51481eb32941..83d70d7d82aac4a3252a0b4cb24af705b01d3635
> 100644
> --- a/rust/hw/char/pl011/src/device_class.rs
> +++ b/rust/hw/char/pl011/src/device_class.rs
> @@ -8,11 +8,8 @@
> };
>
> use qemu_api::{
> - bindings::{qdev_prop_bool, qdev_prop_chr},
> - prelude::*,
> - vmstate::VMStateDescription,
> - vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct,
> vmstate_subsections, vmstate_unused,
> - zeroable::Zeroable,
> + prelude::*, vmstate::VMStateDescription, vmstate_clock,
> vmstate_fields, vmstate_of,
> + vmstate_struct, vmstate_subsections, vmstate_unused,
> zeroable::Zeroable,
> };
>
I would also merge the files at this point, but no hurry.
+#[derive(Debug)]
> +struct DeviceProperty {
> + name: Option<syn::LitCStr>,
> + qdev_prop: Option<syn::Path>,
> + assert_type: Option<proc_macro2::TokenStream>,
> + bitnr: Option<syn::Expr>,
> + defval: Option<syn::Expr>,
> +}
> +
> +impl Parse for DeviceProperty {
>
Can you look into using https://docs.rs/crate/attrs/latest for parsing?
(attrs doesn't support LitCStr btw)
+#[proc_macro_derive(DeviceProperties, attributes(property, bool_property))]
> +pub fn derive_device_properties(input: TokenStream) -> TokenStream {
>
Do you need to handle errors in the parsing of attributes?...
+ _other => unreachable!(),
>
... Yes, you do—there is code already in qemu_macros, used by
derive_object_or_error(), to get the fields of a struct with proper error
handling.
let prop_name = prop.name.as_ref().unwrap();
+ let field_name = field.ident.as_ref().unwrap();
+ let qdev_prop = prop.qdev_prop.as_ref().unwrap();
+ let bitnr = prop.bitnr.as_ref().unwrap_or(&zero);
+ let set_default = prop.defval.is_some();
+ let defval = prop.defval.as_ref().unwrap_or(&zero);+
> bitnr: #bitnr,
> +
You also need to use let...else here instead of unwrap(), since panicking
provides worse error messages.
set_default: #set_default,
> + defval: ::qemu_api::bindings::Property__bindgen_ty_1 { u:
> #defval as u64 },
>
If you like it, you can write this also as
#(bitnr: #bitnr,)?
#(set_default: true, defval: ...)?
and keep bitnr and defval as Options, since the None case is handled by the
default below.
Paolo
+ ..::qemu_api::zeroable::Zeroable::ZERO
> + }
> + });
> + }
> + let properties_expanded = quote_spanned! {span.into()=>
> + #(#assertions)*
> +
> + impl ::qemu_api::qdev::DevicePropertiesImpl for #name {
> + fn properties() -> &'static [::qemu_api::bindings::Property] {
> + static PROPERTIES: &'static
> [::qemu_api::bindings::Property] = &[#(#properties_expanded),*];
> +
> + PROPERTIES
> + }
> + }
> + };
> +
> + TokenStream::from(properties_expanded)
> +}
> +
> #[proc_macro_derive(Wrapper)]
> pub fn derive_opaque(input: TokenStream) -> TokenStream {
> let input = parse_macro_input!(input as DeriveInput);
> diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
> index
> 1279d7a58d50e1bf6c8d2e6f00d7229bbb19e003..2fd8b2750ffabcaa1065558d38a700e35fbc9136
> 100644
> --- a/rust/qemu-api/src/qdev.rs
> +++ b/rust/qemu-api/src/qdev.rs
> @@ -100,8 +100,19 @@ pub trait ResettablePhasesImpl {
> T::EXIT.unwrap()(unsafe { state.as_ref() }, typ);
> }
>
> +pub trait DevicePropertiesImpl {
> + /// An array providing the properties that the user can set on the
> + /// device. Not a `const` because referencing statics in constants
> + /// is unstable until Rust 1.83.0.
> + fn properties() -> &'static [Property] {
> + &[]
> + }
> +}
> +
> /// Trait providing the contents of [`DeviceClass`].
> -pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl +
> IsA<DeviceState> {
> +pub trait DeviceImpl:
> + ObjectImpl + ResettablePhasesImpl + DevicePropertiesImpl +
> IsA<DeviceState>
> +{
> /// _Realization_ is the second stage of device creation. It contains
> /// all operations that depend on device properties and can fail
> (note:
> /// this is not yet supported for Rust devices).
> @@ -110,13 +121,6 @@ pub trait DeviceImpl: ObjectImpl +
> ResettablePhasesImpl + IsA<DeviceState> {
> /// with the function pointed to by `REALIZE`.
> const REALIZE: Option<fn(&Self)> = None;
>
> - /// An array providing the properties that the user can set on the
> - /// device. Not a `const` because referencing statics in constants
> - /// is unstable until Rust 1.83.0.
> - fn properties() -> &'static [Property] {
> - &[]
> - }
> -
> /// A `VMStateDescription` providing the migration format for the
> device
> /// Not a `const` because referencing statics in constants is unstable
> /// until Rust 1.83.0.
> @@ -171,7 +175,7 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
> if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
> self.vmsd = vmsd;
> }
> - let prop = <T as DeviceImpl>::properties();
> + let prop = <T as DevicePropertiesImpl>::properties();
> if !prop.is_empty() {
> unsafe {
> bindings::device_class_set_props_n(self, prop.as_ptr(),
> prop.len());
> diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
> index
> a658a49fcfdda8fa4b9d139c10afb6ff3243790b..e8eadfd6e9add385ffc97de015b84aae825c18ee
> 100644
> --- a/rust/qemu-api/tests/tests.rs
> +++ b/rust/qemu-api/tests/tests.rs
> @@ -9,7 +9,7 @@
> cell::{self, BqlCell},
> declare_properties, define_property,
> prelude::*,
> - qdev::{DeviceImpl, DeviceState, Property, ResettablePhasesImpl},
> + qdev::{DeviceImpl, DevicePropertiesImpl, DeviceState, Property,
> ResettablePhasesImpl},
> qom::{ObjectImpl, ParentField},
> sysbus::SysBusDevice,
> vmstate::VMStateDescription,
> @@ -68,10 +68,13 @@ impl ObjectImpl for DummyState {
>
> impl ResettablePhasesImpl for DummyState {}
>
> -impl DeviceImpl for DummyState {
> +impl DevicePropertiesImpl for DummyState {
> fn properties() -> &'static [Property] {
> &DUMMY_PROPERTIES
> }
> +}
> +
> +impl DeviceImpl for DummyState {
> fn vmsd() -> Option<&'static VMStateDescription> {
> Some(&VMSTATE)
> }
> @@ -85,6 +88,8 @@ pub struct DummyChildState {
>
> qom_isa!(DummyChildState: Object, DeviceState, DummyState);
>
> +impl DevicePropertiesImpl for DummyChildState {}
> +
> pub struct DummyChildClass {
> parent_class: <DummyState as ObjectType>::Class,
> }
>
> ---
> base-commit: 2af4a82ab2cce3412ffc92cd4c96bd870e33bc8e
> change-id: 20250522-rust-qdev-properties-728e8f6a468e
>
> --
> γαῖα πυρί μιχθήτω
>
>
[-- Attachment #2: Type: text/html, Size: 14047 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro
2025-05-23 13:05 ` Paolo Bonzini
@ 2025-05-28 10:49 ` Manos Pitsidianakis
2025-05-28 11:38 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Manos Pitsidianakis @ 2025-05-28 10:49 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, qemu-rust, Alex Bennée, Zhao Liu
On Fri, May 23, 2025 at 4:06 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
>
> Il gio 22 mag 2025, 10:12 Manos Pitsidianakis <manos.pitsidianakis@linaro.org> ha scritto:
>>
>> This is unnecessary though, because once we have the
>> const_refs_to_static feature we can introduce a QdevProp trait that
>> returns a reference to a type's qdev_prop_* global variable. We cannot
>> do this now because for our minimum Rust version we cannot refer to
>> statics from const values.
>
>
> Why don't you already write it assuming const_refs_to_static? If needed we can add a hack to make the VALUE an enum, similar to what is already in vmstate.rs.
>
>> So, this patch is not for merging yet, I will wait until we upgrade the
>> Rust version and re-spin it with a proper trait-based implementation (and
>> also split it into individual steps/patches).
>
>
> It's not too large, overall.
>
>> #[repr(C)]
>> -#[derive(qemu_api_macros::Object)]
>> +#[derive(qemu_api_macros::Object, qemu_api_macros::DeviceProperties)]
>
>
> Is there more to be derived that is useful for Devices? Maybe the macro can be DeviceState or Device.
VMStateDescription and the realize callback, I think.
>
>> + #[property(name = c"chardev", qdev_prop = qemu_api::bindings::qdev_prop_chr)]
>
>
> Can you change the name to be a "normal" string and change it to a C literal in the macro?
That'd be neat, it should be possible to create a cstr literal token
and error out if the input str literal cannot be represented as a
cstr.
>
>> diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
>> index d328d846323f6080a9573053767e51481eb32941..83d70d7d82aac4a3252a0b4cb24af705b01d3635 100644
>> --- a/rust/hw/char/pl011/src/device_class.rs
>> +++ b/rust/hw/char/pl011/src/device_class.rs
>> @@ -8,11 +8,8 @@
>> };
>>
>> use qemu_api::{
>> - bindings::{qdev_prop_bool, qdev_prop_chr},
>> - prelude::*,
>> - vmstate::VMStateDescription,
>> - vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_unused,
>> - zeroable::Zeroable,
>> + prelude::*, vmstate::VMStateDescription, vmstate_clock, vmstate_fields, vmstate_of,
>> + vmstate_struct, vmstate_subsections, vmstate_unused, zeroable::Zeroable,
>> };
>
>
> I would also merge the files at this point, but no hurry.
>
>> +#[derive(Debug)]
>> +struct DeviceProperty {
>> + name: Option<syn::LitCStr>,
>> + qdev_prop: Option<syn::Path>,
>> + assert_type: Option<proc_macro2::TokenStream>,
>> + bitnr: Option<syn::Expr>,
>> + defval: Option<syn::Expr>,
>> +}
>> +
>> +impl Parse for DeviceProperty {
>
>
> Can you look into using https://docs.rs/crate/attrs/latest for parsing?
>
> (attrs doesn't support LitCStr btw)
I think we can do without it for now, even if this patch is not
cleaned up (for example it has unwraps instead of proper panic
messages or error handling) it does not end up very complex as far as
attribute parsing is concerned.
I'm fine with either approach though.
>
>> +#[proc_macro_derive(DeviceProperties, attributes(property, bool_property))]
>> +pub fn derive_device_properties(input: TokenStream) -> TokenStream {
>
>
> Do you need to handle errors in the parsing of attributes?...
>
>> + _other => unreachable!(),
>
This should be literally unreachable IIUC, because only property names
declared in the attributes part of `#[proc_macro_derive(...
attributes(__))]` would get accepted by the compiler and passed to the
derive macro.
>
> ... Yes, you do—there is code already in qemu_macros, used by derive_object_or_error(), to get the fields of a struct with proper error handling.
>
>> let prop_name = prop.name.as_ref().unwrap();
>>
>> + let field_name = field.ident.as_ref().unwrap();
>>
>> + let qdev_prop = prop.qdev_prop.as_ref().unwrap();
>>
>> + let bitnr = prop.bitnr.as_ref().unwrap_or(&zero);
>>
>> + let set_default = prop.defval.is_some();
>>
>> + let defval = prop.defval.as_ref().unwrap_or(&zero);+ bitnr: #bitnr,
>> +
>
>
> You also need to use let...else here instead of unwrap(), since panicking provides worse error messages.
>
>> set_default: #set_default,
>> + defval: ::qemu_api::bindings::Property__bindgen_ty_1 { u: #defval as u64 },
>
>
> If you like it, you can write this also as
>
> #(bitnr: #bitnr,)?
> #(set_default: true, defval: ...)?
>
> and keep bitnr and defval as Options, since the None case is handled by the default below.
>
> Paolo
Thanks I will take another look!
>
>
>> + ..::qemu_api::zeroable::Zeroable::ZERO
>> + }
>> + });
>> + }
>> + let properties_expanded = quote_spanned! {span.into()=>
>> + #(#assertions)*
>> +
>> + impl ::qemu_api::qdev::DevicePropertiesImpl for #name {
>> + fn properties() -> &'static [::qemu_api::bindings::Property] {
>> + static PROPERTIES: &'static [::qemu_api::bindings::Property] = &[#(#properties_expanded),*];
>> +
>> + PROPERTIES
>> + }
>> + }
>> + };
>> +
>> + TokenStream::from(properties_expanded)
>> +}
>> +
>> #[proc_macro_derive(Wrapper)]
>> pub fn derive_opaque(input: TokenStream) -> TokenStream {
>> let input = parse_macro_input!(input as DeriveInput);
>> diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
>> index 1279d7a58d50e1bf6c8d2e6f00d7229bbb19e003..2fd8b2750ffabcaa1065558d38a700e35fbc9136 100644
>> --- a/rust/qemu-api/src/qdev.rs
>> +++ b/rust/qemu-api/src/qdev.rs
>> @@ -100,8 +100,19 @@ pub trait ResettablePhasesImpl {
>> T::EXIT.unwrap()(unsafe { state.as_ref() }, typ);
>> }
>>
>> +pub trait DevicePropertiesImpl {
>> + /// An array providing the properties that the user can set on the
>> + /// device. Not a `const` because referencing statics in constants
>> + /// is unstable until Rust 1.83.0.
>> + fn properties() -> &'static [Property] {
>> + &[]
>> + }
>> +}
>> +
>> /// Trait providing the contents of [`DeviceClass`].
>> -pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
>> +pub trait DeviceImpl:
>> + ObjectImpl + ResettablePhasesImpl + DevicePropertiesImpl + IsA<DeviceState>
>> +{
>> /// _Realization_ is the second stage of device creation. It contains
>> /// all operations that depend on device properties and can fail (note:
>> /// this is not yet supported for Rust devices).
>> @@ -110,13 +121,6 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
>> /// with the function pointed to by `REALIZE`.
>> const REALIZE: Option<fn(&Self)> = None;
>>
>> - /// An array providing the properties that the user can set on the
>> - /// device. Not a `const` because referencing statics in constants
>> - /// is unstable until Rust 1.83.0.
>> - fn properties() -> &'static [Property] {
>> - &[]
>> - }
>> -
>> /// A `VMStateDescription` providing the migration format for the device
>> /// Not a `const` because referencing statics in constants is unstable
>> /// until Rust 1.83.0.
>> @@ -171,7 +175,7 @@ pub fn class_init<T: DeviceImpl>(&mut self) {
>> if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
>> self.vmsd = vmsd;
>> }
>> - let prop = <T as DeviceImpl>::properties();
>> + let prop = <T as DevicePropertiesImpl>::properties();
>> if !prop.is_empty() {
>> unsafe {
>> bindings::device_class_set_props_n(self, prop.as_ptr(), prop.len());
>> diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
>> index a658a49fcfdda8fa4b9d139c10afb6ff3243790b..e8eadfd6e9add385ffc97de015b84aae825c18ee 100644
>> --- a/rust/qemu-api/tests/tests.rs
>> +++ b/rust/qemu-api/tests/tests.rs
>> @@ -9,7 +9,7 @@
>> cell::{self, BqlCell},
>> declare_properties, define_property,
>> prelude::*,
>> - qdev::{DeviceImpl, DeviceState, Property, ResettablePhasesImpl},
>> + qdev::{DeviceImpl, DevicePropertiesImpl, DeviceState, Property, ResettablePhasesImpl},
>> qom::{ObjectImpl, ParentField},
>> sysbus::SysBusDevice,
>> vmstate::VMStateDescription,
>> @@ -68,10 +68,13 @@ impl ObjectImpl for DummyState {
>>
>> impl ResettablePhasesImpl for DummyState {}
>>
>> -impl DeviceImpl for DummyState {
>> +impl DevicePropertiesImpl for DummyState {
>> fn properties() -> &'static [Property] {
>> &DUMMY_PROPERTIES
>> }
>> +}
>> +
>> +impl DeviceImpl for DummyState {
>> fn vmsd() -> Option<&'static VMStateDescription> {
>> Some(&VMSTATE)
>> }
>> @@ -85,6 +88,8 @@ pub struct DummyChildState {
>>
>> qom_isa!(DummyChildState: Object, DeviceState, DummyState);
>>
>> +impl DevicePropertiesImpl for DummyChildState {}
>> +
>> pub struct DummyChildClass {
>> parent_class: <DummyState as ObjectType>::Class,
>> }
>>
>> ---
>> base-commit: 2af4a82ab2cce3412ffc92cd4c96bd870e33bc8e
>> change-id: 20250522-rust-qdev-properties-728e8f6a468e
>>
>> --
>> γαῖα πυρί μιχθήτω
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro
2025-05-28 10:49 ` Manos Pitsidianakis
@ 2025-05-28 11:38 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2025-05-28 11:38 UTC (permalink / raw)
To: Manos Pitsidianakis; +Cc: qemu-devel, qemu-rust, Alex Bennée, Zhao Liu
On 5/28/25 12:49, Manos Pitsidianakis wrote:
>> It's not too large, overall.
>>
>>> #[repr(C)]
>>> -#[derive(qemu_api_macros::Object)]
>>> +#[derive(qemu_api_macros::Object, qemu_api_macros::DeviceProperties)]
>>
>>
>> Is there more to be derived that is useful for Devices? Maybe the
>> macro can be DeviceState or Device.
>
> VMStateDescription and the realize callback, I think.
Ok, pick the name that you prefer.
>> Can you change the name to be a "normal" string and change it to a C literal in the macro?
>
> That'd be neat, it should be possible to create a cstr literal token
> and error out if the input str literal cannot be represented as a
> cstr.
Yes, shouldn't be hard. If it is hard, you could resurrect the c_str module
and use ::qemu_api::c_str!(#string).
>> I would also merge the files at this point, but no hurry.
>>
>>> +#[derive(Debug)]
>>> +struct DeviceProperty {
>>> + name: Option<syn::LitCStr>,
>>> + qdev_prop: Option<syn::Path>,
>>> + assert_type: Option<proc_macro2::TokenStream>,
>>> + bitnr: Option<syn::Expr>,
>>> + defval: Option<syn::Expr>,
>>> +}
>>> +
>>> +impl Parse for DeviceProperty {
>>
>> Can you look into using https://docs.rs/crate/attrs/latest for parsing?
>>
>> (attrs doesn't support LitCStr btw)
>
> I think we can do without it for now, even if this patch is not
> cleaned up (for example it has unwraps instead of proper panic
> messages or error handling) it does not end up very complex as far as
> attribute parsing is concerned.
>
> I'm fine with either approach though.
Ok, I'm fine with the manual parsing as well; just use Result<>
instead of panicking.
>>> +#[proc_macro_derive(DeviceProperties, attributes(property, bool_property))]
>>> +pub fn derive_device_properties(input: TokenStream) -> TokenStream {
>>
>>
>> Do you need to handle errors in the parsing of attributes?...
>>
>>> + _other => unreachable!(),
>
> This should be literally unreachable IIUC, because only property names
> declared in the attributes part of `#[proc_macro_derive(...
> attributes(__))]` would get accepted by the compiler and passed to the
> derive macro.
You're referring to
+ other => unreachable!("Got unexpected DeviceProperty attribute `{}`", other),
and indeed this one is good as is. The "_other => unreachable!()"
line above, instead, comes from matching on input.data and it can
be replaced with the existing function get_fields().
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-28 11:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-22 8:12 [PATCH WIP RFC] rust: add qdev DeviceProperties derive macro Manos Pitsidianakis
2025-05-22 11:17 ` Zhao Liu
2025-05-23 13:05 ` Paolo Bonzini
2025-05-28 10:49 ` Manos Pitsidianakis
2025-05-28 11:38 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).