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

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

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

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

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

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
participants (3)
-
Erik Skultety
-
Michal Privoznik
-
Tomáš Golembiovský