1. Patchew
  2. Libvirt
  3. View series

[PATCH] libxl: support getting and setting parameters for the Credit2

Dario Faggioli posted 1 patch 4 years, 2 months ago
Test syntax-check failed
Failed in applying to current master (apply log)
src/libxl/libxl_driver.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
[PATCH] libxl: support getting and setting parameters for the Credit2
Posted by Dario Faggioli 4 years, 2 months ago 
With Credit2 being Xen default scheduler, it's definitely the case to
allow Credit2's scheduling parameters to be get and set via libvirt.

This is easy, as Credit and Credit2 have (at least as of now) the very
same parameters ('weight' and 'cap'). So we can just let credit2 pass
the scheduler-type check and the same code will work for both.

Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
---
Cc: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_driver.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index e0a8ec5c24..78ca77d0f6 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -75,6 +75,7 @@ VIR_LOG_INIT("libxl.libxl_driver");
 
 /* Number of Xen scheduler parameters */
 #define XEN_SCHED_CREDIT_NPARAM   2
+#define XEN_SCHED_CREDIT2_NPARAM  2
 
 #define LIBXL_CHECK_DOM0_GOTO(name, label) \
     do { \
@@ -4586,6 +4587,8 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
         break;
     case LIBXL_SCHEDULER_CREDIT2:
         name = "credit2";
+        if (nparams)
+            *nparams = XEN_SCHED_CREDIT2_NPARAM;
         break;
     case LIBXL_SCHEDULER_ARINC653:
         name = "arinc653";
@@ -4632,11 +4635,11 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
     if (virDomainObjCheckActive(vm) < 0)
         goto cleanup;
 
+    /* Only credit and credit2 are supported for now. */
     sched_id = libxl_get_scheduler(cfg->ctx);
-
-    if (sched_id != LIBXL_SCHEDULER_CREDIT) {
+    if (sched_id != LIBXL_SCHEDULER_CREDIT && sched_id != LIBXL_SCHEDULER_CREDIT2) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Only 'credit' scheduler is supported"));
+                       _("Only 'credit' and 'credit2' schedulers are supported"));
         goto cleanup;
     }
 
@@ -4657,6 +4660,9 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
             goto cleanup;
     }
 
+    /* Credit and Credit2 have the same number (two) of parameters,
+     * so this is ok for both, at least as long as that stays true. */
+    G_STATIC_ASSERT(XEN_SCHED_CREDIT_NPARAM == XEN_SCHED2_CREDIT_NPARAM);
     if (*nparams > XEN_SCHED_CREDIT_NPARAM)
         *nparams = XEN_SCHED_CREDIT_NPARAM;
     ret = 0;
@@ -4711,9 +4717,11 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
 
     sched_id = libxl_get_scheduler(cfg->ctx);
 
-    if (sched_id != LIBXL_SCHEDULER_CREDIT) {
+    /* Only credit and credit2 are supported for now. */
+    sched_id = libxl_get_scheduler(cfg->ctx);
+    if (sched_id != LIBXL_SCHEDULER_CREDIT && sched_id != LIBXL_SCHEDULER_CREDIT2) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Only 'credit' scheduler is supported"));
+                       _("Only 'credit' and 'credit2' schedulers are supported"));
         goto endjob;
     }
 
-- 
2.24.1

-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)
Re: [PATCH] libxl: support getting and setting parameters for the Credit2
Posted by Jim Fehlig 4 years, 2 months ago 
On 1/29/20 4:05 AM, Dario Faggioli wrote:
> With Credit2 being Xen default scheduler, it's definitely the case to
> allow Credit2's scheduling parameters to be get and set via libvirt.

Indeed. Thanks for the patch!

> This is easy, as Credit and Credit2 have (at least as of now) the very
> same parameters ('weight' and 'cap'). So we can just let credit2 pass
> the scheduler-type check and the same code will work for both.
> 
> Signed-off-by: Dario Faggioli <dfaggioli@suse.com>
> ---
> Cc: Jim Fehlig <jfehlig@suse.com>
> ---
>   src/libxl/libxl_driver.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index e0a8ec5c24..78ca77d0f6 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -75,6 +75,7 @@ VIR_LOG_INIT("libxl.libxl_driver");
>   
>   /* Number of Xen scheduler parameters */
>   #define XEN_SCHED_CREDIT_NPARAM   2
> +#define XEN_SCHED_CREDIT2_NPARAM  2

Since the number of parameters supported is the same, I would prefer using 
XEN_SCHED_CREDIT_NPARAM and delay introducing the new define until credit2 
supports more parameters.

>   #define LIBXL_CHECK_DOM0_GOTO(name, label) \
>       do { \
> @@ -4586,6 +4587,8 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
>           break;
>       case LIBXL_SCHEDULER_CREDIT2:
>           name = "credit2";
> +        if (nparams)
> +            *nparams = XEN_SCHED_CREDIT2_NPARAM;
>           break;
>       case LIBXL_SCHEDULER_ARINC653:
>           name = "arinc653";
> @@ -4632,11 +4635,11 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
>       if (virDomainObjCheckActive(vm) < 0)
>           goto cleanup;
>   
> +    /* Only credit and credit2 are supported for now. */
>       sched_id = libxl_get_scheduler(cfg->ctx);
> -
> -    if (sched_id != LIBXL_SCHEDULER_CREDIT) {
> +    if (sched_id != LIBXL_SCHEDULER_CREDIT && sched_id != LIBXL_SCHEDULER_CREDIT2) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("Only 'credit' scheduler is supported"));
> +                       _("Only 'credit' and 'credit2' schedulers are supported"));
>           goto cleanup;
>       }
>   
> @@ -4657,6 +4660,9 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
>               goto cleanup;
>       }
>   
> +    /* Credit and Credit2 have the same number (two) of parameters,
> +     * so this is ok for both, at least as long as that stays true. */
> +    G_STATIC_ASSERT(XEN_SCHED_CREDIT_NPARAM == XEN_SCHED2_CREDIT_NPARAM);

s/XEN_SCHED2_CREDIT_NPARAM/XEN_SCHED_CREDIT2_NPARAM/

But this hunk can be dropped if using XEN_SCHED_CREDIT_NPARAM for credit and 
credit2.

>       if (*nparams > XEN_SCHED_CREDIT_NPARAM)
>           *nparams = XEN_SCHED_CREDIT_NPARAM;
>       ret = 0;
> @@ -4711,9 +4717,11 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom,
>   
>       sched_id = libxl_get_scheduler(cfg->ctx);
>   
> -    if (sched_id != LIBXL_SCHEDULER_CREDIT) {
> +    /* Only credit and credit2 are supported for now. */
> +    sched_id = libxl_get_scheduler(cfg->ctx);

We just retrieved sched_id a few lines above, no need to fetch it again.

> +    if (sched_id != LIBXL_SCHEDULER_CREDIT && sched_id != LIBXL_SCHEDULER_CREDIT2) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("Only 'credit' scheduler is supported"));
> +                       _("Only 'credit' and 'credit2' schedulers are supported"));
>           goto endjob;
>   >    }

No need to respin this trivial patch

Reviewed-by: Jim Fehlig<jfehlig@suse.com>

I've fixed up the minor issues and pushed it.

Regards,
Jim

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.