Re: [PATCH v3] migration/xbzrle: add encoding rate

[Richard Henderson]

On Wed, 29 Apr 2020 at 18:54, Wei Wang <wei.w.wang@intel.com> wrote:
> +        if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
> +            xbzrle_counters.encoding_rate = 0;
> +        } else if (!encoded_size) {
> +            xbzrle_counters.encoding_rate = UINT64_MAX;
> +        } else {
> +            xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
> +        }

With clang 10, this produces

  CC      aarch64-softmmu/migration/ram.o
/home/rth/qemu/qemu/migration/ram.c:919:45: error: implicit conversion
from 'unsigned long' to 'double' changes value from
18446744073709551615 to 18446744073709551616
[-Werror,-Wimplicit-int-float-conversion]
            xbzrle_counters.encoding_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>:36:1: note: expanded from here
18446744073709551615UL
^~~~~~~~~~~~~~~~~~~~~~
1 error generated.

UINT64_MAX apprears both arbitrary and wrong.

The only way I can imagine encoded_size == 0 is unencoded_size == 0,
so 0 seems like the correct answer.  Moreover, it really seems like
the first test sufficiently covers that possibility.

In addition, the only user of this value is

> +        monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
> +                       info->xbzrle_cache->encoding_rate);

which would be quite happy to print NaN even if the 0/0 computation
were to run.  Though as I say above, I don't think that's reachable.


r~