[Qemu-devel] [PATCH] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Qemu-devel] [PATCH] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Wangting (Kathy)]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

In qemu_put_buffer(), bytes_xfer += size is wrong, it will be more than expected, and should be bytes_xfer += l.

Signed-off-by: zhangmin <zhangmin6@huawei.com<mailto:zhangmin6@huawei.com>>
---
savevm.c |    2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/savevm.c b/savevm.c
index 2f631d4..3f912dd 100644
--- a/savevm.c
+++ b/savevm.c
@@ -794,7 +794,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
         if (l > size)
             l = size;
         memcpy(f->buf + f->buf_index, buf, l);
-        f->bytes_xfer += size;
+        f->bytes_xfer += l;
         if (f->ops->writev_buffer) {
             add_to_iovec(f, f->buf + f->buf_index, l);
         }
--
1.7.3.1.msysgit.0

[-- Attachment #2: Type: text/html, Size: 9146 bytes --]

Re: [Qemu-devel] [PATCH] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Paolo Bonzini]

Il 19/11/2013 06:53, Wangting (Kathy) ha scritto:
> In qemu_put_buffer(), bytes_xfer += size is wrong,it will be more than
> expected,and should be bytes_xfer += l.
> 
> Signed-off-by: zhangmin <zhangmin6@huawei.com>
> ---
> savevm.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index 2f631d4..3f912dd 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -794,7 +794,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>          if (l > size)
>              l = size;
>          memcpy(f->buf + f->buf_index, buf, l);
> -        f->bytes_xfer += size;
> +        f->bytes_xfer += l;
>          if (f->ops->writev_buffer) {
>              add_to_iovec(f, f->buf + f->buf_index, l);
>          }
> -- 
> 1.7.3.1.msysgit.0

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH for-1.7?] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Stefan Weil]

Am 19.11.2013 09:19, schrieb Paolo Bonzini:
> Il 19/11/2013 06:53, Wangting (Kathy) ha scritto:
>> In qemu_put_buffer(), bytes_xfer += size is wrong,it will be more than
>> expected,and should be bytes_xfer += l.
>>
>> Signed-off-by: zhangmin <zhangmin6@huawei.com>
>> ---
>> savevm.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/savevm.c b/savevm.c
>> index 2f631d4..3f912dd 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -794,7 +794,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>>          if (l > size)
>>              l = size;
>>          memcpy(f->buf + f->buf_index, buf, l);
>> -        f->bytes_xfer += size;
>> +        f->bytes_xfer += l;
>>          if (f->ops->writev_buffer) {
>>              add_to_iovec(f, f->buf + f->buf_index, l);
>>          }
>> -- 
>> 1.7.3.1.msysgit.0
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>


Should this patch be included in QEMU 1.7? It's obviously a bug fix, so
I assume yes.

Re: [Qemu-devel] [PATCH for-1.7?] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Paolo Bonzini]

Il 19/11/2013 18:55, Stefan Weil ha scritto:
>>> >> diff --git a/savevm.c b/savevm.c
>>> >> index 2f631d4..3f912dd 100644
>>> >> --- a/savevm.c
>>> >> +++ b/savevm.c
>>> >> @@ -794,7 +794,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>>> >>          if (l > size)
>>> >>              l = size;
>>> >>          memcpy(f->buf + f->buf_index, buf, l);
>>> >> -        f->bytes_xfer += size;
>>> >> +        f->bytes_xfer += l;
>>> >>          if (f->ops->writev_buffer) {
>>> >>              add_to_iovec(f, f->buf + f->buf_index, l);
>>> >>          }
>>> >> -- 
>>> >> 1.7.3.1.msysgit.0
>> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Should this patch be included in QEMU 1.7? It's obviously a bug fix, so
> I assume yes.
> 

The committer didn't say what it fixes or whether it fixes anything.
But I'd guess it is something related to block migration.  In that case
yes, it should be included.

Paolo

Re: [Qemu-devel] [PATCH for-1.7?] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Wangting (Kathy)]

Hi Paolo and Stefan,

I am really sorry for my email before that I didn't say clearly about the bug fix.

