All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] migration: Improve subsections detection
@ 2011-10-06 16:21 Juan Quintela
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 1/5] savevm: teach qemu_fill_buffer to do partial refills Juan Quintela
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 16:21 UTC (permalink / raw)
  To: qemu-devel

Hi

v2:
- rename "used" to "remaining" (Alex suggestion)
- implement qemu_get_{byte,buffer} on top of qemu_peek_{byte, buffer}
  (Anthony suggestion)
- fix qemu_peek_buffe_logic (Alex  discovered the problem)

v1:
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 (5):
  savevm: teach qemu_fill_buffer to do partial refills
  savevm: some coding style cleanups
  savevm: define qemu_get_byte() using qemu_peek_byte()
  savevm: improve subsections detection on load
  Revert "savevm: fix corruption in vmstate_subsection_load()."

 savevm.c |  144 ++++++++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 94 insertions(+), 50 deletions(-)

-- 
1.7.6.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 1/5] savevm: teach qemu_fill_buffer to do partial refills
  2011-10-06 16:21 [Qemu-devel] [PATCH 0/5] migration: Improve subsections detection Juan Quintela
@ 2011-10-06 16:21 ` Juan Quintela
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 2/5] savevm: some coding style cleanups Juan Quintela
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 16:21 UTC (permalink / raw)
  To: qemu-devel

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

v2: rename "used" to "pending" (Alex Williams)

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

diff --git a/savevm.c b/savevm.c
index 46f2447..743c304 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 pending;

     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);
+    pending = f->buf_size - f->buf_index;
+    if (pending > 0) {
+        memmove(f->buf, f->buf + f->buf_index, pending);
+    }
+    f->buf_index = 0;
+    f->buf_size = pending;
+
+    len = f->get_buffer(f->opaque, f->buf + pending, f->buf_offset,
+                        IO_BUF_SIZE - pending);
     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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 2/5] savevm: some coding style cleanups
  2011-10-06 16:21 [Qemu-devel] [PATCH 0/5] migration: Improve subsections detection Juan Quintela
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 1/5] savevm: teach qemu_fill_buffer to do partial refills Juan Quintela
@ 2011-10-06 16:21 ` Juan Quintela
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 3/5] savevm: define qemu_get_byte() using qemu_peek_byte() Juan Quintela
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 16:21 UTC (permalink / raw)
  To: qemu-devel

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>
---
 savevm.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/savevm.c b/savevm.c
index 743c304..4069b34 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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 3/5] savevm: define qemu_get_byte() using qemu_peek_byte()
  2011-10-06 16:21 [Qemu-devel] [PATCH 0/5] migration: Improve subsections detection Juan Quintela
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 1/5] savevm: teach qemu_fill_buffer to do partial refills Juan Quintela
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 2/5] savevm: some coding style cleanups Juan Quintela
@ 2011-10-06 16:21 ` Juan Quintela
  2011-10-06 16:26   ` Paolo Bonzini
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load Juan Quintela
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 5/5] Revert "savevm: fix corruption in vmstate_subsection_load()." Juan Quintela
  4 siblings, 1 reply; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 16:21 UTC (permalink / raw)
  To: qemu-devel

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

diff --git a/savevm.c b/savevm.c
index 4069b34..94628c6 100644
--- a/savevm.c
+++ b/savevm.c
@@ -578,17 +578,14 @@ static int qemu_peek_byte(QEMUFile *f)

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

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

 int64_t qemu_ftell(QEMUFile *f)
