On Tue, Oct 15, 2019 at 10:45:53AM +0200, Juan Quintela wrote: > Peter Xu <pet...@redhat.com> 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 <pet...@redhat.com> > > 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
