On 04.09.2013, at 03:13, Alexey Kardashevskiy wrote:
> On 09/03/2013 07:22 PM, Andreas Färber wrote:
>> Am 03.09.2013 11:07, schrieb Alexey Kardashevskiy:
>>> On 09/03/2013 06:42 PM, Andreas Färber wrote:
>>>> Am 03.09.2013 09:31, schrieb Alexey Kardashevskiy:
>>>>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>>>>> index 12e1512..d1ffc7f 100644
>>>>> --- a/target-ppc/machine.c
>>>>> +++ b/target-ppc/machine.c
>> [...]
>>>>> +static const VMStateDescription vmstate_timebase = {
>>>>> + .name = "cpu/timebase",
>>>>> + .version_id = 1,
>>>>> + .minimum_version_id = 1,
>>>>> + .minimum_version_id_old = 1,
>>>>> + .pre_save = timebase_pre_save,
>>>>> + .post_load = timebase_post_load,
>>>>> + .fields = (VMStateField []) {
>>>>> + VMSTATE_UINT64(timebase, ppc_tb_t),
>>>>> + VMSTATE_INT64(tb_offset, ppc_tb_t),
>>>>> + VMSTATE_UINT64(time_of_the_day, ppc_tb_t),
>>>>> + VMSTATE_UINT32_EQUAL(tb_freq, ppc_tb_t),
>>>>> + VMSTATE_END_OF_LIST()
>>>>> + },
>>>>> +};
>>>>> +
>>>>> const VMStateDescription vmstate_ppc_cpu = {
>>>>> .name = "cpu",
>>>>> .version_id = 5,
>>>>> @@ -498,6 +538,10 @@ const VMStateDescription vmstate_ppc_cpu = {
>>>>> VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
>>>>> VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
>>>>> VMSTATE_UINT32_EQUAL(env.nb_BATs, PowerPCCPU),
>>>>> +
>>>>> + /* Time offset */
>>>>> + VMSTATE_STRUCT_POINTER(env.tb_env, PowerPCCPU,
>>>>> + vmstate_timebase, ppc_tb_t *),
>>>>> VMSTATE_END_OF_LIST()
>>>>> },
>>>>> .subsections = (VMStateSubsection []) {
>>>>
>>>> Breaks the migration format. ;) You need to bump version_id and use a
>>>> macro that accepts the version the field was added in as argument.
>>>
>>> Risking of being called ignorant, I'll still ask - is the patch below what
>>> you mean? I could not find VMSTATE_STRUCT_POINTER_V and I do not believe it
>>> is not there already.
>>
>> Usually the way we do it is to have VMSTATE_STRUCT_POINTER() call
>> VMSTATE_STRUCT_POINTER_V() and thus VMSTATE_STRUCT_POINTER_TEST() call a
>> new VMSTATE_STRUCT_POINTER_TEST_V(), to avoid code duplication of the
>> core array entry. CC'ing Juan. Please do the VMState preparation in a
>> separate patch.
>>
>> ppc usage looks fine.
>>
>>> btw why would it break? Just asking. Is it because the source may send what
>>> the destination cannot handle? Named fields should stop the migration the
>>> same way as version mismatch would have done.
>>
>> Nope, field names do not get transmitted, only the section names.
>> (Otherwise random code refactorings could break the migration format.)
>>
>>> Or the source won't sent what the destination expects and we do not want
>>> this destination guest to continue?
>>
>> There's an incoming stream of data from either live migration or a file,
>> and QEMU must decide whether it can read and how to interpret the raw
>> bytestream. It shouldn't just read random bytes into a new field when
>> they were written from a different field.
>>
>>> Once I was told that migration between different versions of QEMU is not
>>> supported - so what is the point of .version_id field at all?
>>
>> Not sure who told such a thing and in what context. On x86 we try to
>> avoid version_id bumps by adding subsections to allow migration in both
>> ways (including from newer to older) but for ppc, arm and all others we
>> do require that new fields are marked as such. Whether migration is
>> officially supported is a different matter from the VMState wire format.
>
>
> Why is the approach different for x86 and ppc here? I can convert
> VMSTATE_STRUCT_POINTER to a subsection, why should not I? Or ppc is not
> mature enough and therefore there is no need to keep compatibility? Thanks.
Just bump the version.
Alex
