[Qemu-devel] [PATCH] vmstate: fix vmstate_subsection_load

[Qemu-devel] [PATCH] vmstate: fix vmstate_subsection_load

[TeLeMan]

 If the new version adds the new subsection for some vmstate, the old
version will load the new version's vmstate unsuccessfully. So we have
to ignore the unrecognized subsections.

Signed-off-by: TeLeMan <geleman@gmail.com>
---
 savevm.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/savevm.c b/savevm.c
index 9a8328d..3e1aa73 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1581,12 +1581,11 @@ static int vmstate_subsection_load(QEMUFile
*f, const VMStateDescription *vmsd,
         version_id = qemu_get_be32(f);

         sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
-        if (sub_vmsd == NULL) {
-            return -ENOENT;
-        }
-        ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
-        if (ret) {
-            return ret;
+        if (sub_vmsd) {
+            ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
+            if (ret) {
+                return ret;
+            }
         }
     }
     return 0;
-- 
1.6.5.1.1367.gcd48

--
SUN OF A BEACH

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[Juan Quintela]

TeLeMan <geleman@gmail.com> wrote:
>  If the new version adds the new subsection for some vmstate, the old
> version will load the new version's vmstate unsuccessfully. So we have
> to ignore the unrecognized subsections.

No.  That was the whole point of subsections.  If one subsection is
sent, target machine has to understand it.  If it don't understand it,
it fails.

If subsection is not needed, it is the responsability of the source to
not send it.

This was one of the design requirements.  Subsections are optional but
it is the source which decides which ones to send/not to send.  The
target has to understand everything that it gets or fail the migration.

Later, Juan.

> Signed-off-by: TeLeMan <geleman@gmail.com>
> ---
>  savevm.c |   11 +++++------
>  1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 9a8328d..3e1aa73 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1581,12 +1581,11 @@ static int vmstate_subsection_load(QEMUFile
> *f, const VMStateDescription *vmsd,
>          version_id = qemu_get_be32(f);
>
>          sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
> -        if (sub_vmsd == NULL) {
> -            return -ENOENT;
> -        }
> -        ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
> -        if (ret) {
> -            return ret;
> +        if (sub_vmsd) {
> +            ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
> +            if (ret) {
> +                return ret;
> +            }
>          }
>      }
>      return 0;

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[TeLeMan]

On Wed, Jul 28, 2010 at 18:43, Juan Quintela <quintela@redhat.com> wrote:
> TeLeMan <geleman@gmail.com> wrote:
>>  If the new version adds the new subsection for some vmstate, the old
>> version will load the new version's vmstate unsuccessfully. So we have
>> to ignore the unrecognized subsections.
>
> No.  That was the whole point of subsections.  If one subsection is
> sent, target machine has to understand it.  If it don't understand it,
> it fails.
>
> If subsection is not needed, it is the responsability of the source to
> not send it.
>
> This was one of the design requirements.  Subsections are optional but
> it is the source which decides which ones to send/not to send.  The
> target has to understand everything that it gets or fail the migration.
>

If the target must understand everything, the vmstate's version will
be useless because the old version target maybe cannot load the new
version target's vmstate.

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[Juan Quintela]

TeLeMan <geleman@gmail.com> wrote:
> On Wed, Jul 28, 2010 at 18:43, Juan Quintela <quintela@redhat.com> wrote:
>> TeLeMan <geleman@gmail.com> wrote:
>>>  If the new version adds the new subsection for some vmstate, the old
>>> version will load the new version's vmstate unsuccessfully. So we have
>>> to ignore the unrecognized subsections.
>>
>> No.  That was the whole point of subsections.  If one subsection is
>> sent, target machine has to understand it.  If it don't understand it,
>> it fails.
>>
>> If subsection is not needed, it is the responsability of the source to
>> not send it.
>>
>> This was one of the design requirements.  Subsections are optional but
>> it is the source which decides which ones to send/not to send.  The
>> target has to understand everything that it gets or fail the migration.
>>
>
> If the target must understand everything, the vmstate's version will
> be useless because the old version target maybe cannot load the new
> version target's vmstate.

That is the whole point.  See the ide example that I also sent.

We have an old version of qemu, that don't understand pio ide fields.
We get a new qemu version that has pio ide fields in the state.

notice that migration in the middle of a pio operation is a very rare
event.  for instance, if you are using linux as a guest, pio is only
used during boot, and not so many times (around 30000 calls during
boot).

Without subsections, you will be unable to migrate from new qemu to old
qemu, because old qemu don't understand the pio fields.

With subsections, if we are in the middle of a pio operation, we sent
pio subsection and migration to old qemu will fail.

But if we aren't in the middle of a pio operation, pio information will
not be sent and migration will succeed.

This is special important for the stable branch, where we are supposed
to be backwards compatible, but sometimes we find a bug, and we really
have to change the savevm format.

This allows us that after creating the subsection, we can still migrate
to the old version the majority of the time.

Notice that this bugfixes are normally rare cases, because if it were
the normal case, we would have already detected it before the release.

If we sent the subsection, it means that target needs to understand it,
or state will be broken (one way or another).

I hope this hepls to understand how subsections are supposed to work.

Thanks for the comments, Juan.

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[TeLeMan]

On Wed, Jul 28, 2010 at 19:51, Juan Quintela <quintela@redhat.com> wrote:
> TeLeMan <geleman@gmail.com> wrote:
>> On Wed, Jul 28, 2010 at 18:43, Juan Quintela <quintela@redhat.com> wrote:
>>> TeLeMan <geleman@gmail.com> wrote:
>>>>  If the new version adds the new subsection for some vmstate, the old
>>>> version will load the new version's vmstate unsuccessfully. So we have
>>>> to ignore the unrecognized subsections.
>>>
>>> No.  That was the whole point of subsections.  If one subsection is
>>> sent, target machine has to understand it.  If it don't understand it,
>>> it fails.
>>>
>>> If subsection is not needed, it is the responsability of the source to
>>> not send it.
>>>
>>> This was one of the design requirements.  Subsections are optional but
>>> it is the source which decides which ones to send/not to send.  The
>>> target has to understand everything that it gets or fail the migration.
>>>
>>
>> If the target must understand everything, the vmstate's version will
>> be useless because the old version target maybe cannot load the new
>> version target's vmstate.
>
> That is the whole point.  See the ide example that I also sent.
>
> We have an old version of qemu, that don't understand pio ide fields.
> We get a new qemu version that has pio ide fields in the state.
>
> notice that migration in the middle of a pio operation is a very rare
> event.  for instance, if you are using linux as a guest, pio is only
> used during boot, and not so many times (around 30000 calls during
> boot).
>
> Without subsections, you will be unable to migrate from new qemu to old
> qemu, because old qemu don't understand the pio fields.
>
> With subsections, if we are in the middle of a pio operation, we sent
> pio subsection and migration to old qemu will fail.
>
> But if we aren't in the middle of a pio operation, pio information will
> not be sent and migration will succeed.
>
> This is special important for the stable branch, where we are supposed
> to be backwards compatible, but sometimes we find a bug, and we really
> have to change the savevm format.
>
> This allows us that after creating the subsection, we can still migrate
> to the old version the majority of the time.
>
> Notice that this bugfixes are normally rare cases, because if it were
> the normal case, we would have already detected it before the release.
>
> If we sent the subsection, it means that target needs to understand it,
> or state will be broken (one way or another).
>
> I hope this hepls to understand how subsections are supposed to work.
>
> Thanks for the comments, Juan.
>
I see, thanks a lot. But I still hope to have the similar subsection
that can be ignored simply.

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[Juan Quintela]

TeLeMan <geleman@gmail.com> wrote:
> On Wed, Jul 28, 2010 at 19:51, Juan Quintela <quintela@redhat.com> wrote:
>> I hope this hepls to understand how subsections are supposed to work.
>>
>> Thanks for the comments, Juan.
>>
> I see, thanks a lot. But I still hope to have the similar subsection
> that can be ignored simply.

Then it is better not to be sent in the 1st place.

Do you have any example of why you want to do?  When I dessigned
subsections, I looked at all the changes that we had from 0.11 to 0.12,
subsections can handle all of them.  Just curious about what you need.\

Notice that ignoring subsections at this point is not trivial, as
(sub)sections don't have a size field.  Working on getting size there,
but it is a long term project (it requires 1st to change everything to
VMState to be able to change how QEMUFile works).

Later, Juan.

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[TeLeMan]

--
SUN OF A BEACH



On Wed, Jul 28, 2010 at 20:32, Juan Quintela <quintela@redhat.com> wrote:
> TeLeMan <geleman@gmail.com> wrote:
>> On Wed, Jul 28, 2010 at 19:51, Juan Quintela <quintela@redhat.com> wrote:
>>> I hope this hepls to understand how subsections are supposed to work.
>>>
>>> Thanks for the comments, Juan.
>>>
>> I see, thanks a lot. But I still hope to have the similar subsection
>> that can be ignored simply.
>
> Then it is better not to be sent in the 1st place.
>
> Do you have any example of why you want to do?  When I dessigned
> subsections, I looked at all the changes that we had from 0.11 to 0.12,
> subsections can handle all of them.  Just curious about what you need.\
>
> Notice that ignoring subsections at this point is not trivial, as
> (sub)sections don't have a size field.  Working on getting size there,
> but it is a long term project (it requires 1st to change everything to
> VMState to be able to change how QEMUFile works).

I have some extra data to be saved to vmstate and I want the new
vmstate to be compatible with the official version. You are right, I
thought it was simple. Now I discard my thought.

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[Paolo Bonzini]

On 07/28/2010 02:51 PM, TeLeMan wrote:
> On Wed, Jul 28, 2010 at 20:32, Juan Quintela<quintela@redhat.com>  wrote:
>> TeLeMan<geleman@gmail.com>  wrote:
>>> On Wed, Jul 28, 2010 at 19:51, Juan Quintela<quintela@redhat.com>  wrote:
>>>> I hope this hepls to understand how subsections are supposed to work.
>>>>
>>>> Thanks for the comments, Juan.
>>>>
>>> I see, thanks a lot. But I still hope to have the similar subsection
>>> that can be ignored simply.
>>
>> Then it is better not to be sent in the 1st place.
>>
>> Do you have any example of why you want to do?  When I dessigned
>> subsections, I looked at all the changes that we had from 0.11 to 0.12,
>> subsections can handle all of them.  Just curious about what you need.\
>>
>> Notice that ignoring subsections at this point is not trivial, as
>> (sub)sections don't have a size field.  Working on getting size there,
>> but it is a long term project (it requires 1st to change everything to
>> VMState to be able to change how QEMUFile works).
>
> I have some extra data to be saved to vmstate and I want the new
> vmstate to be compatible with the official version. You are right, I
> thought it was simple. Now I discard my thought.

Even if they are mandatory, subsections still improve the situation 
here, because they provide a clean way to "branch" off an upstream 
vmstate version.  At least the failure will be clear, because an 
unsupported subsection is easily detected when migrating to (or 
restoring with) upstream.

Instead, for example RHEL5.5's "version 9" cpu save format will often 
crash upstream version 9 with a SIGSEGV.

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

[Juan Quintela]

Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 07/28/2010 02:51 PM, TeLeMan wrote:

> Even if they are mandatory, subsections still improve the situation
> here, because they provide a clean way to "branch" off an upstream
> vmstate version.  At least the failure will be clear, because an
> unsupported subsection is easily detected when migrating to (or
> restoring with) upstream.
>
> Instead, for example RHEL5.5's "version 9" cpu save format will often
> crash upstream version 9 with a SIGSEGV.

Old kvm (pre start of merge with qemu) and qemu device versions are out
of sync.  You can't migrate between them at all.  Basically you can't
migrate from anything older than qemu-0.12.  In qemu-0.12 firmware for
devices got moved to pci bars and memory layout changed in such a way
that it is not compatible at all.

Later, Juan.