1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] qemu: add entry for balloon stat stat-disk-caches

Tomáš Golembiovský posted 1 patch 5 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/59402afcb0b4f436837e99b62e7097dbaee741e7.1528198855.git.tgolembi@redhat.com
Test syntax-check failed
There is a newer version of this series
include/libvirt/libvirt-domain.h | 9 ++++++++-
src/qemu/qemu_monitor_json.c     | 2 ++
2 files changed, 10 insertions(+), 1 deletion(-)
[libvirt] [PATCH] qemu: add entry for balloon stat stat-disk-caches
Posted by Tomáš Golembiovský 5 years, 11 months ago 
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 include/libvirt/libvirt-domain.h | 9 ++++++++-
 src/qemu/qemu_monitor_json.c     | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index da773b76cb..b96c018a90 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -628,11 +628,18 @@ typedef enum {
     /* Timestamp of the last update of statistics, in seconds. */
     VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE     = 9,
 
+    /*
+     * The amount of memory, in bytes, that can be quickly reclaimed without
+     * additional I/O. Typically these pages are used for caching files from
+     * disk.
+     */
+    VIR_DOMAIN_MEMORY_STAT_DISK_CACHES     = 10,
+
     /*
      * The number of statistics supported by this version of the interface.
      * To add new statistics, add them to the enum and increase this value.
      */
-    VIR_DOMAIN_MEMORY_STAT_NR              = 10,
+    VIR_DOMAIN_MEMORY_STAT_NR              = 11,
 
 # ifdef VIR_ENUM_SENTINELS
     VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 42d7b9c5e9..b0a65d8af9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2069,6 +2069,8 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
                       VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 1024);
     GET_BALLOON_STATS(statsdata, "stat-available-memory",
                       VIR_DOMAIN_MEMORY_STAT_USABLE, 1024);
+    GET_BALLOON_STATS(statsdata, "stat-disk-caches",
+                      VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 1024);
     GET_BALLOON_STATS(data, "last-update",
                       VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 1);
     ret = got;
-- 
2.17.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add entry for balloon stat stat-disk-caches
Posted by Erik Skultety 5 years, 11 months ago 
On Tue, Jun 05, 2018 at 01:41:02PM +0200, Tomáš Golembiovský wrote:
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  include/libvirt/libvirt-domain.h | 9 ++++++++-
>  src/qemu/qemu_monitor_json.c     | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index da773b76cb..b96c018a90 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -628,11 +628,18 @@ typedef enum {
>      /* Timestamp of the last update of statistics, in seconds. */
>      VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE     = 9,
>
> +    /*
> +     * The amount of memory, in bytes, that can be quickly reclaimed without
> +     * additional I/O. Typically these pages are used for caching files from
> +     * disk.
> +     */
> +    VIR_DOMAIN_MEMORY_STAT_DISK_CACHES     = 10,
> +
>      /*
>       * The number of statistics supported by this version of the interface.
>       * To add new statistics, add them to the enum and increase this value.
>       */
> -    VIR_DOMAIN_MEMORY_STAT_NR              = 10,
> +    VIR_DOMAIN_MEMORY_STAT_NR              = 11,

this is a public header, this must never change, otherwise you break backwards
compatibility...

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add entry for balloon stat stat-disk-caches
Posted by Michal Privoznik 5 years, 11 months ago 
On 06/05/2018 03:08 PM, Erik Skultety wrote:
> On Tue, Jun 05, 2018 at 01:41:02PM +0200, Tomáš Golembiovský wrote:
>> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
>> ---
>>  include/libvirt/libvirt-domain.h | 9 ++++++++-
>>  src/qemu/qemu_monitor_json.c     | 2 ++
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>> index da773b76cb..b96c018a90 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -628,11 +628,18 @@ typedef enum {
>>      /* Timestamp of the last update of statistics, in seconds. */
>>      VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE     = 9,
>>
>> +    /*
>> +     * The amount of memory, in bytes, that can be quickly reclaimed without
>> +     * additional I/O. Typically these pages are used for caching files from
>> +     * disk.
>> +     */
>> +    VIR_DOMAIN_MEMORY_STAT_DISK_CACHES     = 10,
>> +
>>      /*
>>       * The number of statistics supported by this version of the interface.
>>       * To add new statistics, add them to the enum and increase this value.
>>       */
>> -    VIR_DOMAIN_MEMORY_STAT_NR              = 10,
>> +    VIR_DOMAIN_MEMORY_STAT_NR              = 11,
> 
> this is a public header, this must never change, otherwise you break backwards
> compatibility...

Not true. This is meant to work roughly like this:

  virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];

  nr_stats = virDomainMemoryStats(dom, stats, VIR_DOMAIN_MEMORY_STAT_NR, 0);

And depending what version you are compiled with you will get different
number of results. Important to say that to maintain backward
compatibility we have a rule that says _NR can only grow and never
shrink. Tomas' patch is actually correct (in this aspect).
View examples:

200a40f94ec9427eb7187d9d5396ad3a3f2925c8
65bf044686c7502ba16f1bee5763fd3e448994fd


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add entry for balloon stat stat-disk-caches
Posted by Erik Skultety 5 years, 11 months ago 
On Tue, Jun 05, 2018 at 03:36:14PM +0200, Michal Privoznik wrote:
> On 06/05/2018 03:08 PM, Erik Skultety wrote:
> > On Tue, Jun 05, 2018 at 01:41:02PM +0200, Tomáš Golembiovský wrote:
> >> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> >> ---
> >>  include/libvirt/libvirt-domain.h | 9 ++++++++-
> >>  src/qemu/qemu_monitor_json.c     | 2 ++
> >>  2 files changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> >> index da773b76cb..b96c018a90 100644
> >> --- a/include/libvirt/libvirt-domain.h
> >> +++ b/include/libvirt/libvirt-domain.h
> >> @@ -628,11 +628,18 @@ typedef enum {
> >>      /* Timestamp of the last update of statistics, in seconds. */
> >>      VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE     = 9,
> >>
> >> +    /*
> >> +     * The amount of memory, in bytes, that can be quickly reclaimed without
> >> +     * additional I/O. Typically these pages are used for caching files from
> >> +     * disk.
> >> +     */
> >> +    VIR_DOMAIN_MEMORY_STAT_DISK_CACHES     = 10,
> >> +
> >>      /*
> >>       * The number of statistics supported by this version of the interface.
> >>       * To add new statistics, add them to the enum and increase this value.
> >>       */
> >> -    VIR_DOMAIN_MEMORY_STAT_NR              = 10,
> >> +    VIR_DOMAIN_MEMORY_STAT_NR              = 11,
> >
> > this is a public header, this must never change, otherwise you break backwards
> > compatibility...
>
> Not true. This is meant to work roughly like this:
>
>   virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
>
>   nr_stats = virDomainMemoryStats(dom, stats, VIR_DOMAIN_MEMORY_STAT_NR, 0);
>
> And depending what version you are compiled with you will get different
> number of results. Important to say that to maintain backward
> compatibility we have a rule that says _NR can only grow and never
> shrink. Tomas' patch is actually correct (in this aspect).
> View examples:

Doh! If only I read the commentary right above the enum it would tell me the
very same thing, I have to admit, I didn't even read properly, I saw a value
being changed and I immediately responded, sorry, I take it back.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add entry for balloon stat stat-disk-caches
Posted by Michal Privoznik 5 years, 11 months ago 
On 06/05/2018 01:41 PM, Tomáš Golembiovský (by way of Tomáš Golembiovský
<tgolembi@redhat.com>) wrote:
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  include/libvirt/libvirt-domain.h | 9 ++++++++-
>  src/qemu/qemu_monitor_json.c     | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index da773b76cb..b96c018a90 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -628,11 +628,18 @@ typedef enum {
>      /* Timestamp of the last update of statistics, in seconds. */
>      VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE     = 9,
>  
> +    /*
> +     * The amount of memory, in bytes, that can be quickly reclaimed without
> +     * additional I/O. Typically these pages are used for caching files from
> +     * disk.
> +     */
> +    VIR_DOMAIN_MEMORY_STAT_DISK_CACHES     = 10,
> +
>      /*
>       * The number of statistics supported by this version of the interface.
>       * To add new statistics, add them to the enum and increase this value.
>       */
> -    VIR_DOMAIN_MEMORY_STAT_NR              = 10,
> +    VIR_DOMAIN_MEMORY_STAT_NR              = 11,
>  
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 42d7b9c5e9..b0a65d8af9 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2069,6 +2069,8 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
>                        VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 1024);
>      GET_BALLOON_STATS(statsdata, "stat-available-memory",
>                        VIR_DOMAIN_MEMORY_STAT_USABLE, 1024);
> +    GET_BALLOON_STATS(statsdata, "stat-disk-caches",
> +                      VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 1024);
>      GET_BALLOON_STATS(data, "last-update",
>                        VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 1);
>      ret = got;
> 

No, this must go at the end because if there's a client requesting 9
items, they would get "last-update" as the 9th item. But with this
change they would no longer get "last-update" rather "stat-disk-caches".
IOW, you need to honour the number of stat item from the header file.

Also, it would be nice if virsh reports this new stat.

Michal

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.