[Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Juan Quintela]

Hi

This series move the subsections detection code form:
- Look that it starts form 5
To:
- Look that it starts form 5 (SUBSECTION)
- Look at the length
- Look that length is bigger than section name
- Look at the idstr and see that it starts with the subsection name.

Please review.

Later, Juan.

Juan Quintela (4):
  savevm: teach qemu_fill_buffer to do partial refills
  savevm: some coding style cleanups
  savevm: improve subsections detection on load
  Revert "savevm: fix corruption in vmstate_subsection_load()."

 savevm.c |  112 ++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 84 insertions(+), 28 deletions(-)

-- 
1.7.6.4

[Qemu-devel] [PATCH 1/4] savevm: teach qemu_fill_buffer to do partial refills

[Juan Quintela]

We will need on next patch to be able to lookahead on next patch

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/savevm.c b/savevm.c
index 46f2447..31131df 100644
--- a/savevm.c
+++ b/savevm.c
@@ -455,6 +455,7 @@ void qemu_fflush(QEMUFile *f)
 static void qemu_fill_buffer(QEMUFile *f)
 {
     int len;
+    int used;

     if (!f->get_buffer)
         return;
@@ -462,10 +463,17 @@ static void qemu_fill_buffer(QEMUFile *f)
     if (f->is_write)
         abort();

-    len = f->get_buffer(f->opaque, f->buf, f->buf_offset, IO_BUF_SIZE);
+    used = f->buf_size - f->buf_index;
+    if (used > 0) {
+        memmove(f->buf, f->buf + f->buf_index, used);
+    }
+    f->buf_index = 0;
+    f->buf_size = used;
+
+    len = f->get_buffer(f->opaque, f->buf + used, f->buf_offset,
+                        IO_BUF_SIZE - used);
     if (len > 0) {
-        f->buf_index = 0;
-        f->buf_size = len;
+        f->buf_size += len;
         f->buf_offset += len;
     } else if (len != -EAGAIN)
         f->has_error = 1;
-- 
1.7.6.4

[Qemu-devel] [PATCH 2/4] savevm: some coding style cleanups

[Juan Quintela]

This patch will make moving code on next patches and having checkpatch
happy easier.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/savevm.c b/savevm.c
index 31131df..5fee4e2 100644
--- a/savevm.c
+++ b/savevm.c
@@ -536,8 +536,9 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
 {
     int size, l;

-    if (f->is_write)
+    if (f->is_write) {
         abort();
+    }

     size = size1;
     while (size > 0) {
@@ -545,11 +546,13 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
         if (l == 0) {
             qemu_fill_buffer(f);
             l = f->buf_size - f->buf_index;
-            if (l == 0)
+            if (l == 0) {
                 break;
+            }
         }
-        if (l > size)
+        if (l > size) {
             l = size;
+        }
         memcpy(buf, f->buf + f->buf_index, l);
         f->buf_index += l;
         buf += l;
@@ -560,26 +563,30 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)

 static int qemu_peek_byte(QEMUFile *f)
 {
-    if (f->is_write)
+    if (f->is_write) {
         abort();
+    }

     if (f->buf_index >= f->buf_size) {
         qemu_fill_buffer(f);
-        if (f->buf_index >= f->buf_size)
+        if (f->buf_index >= f->buf_size) {
             return 0;
+        }
     }
     return f->buf[f->buf_index];
 }

 int qemu_get_byte(QEMUFile *f)
 {
-    if (f->is_write)
+    if (f->is_write) {
         abort();
+    }

     if (f->buf_index >= f->buf_size) {
         qemu_fill_buffer(f);
-        if (f->buf_index >= f->buf_size)
+        if (f->buf_index >= f->buf_size) {
             return 0;
+        }
     }
     return f->buf[f->buf_index++];
 }
-- 
1.7.6.4

[Qemu-devel] [PATCH 3/4] savevm: improve subsections detection on load

[Juan Quintela]

We add qemu_peek_buffer, that is identical to qemu_get_buffer, just
that it don't update f->buf_index.

We add a paramenter to qemu_peek_byte() to be able to peek more than
one byte.

Once this is done, to see if we have a subsection we look:
- 1st byte is QEMU_VM_SUBSECTION
- 2nd byte is a length, and is bigger than section name
- 3rd element is a string that starts with section_name

So, we shouldn't have false positives (yes, content could still get us
wrong but probabilities are really low).

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/savevm.c b/savevm.c
index 5fee4e2..db6ea12 100644
--- a/savevm.c
+++ b/savevm.c
@@ -532,6 +532,37 @@ void qemu_put_byte(QEMUFile *f, int v)
         qemu_fflush(f);
 }