-- 
1.7.6.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load
  2011-10-06 16:21 [Qemu-devel] [PATCH 0/5] migration: Improve subsections detection Juan Quintela
                   ` (2 preceding siblings ...)
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 3/5] savevm: define qemu_get_byte() using qemu_peek_byte() Juan Quintela
@ 2011-10-06 16:21 ` Juan Quintela
  2011-10-06 16:26   ` Paolo Bonzini
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 5/5] Revert "savevm: fix corruption in vmstate_subsection_load()." Juan Quintela
  4 siblings, 1 reply; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 16:21 UTC (permalink / raw)
  To: qemu-devel

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).

v2:
- Alex Williamsom found that we could get negative values on index.
- Rework code to fix that part.
- Rewrite qemu_get_buffer() using qemu_peek_buffer()

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

diff --git a/savevm.c b/savevm.c
index 94628c6..28c0a43 100644
--- a/savevm.c
+++ b/savevm.c
@@ -532,59 +532,85 @@ void qemu_put_byte(QEMUFile *f, int v)
         qemu_fflush(f);
 }

-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
+static void qemu_file_skip(QEMUFile *f, int size)
 {
-    int size, l;
+    if (f->buf_index + size < f->buf_size) {
+        f->buf_index += size;
+    }
+}
+
+static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+{
+    int pending;
+    int index;

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

-    size = size1;
-    while (size > 0) {
-        l = f->buf_size - f->buf_index;
-        if (l == 0) {
-            qemu_fill_buffer(f);
-            l = f->buf_size - f->buf_index;
-            if (l == 0) {
-                break;
-            }
-        }
-        if (l > size) {
-            l = size;
+    index = f->buf_index + offset;
+    pending = f->buf_size - index;
+    if (pending < size) {
+        qemu_fill_buffer(f);
+        index = f->buf_index + offset;
+        pending = f->buf_size - index;
+    }
+
+    if (pending <= 0) {
+        return 0;
+    }
+    if (size > pending) {
+        size = pending;
+    }
+
+    memcpy(buf, f->buf + index, size);
+    return size;
+}
+
+int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
+{
+    int pending = size;
+    int done = 0;
+
+    while (pending > 0) {
+        int res;
+
+        res = qemu_peek_buffer(f, buf, pending, 0);
+        if (res == 0) {
+            return 0;
         }
-        memcpy(buf, f->buf + f->buf_index, l);
-        f->buf_index += l;
-        buf += l;
-        size -= l;
+        qemu_file_skip(f, res);
+        buf += res;
+        pending -= res;
+        done += res;
     }
-    return size1 - size;
+    return done;
 }

-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)
 {
     int result;

-    result = qemu_peek_byte(f);
-
-    if (f->buf_index < f->buf_size) {
-        f->buf_index++;
-    }
+    result = qemu_peek_byte(f, 0);
+    qemu_file_skip(f, 1);
     return result;
 }

@@ -1684,22 +1710,36 @@ 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_file_skip(f, 1); /* subsection */
+        qemu_file_skip(f, 1); /* len */
+        qemu_file_skip(f, len); /* idstr */
+        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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 5/5] Revert "savevm: fix corruption in vmstate_subsection_load()."
  2011-10-06 16:21 [Qemu-devel] [PATCH 0/5] migration: Improve subsections detection Juan Quintela
                   ` (3 preceding siblings ...)
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load Juan Quintela
@ 2011-10-06 16:21 ` Juan Quintela
  4 siblings, 0 replies; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 16:21 UTC (permalink / raw)
  To: qemu-devel

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>
---
 savevm.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/savevm.c b/savevm.c
index 28c0a43..1c62269 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1704,12 +1704,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;
@@ -1731,7 +1725,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;
         }
@@ -1740,7 +1734,6 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
         qemu_file_skip(f, len); /* idstr */
         version_id = qemu_get_be32(f);

-        assert(!sub_vmsd->subsections);
         ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
         if (ret) {
             return ret;
@@ -1764,7 +1757,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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] savevm: define qemu_get_byte() using qemu_peek_byte()
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 3/5] savevm: define qemu_get_byte() using qemu_peek_byte() Juan Quintela
@ 2011-10-06 16:26   ` Paolo Bonzini
  2011-10-06 22:40     ` Juan Quintela
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2011-10-06 16:26 UTC (permalink / raw)
  To: Juan Quintela; +Cc: qemu-devel

On 10/06/2011 06:21 PM, Juan Quintela wrote:
> +    result = qemu_peek_byte(f);
> +
> +    if (f->buf_index<  f->buf_size) {
> +        f->buf_index++;
>       }

This should really be an assert that f->buf_index < f->buf_size, 
otherwise qemu_peek_byte has read garbage.

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load
  2011-10-06 16:21 ` [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load Juan Quintela
@ 2011-10-06 16:26   ` Paolo Bonzini
  2011-10-06 22:42     ` Juan Quintela
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2011-10-06 16:26 UTC (permalink / raw)
  To: Juan Quintela; +Cc: qemu-devel

On 10/06/2011 06:21 PM, Juan Quintela wrote:
> +
> +int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
> +{
> +    int pending = size;
> +    int done = 0;
> +
> +    while (pending>  0) {
> +        int res;
> +
> +        res = qemu_peek_buffer(f, buf, pending, 0);
> +        if (res == 0) {
> +            return 0;
>           }
> -        memcpy(buf, f->buf + f->buf_index, l);
> -        f->buf_index += l;
> -        buf += l;
> -        size -= l;
> +        qemu_file_skip(f, res);
> +        buf += res;
> +        pending -= res;
> +        done += res;
>       }
> -    return size1 - size;
> +    return done;
>   }

This changes semantics for reads above 32KB.  It should be in the commit 
message, or preferably v1 could be committed instead. :)

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 3/5] savevm: define qemu_get_byte() using qemu_peek_byte()
  2011-10-06 16:26   ` Paolo Bonzini
@ 2011-10-06 22:40     ` Juan Quintela
  0 siblings, 0 replies; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 22:40 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 10/06/2011 06:21 PM, Juan Quintela wrote:
