[v1] virtio,pci,pc: features, fixes, tests

[PULL 28/97] rust: bindings: allow any number of params

Michael S. Tsirkin posted 97 patches 5 months ago

Diff against v1
Download mbox

Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Peter Maydell <peter.maydell@linaro.org>, Beniamino Galvani <b.galvani@gmail.com>, Strahinja Jankovic <strahinja.p.jankovic@gmail.com>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Shannon Zhao <shannon.zhaosl@gmail.com>, John Snow <jsnow@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, Stefano Garzarella <sgarzare@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, Helge Deller <deller@gmx.de>, Gerd Hoffmann <kraxel@redhat.com>, Jason Wang <jasowang@redhat.com>, Yi Liu <yi.l.liu@intel.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, Song Gao <gaosong@loongson.cn>, Bibo Mao <maobibo@loongson.cn>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Pavel Pisa <pisa@cmp.felk.cvut.cz>, Francisco Iglesias <francisco.iglesias@amd.com>, Vikram Garhwal <vikram.garhwal@bytedance.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Bernhard Beschow <shentey@gmail.com>, Yoshinori Sato <yoshinori.sato@nifty.com>, Magnus Damm <magnus.damm@gmail.com>, Sunil V L <sunilvl@ventanamicro.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Alexander Graf <agraf@csgraf.de>, Phil Dennis-Jordan <phil@philjordan.eu>, "Alex Bennée" <alex.bennee@linaro.org>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Peter Xu <peterx@redhat.com>, Riku Voipio <riku.voipio@iki.fi>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Huacai Chen <chenhuacai@kernel.org>, Aleksandar Rikalo <arikalo@gmail.com>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>

Expand all Fold all

[PULL 28/97] rust: bindings: allow any number of params

Posted by Michael S. Tsirkin 5 months ago

We are going to be adding more parameters, and this makes
rust unhappy:
    Functions with lots of parameters are considered bad style and reduce
    readability (“what does the 5th parameter mean?”). Consider grouping
    some parameters into a new type.

Specifically:

error: this function has too many arguments (8/7)
    --> /builds/mstredhat/qemu/build/rust/qemu-api/rust-qemu-api-tests.p/structured/bindings.inc.rs:3840:5
     |
