[v1] test_driver: implement virDomainMemoryStats

[libvirt] [PATCH] test_driver: implement virDomainMemoryStats

Posted by Ilias Stamatis 4 years, 9 months ago

Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
---
 src/test/test_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index fcb80c9e47..55976eedf1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -6845,6 +6845,57 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
     return 0;
 }

+
+static int
+testDomainMemoryStats(virDomainPtr dom,
+                      virDomainMemoryStatPtr stats,
+                      unsigned int nr_stats,
+                      unsigned int flags)
+{
+    virDomainObjPtr vm;
+    int ret = -1;
+
+    virCheckFlags(0, -1);
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        return -1;
+
+    if (virDomainObjCheckActive(vm) < 0)
+        goto cleanup;
+
+    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, 1024000);
+        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, 791584);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 977816);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_USABLE, 726244);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 627319920);
+        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 164964);
+        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, 1131076);
+    }
+
+#undef STATS_SET_PARAM
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    return ret;
+}
+
+
 static int
 testDomainMemoryPeek(virDomainPtr dom,
                      unsigned long long start,
@@ -7799,6 +7850,7 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainManagedSave = testDomainManagedSave, /* 1.1.4 */
     .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.4 */
     .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.4 */
+    .domainMemoryStats = testDomainMemoryStats, /* 5.6.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] test_driver: implement virDomainMemoryStats

Posted by Erik Skultety 4 years, 9 months ago

On Tue, Jul 16, 2019 at 11:36:31PM +0200, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> ---
>  src/test/test_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index fcb80c9e47..55976eedf1 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -6845,6 +6845,57 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
>      return 0;
>  }
>
> +
> +static int
> +testDomainMemoryStats(virDomainPtr dom,
> +                      virDomainMemoryStatPtr stats,
> +                      unsigned int nr_stats,
> +                      unsigned int flags)
> +{
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(vm = testDomObjFromDomain(dom)))
> +        return -1;
> +
> +    if (virDomainObjCheckActive(vm) < 0)
> +        goto cleanup;
> +
> +    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, 1024000);

^This one...

> +        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, 791584);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 977816);

...and this one should be taken from the definition from what is reported in
<currentMemory>. The reasoning behind that is that you can have multiple
machines defined and those values wouldn't make sense.
The rest can be semi-random in that the values need to be less than those I
mentioned.

> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_USABLE, 726244);
> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 627319920);

I don't object to ^this unless it is affected by balloon period setting. If it
is, then it might make sense to take it into account. I haven't checked that
myself though, so it may simply not be worth anyway.

Erik

> +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 164964);
> +        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, 1131076);
> +    }
> +
> +#undef STATS_SET_PARAM
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    return ret;
> +}
> +
> +
>  static int
>  testDomainMemoryPeek(virDomainPtr dom,
>                       unsigned long long start,
> @@ -7799,6 +7850,7 @@ static virHypervisorDriver testHypervisorDriver = {
>      .domainManagedSave = testDomainManagedSave, /* 1.1.4 */
>      .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.4 */
>      .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.4 */
> +    .domainMemoryStats = testDomainMemoryStats, /* 5.6.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

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

Re: [libvirt] [PATCH] test_driver: implement virDomainMemoryStats

Posted by Ilias Stamatis 4 years, 9 months ago

On Wed, Jul 24, 2019 at 4:19 PM Erik Skultety <eskultet@redhat.com> wrote:
>
> On Tue, Jul 16, 2019 at 11:36:31PM +0200, Ilias Stamatis wrote:
> > Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> > ---
> >  src/test/test_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index fcb80c9e47..55976eedf1 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -6845,6 +6845,57 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
> >      return 0;
> >  }
> >
> > +
> > +static int
> > +testDomainMemoryStats(virDomainPtr dom,
> > +                      virDomainMemoryStatPtr stats,
> > +                      unsigned int nr_stats,
> > +                      unsigned int flags)
> > +{
> > +    virDomainObjPtr vm;
> > +    int ret = -1;
> > +
> > +    virCheckFlags(0, -1);
> > +
> > +    if (!(vm = testDomObjFromDomain(dom)))
> > +        return -1;
> > +
> > +    if (virDomainObjCheckActive(vm) < 0)
> > +        goto cleanup;
> > +
> > +    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, 1024000);
>
> ^This one...
>
> > +        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, 791584);
> > +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 977816);
>
> ...and this one should be taken from the definition from what is reported in
> <currentMemory>. The reasoning behind that is that you can have multiple
> machines defined and those values wouldn't make sense.

Right, I'll fix that.

> The rest can be semi-random in that the values need to be less than those I
> mentioned.

My concern here again is that we will have to send different values
every time depending on the config. E.g. currentMemory - 45000.

But is that good for testing?

Ilias

>
> > +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_USABLE, 726244);
> > +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE, 627319920);
>
> I don't object to ^this unless it is affected by balloon period setting. If it
> is, then it might make sense to take it into account. I haven't checked that
> myself though, so it may simply not be worth anyway.
>
> Erik
>
> > +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_DISK_CACHES, 164964);
> > +        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, 1131076);
> > +    }
> > +
> > +#undef STATS_SET_PARAM
> > +
> > + cleanup:
> > +    virDomainObjEndAPI(&vm);
> > +    return ret;
> > +}
> > +
> > +
> >  static int
> >  testDomainMemoryPeek(virDomainPtr dom,
> >                       unsigned long long start,
> > @@ -7799,6 +7850,7 @@ static virHypervisorDriver testHypervisorDriver = {
> >      .domainManagedSave = testDomainManagedSave, /* 1.1.4 */
> >      .domainHasManagedSaveImage = testDomainHasManagedSaveImage, /* 1.1.4 */
> >      .domainManagedSaveRemove = testDomainManagedSaveRemove, /* 1.1.4 */
> > +    .domainMemoryStats = testDomainMemoryStats, /* 5.6.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

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

Re: [libvirt] [PATCH] test_driver: implement virDomainMemoryStats

Posted by Erik Skultety 4 years, 8 months ago

On Fri, Jul 26, 2019 at 03:47:55PM +0200, Ilias Stamatis wrote:
> On Wed, Jul 24, 2019 at 4:19 PM Erik Skultety <eskultet@redhat.com> wrote:
> >
> > On Tue, Jul 16, 2019 at 11:36:31PM +0200, Ilias Stamatis wrote:
> > > Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> > > ---
> > >  src/test/test_driver.c | 52 ++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 52 insertions(+)
> > >
> > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > > index fcb80c9e47..55976eedf1 100644
> > > --- a/src/test/test_driver.c
> > > +++ b/src/test/test_driver.c
> > > @@ -6845,6 +6845,57 @@ testDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags)
> > >      return 0;
> > >  }
> > >
> > > +
> > > +static int
> > > +testDomainMemoryStats(virDomainPtr dom,
> > > +                      virDomainMemoryStatPtr stats,
> > > +                      unsigned int nr_stats,
> > > +                      unsigned int flags)
> > > +{
> > > +    virDomainObjPtr vm;
> > > +    int ret = -1;
> > > +
> > > +    virCheckFlags(0, -1);
> > > +
> > > +    if (!(vm = testDomObjFromDomain(dom)))
> > > +        return -1;
> > > +
> > > +    if (virDomainObjCheckActive(vm) < 0)
> > > +        goto cleanup;
> > > +
> > > +    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, 1024000);
> >
> > ^This one...
> >
> > > +        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, 791584);
> > > +        STATS_SET_PARAM(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 977816);
> >
> > ...and this one should be taken from the definition from what is reported in
> > <currentMemory>. The reasoning behind that is that you can have multiple
> > machines defined and those values wouldn't make sense.
>
> Right, I'll fix that.
>
> > The rest can be semi-random in that the values need to be less than those I
> > mentioned.
>
> My concern here again is that we will have to send different values
> every time depending on the config. E.g. currentMemory - 45000.
>
> But is that good for testing?

Yes, because it's deterministic, it's based on the config value, if you don't
change the config in your test environment, the return values will always be
the same and will keep it stable.

Erik

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