[Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data

[Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Make qemu_peek_buffer repeatedly call fill_buffer until it gets
all the data it requires, or until there is an error.

  At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
  isn't enough data waiting, however the kernel is entitled to return
  just a few bytes, and still leave qemu_peek_buffer with less bytes
  than it needed.  I've seen this fail in a dev world, and I think it
  could theoretically fail in the peeking of the subsection headers in
  the current world.

Comment qemu_peek_byte to point out it's not guaranteed to work for
  non-continuous peeks

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

v4:
  Limit the size qemu_get_buffer passes to qemu_fill_buffer on each loop

v3:
  Make qemu_fill_buffer return ssize_t
  Remove the other size_t cleanup from this patch - I'll get back to them later
  Comment additions/cleanups from review
  Stretch my -ve

 include/migration/qemu-file.h |  5 ++++
 qemu-file.c                   | 53 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a191fb6..c90f529 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -123,6 +123,11 @@ void qemu_put_be32(QEMUFile *f, unsigned int v);
 void qemu_put_be64(QEMUFile *f, uint64_t v);
 int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
 int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
+/*
+ * Note that you can only peek continuous bytes from where the current pointer
+ * is; you aren't guaranteed to be able to peak to +n bytes unless you've
+ * previously peeked +n-1.
+ */
 int qemu_peek_byte(QEMUFile *f, int offset);
 int qemu_get_byte(QEMUFile *f);
 void qemu_file_skip(QEMUFile *f, int size);
diff --git a/qemu-file.c b/qemu-file.c
index e5ec798..6ae37d0 100644
--- a/qemu-file.c
+++ b/qemu-file.c
@@ -529,7 +529,15 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
     return RAM_SAVE_CONTROL_NOT_SUPP;
 }
 
-static void qemu_fill_buffer(QEMUFile *f)
+/*
+ * Attempt to fill the buffer from the underlying file
+ * Returns the number of bytes read, or negative value for an error.
+ *
+ * Note that it can return a partially full buffer even in a not error/not EOF
+ * case if the underlying file descriptor gives a short read, and that can
+ * happen even on a blocking fd.
+ */
+static ssize_t qemu_fill_buffer(QEMUFile *f)
 {
     int len;
     int pending;
@@ -553,6 +561,8 @@ static void qemu_fill_buffer(QEMUFile *f)
     } else if (len != -EAGAIN) {
         qemu_file_set_error(f, len);
     }
+
+    return len;
 }
 
 int qemu_get_fd(QEMUFile *f)
@@ -683,17 +693,39 @@ void qemu_file_skip(QEMUFile *f, int size)
     }
 }
 
