Hi,

On Sat, 8 Mar 2025 at 04:18, Peter Xu <pet...@redhat.com> wrote:
> Please move all of them at the entry of postcopy_start().

* Okay.

> [1]
>
> > +int qemu_savevm_state_complete_multifd(QEMUFile *f)
> I still like the name I provided because it's more generic, above [1].

===
>  Maybe the easiest as of now is one more hook like
> qemu_savevm_state_complete_precopy_prepare(),
===

* ie. use  ->  'qemu_savevm_state_complete_precopy_prepare'  name?
Should it be  _postcopy_prepare?

>> +        if (strcmp(se->idstr, "ram")) {
>> +            continue;
>> +        }
>> +        save_section_header(f, se, QEMU_VM_SECTION_END);
>
> Maybe it should be SECTION_PART.

* Will try with SECTION_PART.

>  So we provide all iterator a chance to
> do something right before switching to postcopy but before VM stop.  RAM
> can register.

* ...all iterators a chance to do something? Above function only
processes "ram" entries -> strcmp(se->idstr, "ram").

* ...RAM can register?

> > +
> > +        ret = se->ops->save_live_complete_precopy(f, se->opaque);
>
> Maybe we don't want to run the whole iteration but only flush.  I think for
> simplicity we can make a new hook.

* ..new hook? I tried calling multifd_ram_flush_and_sync() in place of
->save_live_complete_precopy(),  but it crashes QEMU.

> > +        end_ts_each = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
> > +        trace_vmstate_downtime_save("iterable", se->idstr, se->instance_id,
> > +                                    end_ts_each - start_ts_each);
>
> We do not need to account
> If you do flush and sync, IMHO we can keep the threads there and remove
> this shutdown, as long as we are sure it'll be properly shutdown when
> cleanup. With the assertion in dest threads, I think it should be OK.

* Okay.

Thank you.
---
  - Prasad


Reply via email to