[Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Peter Maydell]

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.

Handle NULL 'needed' function pointers in subsections in the same
way that we do for top level VMStateDescription structures:
treat the subsection as always being needed.

This doesn't change behaviour for the current set of devices
in the tree, because all subsections declare a 'needed' function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
NB: last para in the commit message is only true once the
"Arm migration fixes for 3.0" patchset has been committed.
We could optionally drop some of the "use a dummy needed fn"
changes once this is in...

I thought I'd sent this out a few days back, but apparently not,
since it's not on-list...

 migration/vmstate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 6b9079bb51e..0bc240a3175 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -418,7 +418,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
 static const VMStateDescription *
 vmstate_get_subsection(const VMStateDescription **sub, char *idstr)
 {
-    while (sub && *sub && (*sub)->needed) {
+    while (sub && *sub) {
         if (strcmp(idstr, (*sub)->name) == 0) {
             return *sub;
         }
@@ -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;
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Juan Quintela]

Peter Maydell <peter.maydell@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.
>
> Handle NULL 'needed' function pointers in subsections in the same
> way that we do for top level VMStateDescription structures:
> treat the subsection as always being needed.
>
> This doesn't change behaviour for the current set of devices
> in the tree, because all subsections declare a 'needed' function.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> NB: last para in the commit message is only true once the
> "Arm migration fixes for 3.0" patchset has been committed.
> We could optionally drop some of the "use a dummy needed fn"
> changes once this is in...
>
> I thought I'd sent this out a few days back, but apparently not,
> since it's not on-list...
>
>  migration/vmstate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 6b9079bb51e..0bc240a3175 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -418,7 +418,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
>  static const VMStateDescription *
>  vmstate_get_subsection(const VMStateDescription **sub, char *idstr)
>  {
> -    while (sub && *sub && (*sub)->needed) {
> +    while (sub && *sub) {
>          if (strcmp(idstr, (*sub)->name) == 0) {
>              return *sub;
>          }

Ack to this one.  Code supposed that needed was alwasy !=0.  We don't
really care on the reception side.  Good spotted.

> @@ -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?

Later, Juan.

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Peter Maydell]

On 7 August 2018 at 14:17, Juan Quintela <quintela@redhat.com> wrote:
> Peter Maydell <peter.maydell@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

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Peter Maydell]

On 7 August 2018 at 14:21, Peter Maydell <peter.maydell@linaro.org> wrote:
>  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.

We should also sanity-check the subsection names at device
init, because there's a hidden requirement that they start
with the name of the parent VMSD, and if you get that wrong
then nothing complains except that trying to reload the
vm state silently doesn't work right.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 August 2018 at 14:17, Juan Quintela <quintela@redhat.com> wrote:
>> Peter Maydell <peter.maydell@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.

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Peter Maydell]

On 7 August 2018 at 15:32, Juan Quintela <quintela@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 7 August 2018 at 14:17, Juan Quintela <quintela@redhat.com> wrote:
>> 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.

I can see these plausible API choices:
 (1) .needed for a subsection VMSD has the same behaviour as
     .needed for a top-level VMSD: if not present, means "always"
 (2) .needed is mandatory for a subsection VMSD, and failure
     to provide it is a bug which we diagnose as early as
     possible

If I understand you correctly you're suggesting
 (3) .needed is not mandatory and if not present means
     "never transfer the subsection"
which I think would be confusing.

I could live with (2) but I would end up writing a lot
of .needed functions like

static bool needed_always(void *opaque)
{
    return true;
}

which seems a bit unnecessary. (I have another patchset
in the works which will have one of these, assuming this
patch doesn't go into master first.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 August 2018 at 15:32, Juan Quintela <quintela@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 7 August 2018 at 14:17, Juan Quintela <quintela@redhat.com> wrote:
>>> 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.
>
> I can see these plausible API choices:
>  (1) .needed for a subsection VMSD has the same behaviour as
>      .needed for a top-level VMSD: if not present, means "always"
>  (2) .needed is mandatory for a subsection VMSD, and failure
>      to provide it is a bug which we diagnose as early as
>      possible

I vote for (2)


> If I understand you correctly you're suggesting
>  (3) .needed is not mandatory and if not present means
>      "never transfer the subsection"
> which I think would be confusing.

No.  I am suggestion that not having "needed" is a bug.

> I could live with (2) but I would end up writing a lot
> of .needed functions like
>
> static bool needed_always(void *opaque)
> {
>     return true;
> }

vmstate_needed_always() can be exported.
>
> which seems a bit unnecessary. (I have another patchset
> in the works which will have one of these, assuming this
> patch doesn't go into master first.)

But what I wonder is _why_ we want subsections that are always there.
We have:

- normal sections
  int32_t foo
  int64_t bad;

- subsections (always optional)
  if needed(whatever)
      int16_t foo2
      int32_t bar2

So needed_always means that it is never optional, it is always sent.
Here we have two cases:

- migration from old qemu (subsction don't exist) to new qemu works
  then the subsection clearly is mandatory

- migration from old qemu to new qemu don't work, because there are
  missing bits:
    then there is not problem adding the fields to the toplevel section
    and increasing the version number, because we can't migrate from
  older versions.

I can't see how to can make both claims true at the same time.

Later, Juan.

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Peter Maydell]

On 7 August 2018 at 15:49, Juan Quintela <quintela@redhat.com> wrote:
> But what I wonder is _why_ we want subsections that are always there.
> We have:
>
> - normal sections
>   int32_t foo
>   int64_t bad;
>
> - subsections (always optional)
>   if needed(whatever)
>       int16_t foo2
>       int32_t bar2
>
> So needed_always means that it is never optional, it is always sent.
> Here we have two cases:
>
> - migration from old qemu (subsction don't exist) to new qemu works
>   then the subsection clearly is mandatory
>
> - migration from old qemu to new qemu don't work, because there are
>   missing bits:
>     then there is not problem adding the fields to the toplevel section
>     and increasing the version number, because we can't migrate from
>   older versions.
>
> I can't see how to can make both claims true at the same time.

The usual situation is:
 * we write a device, but it's missing some bit of functionality
   and so is missing some internal state
 * later, we add that functionality, and the internal state, and
   now we want to add that state to the migration struct
 * we would like to not break migration from the old QEMU to
   the new one in the (fairly common) case where actually the
   guest wasn't using that bit of the device and doesn't care
   about its state

We could do that by putting the fields into the existing
vmstate struct, and marking them as only being present in
newer versions. But using migration version numbers is
brittle, especially if sometimes a feature might be backported
to an earlier version, or if downstreams make changes.
So it's better to use a needed-always subsection, which will give
the desired behaviour:
 * new QEMU -> new QEMU: state is always migrated
 * old QEMU -> new QEMU: migration doesn't fail, and guest
   will work assuming it didn't care about this corner of the
   device's functionality (the device will end up with state
   as it was at reset, or possibly special-cased via
   pre_load/post_load hooks for the "section not present" case)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 August 2018 at 15:49, Juan Quintela <quintela@redhat.com> wrote:
>> But what I wonder is _why_ we want subsections that are always there.
>> We have:
>>
>> - normal sections
>>   int32_t foo
>>   int64_t bad;
>>
>> - subsections (always optional)
>>   if needed(whatever)
>>       int16_t foo2
>>       int32_t bar2
>>
>> So needed_always means that it is never optional, it is always sent.
>> Here we have two cases:
>>
>> - migration from old qemu (subsction don't exist) to new qemu works
>>   then the subsection clearly is mandatory
>>
>> - migration from old qemu to new qemu don't work, because there are
>>   missing bits:
>>     then there is not problem adding the fields to the toplevel section
>>     and increasing the version number, because we can't migrate from
>>   older versions.
>>
>> I can't see how to can make both claims true at the same time.
>
> The usual situation is:
>  * we write a device, but it's missing some bit of functionality
>    and so is missing some internal state
>  * later, we add that functionality, and the internal state, and
>    now we want to add that state to the migration struct
>  * we would like to not break migration from the old QEMU to
>    the new one in the (fairly common) case where actually the
>    guest wasn't using that bit of the device and doesn't care
>    about its state
>
> We could do that by putting the fields into the existing
> vmstate struct, and marking them as only being present in
> newer versions. But using migration version numbers is
> brittle, especially if sometimes a feature might be backported
> to an earlier version, or if downstreams make changes.
> So it's better to use a needed-always subsection, which will give
> the desired behaviour:
>  * new QEMU -> new QEMU: state is always migrated
>  * old QEMU -> new QEMU: migration doesn't fail, and guest
>    will work assuming it didn't care about this corner of the
>    device's functionality (the device will end up with state
>    as it was at reset, or possibly special-cased via
>    pre_load/post_load hooks for the "section not present" case)

we break by definiton new QEMU -M <old machine type> into old QEMU.

That is the whole point why I don't like that.  But I understand your
point.  We have to decide if we care about making:

QEMU-x.y -M board-x.y
QEMU-z.y -M board-x.y

working more or less.

Later, Juan.

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Peter Maydell]

On 7 August 2018 at 16:05, Juan Quintela <quintela@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> So it's better to use a needed-always subsection, which will give
>> the desired behaviour:
>>  * new QEMU -> new QEMU: state is always migrated
>>  * old QEMU -> new QEMU: migration doesn't fail, and guest
>>    will work assuming it didn't care about this corner of the
>>    device's functionality (the device will end up with state
>>    as it was at reset, or possibly special-cased via
>>    pre_load/post_load hooks for the "section not present" case)
>
> we break by definiton new QEMU -M <old machine type> into old QEMU.

Is that supposed to work? I always thought that we never
supported migration back to an older QEMU version like that.

In any case, for all the devices here the machine types are
not versioned.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 7 August 2018 at 16:05, Juan Quintela <quintela@redhat.com> wrote:
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >> So it's better to use a needed-always subsection, which will give
> >> the desired behaviour:
> >>  * new QEMU -> new QEMU: state is always migrated
> >>  * old QEMU -> new QEMU: migration doesn't fail, and guest
> >>    will work assuming it didn't care about this corner of the
> >>    device's functionality (the device will end up with state
> >>    as it was at reset, or possibly special-cased via
> >>    pre_load/post_load hooks for the "section not present" case)
> >
> > we break by definiton new QEMU -M <old machine type> into old QEMU.
> 
> Is that supposed to work? I always thought that we never
> supported migration back to an older QEMU version like that.

Downstream we do a lot of hard work to keep that working for versioned 
machine types;  using subsections makes this a lot easier and if it's
done right in the first place then it works upstream as well.

> In any case, for all the devices here the machine types are
> not versioned.

Right.

Dave

> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
D> On 7 August 2018 at 16:05, Juan Quintela <quintela@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>> So it's better to use a needed-always subsection, which will give
>>> the desired behaviour:
>>>  * new QEMU -> new QEMU: state is always migrated
>>>  * old QEMU -> new QEMU: migration doesn't fail, and guest
>>>    will work assuming it didn't care about this corner of the
>>>    device's functionality (the device will end up with state
>>>    as it was at reset, or possibly special-cased via
>>>    pre_load/post_load hooks for the "section not present" case)
>>
>> we break by definiton new QEMU -M <old machine type> into old QEMU.
>
> Is that supposed to work? I always thought that we never
> supported migration back to an older QEMU version like that.