+/*
+ * Read 'size' bytes from file (at 'offset') into buf without moving the
+ * pointer.
+ *
+ * It will return size bytes unless there was an error, in which case it will
+ * return as many as it managed to read (assuming blocking fd's which
+ * all current QEMUFile are)
+ */
 int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
 {
     int pending;
     int index;
 
     assert(!qemu_file_is_writable(f));
+    assert(offset < IO_BUF_SIZE);
+    assert(size <= IO_BUF_SIZE - offset);
 
+    /* The 1st byte to read from */
     index = f->buf_index + offset;
+    /* The number of available bytes starting at index */
     pending = f->buf_size - index;
-    if (pending < size) {
-        qemu_fill_buffer(f);
+
+    /*
+     * qemu_fill_buffer might return just a few bytes, even when there isn't
+     * an error, so loop collecting them until we get enough.
+     */
+    while (pending < size) {
+        int received = qemu_fill_buffer(f);
+
+        if (received <= 0) {
+            break;
+        }
+
         index = f->buf_index + offset;
         pending = f->buf_size - index;
     }
@@ -709,6 +741,14 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
     return size;
 }
 
+/*
+ * Read 'size' bytes of data from the file into buf.
+ * 'size' can be larger than the internal buffer.
+ *
+ * It will return size bytes unless there was an error, in which case it will
+ * return as many as it managed to read (assuming blocking fd's which
+ * all current QEMUFile are)
+ */
 int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
 {
     int pending = size;
@@ -717,7 +757,7 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
     while (pending > 0) {
         int res;
 
-        res = qemu_peek_buffer(f, buf, pending, 0);
+        res = qemu_peek_buffer(f, buf, MIN(pending, IO_BUF_SIZE), 0);
         if (res == 0) {
             return done;
         }
@@ -729,11 +769,16 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
     return done;
 }
 
+/*
+ * Peeks a single byte from the buffer; this isn't guaranteed to work if
+ * offset leaves a gap after the previous read/peeked data.
+ */
 int qemu_peek_byte(QEMUFile *f, int offset)
 {
     int index = f->buf_index + offset;
 
     assert(!qemu_file_is_writable(f));
+    assert(offset < IO_BUF_SIZE);
 
     if (index >= f->buf_size) {
         qemu_fill_buffer(f);
-- 
1.9.0

Re: [Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data

[Dr. David Alan Gilbert]

* ???? (chenliang0016@icloud.com) wrote:
> 
> ?? 2014??4??8????????10:29??Dr. David Alan Gilbert (git) <dgilbert@redhat.com> ??????
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Make qemu_peek_buffer repeatedly call fill_buffer until it gets
> > all the data it requires, or until there is an error.
> > 
> >  At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
> >  isn't enough data waiting, however the kernel is entitled to return
> >  just a few bytes, and still leave qemu_peek_buffer with less bytes
> >  than it needed.  I've seen this fail in a dev world, and I think it
> >  could theoretically fail in the peeking of the subsection headers in
> >  the current world.
> hmm, I also have got some errors(infrequently). Maybe It is one point.
> Could you show some messages about the error?

I've only seen the errors in my visitor/ber world where I use the peek_buffer
a lot more; but the one place it is used in the existing code is in
the code to check if we have the start of a subsection; if that goes wrong
I'm not sure what error would be produced.

Dave

> 
> However, this patch avoids one potential issue, thanks.
> 
> Reviewed-by: ChenLiang <chenliang0016@icloud.com>
> > 
> > Comment qemu_peek_byte to point out it's not guaranteed to work for
> >  non-continuous peeks
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> > 
> > v4:
> >  Limit the size qemu_get_buffer passes to qemu_fill_buffer on each loop
> > 
> > v3:
> >  Make qemu_fill_buffer return ssize_t
> >  Remove the other size_t cleanup from this patch - I'll get back to them later
> >  Comment additions/cleanups from review
> >  Stretch my -ve
> > 
> > include/migration/qemu-file.h |  5 ++++
> > qemu-file.c                   | 53 +++++++++++++++++++++++++++++++++++++++----
> > 2 files changed, 54 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> > index a191fb6..c90f529 100644
> > --- a/include/migration/qemu-file.h
> > +++ b/include/migration/qemu-file.h
> > @@ -123,6 +123,11 @@ void qemu_put_be32(QEMUFile *f, unsigned int v);
> > void qemu_put_be64(QEMUFile *f, uint64_t v);
> > int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
> > int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
> > +/*
> > + * Note that you can only peek continuous bytes from where the current pointer
> > + * is; you aren't guaranteed to be able to peak to +n bytes unless you've
> > + * previously peeked +n-1.
> > + */
> > int qemu_peek_byte(QEMUFile *f, int offset);
> > int qemu_get_byte(QEMUFile *f);
> > void qemu_file_skip(QEMUFile *f, int size);
> > diff --git a/qemu-file.c b/qemu-file.c
> > index e5ec798..6ae37d0 100644
> > --- a/qemu-file.c
> > +++ b/qemu-file.c
> > @@ -529,7 +529,15 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
> >     return RAM_SAVE_CONTROL_NOT_SUPP;
> > }
> > 
> > -static void qemu_fill_buffer(QEMUFile *f)
> > +/*
> > + * Attempt to fill the buffer from the underlying file
> > + * Returns the number of bytes read, or negative value for an error.
> > + *
> > + * Note that it can return a partially full buffer even in a not error/not EOF
> > + * case if the underlying file descriptor gives a short read, and that can
> > + * happen even on a blocking fd.
> > + */
> > +static ssize_t qemu_fill_buffer(QEMUFile *f)
> > {
> >     int len;
> >     int pending;
> > @@ -553,6 +561,8 @@ static void qemu_fill_buffer(QEMUFile *f)
> >     } else if (len != -EAGAIN) {
> >         qemu_file_set_error(f, len);
> >     }
> > +
> > +    return len;
> > }
> > 
> > int qemu_get_fd(QEMUFile *f)
> > @@ -683,17 +693,39 @@ void qemu_file_skip(QEMUFile *f, int size)
> >     }
> > }
> > 
> > +/*
> > + * Read 'size' bytes from file (at 'offset') into buf without moving the
> > + * pointer.
> > + *
> > + * It will return size bytes unless there was an error, in which case it will
> > + * return as many as it managed to read (assuming blocking fd's which
> > + * all current QEMUFile are)
> > + */
> > int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
> > {
> >     int pending;
> >     int index;
> > 
> >     assert(!qemu_file_is_writable(f));
> > +    assert(offset < IO_BUF_SIZE);
> > +    assert(size <= IO_BUF_SIZE - offset);
> > 
> > +    /* The 1st byte to read from */
> >     index = f->buf_index + offset;
> > +    /* The number of available bytes starting at index */
> >     pending = f->buf_size - index;
> > -    if (pending < size) {
> > -        qemu_fill_buffer(f);
> > +
> > +    /*
> > +     * qemu_fill_buffer might return just a few bytes, even when there isn't
> > +     * an error, so loop collecting them until we get enough.
> > +     */
> > +    while (pending < size) {
> > +        int received = qemu_fill_buffer(f);
> > +
> > +        if (received <= 0) {
> > +            break;
> > +        }
> > +
> >         index = f->buf_index + offset;
> >         pending = f->buf_size - index;
> >     }
> > @@ -709,6 +741,14 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
> >     return size;
> > }
> > 
> > +/*
> > + * Read 'size' bytes of data from the file into buf.
> > + * 'size' can be larger than the internal buffer.
> > + *
> > + * It will return size bytes unless there was an error, in which case it will
> > + * return as many as it managed to read (assuming blocking fd's which
> > + * all current QEMUFile are)
> > + */
> > int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
> > {
> >     int pending = size;
> > @@ -717,7 +757,7 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
> >     while (pending > 0) {
> >         int res;
> > 
> > -        res = qemu_peek_buffer(f, buf, pending, 0);
> > +        res = qemu_peek_buffer(f, buf, MIN(pending, IO_BUF_SIZE), 0);
> >         if (res == 0) {
> >             return done;
> >         }
> > @@ -729,11 +769,16 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
> >     return done;
> > }
> > 
> > +/*
> > + * Peeks a single byte from the buffer; this isn't guaranteed to work if
> > + * offset leaves a gap after the previous read/peeked data.
> > + */
> > int qemu_peek_byte(QEMUFile *f, int offset)
> > {
> >     int index = f->buf_index + offset;
> > 
> >     assert(!qemu_file_is_writable(f));
> > +    assert(offset < IO_BUF_SIZE);
> > 
> >     if (index >= f->buf_size) {
> >         qemu_fill_buffer(f);
> > -- 
> > 1.9.0
> > 
> > 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data

[陈梁]

> * ???? (chenliang0016@icloud.com) wrote:
>> 
>> ?? 2014??4??8????????10:29??Dr. David Alan Gilbert (git) <dgilbert@redhat.com> ??????
>> 
>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>> 
>>> Make qemu_peek_buffer repeatedly call fill_buffer until it gets
>>> all the data it requires, or until there is an error.
>>> 
>>> At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
>>> isn't enough data waiting, however the kernel is entitled to return
>>> just a few bytes, and still leave qemu_peek_buffer with less bytes
>>> than it needed.  I've seen this fail in a dev world, and I think it
>>> could theoretically fail in the peeking of the subsection headers in
>>> the current world.
>> hmm, I also have got some errors(infrequently). Maybe It is one point.
>> Could you show some messages about the error?
> 
> I've only seen the errors in my visitor/ber world where I use the peek_buffer
> a lot more; but the one place it is used in the existing code is in
> the code to check if we have the start of a subsection; if that goes wrong
> I'm not sure what error would be produced.
> 
> Dave
In my observation, error is more likely to happen if migration thread is blocked too long.
BTW, xbzrle and auto convergence are disable.

Best regards
ChenLiang
> 
>> 
>> However, this patch avoids one potential issue, thanks.
>> 
>> Reviewed-by: ChenLiang <chenliang0016@icloud.com>
>>> 
>>> Comment qemu_peek_byte to point out it's not guaranteed to work for
>>> non-continuous peeks
>>> 
>>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> ---
>>> 
>>> v4:
>>> Limit the size qemu_get_buffer passes to qemu_fill_buffer on each loop
>>> 
>>> v3:
>>> Make qemu_fill_buffer return ssize_t
>>> Remove the other size_t cleanup from this patch - I'll get back to them later
>>> Comment additions/cleanups from review
>>> Stretch my -ve
>>> 
>>> include/migration/qemu-file.h |  5 ++++
>>> qemu-file.c                   | 53 +++++++++++++++++++++++++++++++++++++++----
>>> 2 files changed, 54 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
>>> index a191fb6..c90f529 100644
>>> --- a/include/migration/qemu-file.h
>>> +++ b/include/migration/qemu-file.h
>>> @@ -123,6 +123,11 @@ void qemu_put_be32(QEMUFile *f, unsigned int v);
>>> void qemu_put_be64(QEMUFile *f, uint64_t v);
>>> int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
>>> int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
>>> +/*
>>> + * Note that you can only peek continuous bytes from where the current pointer
>>> + * is; you aren't guaranteed to be able to peak to +n bytes unless you've
>>> + * previously peeked +n-1.
>>> + */
>>> int qemu_peek_byte(QEMUFile *f, int offset);
>>> int qemu_get_byte(QEMUFile *f);
>>> void qemu_file_skip(QEMUFile *f, int size);
>>> diff --git a/qemu-file.c b/qemu-file.c
>>> index e5ec798..6ae37d0 100644
>>> --- a/qemu-file.c
>>> +++ b/qemu-file.c
>>> @@ -529,7 +529,15 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
>>>    return RAM_SAVE_CONTROL_NOT_SUPP;
>>> }
>>> 
>>> -static void qemu_fill_buffer(QEMUFile *f)
>>> +/*
>>> + * Attempt to fill the buffer from the underlying file
>>> + * Returns the number of bytes read, or negative value for an error.
>>> + *
>>> + * Note that it can return a partially full buffer even in a not error/not EOF
>>> + * case if the underlying file descriptor gives a short read, and that can
>>> + * happen even on a blocking fd.
>>> + */
>>> +static ssize_t qemu_fill_buffer(QEMUFile *f)
>>> {
>>>    int len;
>>>    int pending;
>>> @@ -553,6 +561,8 @@ static void qemu_fill_buffer(QEMUFile *f)
>>>    } else if (len != -EAGAIN) {
>>>        qemu_file_set_error(f, len);
>>>    }
>>> +
>>> +    return len;
>>> }
>>> 
>>> int qemu_get_fd(QEMUFile *f)
>>> @@ -683,17 +693,39 @@ void qemu_file_skip(QEMUFile *f, int size)
>>>    }
>>> }
>>> 
>>> +/*
>>> + * Read 'size' bytes from file (at 'offset') into buf without moving the
>>> + * pointer.
>>> + *
>>> + * It will return size bytes unless there was an error, in which case it will
>>> + * return as many as it managed to read (assuming blocking fd's which
>>> + * all current QEMUFile are)
>>> + */
>>> int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
>>> {
>>>    int pending;
>>>    int index;
>>> 
>>>    assert(!qemu_file_is_writable(f));
>>> +    assert(offset < IO_BUF_SIZE);
>>> +    assert(size <= IO_BUF_SIZE - offset);
>>> 
>>> +    /* The 1st byte to read from */
>>>    index = f->buf_index + offset;
>>> +    /* The number of available bytes starting at index */
>>>    pending = f->buf_size - index;
>>> -    if (pending < size) {
>>> -        qemu_fill_buffer(f);
>>> +
>>> +    /*
>>> +     * qemu_fill_buffer might return just a few bytes, even when there isn't
>>> +     * an error, so loop collecting them until we get enough.
>>> +     */
>>> +    while (pending < size) {
>>> +        int received = qemu_fill_buffer(f);
>>> +
>>> +        if (received <= 0) {
>>> +            break;
>>> +        }
>>> +
>>>        index = f->buf_index + offset;
>>>        pending = f->buf_size - index;
>>>    }
>>> @@ -709,6 +741,14 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
>>>    return size;
>>> }
>>> 
>>> +/*
>>> + * Read 'size' bytes of data from the file into buf.
>>> + * 'size' can be larger than the internal buffer.
>>> + *
>>> + * It will return size bytes unless there was an error, in which case it will
>>> + * return as many as it managed to read (assuming blocking fd's which
>>> + * all current QEMUFile are)
>>> + */
>>> int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
>>> {
>>>    int pending = size;
>>> @@ -717,7 +757,7 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
>>>    while (pending > 0) {
>>>        int res;
>>> 
>>> -        res = qemu_peek_buffer(f, buf, pending, 0);
>>> +        res = qemu_peek_buffer(f, buf, MIN(pending, IO_BUF_SIZE), 0);
>>>        if (res == 0) {
>>>            return done;
>>>        }
>>> @@ -729,11 +769,16 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
>>>    return done;
>>> }
>>> 
>>> +/*
>>> + * Peeks a single byte from the buffer; this isn't guaranteed to work if
>>> + * offset leaves a gap after the previous read/peeked data.
>>> + */
>>> int qemu_peek_byte(QEMUFile *f, int offset)
>>> {
>>>    int index = f->buf_index + offset;
>>> 
>>>    assert(!qemu_file_is_writable(f));
>>> +    assert(offset < IO_BUF_SIZE);
>>> 
>>>    if (index >= f->buf_size) {
>>>        qemu_fill_buffer(f);
>>> -- 
>>> 1.9.0
>>> 
>>> 
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data

[Dr. David Alan Gilbert]

* ???? (chenliang0016@icloud.com) wrote:
> 
> > * ???? (chenliang0016@icloud.com) wrote:
> >> 
> >> ?? 2014??4??8????????10:29??Dr. David Alan Gilbert (git) <dgilbert@redhat.com> ??????
> >> 
> >>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >>> 
> >>> Make qemu_peek_buffer repeatedly call fill_buffer until it gets
> >>> all the data it requires, or until there is an error.
> >>> 
> >>> At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
> >>> isn't enough data waiting, however the kernel is entitled to return
> >>> just a few bytes, and still leave qemu_peek_buffer with less bytes
> >>> than it needed.  I've seen this fail in a dev world, and I think it
> >>> could theoretically fail in the peeking of the subsection headers in
> >>> the current world.
> >> hmm, I also have got some errors(infrequently). Maybe It is one point.
> >> Could you show some messages about the error?
> > 
> > I've only seen the errors in my visitor/ber world where I use the peek_buffer
> > a lot more; but the one place it is used in the existing code is in
> > the code to check if we have the start of a subsection; if that goes wrong
> > I'm not sure what error would be produced.
> > 
> > Dave
> In my observation, error is more likely to happen if migration thread is blocked too long.
> BTW, xbzrle and auto convergence are disable.

What error do you see?

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data

[陈梁]

> * ???? (chenliang0016@icloud.com) wrote:
>> 
>>> * ???? (chenliang0016@icloud.com) wrote:
>>>> 
>>>> ?? 2014??4??8????????10:29??Dr. David Alan Gilbert (git) <dgilbert@redhat.com> ??????
>>>> 
>>>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>>>> 
>>>>> Make qemu_peek_buffer repeatedly call fill_buffer until it gets
>>>>> all the data it requires, or until there is an error.
>>>>> 
>>>>> At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
>>>>> isn't enough data waiting, however the kernel is entitled to return
>>>>> just a few bytes, and still leave qemu_peek_buffer with less bytes
>>>>> than it needed.  I've seen this fail in a dev world, and I think it
>>>>> could theoretically fail in the peeking of the subsection headers in
>>>>> the current world.
>>>> hmm, I also have got some errors(infrequently). Maybe It is one point.
>>>> Could you show some messages about the error?
>>> 
>>> I've only seen the errors in my visitor/ber world where I use the peek_buffer
>>> a lot more; but the one place it is used in the existing code is in
>>> the code to check if we have the start of a subsection; if that goes wrong
>>> I'm not sure what error would be produced.
>>> 
>>> Dave
>> In my observation, error is more likely to happen if migration thread is blocked too long.
>> BTW, xbzrle and auto convergence are disable.
> 
> What error do you see?
> 
> Dave
It likes section id 1 load failed at dest side.
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 1/1] Make qemu_peek_buffer loop until it gets it's data

[Dr. David Alan Gilbert]

* ???? (chenliang0016@icloud.com) wrote:
> 
> > * ???? (chenliang0016@icloud.com) wrote:
> >> 
> >>> * ???? (chenliang0016@icloud.com) wrote:
> >>>> 
> >>>> ?? 2014??4??8????????10:29??Dr. David Alan Gilbert (git) <dgilbert@redhat.com> ??????
> >>>> 
> >>>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >>>>> 
> >>>>> Make qemu_peek_buffer repeatedly call fill_buffer until it gets
> >>>>> all the data it requires, or until there is an error.
> >>>>> 
> >>>>> At the moment, qemu_peek_buffer will try one qemu_fill_buffer if there
> >>>>> isn't enough data waiting, however the kernel is entitled to return
> >>>>> just a few bytes, and still leave qemu_peek_buffer with less bytes
> >>>>> than it needed.  I've seen this fail in a dev world, and I think it
> >>>>> could theoretically fail in the peeking of the subsection headers in
> >>>>> the current world.
> >>>> hmm, I also have got some errors(infrequently). Maybe It is one point.
> >>>> Could you show some messages about the error?
> >>> 
> >>> I've only seen the errors in my visitor/ber world where I use the peek_buffer
> >>> a lot more; but the one place it is used in the existing code is in
> >>> the code to check if we have the start of a subsection; if that goes wrong
> >>> I'm not sure what error would be produced.
> >>> 
> >>> Dave
> >> In my observation, error is more likely to happen if migration thread is blocked too long.
> >> BTW, xbzrle and auto convergence are disable.
> > 
> > What error do you see?
> > 
> > Dave
> It likes section id 1 load failed at dest side.

Our migration errors aren't particularly descriptive :-(
However, try it with this patch - I'd be interested if it makes
your problem go away.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK