[libvirt] [PATCH v2] test_driver: implement virDomainMemoryStats

Ilias Stamatis posted 1 patch 4 years, 8 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190801220719.10858-1-stamatis.iliass@gmail.com
There is a newer version of this series
src/test/test_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
[libvirt] [PATCH v2] test_driver: implement virDomainMemoryStats
Posted by Ilias Stamatis 4 years, 8 months ago
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
---
 src/test/test_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index aae9875194..0e0acf2baf 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7367,6 +7367,59 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
     return 0;
 }

+
+static int
+testDomainMemoryStats(virDomainPtr dom,
+                      virDomainMemoryStatPtr stats,
+                      unsigned int nr_stats,
+                      unsigned int flags)
+{
+    virDomainObjPtr vm = NULL;
+    int cur_memory;
+    int ret = -1;
+
+    virCheckFlags(0, -1);
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        return -1;
+
+    if (virDomainObjCheckActive(vm) < 0)
+        goto cleanup;
+
+    cur_memory = vm->def->mem.cur_balloon;
+    ret = 0;
+
+#define STATS_SET_PARAM(name, value) \
+    if (ret < nr_stats) { \
+        stats[ret].tag = name; \
+        stats[ret].val = value; \
+        ret++; \
+    }
+
+    if (virDomainDefHasMemballoon(vm->def)) {
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, cur_memory);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_SWAP_IN, 0);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, 0);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, 0);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, 0);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_UNUSED, cur_memory / 2);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, cur_memory);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_USABLE, cur_memory / 2);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 627319920);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, cur_memory / 10);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC, 0);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL, 0);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_RSS, cur_memory);
+    }
+
+#undef STATS_SET_PARAM
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
+
+
 static int
 testDomainMemoryPeek(virDomainPtr dom,
                      unsigned long long start,
@@ -8697,6 +8750,7 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainManagedSave = testDomainManagedSave, /* 1.1.4 */
     .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.4 */
     .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.4 */
+    .domainMemoryStats = testDomainMemoryStats, /* 5.7.0 */
     .domainMemoryPeek = testDomainMemoryPeek, /* 5.4.0 */

     .domainSnapshotNum = testDomainSnapshotNum, /* 1.1.4 */
--
2.22.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] test_driver: implement virDomainMemoryStats
Posted by Erik Skultety 4 years, 8 months ago
On Fri, Aug 02, 2019 at 12:07:19AM +0200, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> ---
>  src/test/test_driver.c | 54 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index aae9875194..0e0acf2baf 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -7367,6 +7367,59 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
>      return 0;
>  }
>
> +
> +static int
> +testDomainMemoryStats(virDomainPtr dom,
> +                      virDomainMemoryStatPtr stats,
> +                      unsigned int nr_stats,
> +                      unsigned int flags)
> +{
> +    virDomainObjPtr vm = NULL;
> +    int cur_memory;
> +    int ret = -1;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(vm = testDomObjFromDomain(dom)))
> +        return -1;
> +
> +    if (virDomainObjCheckActive(vm) < 0)
> +        goto cleanup;
> +
> +    cur_memory = vm->def->mem.cur_balloon;
> +    ret = 0;
> +
> +#define STATS_SET_PARAM(name, value) \
> +    if (ret < nr_stats) { \
> +        stats[ret].tag = name; \
> +        stats[ret].val = value; \
> +        ret++; \
> +    }
> +
> +    if (virDomainDefHasMemballoon(vm->def)) {
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, cur_memory);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_SWAP_IN, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_UNUSED, cur_memory / 2);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, cur_memory);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_USABLE, cur_memory / 2);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 627319920);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, cur_memory / 10);

Let's divide by 4 or 8 ^here, compiler is usually smart enough with such
divisions and prefers bitwise operations instead.

> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGALLOC, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_HUGETLB_PGFAIL, 0);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_RSS, cur_memory);

I'd divide here by 2 ^here as well, since it's a bit odd that the residential
memory of the process equals it's current balloon and yet it still has some
usable memory.

It would also be beneficial if we introduced a default memballoon device in the
test config, otherwise this API returns nothing.

Since we're still in the freeze, I suggest you re-spin the series with a patch
introducing the memballoon device and adjusting according to my comments above,
still, you can consider my:

Reviewed-by: Erik Skultety <eskultet@redhat.com>

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