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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 14, 2019 at 01:08:19AM +0800, Wang Huaqiang wrote:
> From: Huaqiang <huaqiang.wang@xxxxxxxxx>
> 
> 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@xxxxxxxxx>
> ---
>  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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux