[PATCH v3 2/5] migration: fix xbzrle encoding rate calculation

Richard Henderson posted 5 patches 5 years, 7 months ago
Maintainers: Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
There is a newer version of this series
[PATCH v3 2/5] migration: fix xbzrle encoding rate calculation
Posted by Richard Henderson 5 years, 7 months ago
From: Wei Wang <wei.w.wang@intel.com>

It's reported an error of implicit conversion from "unsigned long" to
"double" when compiling with Clang 10. Simply make the encoding rate 0
when the encoded_size is 0.

Fixes: e460a4b1a4
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 migration/ram.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 41cc530d9d..069b6e30bc 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -913,10 +913,8 @@ static void migration_update_rates(RAMState *rs, int64_t end_time)
         unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
                          TARGET_PAGE_SIZE;
         encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
-        if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
+        if (xbzrle_counters.pages == rs->xbzrle_pages_prev || !encoded_size) {
             xbzrle_counters.encoding_rate = 0;
-        } else if (!encoded_size) {
-            xbzrle_counters.encoding_rate = UINT64_MAX;
         } else {
             xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
         }
-- 
2.25.1


Re: [PATCH v3 2/5] migration: fix xbzrle encoding rate calculation
Posted by Philippe Mathieu-Daudé 5 years, 7 months ago
On 6/17/20 6:37 AM, Richard Henderson wrote:
> From: Wei Wang <wei.w.wang@intel.com>
> 
> It's reported an error of implicit conversion from "unsigned long" to
> "double" when compiling with Clang 10. Simply make the encoding rate 0
> when the encoded_size is 0.

There is a similar error in tests/qht-bench.c, see the asan log:
https://patchew.org/QEMU/20200617072539.32686-1-f4bug@amsat.org/

/tmp/qemu-test/src/tests/qht-bench.c:287:29: error: implicit conversion
from 'unsigned long' to 'double' changes value from 18446744073709551615
to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion]
        *threshold = rate * UINT64_MAX;
                          ~ ^~~~~~~~~~
/usr/include/stdint.h:130:23: note: expanded from macro 'UINT64_MAX'
# define UINT64_MAX             (__UINT64_C(18446744073709551615))
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/stdint.h:107:25: note: expanded from macro '__UINT64_C'
#  define __UINT64_C(c) c ## UL
                        ^~~~~~~
<scratch space>:33:1: note: expanded from here
18446744073709551615UL
^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: tests/qht-bench.o] Error 1

> 
> Fixes: e460a4b1a4
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  migration/ram.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 41cc530d9d..069b6e30bc 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -913,10 +913,8 @@ static void migration_update_rates(RAMState *rs, int64_t end_time)
>          unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
>                           TARGET_PAGE_SIZE;
>          encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
> -        if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
> +        if (xbzrle_counters.pages == rs->xbzrle_pages_prev || !encoded_size) {
>              xbzrle_counters.encoding_rate = 0;
> -        } else if (!encoded_size) {
> -            xbzrle_counters.encoding_rate = UINT64_MAX;
>          } else {
>              xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
>          }
> 


Re: [PATCH v3 2/5] migration: fix xbzrle encoding rate calculation
Posted by Alex Bennée 5 years, 7 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> From: Wei Wang <wei.w.wang@intel.com>
>
> It's reported an error of implicit conversion from "unsigned long" to
> "double" when compiling with Clang 10. Simply make the encoding rate 0
> when the encoded_size is 0.
>
> Fixes: e460a4b1a4
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée