Re: [PATCH 02/10] rust: qom: add reference counting functionality

[Zhao Liu]

On Wed, Feb 05, 2025 at 10:10:01AM +0100, Paolo Bonzini wrote:
> Date: Wed, 5 Feb 2025 10:10:01 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [PATCH 02/10] rust: qom: add reference counting functionality
> 
> On 2/5/25 10:13, Zhao Liu wrote:
> > > > * The use of from():
> > > > 
> > > >                  let clk = bindings::qdev_init_clock_in(...)
> > > >                  Owned::from(&*clk)
> > > 
> > > In this case the C side wants to manage the reference that
> > > qdev_init_clock_in() returns; it is dropped in
> > > qdev_finalize_clocklist(). So Rust code needs to increase the
> > > refcount.
> > 
> > Pls forgive me for one more question about qdev_init_clock_in() on the C
> > side. :-)
> > 
> > qdev_init_clock_in() didn't unref `clk` after object_property_add_child(),
> > so it is intentional, to make the ref count of `clk` be 2:
> >   * 1 count is held by clocklist until qdev_finalize_clocklist().
> >   * another 1 is held by its parent via QOM Child<>.
> > 
> > Am I understanding it correctly?
> 
> Yes, that's more precise.  In Rust it will be 3, the two above plus the
> Owned<Clock>.

Thanks!

> Ah wait... qdev_finalize_clocklist() is only called _after_ the Rust struct
> is Drop::drop-ped, because device_finalize() is called after the subclass's
> instance_finalize.
> 
> So the result of qdev_init_clock_in(), strictly speaking, does not have to
> be an Owned<Clock>.  It can also be a &'device Clock; either is possible.
> Would you prefer that, or do you think it's enough to add a comment?
> 

I prefer the latter, i.e., keep current Owned<Clock> and add a comment
to mention something like "Although strictly speaking, it does not have
to be an Owned<Clock>. Owned<> is still worth favor to use to protect
Rust code from FFI. When unsure whether to use Owned<>, then use Owned<>."

-Zhao