Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 7 August 2018 at 14:17, Juan Quintela <quint...@redhat.com> wrote:
>> Peter Maydell <peter.mayd...@linaro.org> wrote:
>>> Currently the vmstate subsection handling code treats a subsection
>>> with no 'needed' function pointer as if it were the subsection
>>> list terminator, so the subsection is never transferred and nor
>>> is any subsection following it in the list.
>
>>> @@ -486,8 +486,8 @@ static int vmstate_subsection_save(QEMUFile *f, const
>>> VMStateDescription *vmsd,
>>> int ret = 0;
>>>
>>> trace_vmstate_subsection_save_top(vmsd->name);
>>> - while (sub && *sub && (*sub)->needed) {
>>> - if ((*sub)->needed(opaque)) {
>>> + while (sub && *sub) {
>>> + if (vmstate_save_needed(*sub, opaque)) {
>>> const VMStateDescription *vmsdsub = *sub;
>>> uint8_t len;
>>
>>
>> I am not so sure about this one. Why are we having a subsection without
>> a ->needed function? I don't know why that it is useful for. if we
>> don't have a needed function, then it is just better to just add that
>> "subsection" to the normal section, increase the version number and live
>> with that, no?
>
> No, because then you can't migrate from an older QEMU without
> the subsection to one with it. AIUI version-number bumps are
> deprecated if you care about migration compatibility, because
> they don't work if you're potentially going to be backporting
> changes to older versions.
>
> Also, what prompted me to write this patch was that for 3.0
> I had to fix several bugs where subsections had been written
> with no needed function and the migration code was (a)
> silently ignoring the subsection and (b) silently ignoring
> any subsection following. If you want to make it a bug to
b) is clearly a bug.
a) ... I am not sure that this should be correct.
> not have a needed function, you need to have code that checks
> for this in device init and aborts, so that device authors
> don't fall into this trap.
Here you have a good point.
Later, Juan.
