qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Marc-André Lureau" <marcandre.lureau@redhat.com>
To: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, "Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-rust@nongnu.org,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: Re: [RFC 00/18] rust: split qemu-api
Date: Tue, 26 Aug 2025 19:22:45 +0400	[thread overview]
Message-ID: <CAMxuvaxLPuvGeO897jdWYHRW+3s33ki0-vhxtB7=JMsvAmyumA@mail.gmail.com> (raw)
In-Reply-To: <CAAjaMXYwEwpaybjEiA6tBCartTrzoAqsKNzHSfrs4f2wJx-wjA@mail.gmail.com>

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

Hi

On Tue, Aug 26, 2025 at 6:56 PM Manos Pitsidianakis <
manos.pitsidianakis@linaro.org> wrote:

> On Tue, Aug 26, 2025 at 5:44 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 8/26/25 16:04, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Hi,
> > >
> > > qemu-api is providing bindings for various internal libraries.
> Instead, the
> > > bindings requirements should match the various libraries and use the
> minimal set
> > > of dependencies.
> > >
> > > An initial Rust-only "common" crate is introduced, then "util" (for
> libqemuutil,
> > > without bql), "migration" (so it doesn't depend on bql), "bql", "qom"
> (arguably,
> > > bql shouldn't be required?), and "chardev", "system", "hwcore".
> Finally the
> > > qemu-api crates are renamed and repurposed.
> > >
> > > This involves a lot of code churn, so hopefully it can be reviewed and
> merged
> > > early and iterated upon :)
> >
> > The one comment that I would like to handle before merging, is that I'd
> > prefer to keep the preludes and, in fact, even add more exports to them
> > since they can now be chosen per-crate.  Ideally, many of the crates
> > you've crated would be accessed through many "use xyz::prelude::*"
> > statements.
>
> I kind of agree with this sentiment. What this series basically does
> is unwrapping most modules to standalone crates. Semantically, they
> make sense to be their own crates just like they made sense being
> their own modules before). But I'm not sure what developer benefit
>

Indeed, that's the point. The benefit is that you can compile and link only
what you need, just like how qemu code and libraries are organized. Modules
become quickly tangled between each other if you don't do it.


> this brings, it's like splitting Rust's std crate to separate crates
> (ergonomically speaking). Big crates like tokio split implementation
> to crates and then re-export them under tokio::* to ease compilation
> parallelism which we should definitely try to do as well. However in
> our case our Rust wrappers are very small. Let's do this if they ever
> end up growing unwieldy.
>

It will become difficult to manage. It's already hard to know which type
comes from which library/header. Better to do the split early imho. It was
nice to have a single crate in the early days, but we should prepare to
scale up now.