>> +    result = qemu_peek_byte(f);
>> +
>> +    if (f->buf_index<  f->buf_size) {
>> +        f->buf_index++;
>>       }
>
> This should really be an assert that f->buf_index < f->buf_size,
> otherwise qemu_peek_byte has read garbage.

That is a change from current behaviour.  qemu_get_byte() returns 0 in
the case that there is nothing to read.  Yes, it is ugly.

Later, Juan.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load
  2011-10-06 16:26   ` Paolo Bonzini
@ 2011-10-06 22:42     ` Juan Quintela
  2011-10-07  6:37       ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Juan Quintela @ 2011-10-06 22:42 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 10/06/2011 06:21 PM, Juan Quintela wrote:
>> +
>> +int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
>> +{
>> +    int pending = size;
>> +    int done = 0;
>> +
>> +    while (pending>  0) {
>> +        int res;
>> +
>> +        res = qemu_peek_buffer(f, buf, pending, 0);
>> +        if (res == 0) {
>> +            return 0;

should this line return "done" insntead?

>>           }
>> -        memcpy(buf, f->buf + f->buf_index, l);
>> -        f->buf_index += l;
>> -        buf += l;
>> -        size -= l;
>> +        qemu_file_skip(f, res);
>> +        buf += res;
>> +        pending -= res;
>> +        done += res;
>>       }
>> -    return size1 - size;
>> +    return done;
>>   }
>
> This changes semantics for reads above 32KB.  It should be in the
> commit message, or preferably v1 could be committed instead. :)

how it changes?  My understanding is that we read the same, only change
that I can think of is the one that I have jsut shown (and that is on
the error case).

Later, Juan.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load
  2011-10-06 22:42     ` Juan Quintela
@ 2011-10-07  6:37       ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2011-10-07  6:37 UTC (permalink / raw)
  To: quintela; +Cc: qemu-devel

On 10/07/2011 12:42 AM, Juan Quintela wrote:
>> >  This changes semantics for reads above 32KB.  It should be in the
>> >  commit message, or preferably v1 could be committed instead.:)
> how it changes?  My understanding is that we read the same, only change
> that I can think of is the one that I have jsut shown (and that is on
> the error case).

Yes, you're right.

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load
  2011-10-07 10:53 [Qemu-devel] [PATCH v3 0/5] migration: Improve subsections detection Juan Quintela
@ 2011-10-07 10:53 ` Juan Quintela
  0 siblings, 0 replies; 13+ messages in thread
From: Juan Quintela @ 2011-10-07 10:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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).

v2:
- Alex Williamsom found that we could get negative values on index.
- Rework code to fix that part.
- Rewrite qemu_get_buffer() using qemu_peek_buffer()

v3:
- return "done" on error case

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

diff --git a/savevm.c b/savevm.c
index 94628c6..aafdc7b 100644
--- a/savevm.c
+++ b/savevm.c
@@ -532,59 +532,85 @@ void qemu_put_byte(QEMUFile *f, int v)
         qemu_fflush(f);
 }

