1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] test_driver: virDomainGetPerfEvents

Ilias Stamatis posted 1 patch 4 years, 10 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190628161501.10344-1-stamatis.iliass@gmail.com
src/test/test_driver.c | 48 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
[libvirt] [PATCH] test_driver: virDomainGetPerfEvents
Posted by Ilias Stamatis 4 years, 10 months ago 
Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
---
 src/test/test_driver.c | 48 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4b1f2724a0..215171839c 100755
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3293,6 +3293,53 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
     return ret;
 }

+
+static int
+testDomainGetPerfEvents(virDomainPtr dom,
+                        virTypedParameterPtr *params,
+                        int *nparams,
+                        unsigned int flags)
+{
+    virDomainObjPtr vm = NULL;
+    virDomainDefPtr def = NULL;
+    virTypedParameterPtr par = NULL;
+    size_t i;
+    int maxpar = 0;
+    int npar = 0;
+    int ret = -1;
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG |
+                  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+    if (!(vm = testDomObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
+        goto cleanup;
+
+    if (!(def = virDomainObjGetOneDef(vm, flags)))
+        goto cleanup;
+
+    for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
+        if (virTypedParamsAddBoolean(&par, &npar, &maxpar,
+                                     virPerfEventTypeToString(i),
+                                     def->perf.events[i] == VIR_TRISTATE_BOOL_YES) < 0)
+            goto cleanup;
+    }
+
+    VIR_STEAL_PTR(*params, par);
+    *nparams = npar;
+    npar = 0;
+
+    ret = 0;
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    virTypedParamsFree(par, npar);
+    return ret;
+}
+
+
 static char *testDomainGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED,
                                         int *nparams)
 {
@@ -7287,6 +7334,7 @@ static virHypervisorDriver testHypervisorDriver = {
     .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
     .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
     .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */
+    .domainGetPerfEvents = testDomainGetPerfEvents, /* 5.6.0 */
     .domainGetSchedulerType = testDomainGetSchedulerType, /* 0.3.2 */
     .domainGetSchedulerParameters = testDomainGetSchedulerParameters, /* 0.3.2 */
     .domainGetSchedulerParametersFlags = testDomainGetSchedulerParametersFlags, /* 0.9.2 */
--
2.22.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: virDomainGetPerfEvents
Posted by Ilias Stamatis 4 years, 10 months ago 
On Fri, Jun 28, 2019 at 6:15 PM Ilias Stamatis
<stamatis.iliass@gmail.com> wrote:
>
> Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> ---
>  src/test/test_driver.c | 48 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 4b1f2724a0..215171839c 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3293,6 +3293,53 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
>      return ret;
>  }
>
> +
> +static int
> +testDomainGetPerfEvents(virDomainPtr dom,
> +                        virTypedParameterPtr *params,
> +                        int *nparams,
> +                        unsigned int flags)
> +{
> +    virDomainObjPtr vm = NULL;
> +    virDomainDefPtr def = NULL;
> +    virTypedParameterPtr par = NULL;
> +    size_t i;
> +    int maxpar = 0;
> +    int npar = 0;
> +    int ret = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG |
> +                  VIR_TYPED_PARAM_STRING_OKAY, -1);
> +
> +    if (!(vm = testDomObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> +        goto cleanup;

Actually calling this function here is redundant since it is called
also by virDomainObjGetOneDef just in the next line. So it can be
removed before merging.

This redundant call is also done in the implementation of the same API
from the QEMU driver, so it could probably be safely removed from
there as well.

> +
> +    if (!(def = virDomainObjGetOneDef(vm, flags)))
> +        goto cleanup;
> +
> +    for (i = 0; i < VIR_PERF_EVENT_LAST; i++) {
> +        if (virTypedParamsAddBoolean(&par, &npar, &maxpar,
> +                                     virPerfEventTypeToString(i),
> +                                     def->perf.events[i] == VIR_TRISTATE_BOOL_YES) < 0)
> +            goto cleanup;
> +    }
> +
> +    VIR_STEAL_PTR(*params, par);
> +    *nparams = npar;
> +    npar = 0;
> +
> +    ret = 0;
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    virTypedParamsFree(par, npar);
> +    return ret;
> +}
> +
> +
>  static char *testDomainGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED,
>                                          int *nparams)
>  {
> @@ -7287,6 +7334,7 @@ static virHypervisorDriver testHypervisorDriver = {
>      .domainGetAutostart = testDomainGetAutostart, /* 0.3.2 */
>      .domainSetAutostart = testDomainSetAutostart, /* 0.3.2 */
>      .domainGetDiskErrors = testDomainGetDiskErrors, /* 5.4.0 */
> +    .domainGetPerfEvents = testDomainGetPerfEvents, /* 5.6.0 */
>      .domainGetSchedulerType = testDomainGetSchedulerType, /* 0.3.2 */
>      .domainGetSchedulerParameters = testDomainGetSchedulerParameters, /* 0.3.2 */
>      .domainGetSchedulerParametersFlags = testDomainGetSchedulerParametersFlags, /* 0.9.2 */
> --
> 2.22.0
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: virDomainGetPerfEvents
Posted by Erik Skultety 4 years, 9 months ago 
On Fri, Jun 28, 2019 at 09:38:30PM +0200, Ilias Stamatis wrote:
> On Fri, Jun 28, 2019 at 6:15 PM Ilias Stamatis
> <stamatis.iliass@gmail.com> wrote:

the subject should say "Implement <function>"

> >
> > Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> > ---
> >  src/test/test_driver.c | 48 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 4b1f2724a0..215171839c 100755
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -3293,6 +3293,53 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
> >      return ret;
> >  }
> >
> > +
> > +static int
> > +testDomainGetPerfEvents(virDomainPtr dom,
> > +                        virTypedParameterPtr *params,
> > +                        int *nparams,
> > +                        unsigned int flags)
> > +{
> > +    virDomainObjPtr vm = NULL;
> > +    virDomainDefPtr def = NULL;
> > +    virTypedParameterPtr par = NULL;
> > +    size_t i;
> > +    int maxpar = 0;
> > +    int npar = 0;
> > +    int ret = -1;
> > +
> > +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> > +                  VIR_DOMAIN_AFFECT_CONFIG |
> > +                  VIR_TYPED_PARAM_STRING_OKAY, -1);
> > +
> > +    if (!(vm = testDomObjFromDomain(dom)))
> > +        goto cleanup;
> > +
> > +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> > +        goto cleanup;
>
> Actually calling this function here is redundant since it is called
> also by virDomainObjGetOneDef just in the next line. So it can be
> removed before merging.

Hmm, good catch, looking at the code I don't see a reason for it either. I'll
drop the call before merging.

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

>
> This redundant call is also done in the implementation of the same API
> from the QEMU driver, so it could probably be safely removed from
> there as well.

Care to send a patch for QEMU as well?

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: virDomainGetPerfEvents
Posted by Ilias Stamatis 4 years, 9 months ago 
On Mon, Jul 1, 2019 at 3:55 PM Erik Skultety <eskultet@redhat.com> wrote:
>
> On Fri, Jun 28, 2019 at 09:38:30PM +0200, Ilias Stamatis wrote:
> > On Fri, Jun 28, 2019 at 6:15 PM Ilias Stamatis
> > <stamatis.iliass@gmail.com> wrote:
>
> the subject should say "Implement <function>"

Right. I do that always, just this time it slipped.
>
> > >
> > > Signed-off-by: Ilias Stamatis <stamatis.iliass@gmail.com>
> > > ---
> > >  src/test/test_driver.c | 48 ++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 48 insertions(+)
> > >
> > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > > index 4b1f2724a0..215171839c 100755
> > > --- a/src/test/test_driver.c
> > > +++ b/src/test/test_driver.c
> > > @@ -3293,6 +3293,53 @@ static int testDomainGetDiskErrors(virDomainPtr dom,
> > >      return ret;
> > >  }
> > >
> > > +
> > > +static int
> > > +testDomainGetPerfEvents(virDomainPtr dom,
> > > +                        virTypedParameterPtr *params,
> > > +                        int *nparams,
> > > +                        unsigned int flags)
> > > +{
> > > +    virDomainObjPtr vm = NULL;
> > > +    virDomainDefPtr def = NULL;
> > > +    virTypedParameterPtr par = NULL;
> > > +    size_t i;
> > > +    int maxpar = 0;
> > > +    int npar = 0;
> > > +    int ret = -1;
> > > +
> > > +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> > > +                  VIR_DOMAIN_AFFECT_CONFIG |
> > > +                  VIR_TYPED_PARAM_STRING_OKAY, -1);
> > > +
> > > +    if (!(vm = testDomObjFromDomain(dom)))
> > > +        goto cleanup;
> > > +
> > > +    if (virDomainObjUpdateModificationImpact(vm, &flags) < 0)
> > > +        goto cleanup;
> >
> > Actually calling this function here is redundant since it is called
> > also by virDomainObjGetOneDef just in the next line. So it can be
> > removed before merging.
>
> Hmm, good catch, looking at the code I don't see a reason for it either. I'll
> drop the call before merging.
>
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
>
> >
> > This redundant call is also done in the implementation of the same API
> > from the QEMU driver, so it could probably be safely removed from
> > there as well.
>
> Care to send a patch for QEMU as well?
>

Sure, I will. I just wanted to see if you agreed.

Thanks,
Ilias

> Erik

--
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.