3840 | /     pub fn new_bitfield_1(
3841 | |         secure: std::os::raw::c_uint,
3842 | |         space: std::os::raw::c_uint,
3843 | |         user: std::os::raw::c_uint,
...    |
3848 | |         address_type: std::os::raw::c_uint,
3849 | |     ) -> __BindgenBitfieldUnit<[u8; 4usize]> {
     | |____________________________________________^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
     = note: `-D clippy::too-many-arguments` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`

I didn't want to disable this globally, so I just shut it off for this
file.

Message-Id: <a4c65fb2b735740bda2874c86de31d29a5ae24d2.1752530758.git.mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 rust/qemu-api/src/bindings.rs | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
index 057de4b646..b4692f9b4b 100644
--- a/rust/qemu-api/src/bindings.rs
+++ b/rust/qemu-api/src/bindings.rs
@@ -18,11 +18,15 @@
 
 //! `bindgen`-generated declarations.
 
-#[cfg(MESON)]
-include!("bindings.inc.rs");
+#[allow(clippy::too_many_arguments)]
+mod gen {
+    #[cfg(MESON)]
+    include!("bindings.inc.rs");
 
-#[cfg(not(MESON))]
-include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
+    #[cfg(not(MESON))]
+    include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
+}
+pub use gen::*;
 
 // SAFETY: these are implemented in C; the bindings need to assert that the
 // BQL is taken, either directly or via `BqlCell` and `BqlRefCell`.
-- 
MST

Re: [PULL 28/97] rust: bindings: allow any number of params

Posted by Manos Pitsidianakis 5 months ago

On Tue, Jul 15, 2025 at 2:07 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> We are going to be adding more parameters, and this makes
> rust unhappy:
>     Functions with lots of parameters are considered bad style and reduce
>     readability (“what does the 5th parameter mean?”). Consider grouping
>     some parameters into a new type.
>
> Specifically:
>
> error: this function has too many arguments (8/7)
>     --> /builds/mstredhat/qemu/build/rust/qemu-api/rust-qemu-api-tests.p/structured/bindings.inc.rs:3840:5
>      |
> 3840 | /     pub fn new_bitfield_1(
> 3841 | |         secure: std::os::raw::c_uint,
> 3842 | |         space: std::os::raw::c_uint,
> 3843 | |         user: std::os::raw::c_uint,
> ...    |
> 3848 | |         address_type: std::os::raw::c_uint,
> 3849 | |     ) -> __BindgenBitfieldUnit<[u8; 4usize]> {
>      | |____________________________________________^
>      |
>      = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
>      = note: `-D clippy::too-many-arguments` implied by `-D warnings`
>      = help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`
>
> I didn't want to disable this globally, so I just shut it off for this
> file.
>
> Message-Id: <a4c65fb2b735740bda2874c86de31d29a5ae24d2.1752530758.git.mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  rust/qemu-api/src/bindings.rs | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
> index 057de4b646..b4692f9b4b 100644
> --- a/rust/qemu-api/src/bindings.rs
> +++ b/rust/qemu-api/src/bindings.rs
> @@ -18,11 +18,15 @@
>
>  //! `bindgen`-generated declarations.
>
> -#[cfg(MESON)]
> -include!("bindings.inc.rs");
> +#[allow(clippy::too_many_arguments)]
> +mod gen {
> +    #[cfg(MESON)]
> +    include!("bindings.inc.rs");
>
> -#[cfg(not(MESON))]
> -include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
> +    #[cfg(not(MESON))]
> +    include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
> +}
> +pub use gen::*;
>
>  // SAFETY: these are implemented in C; the bindings need to assert that the
>  // BQL is taken, either directly or via `BqlCell` and `BqlRefCell`.
> --
> MST
>

Hi Michael,

This patch does not seem to have been reviewed.

The clippy allows are in the top of the file, not above the
`include!`. This should be a one line change and the `mod gen` wrap is
unnecessary.
-- 
Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

Re: [PULL 28/97] rust: bindings: allow any number of params

Posted by Michael S. Tsirkin 5 months ago

On Tue, Jul 15, 2025 at 02:30:40AM +0300, Manos Pitsidianakis wrote:
> On Tue, Jul 15, 2025 at 2:07 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > We are going to be adding more parameters, and this makes
> > rust unhappy:
> >     Functions with lots of parameters are considered bad style and reduce
> >     readability (“what does the 5th parameter mean?”). Consider grouping
> >     some parameters into a new type.
> >
> > Specifically:
> >
> > error: this function has too many arguments (8/7)
> >     --> /builds/mstredhat/qemu/build/rust/qemu-api/rust-qemu-api-tests.p/structured/bindings.inc.rs:3840:5
> >      |
> > 3840 | /     pub fn new_bitfield_1(
> > 3841 | |         secure: std::os::raw::c_uint,
> > 3842 | |         space: std::os::raw::c_uint,
> > 3843 | |         user: std::os::raw::c_uint,
> > ...    |
> > 3848 | |         address_type: std::os::raw::c_uint,
> > 3849 | |     ) -> __BindgenBitfieldUnit<[u8; 4usize]> {
> >      | |____________________________________________^
> >      |
> >      = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
> >      = note: `-D clippy::too-many-arguments` implied by `-D warnings`
> >      = help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`
> >
> > I didn't want to disable this globally, so I just shut it off for this
> > file.
> >
> > Message-Id: <a4c65fb2b735740bda2874c86de31d29a5ae24d2.1752530758.git.mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  rust/qemu-api/src/bindings.rs | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
> > index 057de4b646..b4692f9b4b 100644
> > --- a/rust/qemu-api/src/bindings.rs
> > +++ b/rust/qemu-api/src/bindings.rs
> > @@ -18,11 +18,15 @@
> >
> >  //! `bindgen`-generated declarations.
> >
> > -#[cfg(MESON)]
> > -include!("bindings.inc.rs");
> > +#[allow(clippy::too_many_arguments)]
> > +mod gen {
> > +    #[cfg(MESON)]
> > +    include!("bindings.inc.rs");
> >
> > -#[cfg(not(MESON))]
> > -include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
> > +    #[cfg(not(MESON))]
> > +    include!(concat!(env!("OUT_DIR"), "/bindings.inc.rs"));
> > +}
> > +pub use gen::*;
> >
> >  // SAFETY: these are implemented in C; the bindings need to assert that the
> >  // BQL is taken, either directly or via `BqlCell` and `BqlRefCell`.
> > --
> > MST
> >
> 
> Hi Michael,
> 
> This patch does not seem to have been reviewed.
> 
> The clippy allows are in the top of the file, not above the
> `include!`. This should be a one line change and the `mod gen` wrap is
> unnecessary.
> -- 
> Manos Pitsidianakis
> Emulation and Virtualization Engineer at Linaro Ltd

Yea sorry - I really wanted the API changes in and I tried to be
as conservative as possible.

I'll send a revert and the one-liner on top, is that
ok with you?

-- 
MST

[PULL v2 28/97] rust: bindings: allow any number of params

Posted by Michael S. Tsirkin 5 months ago

We are going to be adding more parameters, and this makes
rust unhappy:
    Functions with lots of parameters are considered bad style and reduce
    readability (“what does the 5th parameter mean?”). Consider grouping
    some parameters into a new type.

Specifically:

error: this function has too many arguments (8/7)
    --> /builds/mstredhat/qemu/build/rust/qemu-api/rust-qemu-api-tests.p/structured/bindings.inc.rs:3840:5
     |
3840 | /     pub fn new_bitfield_1(
3841 | |         secure: std::os::raw::c_uint,
3842 | |         space: std::os::raw::c_uint,
3843 | |         user: std::os::raw::c_uint,
...    |
3848 | |         address_type: std::os::raw::c_uint,
3849 | |     ) -> __BindgenBitfieldUnit<[u8; 4usize]> {
     | |____________________________________________^
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
     = note: `-D clippy::too-many-arguments` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <e41344bd22248b0883752ef7a7c459090a3d9cfc.1752560127.git.mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 rust/qemu-api/src/bindings.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/rust/qemu-api/src/bindings.rs b/rust/qemu-api/src/bindings.rs
index 057de4b646..c4f1f755ce 100644
--- a/rust/qemu-api/src/bindings.rs
+++ b/rust/qemu-api/src/bindings.rs
@@ -13,7 +13,8 @@
     clippy::missing_const_for_fn,
     clippy::ptr_offset_with_cast,
     clippy::useless_transmute,
-    clippy::missing_safety_doc
+    clippy::missing_safety_doc,
+    clippy::too_many_arguments
 )]
 
 //! `bindgen`-generated declarations.
-- 
MST