-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
+static void qemu_file_skip(QEMUFile *f, int size)
 {
-    int size, l;
+    if (f->buf_index + size < f->buf_size) {
+        f->buf_index += size;
+    }
+}
+
+static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+{
+    int pending;
+    int index;

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

-    size = size1;
-    while (size > 0) {
-        l = f->buf_size - f->buf_index;
-        if (l == 0) {
-            qemu_fill_buffer(f);
-            l = f->buf_size - f->buf_index;
-            if (l == 0) {
-                break;
-            }
-        }
-        if (l > size) {
-            l = size;
+    index = f->buf_index + offset;
+    pending = f->buf_size - index;
+    if (pending < size) {
+        qemu_fill_buffer(f);
+        index = f->buf_index + offset;
+        pending = f->buf_size - index;
+    }
+
+    if (pending <= 0) {
+        return 0;
+    }
+    if (size > pending) {
+        size = pending;
+    }
+
+    memcpy(buf, f->buf + index, size);
+    return size;
+}
+
+int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
+{
+    int pending = size;
+    int done = 0;
+
+    while (pending > 0) {
+        int res;
+
+        res = qemu_peek_buffer(f, buf, pending, 0);
+        if (res == 0) {
+            return done;
         }
-        memcpy(buf, f->buf + f->buf_index, l);
-        f->buf_index += l;
-        buf += l;
-        size -= l;
+        qemu_file_skip(f, res);
+        buf += res;
+        pending -= res;
+        done += res;
     }
-    return size1 - size;
+    return done;
 }

-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)
 {
     int result;

-    result = qemu_peek_byte(f);
-
-    if (f->buf_index < f->buf_size) {
-        f->buf_index++;
-    }
+    result = qemu_peek_byte(f, 0);
+    qemu_file_skip(f, 1);
     return result;
 }

@@ -1684,22 +1710,36 @@ 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_file_skip(f, 1); /* subsection */
+        qemu_file_skip(f, 1); /* len */
+        qemu_file_skip(f, len); /* idstr */
+        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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load
  2011-10-09 19:49 [Qemu-devel] [PATCH v4 0/5] Improve subsections detection Juan Quintela
@ 2011-10-09 19:50 ` Juan Quintela
  0 siblings, 0 replies; 13+ messages in thread
From: Juan Quintela @ 2011-10-09 19:50 UTC (permalink / raw)
  To: qemu-devel

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).

v2:
- Alex Williamsom found that we could get negative values on index.
- Rework code to fix that part.
- Rewrite qemu_get_buffer() using qemu_peek_buffer()

v3:
- return "done" on error case

v4:
- fix qemu_file_skip() off by one.

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

diff --git a/savevm.c b/savevm.c
index 94628c6..ad58e7c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -532,59 +532,85 @@ void qemu_put_byte(QEMUFile *f, int v)
         qemu_fflush(f);
 }

-int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1)
+static void qemu_file_skip(QEMUFile *f, int size)
 {
-    int size, l;
+    if (f->buf_index + size <= f->buf_size) {
+        f->buf_index += size;
+    }
+}
+
+static int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+{
+    int pending;
+    int index;

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

-    size = size1;
-    while (size > 0) {
-        l = f->buf_size - f->buf_index;
-        if (l == 0) {
-            qemu_fill_buffer(f);
-            l = f->buf_size - f->buf_index;
-            if (l == 0) {
-                break;
-            }
-        }
-        if (l > size) {
-            l = size;
+    index = f->buf_index + offset;
+    pending = f->buf_size - index;
+    if (pending < size) {
+        qemu_fill_buffer(f);
+        index = f->buf_index + offset;
+        pending = f->buf_size - index;
+    }
+
+    if (pending <= 0) {
+        return 0;
+    }
+    if (size > pending) {
+        size = pending;
+    }
+
+    memcpy(buf, f->buf + index, size);
+    return size;
+}
+
+int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
+{
+    int pending = size;
+    int done = 0;
+
+    while (pending > 0) {
+        int res;
+
+        res = qemu_peek_buffer(f, buf, pending, 0);
+        if (res == 0) {
+            return done;
         }
-        memcpy(buf, f->buf + f->buf_index, l);
-        f->buf_index += l;
-        buf += l;
-        size -= l;
+        qemu_file_skip(f, res);
+        buf += res;
+        pending -= res;
+        done += res;
     }
-    return size1 - size;
+    return done;
 }

-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)
 {
     int result;

-    result = qemu_peek_byte(f);
-
-    if (f->buf_index < f->buf_size) {
-        f->buf_index++;
-    }
+    result = qemu_peek_byte(f, 0);
+    qemu_file_skip(f, 1);
     return result;
 }

@@ -1684,22 +1710,36 @@ 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_file_skip(f, 1); /* subsection */
+        qemu_file_skip(f, 1); /* len */
+        qemu_file_skip(f, len); /* idstr */
+        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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-10-09 20:04 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-06 16:21 [Qemu-devel] [PATCH 0/5] migration: Improve subsections detection Juan Quintela
2011-10-06 16:21 ` [Qemu-devel] [PATCH 1/5] savevm: teach qemu_fill_buffer to do partial refills Juan Quintela
2011-10-06 16:21 ` [Qemu-devel] [PATCH 2/5] savevm: some coding style cleanups Juan Quintela
2011-10-06 16:21 ` [Qemu-devel] [PATCH 3/5] savevm: define qemu_get_byte() using qemu_peek_byte() Juan Quintela
2011-10-06 16:26   ` Paolo Bonzini
2011-10-06 22:40     ` Juan Quintela
2011-10-06 16:21 ` [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load Juan Quintela
2011-10-06 16:26   ` Paolo Bonzini
2011-10-06 22:42     ` Juan Quintela
2011-10-07  6:37       ` Paolo Bonzini
2011-10-06 16:21 ` [Qemu-devel] [PATCH 5/5] Revert "savevm: fix corruption in vmstate_subsection_load()." Juan Quintela
  -- strict thread matches above, loose matches on Subject: below --
2011-10-07 10:53 [Qemu-devel] [PATCH v3 0/5] migration: Improve subsections detection Juan Quintela
2011-10-07 10:53 ` [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load Juan Quintela
2011-10-09 19:49 [Qemu-devel] [PATCH v4 0/5] Improve subsections detection Juan Quintela
2011-10-09 19:50 ` [Qemu-devel] [PATCH 4/5] savevm: improve subsections detection on load Juan Quintela

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.