> > Also, if I understood correctly the split util/errno.rs can move to
> > common/.  While it has a dependency on libc, it doesn't need bindgen.
> >
> > There's a bunch of code duplication for the various bindings.rs and
> > build.rs files, which is not ideal but shouldn't grow much more than
> > this.  I wonder if, later, common code across build.rs could be written
> > just once by adding a new crate (e.g. "qemu_meson") to the workspace,
> > that can be used as a build-dependency.
> >
> > Paolo
> >
> > > Marc-André Lureau (18):
> > >    rust: remove unused global qemu "allocator"
> > >    rust: add workspace authors
> > >    rust: split Rust-only "common" crate
> > >    rust: split "util" crate
> > >    rust: move vmstate_clock!() to qdev module
> > >    rust: move VMState handling to QOM module
> > >    rust: move Cell vmstate impl
> > >    rust: split "migration" crate
> > >    rust: split "bql" crate
> > >    rust: split "qom" crate
> > >    rust: split "chardev" crate
> > >    rust: split "system" crate
> > >    rust: split "hwcore" crate
> > >    rust: rename qemu_api_macros -> qemu_macros
> > >    rust/hpet: drop now unneeded qemu_api dep
> > >    rust/pl011: drop dependency on qemu_api
> > >    rust: repurpose qemu_api -> tests
> > >    docs: update rust.rst
> > >
> > >   MAINTAINERS                                   |  12 +-
> > >   docs/devel/rust.rst                           |  51 +--
> > >   meson.build                                   |   4 -
> > >   rust/bql/wrapper.h                            |  27 ++
> > >   rust/chardev/wrapper.h                        |  28 ++
> > >   rust/hw/char/pl011/wrapper.h                  |  51 +++
> > >   rust/hw/core/wrapper.h                        |  32 ++
> > >   rust/{qemu-api => migration}/wrapper.h        |  20 --
> > >   rust/qom/wrapper.h                            |  27 ++
> > >   rust/system/wrapper.h                         |  29 ++
> > >   rust/util/wrapper.h                           |  32 ++
> > >   rust/Cargo.lock                               | 127 ++++++-
> > >   rust/Cargo.toml                               |  16 +-
> > >   rust/bits/Cargo.toml                          |   2 +-
> > >   rust/bits/meson.build                         |   2 +-
> > >   rust/bits/src/lib.rs                          |   4 +-
> > >   rust/{qemu-api => bql}/Cargo.toml             |  13 +-
> > >   rust/{qemu-api => bql}/build.rs               |   2 +-
> > >   rust/bql/meson.build                          |  52 +++
> > >   rust/bql/src/bindings.rs                      |  25 ++
> > >   rust/{qemu-api => bql}/src/cell.rs            | 333
> +++---------------
> > >   rust/bql/src/lib.rs                           |  29 ++
> > >   rust/chardev/Cargo.toml                       |  24 ++
> > >   rust/chardev/build.rs                         |  43 +++
> > >   rust/chardev/meson.build                      |  54 +++
> > >   rust/chardev/src/bindings.rs                  |  36 ++
> > >   rust/{qemu-api => chardev}/src/chardev.rs     |  35 +-
> > >   rust/chardev/src/lib.rs                       |   4 +
> > >   rust/common/Cargo.toml                        |  16 +
> > >   rust/common/meson.build                       |  32 ++
> > >   rust/{qemu-api => common}/src/assertions.rs   |  16 +-
> > >   rust/{qemu-api => common}/src/bitops.rs       |   1 -
> > >   rust/{qemu-api => common}/src/callbacks.rs    |  12 +-
> > >   rust/common/src/lib.rs                        |  17 +
> > >   rust/common/src/opaque.rs                     | 240 +++++++++++++
> > >   rust/{qemu-api => common}/src/uninit.rs       |   2 +-
> > >   rust/common/src/zeroable.rs                   |  18 +
> > >   rust/hw/char/pl011/Cargo.toml                 |  11 +-
> > >   rust/hw/char/pl011/build.rs                   |  43 +++
> > >   rust/hw/char/pl011/meson.build                |  39 +-
> > >   rust/hw/char/pl011/src/bindings.rs            |  27 ++
> > >   rust/hw/char/pl011/src/device.rs              |  49 +--
> > >   rust/hw/char/pl011/src/lib.rs                 |   1 +
> > >   rust/hw/char/pl011/src/registers.rs           |   4 +-
> > >   rust/hw/core/Cargo.toml                       |  26 ++
> > >   rust/hw/core/build.rs                         |  43 +++
> > >   rust/{qemu-api => hw/core}/meson.build        |  86 ++---
> > >   rust/hw/core/src/bindings.rs                  |  41 +++
> > >   rust/{qemu-api => hw/core}/src/irq.rs         |  18 +-
> > >   rust/hw/core/src/lib.rs                       |  12 +
> > >   rust/{qemu-api => hw/core}/src/qdev.rs        |  81 +++--
> > >   rust/{qemu-api => hw/core}/src/sysbus.rs      |  28 +-
> > >   rust/{qemu-api => hw/core}/tests/tests.rs     |  29 +-
> > >   rust/hw/timer/hpet/Cargo.toml                 |  10 +-
> > >   rust/hw/timer/hpet/meson.build                |  12 +-
> > >   rust/hw/timer/hpet/src/device.rs              |  56 ++-
> > >   rust/hw/timer/hpet/src/fw_cfg.rs              |   6 +-
> > >   rust/meson.build                              |  12 +-
> > >   rust/migration/Cargo.toml                     |  21 ++
> > >   rust/migration/build.rs                       |  43 +++
> > >   rust/migration/meson.build                    |  57 +++
> > >   rust/migration/src/bindings.rs                |  48 +++
> > >   rust/migration/src/lib.rs                     |   4 +
> > >   rust/{qemu-api => migration}/src/vmstate.rs   | 166 ++++-----
> > >   rust/qemu-api/.gitignore                      |   2 -
> > >   rust/qemu-api/README.md                       |  19 -
> > >   rust/qemu-api/src/lib.rs                      | 170 ---------
> > >   rust/qemu-api/src/prelude.rs                  |  31 --
> > >   rust/qemu-api/src/zeroable.rs                 |  37 --
> > >   .../Cargo.toml                                |   2 +-
> > >   .../meson.build                               |  10 +-
> > >   .../src/bits.rs                               |   0
> > >   .../src/lib.rs                                |  20 +-
> > >   .../src/tests.rs                              |   8 +-
> > >   rust/qom/Cargo.toml                           |  23 ++
> > >   rust/qom/build.rs                             |  43 +++
> > >   rust/qom/meson.build                          |  61 ++++
> > >   rust/qom/src/bindings.rs                      |  25 ++
> > >   rust/qom/src/lib.rs                           |   4 +
> > >   rust/{qemu-api => qom}/src/qom.rs             |  27 +-
> > >   rust/qom/tests/tests.rs                       |  47 +++
> > >   rust/system/Cargo.toml                        |  22 ++
> > >   rust/system/build.rs                          |  43 +++
> > >   rust/system/meson.build                       |  57 +++
> > >   rust/{qemu-api => system}/src/bindings.rs     |  33 +-
> > >   rust/system/src/lib.rs                        |   4 +
> > >   rust/{qemu-api => system}/src/memory.rs       |  20 +-
> > >   rust/tests/Cargo.toml                         |  30 ++
> > >   rust/tests/meson.build                        |  14 +
> > >   .../tests/vmstate_tests.rs                    |  18 +-
> > >   rust/util/Cargo.toml                          |  23 ++
> > >   rust/util/build.rs                            |  43 +++
> > >   rust/util/meson.build                         |  61 ++++
> > >   rust/util/src/bindings.rs                     |  25 ++
> > >   rust/{qemu-api => util}/src/errno.rs          |  11 +-
> > >   rust/{qemu-api => util}/src/error.rs          |   6 +-
> > >   rust/util/src/lib.rs                          |  10 +
> > >   rust/{qemu-api => util}/src/log.rs            |  12 +-
> > >   rust/{qemu-api => util}/src/module.rs         |   2 +-
> > >   rust/{qemu-api => util}/src/timer.rs          |  12 +-
> > >   100 files changed, 2372 insertions(+), 1044 deletions(-)
> > >   create mode 100644 rust/bql/wrapper.h
> > >   create mode 100644 rust/chardev/wrapper.h
> > >   create mode 100644 rust/hw/char/pl011/wrapper.h
> > >   create mode 100644 rust/hw/core/wrapper.h
> > >   rename rust/{qemu-api => migration}/wrapper.h (77%)
> > >   create mode 100644 rust/qom/wrapper.h
> > >   create mode 100644 rust/system/wrapper.h
> > >   create mode 100644 rust/util/wrapper.h
> > >   rename rust/{qemu-api => bql}/Cargo.toml (52%)
> > >   rename rust/{qemu-api => bql}/build.rs (96%)
> > >   create mode 100644 rust/bql/meson.build
> > >   create mode 100644 rust/bql/src/bindings.rs
> > >   rename rust/{qemu-api => bql}/src/cell.rs (70%)
> > >   create mode 100644 rust/bql/src/lib.rs
> > >   create mode 100644 rust/chardev/Cargo.toml
> > >   create mode 100644 rust/chardev/build.rs
> > >   create mode 100644 rust/chardev/meson.build
> > >   create mode 100644 rust/chardev/src/bindings.rs
> > >   rename rust/{qemu-api => chardev}/src/chardev.rs (91%)
> > >   create mode 100644 rust/chardev/src/lib.rs
> > >   create mode 100644 rust/common/Cargo.toml
> > >   create mode 100644 rust/common/meson.build
> > >   rename rust/{qemu-api => common}/src/assertions.rs (92%)
> > >   rename rust/{qemu-api => common}/src/bitops.rs (98%)
> > >   rename rust/{qemu-api => common}/src/callbacks.rs (97%)
> > >   create mode 100644 rust/common/src/lib.rs
> > >   create mode 100644 rust/common/src/opaque.rs
> > >   rename rust/{qemu-api => common}/src/uninit.rs (98%)
> > >   create mode 100644 rust/common/src/zeroable.rs
> > >   create mode 100644 rust/hw/char/pl011/build.rs
> > >   create mode 100644 rust/hw/char/pl011/src/bindings.rs
> > >   create mode 100644 rust/hw/core/Cargo.toml
> > >   create mode 100644 rust/hw/core/build.rs
> > >   rename rust/{qemu-api => hw/core}/meson.build (52%)
> > >   create mode 100644 rust/hw/core/src/bindings.rs
> > >   rename rust/{qemu-api => hw/core}/src/irq.rs (92%)
> > >   create mode 100644 rust/hw/core/src/lib.rs
> > >   rename rust/{qemu-api => hw/core}/src/qdev.rs (86%)
> > >   rename rust/{qemu-api => hw/core}/src/sysbus.rs (87%)
> > >   rename rust/{qemu-api => hw/core}/tests/tests.rs (88%)
> > >   create mode 100644 rust/migration/Cargo.toml
> > >   create mode 100644 rust/migration/build.rs
> > >   create mode 100644 rust/migration/meson.build
> > >   create mode 100644 rust/migration/src/bindings.rs
> > >   create mode 100644 rust/migration/src/lib.rs
> > >   rename rust/{qemu-api => migration}/src/vmstate.rs (80%)
> > >   delete mode 100644 rust/qemu-api/.gitignore
> > >   delete mode 100644 rust/qemu-api/README.md
> > >   delete mode 100644 rust/qemu-api/src/lib.rs
> > >   delete mode 100644 rust/qemu-api/src/prelude.rs
> > >   delete mode 100644 rust/qemu-api/src/zeroable.rs
> > >   rename rust/{qemu-api-macros => qemu-macros}/Cargo.toml (94%)
> > >   rename rust/{qemu-api-macros => qemu-macros}/meson.build (63%)
> > >   rename rust/{qemu-api-macros => qemu-macros}/src/bits.rs (100%)
> > >   rename rust/{qemu-api-macros => qemu-macros}/src/lib.rs (91%)
> > >   rename rust/{qemu-api-macros => qemu-macros}/src/tests.rs (93%)
> > >   create mode 100644 rust/qom/Cargo.toml
> > >   create mode 100644 rust/qom/build.rs
> > >   create mode 100644 rust/qom/meson.build
> > >   create mode 100644 rust/qom/src/bindings.rs
> > >   create mode 100644 rust/qom/src/lib.rs
> > >   rename rust/{qemu-api => qom}/src/qom.rs (98%)
> > >   create mode 100644 rust/qom/tests/tests.rs
> > >   create mode 100644 rust/system/Cargo.toml
> > >   create mode 100644 rust/system/build.rs
> > >   create mode 100644 rust/system/meson.build
> > >   rename rust/{qemu-api => system}/src/bindings.rs (56%)
> > >   create mode 100644 rust/system/src/lib.rs
> > >   rename rust/{qemu-api => system}/src/memory.rs (95%)
> > >   create mode 100644 rust/tests/Cargo.toml
> > >   create mode 100644 rust/tests/meson.build
> > >   rename rust/{qemu-api => tests}/tests/vmstate_tests.rs (96%)
> > >   create mode 100644 rust/util/Cargo.toml
> > >   create mode 100644 rust/util/build.rs
> > >   create mode 100644 rust/util/meson.build
> > >   create mode 100644 rust/util/src/bindings.rs
> > >   rename rust/{qemu-api => util}/src/errno.rs (98%)
> > >   rename rust/{qemu-api => util}/src/error.rs (98%)
> > >   create mode 100644 rust/util/src/lib.rs
> > >   rename rust/{qemu-api => util}/src/log.rs (93%)
> > >   rename rust/{qemu-api => util}/src/module.rs (97%)
> > >   rename rust/{qemu-api => util}/src/timer.rs (93%)
> > >
> >
>
>

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

  reply	other threads:[~2025-08-26 15:24 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-26 14:04 [RFC 00/18] rust: split qemu-api marcandre.lureau
2025-08-26 14:04 ` [RFC 01/18] rust: remove unused global qemu "allocator" marcandre.lureau
2025-08-26 14:04 ` [RFC 02/18] rust: add workspace authors marcandre.lureau
2025-08-26 14:04 ` [RFC 03/18] rust: split Rust-only "common" crate marcandre.lureau
2025-08-26 14:04 ` [RFC 04/18] rust: split "util" crate marcandre.lureau
2025-08-26 14:04 ` [RFC 05/18] rust: move vmstate_clock!() to qdev module marcandre.lureau
2025-08-26 14:04 ` [RFC 06/18] rust: move VMState handling to QOM module marcandre.lureau
2025-08-26 14:04 ` [RFC 07/18] rust: move Cell vmstate impl marcandre.lureau
2025-08-26 18:28   ` Paolo Bonzini
2025-08-26 19:06     ` Marc-André Lureau
2025-08-27  7:35       ` Paolo Bonzini
2025-08-26 14:04 ` [RFC 08/18] rust: split "migration" crate marcandre.lureau
2025-08-26 14:04 ` [RFC 09/18] rust: split "bql" crate marcandre.lureau
2025-08-26 14:04 ` [RFC 10/18] rust: split "qom" crate marcandre.lureau
2025-08-27  6:55   ` Zhao Liu
2025-08-27  8:57     ` Marc-André Lureau
2025-08-27  9:49       ` Paolo Bonzini
2025-08-26 14:04 ` [RFC 11/18] rust: split "chardev" crate marcandre.lureau
2025-08-26 14:04 ` [RFC 12/18] rust: split "system" crate marcandre.lureau
2025-08-26 14:04 ` [RFC 13/18] rust: split "hwcore" crate marcandre.lureau
2025-08-26 14:04 ` [RFC 14/18] rust: rename qemu_api_macros -> qemu_macros marcandre.lureau
2025-08-26 14:53   ` Paolo Bonzini
2025-08-26 20:30     ` Marc-André Lureau
2025-08-26 14:04 ` [RFC 15/18] rust/hpet: drop now unneeded qemu_api dep marcandre.lureau
2025-08-26 14:04 ` [RFC 16/18] rust/pl011: drop dependency on qemu_api marcandre.lureau
2025-08-26 14:04 ` [RFC 17/18] rust: repurpose qemu_api -> tests marcandre.lureau
2025-08-26 14:04 ` [RFC 18/18] docs: update rust.rst marcandre.lureau
2025-08-26 14:44 ` [RFC 00/18] rust: split qemu-api Paolo Bonzini
2025-08-26 14:55   ` Manos Pitsidianakis
2025-08-26 15:22     ` Marc-André Lureau [this message]
2025-08-26 15:33     ` Paolo Bonzini
2025-08-26 15:15   ` Marc-André Lureau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMxuvaxLPuvGeO897jdWYHRW+3s33ki0-vhxtB7=JMsvAmyumA@mail.gmail.com' \
    --to=marcandre.lureau@redhat.com \
    --cc=berrange@redhat.com \
    --cc=manos.pitsidianakis@linaro.org \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-rust@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).