It is a "best effort" case.  As David told, downstream we try very
hard.  For upstream we do when I found that, but not anywhere else.

> In any case, for all the devices here the machine types are
> not versioned.

This is part of the problem.  For some architectures, we don't even care
about migraiton.  For others, we care about migration but not
cross-version.  And yet in others we care very much (downstream
specially).

And it is not clear what devices support each of them.

Later, Juan.

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Peter Maydell]

On 8 August 2018 at 08:27, Juan Quintela <quintela@redhat.com> wrote:
>
> This is part of the problem.  For some architectures, we don't even care
> about migraiton.  For others, we care about migration but not
> cross-version.  And yet in others we care very much (downstream
> specially).

I think we should care about migration on all architectures
and devices, in the sense that we want savevm/loadvm to work.
This is a really useful debugging and user tool, and when
I'm reviewing devices it's the minimum bar I think new
devices should clear. You then get migration "for free" but
I don't particularly expect it to be used compared to
snapshot save/restore. (Of course some of our existing code
doesn't support this, and we don't have a good way of testing
so bugs creep in easily, but as a principle I think it's
good.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> Peter Maydell <peter.maydell@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.
> >
> > Handle NULL 'needed' function pointers in subsections in the same
> > way that we do for top level VMStateDescription structures:
> > treat the subsection as always being needed.
> >
> > This doesn't change behaviour for the current set of devices
> > in the tree, because all subsections declare a 'needed' function.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > NB: last para in the commit message is only true once the
> > "Arm migration fixes for 3.0" patchset has been committed.
> > We could optionally drop some of the "use a dummy needed fn"
> > changes once this is in...
> >
> > I thought I'd sent this out a few days back, but apparently not,
> > since it's not on-list...
> >
> >  migration/vmstate.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/migration/vmstate.c b/migration/vmstate.c
> > index 6b9079bb51e..0bc240a3175 100644
> > --- a/migration/vmstate.c
> > +++ b/migration/vmstate.c
> > @@ -418,7 +418,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> >  static const VMStateDescription *
> >  vmstate_get_subsection(const VMStateDescription **sub, char *idstr)
> >  {
> > -    while (sub && *sub && (*sub)->needed) {
> > +    while (sub && *sub) {
> >          if (strcmp(idstr, (*sub)->name) == 0) {
> >              return *sub;
> >          }
> 
> Ack to this one.  Code supposed that needed was alwasy !=0.  We don't
> really care on the reception side.  Good spotted.
> 
> > @@ -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?

I think people are probably preferring adding a subsection comparing to 
using the _V macros everywhere.  It's also a lot more robust.

Dave

> Later, Juan.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration: Correctly handle subsections with no 'needed' function

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@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.
> 
> Handle NULL 'needed' function pointers in subsections in the same
> way that we do for top level VMStateDescription structures:
> treat the subsection as always being needed.
> 
> This doesn't change behaviour for the current set of devices
> in the tree, because all subsections declare a 'needed' function.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
> NB: last para in the commit message is only true once the
> "Arm migration fixes for 3.0" patchset has been committed.
> We could optionally drop some of the "use a dummy needed fn"
> changes once this is in...
> 
> I thought I'd sent this out a few days back, but apparently not,
> since it's not on-list...
> 
>  migration/vmstate.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 6b9079bb51e..0bc240a3175 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -418,7 +418,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
>  static const VMStateDescription *
>  vmstate_get_subsection(const VMStateDescription **sub, char *idstr)
>  {
> -    while (sub && *sub && (*sub)->needed) {
> +    while (sub && *sub) {
>          if (strcmp(idstr, (*sub)->name) == 0) {
>              return *sub;
>          }
> @@ -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;
>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK