Re: [RFC PATCH v2 3/5] rust: add PL011 device model

[Manos Pitsidianakis]

On Fri, 14 Jun 2024 20:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On Fri, Jun 14, 2024 at 9:04 AM Manos Pitsidianakis
> <manos.pitsidianakis@linaro.org> wrote:
> > On Thu, 13 Jun 2024 23:57, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >On Thu, Jun 13, 2024 at 11:16 AM Daniel P. Berrangé <berrange@redhat.com> 
> > wrote:
> > >> I guess there's a balance to be had somewhere on the spectrum between doing
> > >> everything against the raw C binding, vs everything against a perfectly
> > >> idiomatic Rust API wrapping the C bniding. The latter might be the ideal,
> > >> but from a pragmmatic POV I doubt we want the barrier to entry to be that
> > >> high.
> > >
> > >Yes, I agree. I guess we could make things work step by step, even
> > >committing something that only focuses on the build system like
> > >Manos's work (I'll review it).
> > >
> > >I can try to look at the basic QOM interface.
> > >
> > >Manos, can you create a page on the wiki? Something like
> > >https://wiki.qemu.org/Features/Meson.
> >
> >
> > Certainly! Just to make sure I understood correctly, you mean a wiki
> > page describing how things work and tracking the progress?
> >
> > I added https://wiki.qemu.org/Features/Meson/Rust
>
> I moved it to https://wiki.qemu.org/Features/Rust/Meson :) and wrote
> https://wiki.qemu.org/Features/Rust/QOM. I got to the point where at
> least this compiles:
>
> qdev_define_type!(c"test-device", TestDevice);
> impl ObjectImpl for TestDevice {}
> impl DeviceImpl for TestDevice {}
>
> fn main() {
>    let d = TestDevice::new();
>    d.cold_reset();
> }
>
> Of course the code makes no sense but it's a start.

Let's not rush into making interfaces without the need for them arising 
first. It's easy to wander off into bikeshedding territory; case in 
point, there has been little discussion on the code of this RFC and much 
more focus on hypotheticals.

For what it's worth, in my opinion looking at glib-rs for inspiration is 
a bad idea, because that project has to support an immutable public 
interface (glib) while we do not.

There's more to discuss about this topic which I am open to continuing 
on IRC instead!

-- Manos Pitsidianakis
Emulation and Virtualization Engineer at Linaro Ltd

> One thing that would be very useful is to have an Error
> implementation. Looking at what Marc-André did for Error*
> (https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lureau@redhat.com/20210907121943.3498701-13-marcandre.lureau@redhat.com/),
> his precise implementation relies on his code to go back and forth
> between Rust representation, borrowed C object with Rust bindings and
> owned C object with Rust bindings. But I think we can at least have
> something like this:
>
> // qemu::Error
> pub struct Error {
>    msg: String,
>    /// Appends the print string of the error to the msg if not None
>    cause: Option<Box<dyn std::error::Error>>,
>    location: Option<(String, u32)>
> }
>
> impl std::error::Error for Error { ... }
>
> impl Error {
>  ...
>  fn into_c_error(self) -> *const bindings::Error { ... }
> }
>
> // qemu::Result<T>
> type Result<T> = Result<T, Error>;
>
> which can be implemented without too much code. This way any "bool
> f(..., Error *)" function (example: realize :)) could be implemented
> as returning qemu::Result<()>.