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

[Paolo Bonzini <pbonzini@redhat.com>]

On 2/12/25 16:29, Kevin Wolf wrote:
> Am 12.02.2025 um 11:01 hat Paolo Bonzini geschrieben:
>> 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.
> 
> There were more problems with splitting the qemu_api crate related to
> bindgen. Basically, you would want the system emulator bindings to
> contain only those things that aren't already part of the common
> bindings. But the system emulator headers will obviously include common
> headers, too, so this becomes tricky.
> 
> If you don't do this, you get two bindings for the same type, but the
> binding types won't be compatible with each other etc.

That might be a good reason to move the bindings to their own crate. 
Then you don't really care if the bindings crate has declarations for 
things that are only for system emulation, because they're just externs.

qemu_api is currently doing "impl Foo" directly on structs defined by 
bindgen, but that can/should be changed.  This way a PhantomPinned field 
can be added, they can be wrapped with UnsafeCell<>, etc. I need to 
understand better what Linux does[1] and document it.

Anyhow this is not a blocker, this patch is easily reverted.

> This approach of just building two separate libraries was a lot easier.
> Apart from the obvious inefficiency, I just hate the need for
> rust_dependency_map everywhere to make the library show up with the
> neutral 'qemu_api' name in both cases. Maybe there is a better approach
> where this could be defined for the library itself rather than for each
> user, but I couldn't find one. meson is still black magic to me.

Yeah that's ugly.  There's no way to define it for the library indeed.

I'd like to have split crates because for example we're now building the 
QOM and block layer code twice as well.  Ideally, I'd like to have 
crates roughly matching the C static_libraries, so for example utils, 
bindings, block, chardev, hw, etc.

Paolo

[1] https://rust-for-linux.github.io/docs/kernel/struct.Opaque.html