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
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.
thanks
-- PMM
