qemu-rust.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Zhao Liu <zhao1.liu@intel.com>
Cc: qemu-devel <qemu-devel@nongnu.org>,
	qemu-rust@nongnu.org, Dapeng Mi <dapeng1.mi@intel.com>
Subject: Re: [PATCH 8/9] rust/hpet: Support migration
Date: Wed, 16 Apr 2025 16:40:54 +0200	[thread overview]
Message-ID: <71e1a026-e94a-4a5b-b3a2-e636fd597e5b@redhat.com> (raw)
In-Reply-To: <Z/+Ee4YlUBSVtArJ@intel.com>

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



Il mer 16 apr 2025, 12:00 Zhao Liu <zhao1.liu@intel.com> ha scritto:

     > So the problem is that, in a "&'a Foo<T>", T must also be "T: 'a".
     > One solution is for vmsd() to return an
     > Option<VMStateDescription<Self>>, and do
    Box::into_raw(Box::new(vmsd))
     > in the class_init method. Once we have const_refs_static, "fn vmsd()"
     > can become a const and the Box is not needed anymore.

    Thanks so much, that's a good idea!

    About `Box::into_raw(Box::new(vmsd))`, do you think it's necessary
    to use
    Box::leak(Box::new(*))? (though the Box<> isn't actively dropped during
    the class's existence)


It's the same; leak and into_raw only differ in the return type. You can 
use leak if you prefer.

     > I attach the conversion I did of the other devices and tests. I
    am not
     > sure if it's possible to avoid having a huge patch to do
    everything at
     > once (except HPET since that can be added separately).

    Thank you again! From my initial thoughts: Splitting is also possible,
    but it requires first renaming VMStateDescription<T> to
    VMStateDescriptionWrapper<T>, then replacing it in pl011 and test (and
    hpet) one by one, and finally renaming it back to VMStateDescription<T>.
    If you prefer this approach, I can help you split your patch below.


Or maybe first you keepvmsd() return 
Option<&bindings::VMStateDescription> and stick a get_vmsd_ref() in "fn 
vmsd()", then in a final patch you make it return 
Option<VMStateDescription<T>> and introduce the Box::leak(Box::new(...)) 
trick.
> > + vmstate_struct!(HPETState, timers[0 .. num_timers], 
> &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, 
> HPETState::validate_num_timers).with_version_id(0),
>
> And it seems like you don't oppose the hack in patch 1? ;-)
It's not bad, but I haven't looked at why it's needed yet (i.e. why a 
constant function would be evaluating a destructor).

Paolo

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

  reply	other threads:[~2025-04-16 14:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-14 14:49 [PATCH 0/9] rust/hpet: Initial support for migration Zhao Liu
2025-04-14 14:49 ` [PATCH 1/9] rust/vmstate: Support field_exists check in vmstate_struct macro Zhao Liu
2025-04-16 14:54   ` Paolo Bonzini
2025-04-14 14:49 ` [PATCH 2/9] rust/vmstate: Support varray's num field wrapped in BqlCell Zhao Liu
2025-04-15 10:54   ` Paolo Bonzini
2025-04-16  9:43     ` Zhao Liu
2025-04-16 12:34       ` Zhao Liu
2025-04-16 14:24         ` Paolo Bonzini
2025-05-16  8:25     ` Zhao Liu
2025-04-14 14:49 ` [PATCH 3/9] rust/vmstate_test: Test varray with " Zhao Liu
2025-04-14 14:49 ` [PATCH 4/9] rust/vmstate_test: Fix typo in test_vmstate_macro_array_of_pointer_wrapped() Zhao Liu
2025-04-14 14:49 ` [PATCH 5/9] rust/timer: Define NANOSECONDS_PER_SECOND binding as u64 Zhao Liu
2025-04-14 14:49 ` [PATCH 6/9] rust/hpet: convert num_timers to u8 type Zhao Liu
2025-04-14 14:49 ` [PATCH 7/9] rust/hpet: convert HPETTimer index " Zhao Liu
2025-04-14 14:49 ` [PATCH 8/9] rust/hpet: Support migration Zhao Liu
2025-04-15 12:01   ` Zhao Liu
2025-04-15 14:21     ` Paolo Bonzini
2025-04-15 17:43       ` Paolo Bonzini
2025-04-16 10:20         ` Zhao Liu
2025-04-16 14:40           ` Paolo Bonzini [this message]
2025-04-16 10:33       ` Zhao Liu
2025-04-14 14:49 ` [PATCH 9/9] rust/hpet: Fix a clippy error Zhao Liu
2025-04-15 10:24 ` [PATCH 0/9] rust/hpet: Initial support for migration Paolo Bonzini

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=71e1a026-e94a-4a5b-b3a2-e636fd597e5b@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=dapeng1.mi@intel.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-rust@nongnu.org \
    --cc=zhao1.liu@intel.com \
    /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).