[libvirt] [PATCH 1/5] util, resctrl: using 64bit interface instead of 32bit for counters

Daniel P. Berrangé berrange at redhat.com
Fri Dec 13 15:00:49 UTC 2019


On Thu, Nov 14, 2019 at 01:08:19AM +0800, Wang Huaqiang wrote:
> From: Huaqiang <huaqiang.wang at intel.com>
> 
> The underlying resctrl monitoring is actually using 64 bit counters,
> not the 32bit one. Correct this by using 64bit interfaces.
> 
> Signed-off-by: Huaqiang <huaqiang.wang at intel.com>
> ---
>  src/qemu/qemu_driver.c |  4 ++--
>  src/util/virfile.c     | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/util/virfile.h     |  2 ++
>  src/util/virresctrl.c  |  6 +++---
>  src/util/virresctrl.h  |  2 +-
>  5 files changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f4ff2ba292..e396358871 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20587,8 +20587,8 @@ qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver,
>                                           "cpu.cache.monitor.%zu.bank.%zu.id", i, j) < 0)
>                  goto cleanup;
>  
> -            if (virTypedParamListAddUInt(params, resdata[i]->stats[j]->vals[0],
> -                                         "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j) < 0)
> +            if (virTypedParamListAddULLong(params, resdata[i]->stats[j]->vals[0],
> +                                           "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j) < 0)
>                  goto cleanup;
>          }
>      }

Urgh, we cannot do this, as it changes API semantics for applications.

Apps are expecting this field to be encoded as UInt & so the change
will break their decoding.

Is this 32 vs 64-bit difference actually a problem in the real world.

eg can the 32-bit value genuinely overflow in real deployments of
this feature ?


If not, then we should not change this at all.



If we do need to change this though, the only option is to leave the
current field unchanged, and document that it can be truncated.

Then introduce a new field with a different name

eg   cpu.cache.monitor.%zu.bank.%zu.bytes64

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list