f->bytes_xfer means the number of bytes which is send from source to destination during the migration. It is limited by the f->xfer_limit which is converted from bandwidth. That means if bytes_xfer is larger than the limit, the current iteration will not do  block and memory migration util the next iteration.

But in function qemu_put_buffer, the statistics is wrong. The size of block migration is 1MB for each block, but the buffer size for sending is IO_BUF_SIZE(32768bytes). When it need to be send, 1MB block will be divided into small trunks, so f->bytes_xfer should be increased by the size of the small trunks every time which is actually send, in function qemu_put_buffer it should be the variable "l". 

Otherwise, f->bytes_xfer will be increased by (small trunks number * 1MB) while sending 1MB block, it is much larger than the size which is actually send, and it will soon be reached to the size of f->xfer_limit, so the current iteration will send less data blocks than expected. 

If you have any questions, please let me know, thank you very much.

Best wishes.

Ting

-----邮件原件-----
发件人: Paolo Bonzini [mailto:pbonzini@redhat.com] 
发送时间: 2013年11月20日 2:08
收件人: Stefan Weil
抄送: Wangting (Kathy); zhangmin (S); Luonengjun; qemu-devel@nongnu.org; Qinling; Chentao (Boby); Wangrui (K); Wubin (H); Anthony Liguori
主题: Re: [Qemu-devel] [PATCH for-1.7?] the calculation of bytes_xfer in qemu_put_buffer() is wrong

Il 19/11/2013 18:55, Stefan Weil ha scritto:
>>> >> diff --git a/savevm.c b/savevm.c
>>> >> index 2f631d4..3f912dd 100644
>>> >> --- a/savevm.c
>>> >> +++ b/savevm.c
>>> >> @@ -794,7 +794,7 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>>> >>          if (l > size)
>>> >>              l = size;
>>> >>          memcpy(f->buf + f->buf_index, buf, l);
>>> >> -        f->bytes_xfer += size;
>>> >> +        f->bytes_xfer += l;
>>> >>          if (f->ops->writev_buffer) {
>>> >>              add_to_iovec(f, f->buf + f->buf_index, l);
>>> >>          }
>>> >> --
>>> >> 1.7.3.1.msysgit.0
>> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Should this patch be included in QEMU 1.7? It's obviously a bug fix, 
> so I assume yes.
> 

The committer didn't say what it fixes or whether it fixes anything.
But I'd guess it is something related to block migration.  In that case yes, it should be included.

Paolo

Re: [Qemu-devel] [PATCH for-1.7?] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Paolo Bonzini]

Il 20/11/2013 06:28, Wangting (Kathy) ha scritto:
> Hi Paolo and Stefan,
> 
> I am really sorry for my email before that I didn't say clearly about
> the bug fix.
> 
> f->bytes_xfer means the number of bytes which is send from source to
> destination during the migration. It is limited by the f->xfer_limit
> which is converted from bandwidth. That means if bytes_xfer is larger
> than the limit, the current iteration will not do  block and memory
> migration util the next iteration.
> 
> But in function qemu_put_buffer, the statistics is wrong. The size of
> block migration is 1MB for each block, but the buffer size for
> sending is IO_BUF_SIZE(32768bytes). When it need to be send, 1MB
> block will be divided into small trunks, so f->bytes_xfer should be
> increased by the size of the small trunks every time which is
> actually send, in function qemu_put_buffer it should be the variable
> "l".
> 
> Otherwise, f->bytes_xfer will be increased by (small trunks number *
> 1MB) while sending 1MB block, it is much larger than the size which
> is actually send, and it will soon be reached to the size of
> f->xfer_limit, so the current iteration will send less data blocks
> than expected.
> 
> If you have any questions, please let me know, thank you very much.

Thank you very much.  Juan, I guess you need to prepare another pull
request.

Paolo

Re: [Qemu-devel] [PATCH] the calculation of bytes_xfer in qemu_put_buffer() is wrong

[Juan Quintela]

"Wangting (Kathy)" <kathy.wangting@huawei.com> wrote:
> In qemu_put_buffer(), bytes_xfer += size is wrong, it will be more
> than expected, and should be bytes_xfer += l.
>
> Signed-off-by: zhangmin <zhangmin6@huawei.com> 
>

Good spot.
Applied and sent pull request to Anthony.