> * ???? (chenliang0...@icloud.com) wrote:
>>
>> ?? 2014??4??8????????10:29??Dr. David Alan Gilbert (git)
>> <dgilb...@redhat.com> ??????
>>
>>> From: "Dr. David Alan Gilbert" <dgilb...@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 <chenliang0...@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 <dgilb...@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 / dgilb...@redhat.com / Manchester, UK