+static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size1, int offset)
+{
+    int size, l;
+    int index = f->buf_index + offset;
+
+    if (f->is_write) {
+        abort();
+    }
+
+    size = size1;
+    while (size > 0) {
+        l = f->buf_size - index;
+        if (l == 0) {
+            qemu_fill_buffer(f);
+            index = f->buf_index + offset;
+            l = f->buf_size - index;
+            if (l == 0) {
+                break;
+            }
+        }
+        if (l > size) {
+            l = size;
+        }
+        memcpy(buf, f->buf + index, l);
+        index += l;
+        buf += l;
+        size -= l;
+    }
+    return size1 - size;
+}
+
 int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
 {
     int size, l;
@@ -561,19 +592,22 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
     return size1 - size;
 }

-static int qemu_peek_byte(QEMUFile *f)
+static int qemu_peek_byte(QEMUFile *f, int offset)
 {
+    int index = f->buf_index + offset;
+
     if (f->is_write) {
         abort();
     }

-    if (f->buf_index >= f->buf_size) {
+    if (index >= f->buf_size) {
         qemu_fill_buffer(f);
-        if (f->buf_index >= f->buf_size) {
+        index = f->buf_index + offset;
+        if (index >= f->buf_size) {
             return 0;
         }
     }
-    return f->buf[f->buf_index];
+    return f->buf[index];
 }

 int qemu_get_byte(QEMUFile *f)
@@ -1687,22 +1721,37 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
         return 0;
     }

-    while (qemu_peek_byte(f) == QEMU_VM_SUBSECTION) {
+    while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
         char idstr[256];
         int ret;
-        uint8_t version_id, len;
+        uint8_t version_id, len, size;
         const VMStateDescription *sub_vmsd;

-        qemu_get_byte(f); /* subsection */
-        len = qemu_get_byte(f);
-        qemu_get_buffer(f, (uint8_t *)idstr, len);
-        idstr[len] = 0;
-        version_id = qemu_get_be32(f);
+        len = qemu_peek_byte(f, 1);
+        if (len < strlen(vmsd->name) + 1) {
+            /* subsection name has be be "section_name/a" */
+            return 0;
+        }
+        size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
+        if (size != len) {
+            return 0;
+        }
+        idstr[size] = 0;

+        if (strncmp(vmsd->name, idstr, strlen(vmsd->name)) != 0) {
+            /* it don't have a valid subsection name */
+            return 0;
+        }
         sub_vmsd = vmstate_get_subsection(sub, idstr);
         if (sub_vmsd == NULL) {
             return -ENOENT;
         }
+        qemu_get_byte(f); /* subsection */
+        qemu_get_byte(f); /* len  */
+        qemu_get_buffer(f, (uint8_t *)idstr, len);
+        idstr[len] = 0;
+        version_id = qemu_get_be32(f);
+
         assert(!sub_vmsd->subsections);
         ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
         if (ret) {
-- 
1.7.6.4

[Qemu-devel] [PATCH 4/4] Revert "savevm: fix corruption in vmstate_subsection_load()."

[Juan Quintela]

This reverts commit eb60260de0b050a5e8ab725e84d377d0b44c43ae.

Conflicts:

	savevm.c

We changed qemu_peek_byte() prototype, just fixed the rejects.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/savevm.c b/savevm.c
index db6ea12..aaa303e 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1715,12 +1715,6 @@ static const VMStateDescription *vmstate_get_subsection(const VMStateSubsection
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
                                    void *opaque)
 {
-    const VMStateSubsection *sub = vmsd->subsections;
-
-    if (!sub || !sub->needed) {
-        return 0;
-    }
-
     while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
         char idstr[256];
         int ret;
@@ -1742,7 +1736,7 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
             /* it don't have a valid subsection name */
             return 0;
         }
-        sub_vmsd = vmstate_get_subsection(sub, idstr);
+        sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
         if (sub_vmsd == NULL) {
             return -ENOENT;
         }
@@ -1752,7 +1746,6 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
         idstr[len] = 0;
         version_id = qemu_get_be32(f);

-        assert(!sub_vmsd->subsections);
         ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
         if (ret) {
             return ret;
@@ -1776,7 +1769,6 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
             qemu_put_byte(f, len);
             qemu_put_buffer(f, (uint8_t *)vmsd->name, len);
             qemu_put_be32(f, vmsd->version_id);
-            assert(!vmsd->subsections);
             vmstate_save_state(f, vmsd, opaque);
         }
         sub++;
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Paolo Bonzini]

On 10/04/2011 04:38 PM, Juan Quintela wrote:
> Hi
>
> This series move the subsections detection code form:
> - Look that it starts form 5
> To:
> - Look that it starts form 5 (SUBSECTION)
> - Look at the length
> - Look that length is bigger than section name
> - Look at the idstr and see that it starts with the subsection name.
>
> Please review.

Looks good as an alternative to the new migration format.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Avi Kivity]

On 10/04/2011 04:38 PM, Juan Quintela wrote:
> Hi
>
> This series move the subsections detection code form:
> - Look that it starts form 5
> To:
> - Look that it starts form 5 (SUBSECTION)
> - Look at the length
> - Look that length is bigger than section name
> - Look at the idstr and see that it starts with the subsection name.
>
> Please review.
>

The original intent with subsections was to register them as a new 
vmstate section, with just a name relationship.

Can we rename .subsections to .old_and_semi_broken_subsections, and 
introduce a new .subsections field that works properly in all cases?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Anthony Liguori]

On 10/04/2011 01:20 PM, Avi Kivity wrote:
> On 10/04/2011 04:38 PM, Juan Quintela wrote:
>> Hi
>>
>> This series move the subsections detection code form:
>> - Look that it starts form 5
>> To:
>> - Look that it starts form 5 (SUBSECTION)
>> - Look at the length
>> - Look that length is bigger than section name
>> - Look at the idstr and see that it starts with the subsection name.
>>
>> Please review.
>>
>
> The original intent with subsections was to register them as a new vmstate
> section, with just a name relationship.
>
> Can we rename .subsections to .old_and_semi_broken_subsections, and introduce a
> new .subsections field that works properly in all cases?

That gets a bit hairy.  We don't really rely on section order today and I think 
this would introduce a section order requirement.  I don't think it's a problem 
and agree it's probably a better path but it does require a little bit of 
consideration.

Regards,

Anthony Liguori

>

Re: [Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Avi Kivity]

On 10/04/2011 08:22 PM, Anthony Liguori wrote:
>> The original intent with subsections was to register them as a new 
>> vmstate
>> section, with just a name relationship.
>>
>> Can we rename .subsections to .old_and_semi_broken_subsections, and 
>> introduce a
>> new .subsections field that works properly in all cases?
>
>
> That gets a bit hairy.  We don't really rely on section order today 
> and I think this would introduce a section order requirement. 


The information is there, since a subsection is contained within its 
section.  In practice working in FIFO order preserves it.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Juan Quintela]

Avi Kivity <avi@redhat.com> wrote:
> On 10/04/2011 04:38 PM, Juan Quintela wrote:
>> Hi
>>
>> This series move the subsections detection code form:
>> - Look that it starts form 5
>> To:
>> - Look that it starts form 5 (SUBSECTION)
>> - Look at the length
>> - Look that length is bigger than section name
>> - Look at the idstr and see that it starts with the subsection name.
>>
>> Please review.
>>
>
> The original intent with subsections was to register them as a new
> vmstate section, with just a name relationship.
>
> Can we rename .subsections to .old_and_semi_broken_subsections, and
> introduce a new .subsections field that works properly in all cases?

Not easily. We can try to do something, but the problem is that
subsection are just that "sub", and we should have to be very careful
with post-load handlers.

Will think about it, thought.

Later, Juan.

Re: [Qemu-devel] [PATCH 0/4] migration: Improve subsections detection

[Avi Kivity]

On 10/04/2011 08:35 PM, Juan Quintela wrote:
> Avi Kivity<avi@redhat.com>  wrote:
> >  On 10/04/2011 04:38 PM, Juan Quintela wrote:
> >>  Hi
> >>
> >>  This series move the subsections detection code form:
> >>  - Look that it starts form 5
> >>  To:
> >>  - Look that it starts form 5 (SUBSECTION)
> >>  - Look at the length
> >>  - Look that length is bigger than section name
> >>  - Look at the idstr and see that it starts with the subsection name.
> >>
> >>  Please review.
> >>
> >
> >  The original intent with subsections was to register them as a new
> >  vmstate section, with just a name relationship.
> >
> >  Can we rename .subsections to .old_and_semi_broken_subsections, and
> >  introduce a new .subsections field that works properly in all cases?
>
> Not easily. We can try to do something, but the problem is that
> subsection are just that "sub", and we should have to be very careful
> with post-load handlers.
>

If necessary the subsection's post-load handler can re-run the main 
post-load handler (or it can even be the same post-load handler, 
unconditionally).  We just have to make sure that post-load handlers are 
idempotent.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 1/4] savevm: teach qemu_fill_buffer to do partial refills

[Anthony Liguori]

On 10/04/2011 09:38 AM, Juan Quintela wrote:
> We will need on next patch to be able to lookahead on next patch
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   savevm.c |   14 +++++++++++---
>   1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 46f2447..31131df 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -455,6 +455,7 @@ void qemu_fflush(QEMUFile *f)
>   static void qemu_fill_buffer(QEMUFile *f)
>   {
>       int len;
> +    int used;
>
>       if (!f->get_buffer)
>           return;
> @@ -462,10 +463,17 @@ static void qemu_fill_buffer(QEMUFile *f)
>       if (f->is_write)
>           abort();
>
> -    len = f->get_buffer(f->opaque, f->buf, f->buf_offset, IO_BUF_SIZE);
> +    used = f->buf_size - f->buf_index;
> +    if (used>  0) {
> +        memmove(f->buf, f->buf + f->buf_index, used);
> +    }
> +    f->buf_index = 0;
> +    f->buf_size = used;
> +
> +    len = f->get_buffer(f->opaque, f->buf + used, f->buf_offset,
> +                        IO_BUF_SIZE - used);
>       if (len>  0) {
> -        f->buf_index = 0;
> -        f->buf_size = len;
> +        f->buf_size += len;
>           f->buf_offset += len;
>       } else if (len != -EAGAIN)
>           f->has_error = 1;

Re: [Qemu-devel] [PATCH 2/4] savevm: some coding style cleanups

[Anthony Liguori]

On 10/04/2011 09:38 AM, Juan Quintela wrote:
> This patch will make moving code on next patches and having checkpatch
> happy easier.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   savevm.c |   21 ++++++++++++++-------
>   1 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 31131df..5fee4e2 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -536,8 +536,9 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
>   {
>       int size, l;
>
> -    if (f->is_write)
> +    if (f->is_write) {
>           abort();
> +    }
>
>       size = size1;
>       while (size>  0) {
> @@ -545,11 +546,13 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
>           if (l == 0) {
>               qemu_fill_buffer(f);
>               l = f->buf_size - f->buf_index;
> -            if (l == 0)
> +            if (l == 0) {
>                   break;
> +            }
>           }
> -        if (l>  size)
> +        if (l>  size) {
>               l = size;
> +        }
>           memcpy(buf, f->buf + f->buf_index, l);
>           f->buf_index += l;
>           buf += l;
> @@ -560,26 +563,30 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
>
>   static int qemu_peek_byte(QEMUFile *f)
>   {
> -    if (f->is_write)
> +    if (f->is_write) {
>           abort();
> +    }
>
>       if (f->buf_index>= f->buf_size) {
>           qemu_fill_buffer(f);
> -        if (f->buf_index>= f->buf_size)
> +        if (f->buf_index>= f->buf_size) {
>               return 0;
> +        }
>       }
>       return f->buf[f->buf_index];
>   }
>
>   int qemu_get_byte(QEMUFile *f)
>   {
> -    if (f->is_write)
> +    if (f->is_write) {
>           abort();
> +    }
>
>       if (f->buf_index>= f->buf_size) {
>           qemu_fill_buffer(f);
> -        if (f->buf_index>= f->buf_size)
> +        if (f->buf_index>= f->buf_size) {
>               return 0;
> +        }
>       }
>       return f->buf[f->buf_index++];
>   }

Re: [Qemu-devel] [PATCH 3/4] savevm: improve subsections detection on load

[Anthony Liguori]

On 10/04/2011 09:38 AM, Juan Quintela wrote:
> We add qemu_peek_buffer, that is identical to qemu_get_buffer, just
> that it don't update f->buf_index.
>
> We add a paramenter to qemu_peek_byte() to be able to peek more than
> one byte.
>
> Once this is done, to see if we have a subsection we look:
> - 1st byte is QEMU_VM_SUBSECTION
> - 2nd byte is a length, and is bigger than section name
> - 3rd element is a string that starts with section_name
>
> So, we shouldn't have false positives (yes, content could still get us
> wrong but probabilities are really low).
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>
> ---
>   savevm.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
>   1 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 5fee4e2..db6ea12 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -532,6 +532,37 @@ void qemu_put_byte(QEMUFile *f, int v)
>           qemu_fflush(f);
>   }
>
> +static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size1, int offset)
> +{
> +    int size, l;
> +    int index = f->buf_index + offset;
> +
> +    if (f->is_write) {
> +        abort();
> +    }
> +
> +    size = size1;
> +    while (size>  0) {
> +        l = f->buf_size - index;
> +        if (l == 0) {
> +            qemu_fill_buffer(f);
> +            index = f->buf_index + offset;
> +            l = f->buf_size - index;
> +            if (l == 0) {
> +                break;
> +            }
> +        }
> +        if (l>  size) {
> +            l = size;
> +        }
> +        memcpy(buf, f->buf + index, l);
> +        index += l;
> +        buf += l;
> +        size -= l;
> +    }
> +    return size1 - size;
> +}
> +
>   int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
>   {
>       int size, l;

Can we implement get_buffer in terms of peek_buffer and just increment 
f->buf_index in get_buffer?

> @@ -561,19 +592,22 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
>       return size1 - size;
>   }
>
> -static int qemu_peek_byte(QEMUFile *f)
> +static int qemu_peek_byte(QEMUFile *f, int offset)
>   {
> +    int index = f->buf_index + offset;
> +
>       if (f->is_write) {
>           abort();
>       }
>
> -    if (f->buf_index>= f->buf_size) {
> +    if (index>= f->buf_size) {
>           qemu_fill_buffer(f);
> -        if (f->buf_index>= f->buf_size) {
> +        index = f->buf_index + offset;
> +        if (index>= f->buf_size) {
>               return 0;
>           }
>       }
> -    return f->buf[f->buf_index];
> +    return f->buf[index];
>   }
>
>   int qemu_get_byte(QEMUFile *f)
> @@ -1687,22 +1721,37 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
>           return 0;
>       }
>
> -    while (qemu_peek_byte(f) == QEMU_VM_SUBSECTION) {
> +    while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
>           char idstr[256];
>           int ret;
> -        uint8_t version_id, len;
> +        uint8_t version_id, len, size;
>           const VMStateDescription *sub_vmsd;
>
> -        qemu_get_byte(f); /* subsection */
> -        len = qemu_get_byte(f);
> -        qemu_get_buffer(f, (uint8_t *)idstr, len);
> -        idstr[len] = 0;
> -        version_id = qemu_get_be32(f);
> +        len = qemu_peek_byte(f, 1);
> +        if (len<  strlen(vmsd->name) + 1) {
> +            /* subsection name has be be "section_name/a" */
> +            return 0;
> +        }
> +        size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
> +        if (size != len) {
> +            return 0;
> +        }
> +        idstr[size] = 0;


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4] Revert "savevm: fix corruption in vmstate_subsection_load()."

[Anthony Liguori]

On 10/04/2011 09:38 AM, Juan Quintela wrote:
> This reverts commit eb60260de0b050a5e8ab725e84d377d0b44c43ae.
>
> Conflicts:
>
> 	savevm.c
>
> We changed qemu_peek_byte() prototype, just fixed the rejects.
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   savevm.c |   10 +---------
>   1 files changed, 1 insertions(+), 9 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index db6ea12..aaa303e 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1715,12 +1715,6 @@ static const VMStateDescription *vmstate_get_subsection(const VMStateSubsection
>   static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
>                                      void *opaque)
>   {
> -    const VMStateSubsection *sub = vmsd->subsections;
> -
> -    if (!sub || !sub->needed) {
> -        return 0;
> -    }
> -
>       while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
>           char idstr[256];
>           int ret;
> @@ -1742,7 +1736,7 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
>               /* it don't have a valid subsection name */
>               return 0;
>           }
> -        sub_vmsd = vmstate_get_subsection(sub, idstr);
> +        sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
>           if (sub_vmsd == NULL) {
>               return -ENOENT;
>           }
> @@ -1752,7 +1746,6 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
>           idstr[len] = 0;
>           version_id = qemu_get_be32(f);
>
> -        assert(!sub_vmsd->subsections);
>           ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
>           if (ret) {
>               return ret;
> @@ -1776,7 +1769,6 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
>               qemu_put_byte(f, len);
>               qemu_put_buffer(f, (uint8_t *)vmsd->name, len);
>               qemu_put_be32(f, vmsd->version_id);
> -            assert(!vmsd->subsections);
>               vmstate_save_state(f, vmsd, opaque);
>           }
>           sub++;

Re: [Qemu-devel] [PATCH 3/4] savevm: improve subsections detection on load

[Paolo Bonzini]

On 10/05/2011 09:45 PM, Anthony Liguori wrote:
>>
>>   int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
>>   {
>>       int size, l;
>
> Can we implement get_buffer in terms of peek_buffer and just increment
> f->buf_index in get_buffer?

No, get_buffer works even if size1 is greater than the size of the 
buffer, peek_buffer doesn't.

Paolo