[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] migration: Boost SaveStateEntry.instance_id to 64 bits
From: |
Peter Xu |
Subject: |
Re: [PATCH 1/2] migration: Boost SaveStateEntry.instance_id to 64 bits |
Date: |
Tue, 15 Oct 2019 18:23:11 +0800 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
On Tue, Oct 15, 2019 at 10:45:53AM +0200, Juan Quintela wrote:
> Peter Xu <address@hidden> wrote:
> > It was "int" and used as 32bits fields (see save_section_header()).
> > It's unsafe already because sizeof(int) could be 2 on i386, I think.
> > So at least uint32_t would suite more. While it also uses "-1" as a
> > placeholder of "we want to generate the instance ID automatically".
> > Hence a more proper value should be int64_t.
> >
> > This will start to be useful after next patch in which we can start to
> > convert a real uint32_t value as instance ID.
> >
> > Signed-off-by: Peter Xu <address@hidden>
>
> Hi
>
> Being more helpful, I think that it is better to just:
>
> * change instance_id to be an uint32_t (notice that for all architectures
> that we support, it is actually int32_t).
>
> * export calculate_new_instance_id() and adjust callers that use -1.
>
> or
>
> * export a new function that just use the calculate_new_instance_id()
>
> A fast search shows:
>
> 10 callers of vmstate_register() with -1
> 1 caller of vmstate_register_with_alias_id with -1 (but it is the one
> that sets all qdev devices).
> 1 caller of vmstate_register_with_alias_id in apic, where it can be -1.
> 1 caller of register_savevm_live() with -1 (spapr)
>
> And call it a day?
>
> What do you think, Juan.
Sure, I can switch instance_id to uint32_t and add a new flag to both
functions (register_savevm_live, vmstate_register_with_alias_id)
Regards,
--
Peter Xu