All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
@ 2012-05-29 13:56 Ian Campbell
  2012-05-29 13:56 ` [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler Ian Campbell
                   ` (5 more replies)
  0 siblings, 6 replies; 30+ messages in thread
From: Ian Campbell @ 2012-05-29 13:56 UTC (permalink / raw)
  To: xen-devel; +Cc: Dario.Faggioli, Ian.Jackson, Juergen Gross, George.Dunlap

This series defines a descriminating value for each scheduler paramter
and uses it to fix a warning when starting a guest:
  "Cpu weight out of range, valid values are within range from 1 to 65535"

It also cleans up a few things and adds some convience interfaces for
cpupools.

Big change since v1 is to combine all the domain sched paramter's
set/get functions into a single pair of set/get.

Also the distinguished default values are all now -1.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler
  2012-05-29 13:56 [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
@ 2012-05-29 13:56 ` Ian Campbell
  2012-05-31 13:19   ` George Dunlap
  2012-05-29 13:57 ` [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params Ian Campbell
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-05-29 13:56 UTC (permalink / raw)
  To: xen-devel; +Cc: Dario.Faggioli, Ian.Jackson, Juergen Gross, George.Dunlap

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1338299619 -3600
# Node ID 980a25d6ad12ba0f10fa616255b9382cc14ce69e
# Parent  12537c670e9ea9e7f73747e203ca318107b82cd9
libxl: add internal function to get a domain's scheduler.

This takes into account cpupools.

Add a helper to get the info for a single cpu pool, refactor libxl_list_cpupool
t use this. While there fix the leaks due to not disposing the partial list on
realloc failure in that function.

Fix the failure of sched_domain_output to free the poolinfo list.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: add libxl_cpupoolinfo_list_free, use it in libxl_cpupool_list error path.
    Also use it in libxl_cpupool_cpuremove_node (which fixes a leak) and in
    libxl_name_to_cpupoolid, sched_domain_output and main_cpupoollist (which
    also fixes a leak).

    Also in libxl_cpupool_cpuremove_node use libxl_cputopology_list_free
    instead of open coding

diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue May 29 12:56:57 2012 +0100
+++ b/tools/libxl/libxl.c	Tue May 29 14:53:39 2012 +0100
@@ -552,41 +552,70 @@ int libxl_domain_info(libxl_ctx *ctx, li
     return 0;
 }
 
+static int cpupool_info(libxl__gc *gc,
+                        libxl_cpupoolinfo *info,
+                        uint32_t poolid,
+                        bool exact /* exactly poolid or >= poolid */)
+{
+    xc_cpupoolinfo_t *xcinfo;
+    int rc = ERROR_FAIL;
+
+    xcinfo = xc_cpupool_getinfo(CTX->xch, poolid);
+    if (xcinfo == NULL)
+        return ERROR_FAIL;
+
+    if (exact && xcinfo->cpupool_id != poolid)
+        goto out;
+
+    info->poolid = xcinfo->cpupool_id;
+    info->sched = xcinfo->sched_id;
+    info->n_dom = xcinfo->n_dom;
+    if (libxl_cpumap_alloc(CTX, &info->cpumap))
+        goto out;
+    memcpy(info->cpumap.map, xcinfo->cpumap, info->cpumap.size);
+
+    rc = 0;
+out:
+    xc_cpupool_infofree(CTX->xch, xcinfo);
+    return rc;
+}
+
+int libxl_cpupool_info(libxl_ctx *ctx,
+                       libxl_cpupoolinfo *info, uint32_t poolid)
+{
+    GC_INIT(ctx);
+    int rc = cpupool_info(gc, info, poolid, true);
+    GC_FREE;
+    return rc;
+}
+
 libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx *ctx, int *nb_pool)
 {
-    libxl_cpupoolinfo *ptr, *tmp;
+    GC_INIT(ctx);
+    libxl_cpupoolinfo info, *ptr, *tmp;
     int i;
-    xc_cpupoolinfo_t *info;
     uint32_t poolid;
 
     ptr = NULL;
 
     poolid = 0;
     for (i = 0;; i++) {
-        info = xc_cpupool_getinfo(ctx->xch, poolid);
-        if (info == NULL)
+        if (cpupool_info(gc, &info, poolid, false))
             break;
         tmp = realloc(ptr, (i + 1) * sizeof(libxl_cpupoolinfo));
         if (!tmp) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating cpupool info");
-            free(ptr);
-            xc_cpupool_infofree(ctx->xch, info);
-            return NULL;
+            libxl_cpupoolinfo_list_free(ptr, i);
+            goto out;
         }
         ptr = tmp;
-        ptr[i].poolid = info->cpupool_id;
-        ptr[i].sched = info->sched_id;
-        ptr[i].n_dom = info->n_dom;
-        if (libxl_cpumap_alloc(ctx, &ptr[i].cpumap)) {
-            xc_cpupool_infofree(ctx->xch, info);
-            break;
-        }
-        memcpy(ptr[i].cpumap.map, info->cpumap, ptr[i].cpumap.size);
-        poolid = info->cpupool_id + 1;
-        xc_cpupool_infofree(ctx->xch, info);
+        ptr[i] = info;
+        poolid = info.poolid + 1;
     }
 
     *nb_pool = i;
+out:
+    GC_FREE;
     return ptr;
 }
 
@@ -3932,14 +3961,10 @@ int libxl_cpupool_cpuremove_node(libxl_c
         }
     }
 
-    for (cpu = 0; cpu < nr_cpus; cpu++)
-        libxl_cputopology_dispose(&topology[cpu]);
-    free(topology);
+    libxl_cputopology_list_free(topology, nr_cpus);
 
 out:
-    for (p = 0; p < n_pools; p++) {
-        libxl_cpupoolinfo_dispose(poolinfo + p);
-    }
+    libxl_cpupoolinfo_list_free(poolinfo, n_pools);
 
     return ret;
 }
diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue May 29 12:56:57 2012 +0100
+++ b/tools/libxl/libxl.h	Tue May 29 14:53:39 2012 +0100
@@ -576,6 +576,7 @@ int libxl_domain_info(libxl_ctx*, libxl_
 libxl_dominfo * libxl_list_domain(libxl_ctx*, int *nb_domain);
 void libxl_dominfo_list_free(libxl_dominfo *list, int nr);
 libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
+void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nr);
 libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm);
 void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
 
@@ -829,6 +830,7 @@ int libxl_cpupool_cpuadd_node(libxl_ctx 
 int libxl_cpupool_cpuremove(libxl_ctx *ctx, uint32_t poolid, int cpu);
 int libxl_cpupool_cpuremove_node(libxl_ctx *ctx, uint32_t poolid, int node, int *cpus);
 int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid);
+int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t poolid);
 
 int libxl_domid_valid_guest(uint32_t domid);
 
diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 29 12:56:57 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Tue May 29 14:53:39 2012 +0100
@@ -93,6 +93,41 @@ int libxl__domain_shutdown_reason(libxl_
     return (info.flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask;
 }
 
+int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
+{
+    xc_domaininfo_t info;
+    int ret;
+
+    ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
+    if (ret != 1)
+        return ERROR_FAIL;
+    if (info.domain != domid)
+        return ERROR_FAIL;
+
+    return info.cpupool;
+}
+
+libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid)
+{
+    uint32_t cpupool = libxl__domain_cpupool(gc, domid);
+    libxl_cpupoolinfo poolinfo;
+    libxl_scheduler sched = LIBXL_SCHEDULER_UNKNOWN;
+    int rc;
+
+    if (cpupool < 0)
+        return sched;
+
+    rc = libxl_cpupool_info(CTX, &poolinfo, cpupool);
+    if (rc < 0)
+        goto out;
+
+    sched = poolinfo.sched;
+
+out:
+    libxl_cpupoolinfo_dispose(&poolinfo);
+    return sched;
+}
+
 int libxl__build_pre(libxl__gc *gc, uint32_t domid,
               libxl_domain_build_info *info, libxl__domain_build_state *state)
 {
diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue May 29 12:56:57 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Tue May 29 14:53:39 2012 +0100
@@ -738,6 +738,8 @@ _hidden int libxl__file_reference_unmap(
 /* from xl_dom */
 _hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
+_hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
+_hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
 #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
     libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Tue May 29 12:56:57 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Tue May 29 14:53:39 2012 +0100
@@ -107,7 +107,9 @@ libxl_bios_type = Enumeration("bios_type
     ])
 
 # Consistent with values defined in domctl.h
+# Except unknown which we have made up
 libxl_scheduler = Enumeration("scheduler", [
+    (0, "unknown"),
     (4, "sedf"),
     (5, "credit"),
     (6, "credit2"),
diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue May 29 12:56:57 2012 +0100
+++ b/tools/libxl/libxl_utils.c	Tue May 29 14:53:39 2012 +0100
@@ -133,9 +133,8 @@ int libxl_name_to_cpupoolid(libxl_ctx *c
             }
             free(poolname);
         }
-        libxl_cpupoolinfo_dispose(poolinfo + i);
     }
-    free(poolinfo);
+    libxl_cpupoolinfo_list_free(poolinfo, nb_pools);
     return ret;
 }
 
@@ -686,6 +685,14 @@ void libxl_vminfo_list_free(libxl_vminfo
     free(list);
 }
 
+void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nr)
+{
+    int i;
+    for (i = 0; i < nr; i++)
+        libxl_cpupoolinfo_dispose(&list[i]);
+    free(list);
+}
+
 int libxl_domid_valid_guest(uint32_t domid)
 {
     /* returns 1 if the value _could_ be a valid guest domid, 0 otherwise
diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue May 29 12:56:57 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue May 29 14:53:39 2012 +0100
@@ -4608,11 +4608,8 @@ static int sched_domain_output(libxl_sch
                 break;
         }
     }
-    if (poolinfo) {
-        for (p = 0; p < n_pools; p++) {
-            libxl_cpupoolinfo_dispose(poolinfo + p);
-        }
-    }
+    if (poolinfo)
+        libxl_cpupoolinfo_list_free(poolinfo, n_pools);
     return 0;
 }
 
@@ -6119,8 +6116,9 @@ int main_cpupoollist(int argc, char **ar
                 printf("\n");
             }
         }
-        libxl_cpupoolinfo_dispose(poolinfo + p);
-    }
+    }
+
+    libxl_cpupoolinfo_list_free(poolinfo, n_pools);
 
     return ret;
 }

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params
  2012-05-29 13:56 [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
  2012-05-29 13:56 ` [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler Ian Campbell
@ 2012-05-29 13:57 ` Ian Campbell
  2012-05-31 13:21   ` George Dunlap
  2012-05-29 13:57 ` [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-05-29 13:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Dario.Faggioli, Ian.Jackson, Juergen Gross, George.Dunlap

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1338299709 -3600
# Node ID 73d8274c0b6859b4528af75a7405e546d659f130
# Parent  980a25d6ad12ba0f10fa616255b9382cc14ce69e
libxl: rename libxl_sched_params to libxl_domain_sched_params

Remove credit scheduler global options from the struct, they were never used
anyway.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: use libxl_domain_sched_params which fits in better with future changes.

diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 29 14:53:39 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Tue May 29 14:55:09 2012 +0100
@@ -42,7 +42,8 @@ libxl_domain_type libxl__domain_type(lib
         return LIBXL_DOMAIN_TYPE_PV;
 }
 
-int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams)
+int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
+                            libxl_domain_sched_params *scparams)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     libxl_scheduler sched;
diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue May 29 14:53:39 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Tue May 29 14:55:09 2012 +0100
@@ -740,7 +740,8 @@ _hidden libxl_domain_type libxl__domain_
 _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
 _hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
-_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
+_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
+                                    libxl_domain_sched_params *scparams);
 #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
     libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
 typedef struct {
diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Tue May 29 14:53:39 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Tue May 29 14:55:09 2012 +0100
@@ -224,11 +224,9 @@ libxl_domain_create_info = Struct("domai
 
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
-libxl_sched_params = Struct("sched_params",[
+libxl_domain_sched_params = Struct("domain_sched_params",[
     ("weight",       integer),
     ("cap",          integer),
-    ("tslice_ms",    integer),
-    ("ratelimit_us", integer),
     ("period",       integer),
     ("slice",        integer),
     ("latency",      integer),
@@ -262,7 +260,7 @@ libxl_domain_build_info = Struct("domain
     # extra parameters pass directly to qemu for HVM guest, NULL terminated
     ("extra_hvm",        libxl_string_list),
     #  parameters for all type of scheduler
-    ("sched_params",     libxl_sched_params),
+    ("sched_params",     libxl_domain_sched_params),
 
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue May 29 14:53:39 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue May 29 14:55:09 2012 +0100
@@ -633,10 +633,6 @@ static void parse_config_data(const char
         b_info->sched_params.weight = l;
     if (!xlu_cfg_get_long (config, "cap", &l, 0))
         b_info->sched_params.cap = l;
-    if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0))
-        b_info->sched_params.tslice_ms = l;
-    if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0))
-        b_info->sched_params.ratelimit_us = l;
     if (!xlu_cfg_get_long (config, "period", &l, 0))
         b_info->sched_params.period = l;
     if (!xlu_cfg_get_long (config, "slice", &l, 0))

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-29 13:56 [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
  2012-05-29 13:56 ` [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler Ian Campbell
  2012-05-29 13:57 ` [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params Ian Campbell
@ 2012-05-29 13:57 ` Ian Campbell
  2012-05-30  7:26   ` Dario Faggioli
  2012-05-31 13:47   ` George Dunlap
  2012-05-29 13:57 ` [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c Ian Campbell
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 30+ messages in thread
From: Ian Campbell @ 2012-05-29 13:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Dario.Faggioli, Ian.Jackson, Juergen Gross, George.Dunlap

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1338299729 -3600
# Node ID 274de8e1e0d116070d34731d93b53ce44530e5a0
# Parent  73d8274c0b6859b4528af75a7405e546d659f130
libxl: make it possible to explicitly specify default sched params

To do so we define a discriminating value which is never a valid real value for
each parameter.

While there:

 - removed libxl_sched_*_domain in favour of libxl_domain_sched_params.
 - use this new functionality for the various xl commands which set sched
   parameters, which saves an explicit read-modify-write in xl.
 - removed call of xc_domain_getinfolist from a few functions which weren't
   actually using the result (looks like a cut and paste error)
 - fix xl which was setting period for a variety of different config keys.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue May 29 14:55:09 2012 +0100
+++ b/tools/libxl/libxl.c	Tue May 29 14:55:29 2012 +0100
@@ -3197,19 +3197,19 @@ libxl_scheduler libxl_get_scheduler(libx
 }
 
 int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo)
+                                  libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit sdom;
     int rc;
 
-    libxl_sched_credit_domain_init(scinfo);
-
     rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
     if (rc != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
         return ERROR_FAIL;
     }
 
+    libxl_domain_sched_params_init(scinfo);
+    scinfo->sched = LIBXL_SCHEDULER_CREDIT;
     scinfo->weight = sdom.weight;
     scinfo->cap = sdom.cap;
 
@@ -3217,7 +3217,7 @@ int libxl_sched_credit_domain_get(libxl_
 }
 
 int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo)
+                                  libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit sdom;
     xc_domaininfo_t domaininfo;
@@ -3231,22 +3231,33 @@ int libxl_sched_credit_domain_set(libxl_
     if (rc != 1 || domaininfo.domain != domid)
         return ERROR_INVAL;
 
-
-    if (scinfo->weight < 1 || scinfo->weight > 65535) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-            "Cpu weight out of range, valid values are within range from 1 to 65535");
-        return ERROR_INVAL;
+    rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
+    if (rc != 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
+        return ERROR_FAIL;
     }
 
-    if (scinfo->cap < 0 || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-            "Cpu cap out of range, valid range is from 0 to %d for specified number of vcpus",
-            ((domaininfo.max_vcpu_id + 1) * 100));
-        return ERROR_INVAL;
+    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
+        if (scinfo->weight < 1 || scinfo->weight > 65535) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "Cpu weight out of range, "
+                       "valid values are within range from 1 to 65535");
+            return ERROR_INVAL;
+        }
+        sdom.weight = scinfo->weight;
     }
 
-    sdom.weight = scinfo->weight;
-    sdom.cap = scinfo->cap;
+    if (scinfo->cap != LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT) {
+        if (scinfo->cap < 0
+            || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                "Cpu cap out of range, "
+                "valid range is from 0 to %d for specified number of vcpus",
+                       ((domaininfo.max_vcpu_id + 1) * 100));
+            return ERROR_INVAL;
+        }
+        sdom.cap = scinfo->cap;
+    }
 
     rc = xc_sched_credit_domain_set(ctx->xch, domid, &sdom);
     if ( rc < 0 ) {
@@ -3319,13 +3330,11 @@ int libxl_sched_credit_params_set(libxl_
 }
 
 int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo)
+                                   libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit2 sdom;
     int rc;
 
-    libxl_sched_credit2_domain_init(scinfo);
-
     rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
     if (rc != 0) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
@@ -3333,36 +3342,37 @@ int libxl_sched_credit2_domain_get(libxl
         return ERROR_FAIL;
     }
 
+    libxl_domain_sched_params_init(scinfo);
+    scinfo->sched = LIBXL_SCHEDULER_CREDIT2;
     scinfo->weight = sdom.weight;
 
     return 0;
 }
 
 int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo)
+                                   libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit2 sdom;
-    xc_domaininfo_t domaininfo;
     int rc;
 
-    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
-    if (rc < 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
+    rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
+    if (rc != 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+                         "getting domain sched credit2");
         return ERROR_FAIL;
     }
-    if (rc != 1 || domaininfo.domain != domid)
-        return ERROR_INVAL;
-
-
-    if (scinfo->weight < 1 || scinfo->weight > 65535) {
-        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
-            "Cpu weight out of range, valid values are within range from "
-            "1 to 65535");
-        return ERROR_INVAL;
+
+    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
+        if (scinfo->weight < 1 || scinfo->weight > 65535) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "Cpu weight out of range, "
+                       "valid values are within range from "
+                       "1 to 65535");
+            return ERROR_INVAL;
+        }
+        sdom.weight = scinfo->weight;
     }
 
-    sdom.weight = scinfo->weight;
-
     rc = xc_sched_credit2_domain_set(ctx->xch, domid, &sdom);
     if ( rc < 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
@@ -3374,7 +3384,7 @@ int libxl_sched_credit2_domain_set(libxl
 }
 
 int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo)
+                                libxl_domain_sched_params *scinfo)
 {
     uint64_t period;
     uint64_t slice;
@@ -3383,8 +3393,6 @@ int libxl_sched_sedf_domain_get(libxl_ct
     uint16_t weight;
     int rc;
 
-    libxl_sched_sedf_domain_init(scinfo);
-
     rc = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
                             &extratime, &weight);
     if (rc != 0) {
@@ -3392,6 +3400,8 @@ int libxl_sched_sedf_domain_get(libxl_ct
         return ERROR_FAIL;
     }
 
+    libxl_domain_sched_params_init(scinfo);
+    scinfo->sched = LIBXL_SCHEDULER_SEDF;
     scinfo->period = period / 1000000;
     scinfo->slice = slice / 1000000;
     scinfo->latency = latency / 1000000;
@@ -3402,24 +3412,37 @@ int libxl_sched_sedf_domain_get(libxl_ct
 }
 
 int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo)
+                                libxl_domain_sched_params *scinfo)
 {
-    xc_domaininfo_t domaininfo;
-    int rc;
-
-    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
-    if (rc < 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
+    uint64_t period;
+    uint64_t slice;
+    uint64_t latency;
+    uint16_t extratime;
+    uint16_t weight;
+
+    int ret;
+
+    ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
+                            &extratime, &weight);
+    if (ret != 0) {
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
         return ERROR_FAIL;
     }
-    if (rc != 1 || domaininfo.domain != domid)
-        return ERROR_INVAL;
-
-
-    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
-                            scinfo->slice * 1000000, scinfo->latency * 1000000,
-                            scinfo->extratime, scinfo->weight);
-    if ( rc < 0 ) {
+
+    if (scinfo->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT)
+        period = scinfo->period * 1000000;
+    if (scinfo->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT)
+        slice = scinfo->slice * 1000000;
+    if (scinfo->latency != LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT)
+        latency = scinfo->latency * 1000000;
+    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
+        extratime = scinfo->extratime;
+    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT)
+        weight = scinfo->weight;
+
+    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
+                            extratime, weight);
+    if ( ret < 0 ) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
         return ERROR_FAIL;
     }
diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue May 29 14:55:09 2012 +0100
+++ b/tools/libxl/libxl.h	Tue May 29 14:55:29 2012 +0100
@@ -775,23 +775,33 @@ int libxl_set_vcpuonline(libxl_ctx *ctx,
 
 libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx);
 
-
-int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo);
-int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_sched_credit_domain *scinfo);
+/* Per-scheduler parameters */
 int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid,
                                   libxl_sched_credit_params *scinfo);
 int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
                                   libxl_sched_credit_params *scinfo);
+
+/* Scheduler Per-domain parameters */
+
+#define LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT    -1
+#define LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT       -1
+#define LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT    -1
+#define LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT     -1
+#define LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT   -1
+#define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
+
+int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_domain_sched_params *scinfo);
+int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_domain_sched_params *scinfo);
 int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo);
+                                   libxl_domain_sched_params *scinfo);
 int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_sched_credit2_domain *scinfo);
+                                   libxl_domain_sched_params *scinfo);
 int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo);
+                                libxl_domain_sched_params *scinfo);
 int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                libxl_sched_sedf_domain *scinfo);
+                                libxl_domain_sched_params *scinfo);
 int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                        libxl_trigger trigger, uint32_t vcpuid);
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 29 14:55:09 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Tue May 29 14:55:29 2012 +0100
@@ -45,34 +45,26 @@ libxl_domain_type libxl__domain_type(lib
 int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
                             libxl_domain_sched_params *scparams)
 {
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    libxl_scheduler sched;
-    libxl_sched_sedf_domain sedf_info;
-    libxl_sched_credit_domain credit_info;
-    libxl_sched_credit2_domain credit2_info;
+    libxl_scheduler sched = scparams->sched;
     int ret;
 
-    sched = libxl_get_scheduler (ctx);
+    if (sched == LIBXL_SCHEDULER_UNKNOWN)
+        sched = libxl__domain_scheduler(gc, domid);
+
     switch (sched) {
     case LIBXL_SCHEDULER_SEDF:
-      sedf_info.period = scparams->period;
-      sedf_info.slice = scparams->slice;
-      sedf_info.latency = scparams->latency;
-      sedf_info.extratime = scparams->extratime;
-      sedf_info.weight = scparams->weight;
-      ret=libxl_sched_sedf_domain_set(ctx, domid, &sedf_info);
-      break;
+        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
+        break;
     case LIBXL_SCHEDULER_CREDIT:
-      credit_info.weight = scparams->weight;
-      credit_info.cap = scparams->cap;
-      ret=libxl_sched_credit_domain_set(ctx, domid, &credit_info);
-      break;
+        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
+        break;
     case LIBXL_SCHEDULER_CREDIT2:
-      credit2_info.weight = scparams->weight;
-      ret=libxl_sched_credit2_domain_set(ctx, domid, &credit2_info);
-      break;
+        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
+        break;
     default:
-      ret=-1;
+        LOG(ERROR, "Unknown scheduler");
+        ret=ERROR_INVAL;
+        break;
     }
     return ret;
 }
diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Tue May 29 14:55:09 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Tue May 29 14:55:29 2012 +0100
@@ -225,12 +225,13 @@ libxl_domain_create_info = Struct("domai
 MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
 
 libxl_domain_sched_params = Struct("domain_sched_params",[
-    ("weight",       integer),
-    ("cap",          integer),
-    ("period",       integer),
-    ("slice",        integer),
-    ("latency",      integer),
-    ("extratime",    integer),
+    ("sched",        libxl_scheduler),
+    ("weight",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
+    ("cap",          integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT'}),
+    ("period",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT'}),
+    ("slice",        integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),
+    ("latency",      integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
+    ("extratime",    integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
     ], dir=DIR_IN)
 
 libxl_domain_build_info = Struct("domain_build_info",[
@@ -425,28 +426,11 @@ libxl_cputopology = Struct("cputopology"
     ("node", uint32),
     ], dir=DIR_OUT)
 
-libxl_sched_credit_domain = Struct("sched_credit_domain", [
-    ("weight", integer),
-    ("cap", integer),
-    ])
-
 libxl_sched_credit_params = Struct("sched_credit_params", [
     ("tslice_ms", integer),
     ("ratelimit_us", integer),
     ], dispose_fn=None)
 
-libxl_sched_credit2_domain = Struct("sched_credit2_domain", [
-    ("weight", integer),
-    ])
-
-libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
-    ("period", integer),
-    ("slice", integer),
-    ("latency", integer),
-    ("extratime", integer),
-    ("weight", integer),
-    ])
-
 libxl_domain_remus_info = Struct("domain_remus_info",[
     ("interval",     integer),
     ("blackhole",    bool),
diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue May 29 14:55:09 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue May 29 14:55:29 2012 +0100
@@ -628,7 +628,8 @@ static void parse_config_data(const char
 
     libxl_domain_build_info_init_type(b_info, c_info->type);
 
-    /* the following is the actual config parsing with overriding values in the structures */
+    /* the following is the actual config parsing with overriding
+     * values in the structures */
     if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
         b_info->sched_params.weight = l;
     if (!xlu_cfg_get_long (config, "cap", &l, 0))
@@ -636,11 +637,11 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_long (config, "period", &l, 0))
         b_info->sched_params.period = l;
     if (!xlu_cfg_get_long (config, "slice", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.slice = l;
     if (!xlu_cfg_get_long (config, "latency", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.latency = l;
     if (!xlu_cfg_get_long (config, "extratime", &l, 0))
-        b_info->sched_params.period = l;
+        b_info->sched_params.extratime = l;
 
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;
@@ -4360,7 +4361,7 @@ int main_sharing(int argc, char **argv)
     return 0;
 }
 
-static int sched_credit_domain_get(int domid, libxl_sched_credit_domain *scinfo)
+static int sched_credit_domain_get(int domid, libxl_domain_sched_params *scinfo)
 {
     int rc;
 
@@ -4371,7 +4372,7 @@ static int sched_credit_domain_get(int d
     return rc;
 }
 
-static int sched_credit_domain_set(int domid, libxl_sched_credit_domain *scinfo)
+static int sched_credit_domain_set(int domid, libxl_domain_sched_params *scinfo)
 {
     int rc;
 
@@ -4407,7 +4408,7 @@ static int sched_credit_params_get(int p
 static int sched_credit_domain_output(int domid)
 {
     char *domname;
-    libxl_sched_credit_domain scinfo;
+    libxl_domain_sched_params scinfo;
     int rc;
 
     if (domid < 0) {
@@ -4424,7 +4425,7 @@ static int sched_credit_domain_output(in
         scinfo.weight,
         scinfo.cap);
     free(domname);
-    libxl_sched_credit_domain_dispose(&scinfo);
+    libxl_domain_sched_params_dispose(&scinfo);
     return 0;
 }
 
@@ -4450,7 +4451,7 @@ static int sched_credit_pool_output(uint
 }
 
 static int sched_credit2_domain_get(
-    int domid, libxl_sched_credit2_domain *scinfo)
+    int domid, libxl_domain_sched_params *scinfo)
 {
     int rc;
 
@@ -4462,7 +4463,7 @@ static int sched_credit2_domain_get(
 }
 
 static int sched_credit2_domain_set(
-    int domid, libxl_sched_credit2_domain *scinfo)
+    int domid, libxl_domain_sched_params *scinfo)
 {
     int rc;
 
@@ -4477,7 +4478,7 @@ static int sched_credit2_domain_output(
     int domid)
 {
     char *domname;
-    libxl_sched_credit2_domain scinfo;
+    libxl_domain_sched_params scinfo;
     int rc;
 
     if (domid < 0) {
@@ -4493,12 +4494,12 @@ static int sched_credit2_domain_output(
         domid,
         scinfo.weight);
     free(domname);
-    libxl_sched_credit2_domain_dispose(&scinfo);
+    libxl_domain_sched_params_dispose(&scinfo);
     return 0;
 }
 
 static int sched_sedf_domain_get(
-    int domid, libxl_sched_sedf_domain *scinfo)
+    int domid, libxl_domain_sched_params *scinfo)
 {
     int rc;
 
@@ -4510,7 +4511,7 @@ static int sched_sedf_domain_get(
 }
 
 static int sched_sedf_domain_set(
-    int domid, libxl_sched_sedf_domain *scinfo)
+    int domid, libxl_domain_sched_params *scinfo)
 {
     int rc;
 
@@ -4524,7 +4525,7 @@ static int sched_sedf_domain_output(
     int domid)
 {
     char *domname;
-    libxl_sched_sedf_domain scinfo;
+    libxl_domain_sched_params scinfo;
     int rc;
 
     if (domid < 0) {
@@ -4545,7 +4546,7 @@ static int sched_sedf_domain_output(
         scinfo.extratime,
         scinfo.weight);
     free(domname);
-    libxl_sched_sedf_domain_dispose(&scinfo);
+    libxl_domain_sched_params_dispose(&scinfo);
     return 0;
 }
 
@@ -4622,7 +4623,6 @@ static int sched_domain_output(libxl_sch
  */
 int main_sched_credit(int argc, char **argv)
 {
-    libxl_sched_credit_domain scinfo;
     const char *dom = NULL;
     const char *cpupool = NULL;
     int weight = 256, cap = 0, opt_w = 0, opt_c = 0;
@@ -4697,7 +4697,7 @@ int main_sched_credit(int argc, char **a
     }
 
     if (opt_s) {
-        libxl_sched_credit_params  scparam;
+        libxl_sched_credit_params scparam;
         uint32_t poolid = 0;
 
         if (cpupool) {
@@ -4733,20 +4733,19 @@ int main_sched_credit(int argc, char **a
     } else {
         find_domain(dom);
 
-        rc = sched_credit_domain_get(domid, &scinfo);
-        if (rc)
-            return -rc;
-
         if (!opt_w && !opt_c) { /* output credit scheduler info */
             sched_credit_domain_output(-1);
             return -sched_credit_domain_output(domid);
         } else { /* set credit scheduler paramaters */
+            libxl_domain_sched_params scinfo;
+            libxl_domain_sched_params_init(&scinfo);
+            scinfo.sched = LIBXL_SCHEDULER_CREDIT;
             if (opt_w)
                 scinfo.weight = weight;
             if (opt_c)
                 scinfo.cap = cap;
             rc = sched_credit_domain_set(domid, &scinfo);
-            libxl_sched_credit_domain_dispose(&scinfo);
+            libxl_domain_sched_params_dispose(&scinfo);
             if (rc)
                 return -rc;
         }
@@ -4757,7 +4756,6 @@ int main_sched_credit(int argc, char **a
 
 int main_sched_credit2(int argc, char **argv)
 {
-    libxl_sched_credit2_domain scinfo;
     const char *dom = NULL;
     const char *cpupool = NULL;
     int weight = 256, opt_w = 0;
@@ -4812,18 +4810,17 @@ int main_sched_credit2(int argc, char **
     } else {
         find_domain(dom);
 
-        rc = sched_credit2_domain_get(domid, &scinfo);
-        if (rc)
-            return -rc;
-
         if (!opt_w) { /* output credit2 scheduler info */
             sched_credit2_domain_output(-1);
             return -sched_credit2_domain_output(domid);
         } else { /* set credit2 scheduler paramaters */
+            libxl_domain_sched_params scinfo;
+            libxl_domain_sched_params_init(&scinfo);
+            scinfo.sched = LIBXL_SCHEDULER_CREDIT2;
             if (opt_w)
                 scinfo.weight = weight;
             rc = sched_credit2_domain_set(domid, &scinfo);
-            libxl_sched_credit2_domain_dispose(&scinfo);
+            libxl_domain_sched_params_dispose(&scinfo);
             if (rc)
                 return -rc;
         }
@@ -4834,7 +4831,6 @@ int main_sched_credit2(int argc, char **
 
 int main_sched_sedf(int argc, char **argv)
 {
-    libxl_sched_sedf_domain scinfo;
     const char *dom = NULL;
     const char *cpupool = NULL;
     int period = 0, opt_p = 0;
@@ -4917,15 +4913,15 @@ int main_sched_sedf(int argc, char **arg
     } else {
         find_domain(dom);
 
-        rc = sched_sedf_domain_get(domid, &scinfo);
-        if (rc)
-            return -rc;
-
         if (!opt_p && !opt_s && !opt_l && !opt_e && !opt_w) {
             /* output sedf scheduler info */
             sched_sedf_domain_output(-1);
             return -sched_sedf_domain_output(domid);
         } else { /* set sedf scheduler paramaters */
+            libxl_domain_sched_params scinfo;
+            libxl_domain_sched_params_init(&scinfo);
+            scinfo.sched = LIBXL_SCHEDULER_SEDF;
+
             if (opt_p) {
                 scinfo.period = period;
                 scinfo.weight = 0;
@@ -4944,7 +4940,7 @@ int main_sched_sedf(int argc, char **arg
                 scinfo.slice = 0;
             }
             rc = sched_sedf_domain_set(domid, &scinfo);
-            libxl_sched_sedf_domain_dispose(&scinfo);
+            libxl_domain_sched_params_dispose(&scinfo);
             if (rc)
                 return -rc;
         }

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c
  2012-05-29 13:56 [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
                   ` (2 preceding siblings ...)
  2012-05-29 13:57 ` [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
@ 2012-05-29 13:57 ` Ian Campbell
  2012-05-31 13:48   ` George Dunlap
  2012-05-29 13:57 ` [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters Ian Campbell
  2012-06-01 11:07 ` [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
  5 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-05-29 13:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Dario.Faggioli, Ian.Jackson, Juergen Gross, George.Dunlap

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1338299813 -3600
# Node ID d89b5eeb94519fdc056f91663676cf012c40b654
# Parent  274de8e1e0d116070d34731d93b53ce44530e5a0
libxl: move libxl__sched_set_params into libxl.c

All the other sched functions are here and I'm just about to make those static
functions as I make libxl__sched_set_params the public function.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 274de8e1e0d1 -r d89b5eeb9451 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue May 29 14:55:29 2012 +0100
+++ b/tools/libxl/libxl.c	Tue May 29 14:56:53 2012 +0100
@@ -3450,6 +3450,33 @@ int libxl_sched_sedf_domain_set(libxl_ct
     return 0;
 }
 
+int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
+                            libxl_domain_sched_params *scparams)
+{
+    libxl_scheduler sched = scparams->sched;
+    int ret;
+
+    if (sched == LIBXL_SCHEDULER_UNKNOWN)
+        sched = libxl__domain_scheduler(gc, domid);
+
+    switch (sched) {
+    case LIBXL_SCHEDULER_SEDF:
+        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
+        break;
+    case LIBXL_SCHEDULER_CREDIT:
+        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
+        break;
+    case LIBXL_SCHEDULER_CREDIT2:
+        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
+        break;
+    default:
+        LOG(ERROR, "Unknown scheduler");
+        ret=ERROR_INVAL;
+        break;
+    }
+    return ret;
+}
+
 int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                        libxl_trigger trigger, uint32_t vcpuid)
 {
diff -r 274de8e1e0d1 -r d89b5eeb9451 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 29 14:55:29 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Tue May 29 14:56:53 2012 +0100
@@ -42,33 +42,6 @@ libxl_domain_type libxl__domain_type(lib
         return LIBXL_DOMAIN_TYPE_PV;
 }
 
-int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
-                            libxl_domain_sched_params *scparams)
-{
-    libxl_scheduler sched = scparams->sched;
-    int ret;
-
-    if (sched == LIBXL_SCHEDULER_UNKNOWN)
-        sched = libxl__domain_scheduler(gc, domid);
-
-    switch (sched) {
-    case LIBXL_SCHEDULER_SEDF:
-        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
-        break;
-    case LIBXL_SCHEDULER_CREDIT:
-        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
-        break;
-    case LIBXL_SCHEDULER_CREDIT2:
-        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
-        break;
-    default:
-        LOG(ERROR, "Unknown scheduler");
-        ret=ERROR_INVAL;
-        break;
-    }
-    return ret;
-}
-
 int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters
  2012-05-29 13:56 [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
                   ` (3 preceding siblings ...)
  2012-05-29 13:57 ` [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c Ian Campbell
@ 2012-05-29 13:57 ` Ian Campbell
  2012-05-31 13:51   ` George Dunlap
  2012-06-01 11:07 ` [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
  5 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-05-29 13:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Dario.Faggioli, Ian.Jackson, Juergen Gross, George.Dunlap

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1338299813 -3600
# Node ID 9094cf509df0e36d8c59ed131937b1ebc07cc2ad
# Parent  d89b5eeb94519fdc056f91663676cf012c40b654
libxl: expose a single get/setter for domain scheduler parameters.

This is consistent with having a single struct for all parameters.

Effectively renames and exports libxl__sched_set_params as
libxl_domain_sched_params_set. libxl_domain_sched_params_get is new.

Improve const correctness of the setters while I'm here.

Use shorter LOG macros when touching a line anyway.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue May 29 14:56:53 2012 +0100
+++ b/tools/libxl/libxl.c	Tue May 29 14:56:53 2012 +0100
@@ -3196,15 +3196,15 @@ libxl_scheduler libxl_get_scheduler(libx
     return sched;
 }
 
-int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_domain_sched_params *scinfo)
+static int sched_credit_domain_get(libxl__gc *gc, uint32_t domid,
+                                   libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit sdom;
     int rc;
 
-    rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
+    rc = xc_sched_credit_domain_get(CTX->xch, domid, &sdom);
     if (rc != 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
+        LOGE(ERROR, "getting domain sched credit");
         return ERROR_FAIL;
     }
 
@@ -3216,32 +3216,31 @@ int libxl_sched_credit_domain_get(libxl_
     return 0;
 }
 
-int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_domain_sched_params *scinfo)
+static int sched_credit_domain_set(libxl__gc *gc, uint32_t domid,
+                                   const libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit sdom;
     xc_domaininfo_t domaininfo;
     int rc;
 
-    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
+    rc = xc_domain_getinfolist(CTX->xch, domid, 1, &domaininfo);
     if (rc < 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
+        LOGE(ERROR, "getting domain info list");
         return ERROR_FAIL;
     }
     if (rc != 1 || domaininfo.domain != domid)
         return ERROR_INVAL;
 
-    rc = xc_sched_credit_domain_get(ctx->xch, domid, &sdom);
+    rc = xc_sched_credit_domain_get(CTX->xch, domid, &sdom);
     if (rc != 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
+        LOGE(ERROR, "getting domain sched credit");
         return ERROR_FAIL;
     }
 
     if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
         if (scinfo->weight < 1 || scinfo->weight > 65535) {
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                       "Cpu weight out of range, "
-                       "valid values are within range from 1 to 65535");
+            LOG(ERROR, "Cpu weight out of range, "
+                "valid values are within range from 1 to 65535");
             return ERROR_INVAL;
         }
         sdom.weight = scinfo->weight;
@@ -3250,18 +3249,17 @@ int libxl_sched_credit_domain_set(libxl_
     if (scinfo->cap != LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT) {
         if (scinfo->cap < 0
             || scinfo->cap > (domaininfo.max_vcpu_id + 1) * 100) {
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                "Cpu cap out of range, "
+            LOG(ERROR, "Cpu cap out of range, "
                 "valid range is from 0 to %d for specified number of vcpus",
-                       ((domaininfo.max_vcpu_id + 1) * 100));
+                ((domaininfo.max_vcpu_id + 1) * 100));
             return ERROR_INVAL;
         }
         sdom.cap = scinfo->cap;
     }
 
-    rc = xc_sched_credit_domain_set(ctx->xch, domid, &sdom);
+    rc = xc_sched_credit_domain_set(CTX->xch, domid, &sdom);
     if ( rc < 0 ) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched credit");
+        LOGE(ERROR, "setting domain sched credit");
         return ERROR_FAIL;
     }
 
@@ -3329,16 +3327,15 @@ int libxl_sched_credit_params_set(libxl_
     return 0;
 }
 
-int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_domain_sched_params *scinfo)
+static int sched_credit2_domain_get(libxl__gc *gc, uint32_t domid,
+                                    libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit2 sdom;
     int rc;
 
-    rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
+    rc = xc_sched_credit2_domain_get(CTX->xch, domid, &sdom);
     if (rc != 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                         "getting domain sched credit2");
+        LOGE(ERROR, "getting domain sched credit2");
         return ERROR_FAIL;
     }
 
@@ -3349,42 +3346,38 @@ int libxl_sched_credit2_domain_get(libxl
     return 0;
 }
 
-int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_domain_sched_params *scinfo)
+static int sched_credit2_domain_set(libxl__gc *gc, uint32_t domid,
+                                    const libxl_domain_sched_params *scinfo)
 {
     struct xen_domctl_sched_credit2 sdom;
     int rc;
 
-    rc = xc_sched_credit2_domain_get(ctx->xch, domid, &sdom);
+    rc = xc_sched_credit2_domain_get(CTX->xch, domid, &sdom);
     if (rc != 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                         "getting domain sched credit2");
+        LOGE(ERROR, "getting domain sched credit2");
         return ERROR_FAIL;
     }
 
     if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
         if (scinfo->weight < 1 || scinfo->weight > 65535) {
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                       "Cpu weight out of range, "
-                       "valid values are within range from "
-                       "1 to 65535");
+            LOG(ERROR, "Cpu weight out of range, "
+                       "valid values are within range from 1 to 65535");
             return ERROR_INVAL;
         }
         sdom.weight = scinfo->weight;
     }
 
-    rc = xc_sched_credit2_domain_set(ctx->xch, domid, &sdom);
+    rc = xc_sched_credit2_domain_set(CTX->xch, domid, &sdom);
     if ( rc < 0 ) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
-                         "setting domain sched credit2");
+        LOGE(ERROR, "setting domain sched credit2");
         return ERROR_FAIL;
     }
 
     return 0;
 }
 
-int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                libxl_domain_sched_params *scinfo)
+static int sched_sedf_domain_get(libxl__gc *gc, uint32_t domid,
+                                 libxl_domain_sched_params *scinfo)
 {
     uint64_t period;
     uint64_t slice;
@@ -3393,10 +3386,10 @@ int libxl_sched_sedf_domain_get(libxl_ct
     uint16_t weight;
     int rc;
 
-    rc = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
+    rc = xc_sedf_domain_get(CTX->xch, domid, &period, &slice, &latency,
                             &extratime, &weight);
     if (rc != 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
+        LOGE(ERROR, "getting domain sched sedf");
         return ERROR_FAIL;
     }
 
@@ -3411,8 +3404,8 @@ int libxl_sched_sedf_domain_get(libxl_ct
     return 0;
 }
 
-int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                libxl_domain_sched_params *scinfo)
+static int sched_sedf_domain_set(libxl__gc *gc, uint32_t domid,
+                                 const libxl_domain_sched_params *scinfo)
 {
     uint64_t period;
     uint64_t slice;
@@ -3422,10 +3415,10 @@ int libxl_sched_sedf_domain_set(libxl_ct
 
     int ret;
 
-    ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
+    ret = xc_sedf_domain_get(CTX->xch, domid, &period, &slice, &latency,
                             &extratime, &weight);
     if (ret != 0) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
+        LOGE(ERROR, "getting domain sched sedf");
         return ERROR_FAIL;
     }
 
@@ -3440,20 +3433,21 @@ int libxl_sched_sedf_domain_set(libxl_ct
     if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT)
         weight = scinfo->weight;
 
-    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
+    ret = xc_sedf_domain_set(CTX->xch, domid, period, slice, latency,
                             extratime, weight);
     if ( ret < 0 ) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
+        LOGE(ERROR, "setting domain sched sedf");
         return ERROR_FAIL;
     }
 
     return 0;
 }
 
-int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
-                            libxl_domain_sched_params *scparams)
+int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
+                                  const libxl_domain_sched_params *scinfo)
 {
-    libxl_scheduler sched = scparams->sched;
+    GC_INIT(ctx);
+    libxl_scheduler sched = scinfo->sched;
     int ret;
 
     if (sched == LIBXL_SCHEDULER_UNKNOWN)
@@ -3461,19 +3455,51 @@ int libxl__sched_set_params(libxl__gc *g
 
     switch (sched) {
     case LIBXL_SCHEDULER_SEDF:
-        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
+        ret=sched_sedf_domain_set(gc, domid, scinfo);
         break;
     case LIBXL_SCHEDULER_CREDIT:
-        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
+        ret=sched_credit_domain_set(gc, domid, scinfo);
         break;
     case LIBXL_SCHEDULER_CREDIT2:
-        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
+        ret=sched_credit2_domain_set(gc, domid, scinfo);
         break;
     default:
         LOG(ERROR, "Unknown scheduler");
         ret=ERROR_INVAL;
         break;
     }
+
+    GC_FREE;
+    return ret;
+}
+
+int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_domain_sched_params *scinfo)
+{
+    GC_INIT(ctx);
+    int ret;
+
+    libxl_domain_sched_params_init(scinfo);
+
+    scinfo->sched = libxl__domain_scheduler(gc, domid);
+
+    switch (scinfo->sched) {
+    case LIBXL_SCHEDULER_SEDF:
+        ret=sched_sedf_domain_get(gc, domid, scinfo);
+        break;
+    case LIBXL_SCHEDULER_CREDIT:
+        ret=sched_credit_domain_get(gc, domid, scinfo);
+        break;
+    case LIBXL_SCHEDULER_CREDIT2:
+        ret=sched_credit2_domain_get(gc, domid, scinfo);
+        break;
+    default:
+        LOG(ERROR, "Unknown scheduler");
+        ret=ERROR_INVAL;
+        break;
+    }
+
+    GC_FREE;
     return ret;
 }
 
diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue May 29 14:56:53 2012 +0100
+++ b/tools/libxl/libxl.h	Tue May 29 14:56:53 2012 +0100
@@ -790,18 +790,11 @@ int libxl_sched_credit_params_set(libxl_
 #define LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT   -1
 #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
 
-int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_domain_sched_params *scinfo);
-int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                  libxl_domain_sched_params *scinfo);
-int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_domain_sched_params *scinfo);
-int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                   libxl_domain_sched_params *scinfo);
-int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
-                                libxl_domain_sched_params *scinfo);
-int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
-                                libxl_domain_sched_params *scinfo);
+int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_domain_sched_params *params);
+int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
+                                  const libxl_domain_sched_params *params);
+
 int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
                        libxl_trigger trigger, uint32_t vcpuid);
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue May 29 14:56:53 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Tue May 29 14:56:53 2012 +0100
@@ -174,7 +174,7 @@ int libxl__build_post(libxl__gc *gc, uin
     char **ents, **hvm_ents;
     int i;
 
-    libxl__sched_set_params (gc, domid, &(info->sched_params));
+    libxl_domain_sched_params_set(CTX, domid, &info->sched_params);
 
     libxl_cpuid_apply_policy(ctx, domid);
     if (info->cpuid != NULL)
diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue May 29 14:56:53 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue May 29 14:56:53 2012 +0100
@@ -4361,24 +4361,33 @@ int main_sharing(int argc, char **argv)
     return 0;
 }
 
-static int sched_credit_domain_get(int domid, libxl_domain_sched_params *scinfo)
+static int sched_domain_get(libxl_scheduler sched, int domid,
+                            libxl_domain_sched_params *scinfo)
 {
     int rc;
 
-    rc = libxl_sched_credit_domain_get(ctx, domid, scinfo);
-    if (rc)
-        fprintf(stderr, "libxl_sched_credit_domain_get failed.\n");
-
-    return rc;
-}
-
-static int sched_credit_domain_set(int domid, libxl_domain_sched_params *scinfo)
+    rc = libxl_domain_sched_params_get(ctx, domid, scinfo);
+    if (rc) {
+        fprintf(stderr, "libxl_domain_sched_params_get failed.\n");
+        return rc;
+    }
+    if (scinfo->sched != sched) {
+        fprintf(stderr, "libxl_domain_sched_params_get returned %s not %s.\n",
+                libxl_scheduler_to_string(scinfo->sched),
+                libxl_scheduler_to_string(sched));
+        return ERROR_INVAL;
+    }
+
+    return 0;
+}
+
+static int sched_domain_set(int domid, const libxl_domain_sched_params *scinfo)
 {
     int rc;
 
-    rc = libxl_sched_credit_domain_set(ctx, domid, scinfo);
+    rc = libxl_domain_sched_params_set(ctx, domid, scinfo);
     if (rc)
-        fprintf(stderr, "libxl_sched_credit_domain_set failed.\n");
+        fprintf(stderr, "libxl_domain_sched_params_set failed.\n");
 
     return rc;
 }
@@ -4415,7 +4424,7 @@ static int sched_credit_domain_output(in
         printf("%-33s %4s %6s %4s\n", "Name", "ID", "Weight", "Cap");
         return 0;
     }
-    rc = sched_credit_domain_get(domid, &scinfo);
+    rc = sched_domain_get(LIBXL_SCHEDULER_CREDIT, domid, &scinfo);
     if (rc)
         return rc;
     domname = libxl_domid_to_name(ctx, domid);
@@ -4450,30 +4459,6 @@ static int sched_credit_pool_output(uint
     return 0;
 }
 
-static int sched_credit2_domain_get(
-    int domid, libxl_domain_sched_params *scinfo)
-{
-    int rc;
-
-    rc = libxl_sched_credit2_domain_get(ctx, domid, scinfo);
-    if (rc)
-        fprintf(stderr, "libxl_sched_credit2_domain_get failed.\n");
-
-    return rc;
-}
-
-static int sched_credit2_domain_set(
-    int domid, libxl_domain_sched_params *scinfo)
-{
-    int rc;
-
-    rc = libxl_sched_credit2_domain_set(ctx, domid, scinfo);
-    if (rc)
-        fprintf(stderr, "libxl_sched_credit2_domain_set failed.\n");
-
-    return rc;
-}
-
 static int sched_credit2_domain_output(
     int domid)
 {
@@ -4485,7 +4470,7 @@ static int sched_credit2_domain_output(
         printf("%-33s %4s %6s\n", "Name", "ID", "Weight");
         return 0;
     }
-    rc = sched_credit2_domain_get(domid, &scinfo);
+    rc = sched_domain_get(LIBXL_SCHEDULER_CREDIT2, domid, &scinfo);
     if (rc)
         return rc;
     domname = libxl_domid_to_name(ctx, domid);
@@ -4498,29 +4483,6 @@ static int sched_credit2_domain_output(
     return 0;
 }
 
-static int sched_sedf_domain_get(
-    int domid, libxl_domain_sched_params *scinfo)
-{
-    int rc;
-
-    rc = libxl_sched_sedf_domain_get(ctx, domid, scinfo);
-    if (rc)
-        fprintf(stderr, "libxl_sched_sedf_domain_get failed.\n");
-
-    return rc;
-}
-
-static int sched_sedf_domain_set(
-    int domid, libxl_domain_sched_params *scinfo)
-{
-    int rc;
-
-    rc = libxl_sched_sedf_domain_set(ctx, domid, scinfo);
-    if (rc)
-        fprintf(stderr, "libxl_sched_sedf_domain_set failed.\n");
-    return rc;
-}
-
 static int sched_sedf_domain_output(
     int domid)
 {
@@ -4533,7 +4495,7 @@ static int sched_sedf_domain_output(
                "Slice", "Latency", "Extra", "Weight");
         return 0;
     }
-    rc = sched_sedf_domain_get(domid, &scinfo);
+    rc = sched_domain_get(LIBXL_SCHEDULER_SEDF, domid, &scinfo);
     if (rc)
         return rc;
     domname = libxl_domid_to_name(ctx, domid);
@@ -4744,7 +4706,7 @@ int main_sched_credit(int argc, char **a
                 scinfo.weight = weight;
             if (opt_c)
                 scinfo.cap = cap;
-            rc = sched_credit_domain_set(domid, &scinfo);
+            rc = sched_domain_set(domid, &scinfo);
             libxl_domain_sched_params_dispose(&scinfo);
             if (rc)
                 return -rc;
@@ -4819,7 +4781,7 @@ int main_sched_credit2(int argc, char **
             scinfo.sched = LIBXL_SCHEDULER_CREDIT2;
             if (opt_w)
                 scinfo.weight = weight;
-            rc = sched_credit2_domain_set(domid, &scinfo);
+            rc = sched_domain_set(domid, &scinfo);
             libxl_domain_sched_params_dispose(&scinfo);
             if (rc)
                 return -rc;
@@ -4939,7 +4901,7 @@ int main_sched_sedf(int argc, char **arg
                 scinfo.period = 0;
                 scinfo.slice = 0;
             }
-            rc = sched_sedf_domain_set(domid, &scinfo);
+            rc = sched_domain_set(domid, &scinfo);
             libxl_domain_sched_params_dispose(&scinfo);
             if (rc)
                 return -rc;

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-29 13:57 ` [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
@ 2012-05-30  7:26   ` Dario Faggioli
  2012-05-30  7:35     ` Ian Campbell
  2012-05-31 13:47   ` George Dunlap
  1 sibling, 1 reply; 30+ messages in thread
From: Dario Faggioli @ 2012-05-30  7:26 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Ian.Jackson, Juergen Gross, George.Dunlap, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 3909 bytes --]

On Tue, 2012-05-29 at 14:57 +0100, Ian Campbell wrote: 
> int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_sched_sedf_domain *scinfo)
> +                                libxl_domain_sched_params *scinfo)
>  {
> -    xc_domaininfo_t domaininfo;
> -    int rc;
> -
> -    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
> -    if (rc < 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> +    uint64_t period;
> +    uint64_t slice;
> +    uint64_t latency;
> +    uint16_t extratime;
> +    uint16_t weight;
> +
> +    int ret;
> +
> +    ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
> +                            &extratime, &weight);
> +    if (ret != 0) {
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
>          return ERROR_FAIL;
>      }
> -    if (rc != 1 || domaininfo.domain != domid)
> -        return ERROR_INVAL;
> -
> -
> -    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
> -                            scinfo->slice * 1000000, scinfo->latency * 1000000,
> -                            scinfo->extratime, scinfo->weight);
> -    if ( rc < 0 ) {
> +
> +    if (scinfo->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT)
> +        period = scinfo->period * 1000000;
> +    if (scinfo->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT)
> +        slice = scinfo->slice * 1000000;
> +    if (scinfo->latency != LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT)
> +        latency = scinfo->latency * 1000000;
> +    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> +        extratime = scinfo->extratime;
> +    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT)
> +        weight = scinfo->weight;
> +
> +    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
> +                            extratime, weight);
>
I only tested this on your last version of this series (and I am still
trying to find the time to test this new one) and this wasn't working.
Reason should be the fact that the SEDF code in Xen wants you to put a
domain either in the "proper-EDF" state _or_ in the
"weighted-best-effort-state", and it discriminates this by checking
whether you're passing a slice+period _or_ a weight.

IOW, if your domain has a slice+period and wants a weight, either you
set slice and period to zero, or the whole thing will fail. In fact,
with this code, the new parameter set will include both the slice+period
(as the domain has them set) and the new weight that is being set by the
call... Does this sound right?

That's why the  current int main_sched_sedf(int argc, char **argv) is
doing this:

            if (opt_p) {
                scinfo.period = period;
                scinfo.weight = 0;
            }
            if (opt_s) {
                scinfo.slice = slice;
                scinfo.weight = 0;
            }
            if (opt_l)
                scinfo.latency = latency;
            if (opt_e)
                scinfo.extratime = extra;
            if (opt_w) {
                scinfo.weight = weight;
                scinfo.period = 0;
                scinfo.slice = 0;
            } 

That being said, I'm not sure at what level we want to enforce something
like the above. The lowest level toolstack seems fine to me, which would
mean putting something like the code above in the config file parsing...
If you agree, I'll try that and let you know whether or not it works.

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-30  7:26   ` Dario Faggioli
@ 2012-05-30  7:35     ` Ian Campbell
  2012-05-30  8:00       ` Dario Faggioli
  0 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-05-30  7:35 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Juergen Gross, Ian Jackson, xen-devel

On Wed, 2012-05-30 at 08:26 +0100, Dario Faggioli wrote:
> On Tue, 2012-05-29 at 14:57 +0100, Ian Campbell wrote: 
> > int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> > -                                libxl_sched_sedf_domain *scinfo)
> > +                                libxl_domain_sched_params *scinfo)
> >  {
> > -    xc_domaininfo_t domaininfo;
> > -    int rc;
> > -
> > -    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
> > -    if (rc < 0) {
> > -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> > +    uint64_t period;
> > +    uint64_t slice;
> > +    uint64_t latency;
> > +    uint16_t extratime;
> > +    uint16_t weight;
> > +
> > +    int ret;
> > +
> > +    ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
> > +                            &extratime, &weight);
> > +    if (ret != 0) {
> > +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
> >          return ERROR_FAIL;
> >      }
> > -    if (rc != 1 || domaininfo.domain != domid)
> > -        return ERROR_INVAL;
> > -
> > -
> > -    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
> > -                            scinfo->slice * 1000000, scinfo->latency * 1000000,
> > -                            scinfo->extratime, scinfo->weight);
> > -    if ( rc < 0 ) {
> > +
> > +    if (scinfo->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT)
> > +        period = scinfo->period * 1000000;
> > +    if (scinfo->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT)
> > +        slice = scinfo->slice * 1000000;
> > +    if (scinfo->latency != LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT)
> > +        latency = scinfo->latency * 1000000;
> > +    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> > +        extratime = scinfo->extratime;
> > +    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT)
> > +        weight = scinfo->weight;
> > +
> > +    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
> > +                            extratime, weight);
> >
> I only tested this on your last version of this series (and I am still
> trying to find the time to test this new one) and this wasn't working.
> Reason should be the fact that the SEDF code in Xen wants you to put a
> domain either in the "proper-EDF" state _or_ in the
> "weighted-best-effort-state", and it discriminates this by checking
> whether you're passing a slice+period _or_ a weight.
> 
> IOW, if your domain has a slice+period and wants a weight, either you
> set slice and period to zero, or the whole thing will fail. In fact,
> with this code, the new parameter set will include both the slice+period
> (as the domain has them set) and the new weight that is being set by the
> call... Does this sound right?

I've no idea, but I trust you..

> That's why the  current int main_sched_sedf(int argc, char **argv) is
> doing this:
> 
>             if (opt_p) {
>                 scinfo.period = period;
>                 scinfo.weight = 0;
>             }
>             if (opt_s) {
>                 scinfo.slice = slice;
>                 scinfo.weight = 0;
>             }
>             if (opt_l)
>                 scinfo.latency = latency;
>             if (opt_e)
>                 scinfo.extratime = extra;
>             if (opt_w) {
>                 scinfo.weight = weight;
>                 scinfo.period = 0;
>                 scinfo.slice = 0;
>             } 
>
> That being said, I'm not sure at what level we want to enforce something
> like the above. The lowest level toolstack seems fine to me, which would
> mean putting something like the code above in the config file parsing...
> If you agree, I'll try that and let you know whether or not it works.

If you could provide an incremental patch that would be much
appreciated.

IMHO it would be fine (and a good idea) for libxl to return ERROR_INVAL
if the conditions aren't met too. If you want to also check it in xl's
config file parsing and either fix it up like the above or error out
then please do.


> 
> Regards,
> Dario
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-30  7:35     ` Ian Campbell
@ 2012-05-30  8:00       ` Dario Faggioli
  2012-06-01 10:14         ` Ian Campbell
  0 siblings, 1 reply; 30+ messages in thread
From: Dario Faggioli @ 2012-05-30  8:00 UTC (permalink / raw)
  To: Ian Campbell; +Cc: George Dunlap, Juergen Gross, Ian Jackson, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1062 bytes --]

On Wed, 2012-05-30 at 08:35 +0100, Ian Campbell wrote: 
> > That being said, I'm not sure at what level we want to enforce something
> > like the above. The lowest level toolstack seems fine to me, which would
> > mean putting something like the code above in the config file parsing...
> > If you agree, I'll try that and let you know whether or not it works.
> 
> If you could provide an incremental patch that would be much
> appreciated.
> 
I sure can. :-)

> IMHO it would be fine (and a good idea) for libxl to return ERROR_INVAL
> if the conditions aren't met too. 
>
Ok, sounds reasonable.

> If you want to also check it in xl's
> config file parsing and either fix it up like the above or error out
> then please do.
>
Let's see what fits better...

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler
  2012-05-29 13:56 ` [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler Ian Campbell
@ 2012-05-31 13:19   ` George Dunlap
  2012-05-31 15:11     ` Ian Jackson
  2012-06-01  9:51     ` Ian Campbell
  0 siblings, 2 replies; 30+ messages in thread
From: George Dunlap @ 2012-05-31 13:19 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Juergen Gross, Ian Jackson, Dario Faggioli, xen-devel

On 29/05/12 14:56, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1338299619 -3600
> # Node ID 980a25d6ad12ba0f10fa616255b9382cc14ce69e
> # Parent  12537c670e9ea9e7f73747e203ca318107b82cd9
> libxl: add internal function to get a domain's scheduler.
>
> This takes into account cpupools.
>
> Add a helper to get the info for a single cpu pool, refactor libxl_list_cpupool
> t use this. While there fix the leaks due to not disposing the partial list on
> realloc failure in that function.
>
> Fix the failure of sched_domain_output to free the poolinfo list.
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

As an aside, I would prefer that the clean-up happen in separate 
patches; having a single patch do several orthogonal things makes it 
hard for me to tell what goes with what, both for review, and in case 
someone in the future needs to do archaeology and figure out what's 
going on.  Not really worth a respin just for that, though.
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
> ---
> v2: add libxl_cpupoolinfo_list_free, use it in libxl_cpupool_list error path.
>      Also use it in libxl_cpupool_cpuremove_node (which fixes a leak) and in
>      libxl_name_to_cpupoolid, sched_domain_output and main_cpupoollist (which
>      also fixes a leak).
>
>      Also in libxl_cpupool_cpuremove_node use libxl_cputopology_list_free
>      instead of open coding
>
> diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Tue May 29 12:56:57 2012 +0100
> +++ b/tools/libxl/libxl.c	Tue May 29 14:53:39 2012 +0100
> @@ -552,41 +552,70 @@ int libxl_domain_info(libxl_ctx *ctx, li
>       return 0;
>   }
>
> +static int cpupool_info(libxl__gc *gc,
> +                        libxl_cpupoolinfo *info,
> +                        uint32_t poolid,
> +                        bool exact /* exactly poolid or>= poolid */)
> +{
> +    xc_cpupoolinfo_t *xcinfo;
> +    int rc = ERROR_FAIL;
> +
> +    xcinfo = xc_cpupool_getinfo(CTX->xch, poolid);
> +    if (xcinfo == NULL)
> +        return ERROR_FAIL;
> +
> +    if (exact&&  xcinfo->cpupool_id != poolid)
> +        goto out;
> +
> +    info->poolid = xcinfo->cpupool_id;
> +    info->sched = xcinfo->sched_id;
> +    info->n_dom = xcinfo->n_dom;
> +    if (libxl_cpumap_alloc(CTX,&info->cpumap))
> +        goto out;
> +    memcpy(info->cpumap.map, xcinfo->cpumap, info->cpumap.size);
> +
> +    rc = 0;
> +out:
> +    xc_cpupool_infofree(CTX->xch, xcinfo);
> +    return rc;
> +}
> +
> +int libxl_cpupool_info(libxl_ctx *ctx,
> +                       libxl_cpupoolinfo *info, uint32_t poolid)
> +{
> +    GC_INIT(ctx);
> +    int rc = cpupool_info(gc, info, poolid, true);
> +    GC_FREE;
> +    return rc;
> +}
> +
>   libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx *ctx, int *nb_pool)
>   {
> -    libxl_cpupoolinfo *ptr, *tmp;
> +    GC_INIT(ctx);
> +    libxl_cpupoolinfo info, *ptr, *tmp;
>       int i;
> -    xc_cpupoolinfo_t *info;
>       uint32_t poolid;
>
>       ptr = NULL;
>
>       poolid = 0;
>       for (i = 0;; i++) {
> -        info = xc_cpupool_getinfo(ctx->xch, poolid);
> -        if (info == NULL)
> +        if (cpupool_info(gc,&info, poolid, false))
>               break;
>           tmp = realloc(ptr, (i + 1) * sizeof(libxl_cpupoolinfo));
>           if (!tmp) {
>               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating cpupool info");
> -            free(ptr);
> -            xc_cpupool_infofree(ctx->xch, info);
> -            return NULL;
> +            libxl_cpupoolinfo_list_free(ptr, i);
> +            goto out;
>           }
>           ptr = tmp;
> -        ptr[i].poolid = info->cpupool_id;
> -        ptr[i].sched = info->sched_id;
> -        ptr[i].n_dom = info->n_dom;
> -        if (libxl_cpumap_alloc(ctx,&ptr[i].cpumap)) {
> -            xc_cpupool_infofree(ctx->xch, info);
> -            break;
> -        }
> -        memcpy(ptr[i].cpumap.map, info->cpumap, ptr[i].cpumap.size);
> -        poolid = info->cpupool_id + 1;
> -        xc_cpupool_infofree(ctx->xch, info);
> +        ptr[i] = info;
> +        poolid = info.poolid + 1;
>       }
>
>       *nb_pool = i;
> +out:
> +    GC_FREE;
>       return ptr;
>   }
>
> @@ -3932,14 +3961,10 @@ int libxl_cpupool_cpuremove_node(libxl_c
>           }
>       }
>
> -    for (cpu = 0; cpu<  nr_cpus; cpu++)
> -        libxl_cputopology_dispose(&topology[cpu]);
> -    free(topology);
> +    libxl_cputopology_list_free(topology, nr_cpus);
>
>   out:
> -    for (p = 0; p<  n_pools; p++) {
> -        libxl_cpupoolinfo_dispose(poolinfo + p);
> -    }
> +    libxl_cpupoolinfo_list_free(poolinfo, n_pools);
>
>       return ret;
>   }
> diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h	Tue May 29 12:56:57 2012 +0100
> +++ b/tools/libxl/libxl.h	Tue May 29 14:53:39 2012 +0100
> @@ -576,6 +576,7 @@ int libxl_domain_info(libxl_ctx*, libxl_
>   libxl_dominfo * libxl_list_domain(libxl_ctx*, int *nb_domain);
>   void libxl_dominfo_list_free(libxl_dominfo *list, int nr);
>   libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
> +void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nr);
>   libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm);
>   void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
>
> @@ -829,6 +830,7 @@ int libxl_cpupool_cpuadd_node(libxl_ctx
>   int libxl_cpupool_cpuremove(libxl_ctx *ctx, uint32_t poolid, int cpu);
>   int libxl_cpupool_cpuremove_node(libxl_ctx *ctx, uint32_t poolid, int node, int *cpus);
>   int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid);
> +int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t poolid);
>
>   int libxl_domid_valid_guest(uint32_t domid);
>
> diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Tue May 29 12:56:57 2012 +0100
> +++ b/tools/libxl/libxl_dom.c	Tue May 29 14:53:39 2012 +0100
> @@ -93,6 +93,41 @@ int libxl__domain_shutdown_reason(libxl_
>       return (info.flags>>  XEN_DOMINF_shutdownshift)&  XEN_DOMINF_shutdownmask;
>   }
>
> +int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
> +{
> +    xc_domaininfo_t info;
> +    int ret;
> +
> +    ret = xc_domain_getinfolist(CTX->xch, domid, 1,&info);
> +    if (ret != 1)
> +        return ERROR_FAIL;
> +    if (info.domain != domid)
> +        return ERROR_FAIL;
> +
> +    return info.cpupool;
> +}
> +
> +libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid)
> +{
> +    uint32_t cpupool = libxl__domain_cpupool(gc, domid);
> +    libxl_cpupoolinfo poolinfo;
> +    libxl_scheduler sched = LIBXL_SCHEDULER_UNKNOWN;
> +    int rc;
> +
> +    if (cpupool<  0)
> +        return sched;
> +
> +    rc = libxl_cpupool_info(CTX,&poolinfo, cpupool);
> +    if (rc<  0)
> +        goto out;
> +
> +    sched = poolinfo.sched;
> +
> +out:
> +    libxl_cpupoolinfo_dispose(&poolinfo);
> +    return sched;
> +}
> +
>   int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>                 libxl_domain_build_info *info, libxl__domain_build_state *state)
>   {
> diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Tue May 29 12:56:57 2012 +0100
> +++ b/tools/libxl/libxl_internal.h	Tue May 29 14:53:39 2012 +0100
> @@ -738,6 +738,8 @@ _hidden int libxl__file_reference_unmap(
>   /* from xl_dom */
>   _hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
>   _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
> +_hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
> +_hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
>   _hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
>   #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
>       libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
> diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Tue May 29 12:56:57 2012 +0100
> +++ b/tools/libxl/libxl_types.idl	Tue May 29 14:53:39 2012 +0100
> @@ -107,7 +107,9 @@ libxl_bios_type = Enumeration("bios_type
>       ])
>
>   # Consistent with values defined in domctl.h
> +# Except unknown which we have made up
>   libxl_scheduler = Enumeration("scheduler", [
> +    (0, "unknown"),
>       (4, "sedf"),
>       (5, "credit"),
>       (6, "credit2"),
> diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_utils.c
> --- a/tools/libxl/libxl_utils.c	Tue May 29 12:56:57 2012 +0100
> +++ b/tools/libxl/libxl_utils.c	Tue May 29 14:53:39 2012 +0100
> @@ -133,9 +133,8 @@ int libxl_name_to_cpupoolid(libxl_ctx *c
>               }
>               free(poolname);
>           }
> -        libxl_cpupoolinfo_dispose(poolinfo + i);
>       }
> -    free(poolinfo);
> +    libxl_cpupoolinfo_list_free(poolinfo, nb_pools);
>       return ret;
>   }
>
> @@ -686,6 +685,14 @@ void libxl_vminfo_list_free(libxl_vminfo
>       free(list);
>   }
>
> +void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nr)
> +{
> +    int i;
> +    for (i = 0; i<  nr; i++)
> +        libxl_cpupoolinfo_dispose(&list[i]);
> +    free(list);
> +}
> +
>   int libxl_domid_valid_guest(uint32_t domid)
>   {
>       /* returns 1 if the value _could_ be a valid guest domid, 0 otherwise
> diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Tue May 29 12:56:57 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c	Tue May 29 14:53:39 2012 +0100
> @@ -4608,11 +4608,8 @@ static int sched_domain_output(libxl_sch
>                   break;
>           }
>       }
> -    if (poolinfo) {
> -        for (p = 0; p<  n_pools; p++) {
> -            libxl_cpupoolinfo_dispose(poolinfo + p);
> -        }
> -    }
> +    if (poolinfo)
> +        libxl_cpupoolinfo_list_free(poolinfo, n_pools);
>       return 0;
>   }
>
> @@ -6119,8 +6116,9 @@ int main_cpupoollist(int argc, char **ar
>                   printf("\n");
>               }
>           }
> -        libxl_cpupoolinfo_dispose(poolinfo + p);
> -    }
> +    }
> +
> +    libxl_cpupoolinfo_list_free(poolinfo, n_pools);
>
>       return ret;
>   }

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params
  2012-05-29 13:57 ` [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params Ian Campbell
@ 2012-05-31 13:21   ` George Dunlap
  2012-05-31 15:12     ` Ian Jackson
  0 siblings, 1 reply; 30+ messages in thread
From: George Dunlap @ 2012-05-31 13:21 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Juergen Gross, Ian Jackson, Dario Faggioli, xen-devel

On 29/05/12 14:57, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1338299709 -3600
> # Node ID 73d8274c0b6859b4528af75a7405e546d659f130
> # Parent  980a25d6ad12ba0f10fa616255b9382cc14ce69e
> libxl: rename libxl_sched_params to libxl_domain_sched_params
>
> Remove credit scheduler global options from the struct, they were never used
> anyway.
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> v2: use libxl_domain_sched_params which fits in better with future changes.
>
> diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Tue May 29 14:53:39 2012 +0100
> +++ b/tools/libxl/libxl_dom.c	Tue May 29 14:55:09 2012 +0100
> @@ -42,7 +42,8 @@ libxl_domain_type libxl__domain_type(lib
>           return LIBXL_DOMAIN_TYPE_PV;
>   }
>
> -int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams)
> +int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> +                            libxl_domain_sched_params *scparams)
>   {
>       libxl_ctx *ctx = libxl__gc_owner(gc);
>       libxl_scheduler sched;
> diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Tue May 29 14:53:39 2012 +0100
> +++ b/tools/libxl/libxl_internal.h	Tue May 29 14:55:09 2012 +0100
> @@ -740,7 +740,8 @@ _hidden libxl_domain_type libxl__domain_
>   _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
>   _hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
>   _hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
> -_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
> +_hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> +                                    libxl_domain_sched_params *scparams);
>   #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
>       libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
>   typedef struct {
> diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Tue May 29 14:53:39 2012 +0100
> +++ b/tools/libxl/libxl_types.idl	Tue May 29 14:55:09 2012 +0100
> @@ -224,11 +224,9 @@ libxl_domain_create_info = Struct("domai
>
>   MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
>
> -libxl_sched_params = Struct("sched_params",[
> +libxl_domain_sched_params = Struct("domain_sched_params",[
>       ("weight",       integer),
>       ("cap",          integer),
> -    ("tslice_ms",    integer),
> -    ("ratelimit_us", integer),
>       ("period",       integer),
>       ("slice",        integer),
>       ("latency",      integer),
> @@ -262,7 +260,7 @@ libxl_domain_build_info = Struct("domain
>       # extra parameters pass directly to qemu for HVM guest, NULL terminated
>       ("extra_hvm",        libxl_string_list),
>       #  parameters for all type of scheduler
> -    ("sched_params",     libxl_sched_params),
> +    ("sched_params",     libxl_domain_sched_params),
>
>       ("u", KeyedUnion(None, libxl_domain_type, "type",
>                   [("hvm", Struct(None, [("firmware",         string),
> diff -r 980a25d6ad12 -r 73d8274c0b68 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Tue May 29 14:53:39 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c	Tue May 29 14:55:09 2012 +0100
> @@ -633,10 +633,6 @@ static void parse_config_data(const char
>           b_info->sched_params.weight = l;
>       if (!xlu_cfg_get_long (config, "cap",&l, 0))
>           b_info->sched_params.cap = l;
> -    if (!xlu_cfg_get_long (config, "tslice_ms",&l, 0))
> -        b_info->sched_params.tslice_ms = l;
> -    if (!xlu_cfg_get_long (config, "ratelimit_us",&l, 0))
> -        b_info->sched_params.ratelimit_us = l;
>       if (!xlu_cfg_get_long (config, "period",&l, 0))
>           b_info->sched_params.period = l;
>       if (!xlu_cfg_get_long (config, "slice",&l, 0))

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-29 13:57 ` [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
  2012-05-30  7:26   ` Dario Faggioli
@ 2012-05-31 13:47   ` George Dunlap
  2012-05-31 15:13     ` Ian Jackson
  1 sibling, 1 reply; 30+ messages in thread
From: George Dunlap @ 2012-05-31 13:47 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Juergen Gross, Ian Jackson, Dario Faggioli, xen-devel

On 29/05/12 14:57, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1338299729 -3600
> # Node ID 274de8e1e0d116070d34731d93b53ce44530e5a0
> # Parent  73d8274c0b6859b4528af75a7405e546d659f130
> libxl: make it possible to explicitly specify default sched params
>
> To do so we define a discriminating value which is never a valid real value for
> each parameter.
>
> While there:
>
>   - removed libxl_sched_*_domain in favour of libxl_domain_sched_params.
>   - use this new functionality for the various xl commands which set sched
>     parameters, which saves an explicit read-modify-write in xl.
>   - removed call of xc_domain_getinfolist from a few functions which weren't
>     actually using the result (looks like a cut and paste error)
>   - fix xl which was setting period for a variety of different config keys.
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

If you do end up re-spinning this series for some reason, you might just 
mention in this description that you're about to change the public 
interface and get rid of the per-scheduler set and get functions in two 
changesets.
>
> diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c       Tue May 29 14:55:09 2012 +0100
> +++ b/tools/libxl/libxl.c       Tue May 29 14:55:29 2012 +0100
> @@ -3197,19 +3197,19 @@ libxl_scheduler libxl_get_scheduler(libx
>   }
>
>   int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_sched_credit_domain *scinfo)
> +                                  libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit sdom;
>       int rc;
>
> -    libxl_sched_credit_domain_init(scinfo);
> -
>       rc = xc_sched_credit_domain_get(ctx->xch, domid,&sdom);
>       if (rc != 0) {
>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
>           return ERROR_FAIL;
>       }
>
> +    libxl_domain_sched_params_init(scinfo);
> +    scinfo->sched = LIBXL_SCHEDULER_CREDIT;
>       scinfo->weight = sdom.weight;
>       scinfo->cap = sdom.cap;
>
> @@ -3217,7 +3217,7 @@ int libxl_sched_credit_domain_get(libxl_
>   }
>
>   int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_sched_credit_domain *scinfo)
> +                                  libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit sdom;
>       xc_domaininfo_t domaininfo;
> @@ -3231,22 +3231,33 @@ int libxl_sched_credit_domain_set(libxl_
>       if (rc != 1 || domaininfo.domain != domid)
>           return ERROR_INVAL;
>
> -
> -    if (scinfo->weight<  1 || scinfo->weight>  65535) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> -            "Cpu weight out of range, valid values are within range from 1 to 65535");
> -        return ERROR_INVAL;
> +    rc = xc_sched_credit_domain_get(ctx->xch, domid,&sdom);
> +    if (rc != 0) {
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
> +        return ERROR_FAIL;
>       }
>
> -    if (scinfo->cap<  0 || scinfo->cap>  (domaininfo.max_vcpu_id + 1) * 100) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> -            "Cpu cap out of range, valid range is from 0 to %d for specified number of vcpus",
> -            ((domaininfo.max_vcpu_id + 1) * 100));
> -        return ERROR_INVAL;
> +    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
> +        if (scinfo->weight<  1 || scinfo->weight>  65535) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "Cpu weight out of range, "
> +                       "valid values are within range from 1 to 65535");
> +            return ERROR_INVAL;
> +        }
> +        sdom.weight = scinfo->weight;
>       }
>
> -    sdom.weight = scinfo->weight;
> -    sdom.cap = scinfo->cap;
> +    if (scinfo->cap != LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT) {
> +        if (scinfo->cap<  0
> +            || scinfo->cap>  (domaininfo.max_vcpu_id + 1) * 100) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                "Cpu cap out of range, "
> +                "valid range is from 0 to %d for specified number of vcpus",
> +                       ((domaininfo.max_vcpu_id + 1) * 100));
> +            return ERROR_INVAL;
> +        }
> +        sdom.cap = scinfo->cap;
> +    }
>
>       rc = xc_sched_credit_domain_set(ctx->xch, domid,&sdom);
>       if ( rc<  0 ) {
> @@ -3319,13 +3330,11 @@ int libxl_sched_credit_params_set(libxl_
>   }
>
>   int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_sched_credit2_domain *scinfo)
> +                                   libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit2 sdom;
>       int rc;
>
> -    libxl_sched_credit2_domain_init(scinfo);
> -
>       rc = xc_sched_credit2_domain_get(ctx->xch, domid,&sdom);
>       if (rc != 0) {
>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> @@ -3333,36 +3342,37 @@ int libxl_sched_credit2_domain_get(libxl
>           return ERROR_FAIL;
>       }
>
> +    libxl_domain_sched_params_init(scinfo);
> +    scinfo->sched = LIBXL_SCHEDULER_CREDIT2;
>       scinfo->weight = sdom.weight;
>
>       return 0;
>   }
>
>   int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_sched_credit2_domain *scinfo)
> +                                   libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit2 sdom;
> -    xc_domaininfo_t domaininfo;
>       int rc;
>
> -    rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo);
> -    if (rc<  0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> +    rc = xc_sched_credit2_domain_get(ctx->xch, domid,&sdom);
> +    if (rc != 0) {
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> +                         "getting domain sched credit2");
>           return ERROR_FAIL;
>       }
> -    if (rc != 1 || domaininfo.domain != domid)
> -        return ERROR_INVAL;
> -
> -
> -    if (scinfo->weight<  1 || scinfo->weight>  65535) {
> -        LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
> -            "Cpu weight out of range, valid values are within range from "
> -            "1 to 65535");
> -        return ERROR_INVAL;
> +
> +    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
> +        if (scinfo->weight<  1 || scinfo->weight>  65535) {
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                       "Cpu weight out of range, "
> +                       "valid values are within range from "
> +                       "1 to 65535");
> +            return ERROR_INVAL;
> +        }
> +        sdom.weight = scinfo->weight;
>       }
>
> -    sdom.weight = scinfo->weight;
> -
>       rc = xc_sched_credit2_domain_set(ctx->xch, domid,&sdom);
>       if ( rc<  0 ) {
>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> @@ -3374,7 +3384,7 @@ int libxl_sched_credit2_domain_set(libxl
>   }
>
>   int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_sched_sedf_domain *scinfo)
> +                                libxl_domain_sched_params *scinfo)
>   {
>       uint64_t period;
>       uint64_t slice;
> @@ -3383,8 +3393,6 @@ int libxl_sched_sedf_domain_get(libxl_ct
>       uint16_t weight;
>       int rc;
>
> -    libxl_sched_sedf_domain_init(scinfo);
> -
>       rc = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency,
>                               &extratime,&weight);
>       if (rc != 0) {
> @@ -3392,6 +3400,8 @@ int libxl_sched_sedf_domain_get(libxl_ct
>           return ERROR_FAIL;
>       }
>
> +    libxl_domain_sched_params_init(scinfo);
> +    scinfo->sched = LIBXL_SCHEDULER_SEDF;
>       scinfo->period = period / 1000000;
>       scinfo->slice = slice / 1000000;
>       scinfo->latency = latency / 1000000;
> @@ -3402,24 +3412,37 @@ int libxl_sched_sedf_domain_get(libxl_ct
>   }
>
>   int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_sched_sedf_domain *scinfo)
> +                                libxl_domain_sched_params *scinfo)
>   {
> -    xc_domaininfo_t domaininfo;
> -    int rc;
> -
> -    rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo);
> -    if (rc<  0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> +    uint64_t period;
> +    uint64_t slice;
> +    uint64_t latency;
> +    uint16_t extratime;
> +    uint16_t weight;
> +
> +    int ret;
> +
> +    ret = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency,
> +&extratime,&weight);
> +    if (ret != 0) {
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
>           return ERROR_FAIL;
>       }
> -    if (rc != 1 || domaininfo.domain != domid)
> -        return ERROR_INVAL;
> -
> -
> -    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
> -                            scinfo->slice * 1000000, scinfo->latency * 1000000,
> -                            scinfo->extratime, scinfo->weight);
> -    if ( rc<  0 ) {
> +
> +    if (scinfo->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT)
> +        period = scinfo->period * 1000000;
> +    if (scinfo->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT)
> +        slice = scinfo->slice * 1000000;
> +    if (scinfo->latency != LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT)
> +        latency = scinfo->latency * 1000000;
> +    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> +        extratime = scinfo->extratime;
> +    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT)
> +        weight = scinfo->weight;
> +
> +    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
> +                            extratime, weight);
> +    if ( ret<  0 ) {
>           LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
>           return ERROR_FAIL;
>       }
> diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h       Tue May 29 14:55:09 2012 +0100
> +++ b/tools/libxl/libxl.h       Tue May 29 14:55:29 2012 +0100
> @@ -775,23 +775,33 @@ int libxl_set_vcpuonline(libxl_ctx *ctx,
>
>   libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx);
>
> -
> -int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_sched_credit_domain *scinfo);
> -int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_sched_credit_domain *scinfo);
> +/* Per-scheduler parameters */
>   int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid,
>                                     libxl_sched_credit_params *scinfo);
>   int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
>                                     libxl_sched_credit_params *scinfo);
> +
> +/* Scheduler Per-domain parameters */
> +
> +#define LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT    -1
> +#define LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT       -1
> +#define LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT    -1
> +#define LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT     -1
> +#define LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT   -1
> +#define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
> +
> +int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
> +                                  libxl_domain_sched_params *scinfo);
> +int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
> +                                  libxl_domain_sched_params *scinfo);
>   int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_sched_credit2_domain *scinfo);
> +                                   libxl_domain_sched_params *scinfo);
>   int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_sched_credit2_domain *scinfo);
> +                                   libxl_domain_sched_params *scinfo);
>   int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_sched_sedf_domain *scinfo);
> +                                libxl_domain_sched_params *scinfo);
>   int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_sched_sedf_domain *scinfo);
> +                                libxl_domain_sched_params *scinfo);
>   int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
>                          libxl_trigger trigger, uint32_t vcpuid);
>   int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
> diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c   Tue May 29 14:55:09 2012 +0100
> +++ b/tools/libxl/libxl_dom.c   Tue May 29 14:55:29 2012 +0100
> @@ -45,34 +45,26 @@ libxl_domain_type libxl__domain_type(lib
>   int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
>                               libxl_domain_sched_params *scparams)
>   {
> -    libxl_ctx *ctx = libxl__gc_owner(gc);
> -    libxl_scheduler sched;
> -    libxl_sched_sedf_domain sedf_info;
> -    libxl_sched_credit_domain credit_info;
> -    libxl_sched_credit2_domain credit2_info;
> +    libxl_scheduler sched = scparams->sched;
>       int ret;
>
> -    sched = libxl_get_scheduler (ctx);
> +    if (sched == LIBXL_SCHEDULER_UNKNOWN)
> +        sched = libxl__domain_scheduler(gc, domid);
> +
>       switch (sched) {
>       case LIBXL_SCHEDULER_SEDF:
> -      sedf_info.period = scparams->period;
> -      sedf_info.slice = scparams->slice;
> -      sedf_info.latency = scparams->latency;
> -      sedf_info.extratime = scparams->extratime;
> -      sedf_info.weight = scparams->weight;
> -      ret=libxl_sched_sedf_domain_set(ctx, domid,&sedf_info);
> -      break;
> +        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
> +        break;
>       case LIBXL_SCHEDULER_CREDIT:
> -      credit_info.weight = scparams->weight;
> -      credit_info.cap = scparams->cap;
> -      ret=libxl_sched_credit_domain_set(ctx, domid,&credit_info);
> -      break;
> +        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
> +        break;
>       case LIBXL_SCHEDULER_CREDIT2:
> -      credit2_info.weight = scparams->weight;
> -      ret=libxl_sched_credit2_domain_set(ctx, domid,&credit2_info);
> -      break;
> +        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
> +        break;
>       default:
> -      ret=-1;
> +        LOG(ERROR, "Unknown scheduler");
> +        ret=ERROR_INVAL;
> +        break;
>       }
>       return ret;
>   }
> diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl       Tue May 29 14:55:09 2012 +0100
> +++ b/tools/libxl/libxl_types.idl       Tue May 29 14:55:29 2012 +0100
> @@ -225,12 +225,13 @@ libxl_domain_create_info = Struct("domai
>   MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
>
>   libxl_domain_sched_params = Struct("domain_sched_params",[
> -    ("weight",       integer),
> -    ("cap",          integer),
> -    ("period",       integer),
> -    ("slice",        integer),
> -    ("latency",      integer),
> -    ("extratime",    integer),
> +    ("sched",        libxl_scheduler),
> +    ("weight",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
> +    ("cap",          integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT'}),
> +    ("period",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT'}),
> +    ("slice",        integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),
> +    ("latency",      integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
> +    ("extratime",    integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
>       ], dir=DIR_IN)
>
>   libxl_domain_build_info = Struct("domain_build_info",[
> @@ -425,28 +426,11 @@ libxl_cputopology = Struct("cputopology"
>       ("node", uint32),
>       ], dir=DIR_OUT)
>
> -libxl_sched_credit_domain = Struct("sched_credit_domain", [
> -    ("weight", integer),
> -    ("cap", integer),
> -    ])
> -
>   libxl_sched_credit_params = Struct("sched_credit_params", [
>       ("tslice_ms", integer),
>       ("ratelimit_us", integer),
>       ], dispose_fn=None)
>
> -libxl_sched_credit2_domain = Struct("sched_credit2_domain", [
> -    ("weight", integer),
> -    ])
> -
> -libxl_sched_sedf_domain = Struct("sched_sedf_domain", [
> -    ("period", integer),
> -    ("slice", integer),
> -    ("latency", integer),
> -    ("extratime", integer),
> -    ("weight", integer),
> -    ])
> -
>   libxl_domain_remus_info = Struct("domain_remus_info",[
>       ("interval",     integer),
>       ("blackhole",    bool),
> diff -r 73d8274c0b68 -r 274de8e1e0d1 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c  Tue May 29 14:55:09 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c  Tue May 29 14:55:29 2012 +0100
> @@ -628,7 +628,8 @@ static void parse_config_data(const char
>
>       libxl_domain_build_info_init_type(b_info, c_info->type);
>
> -    /* the following is the actual config parsing with overriding values in the structures */
> +    /* the following is the actual config parsing with overriding
> +     * values in the structures */
>       if (!xlu_cfg_get_long (config, "cpu_weight",&l, 0))
>           b_info->sched_params.weight = l;
>       if (!xlu_cfg_get_long (config, "cap",&l, 0))
> @@ -636,11 +637,11 @@ static void parse_config_data(const char
>       if (!xlu_cfg_get_long (config, "period",&l, 0))
>           b_info->sched_params.period = l;
>       if (!xlu_cfg_get_long (config, "slice",&l, 0))
> -        b_info->sched_params.period = l;
> +        b_info->sched_params.slice = l;
>       if (!xlu_cfg_get_long (config, "latency",&l, 0))
> -        b_info->sched_params.period = l;
> +        b_info->sched_params.latency = l;
>       if (!xlu_cfg_get_long (config, "extratime",&l, 0))
> -        b_info->sched_params.period = l;
> +        b_info->sched_params.extratime = l;
>
>       if (!xlu_cfg_get_long (config, "vcpus",&l, 0)) {
>           b_info->max_vcpus = l;
> @@ -4360,7 +4361,7 @@ int main_sharing(int argc, char **argv)
>       return 0;
>   }
>
> -static int sched_credit_domain_get(int domid, libxl_sched_credit_domain *scinfo)
> +static int sched_credit_domain_get(int domid, libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> @@ -4371,7 +4372,7 @@ static int sched_credit_domain_get(int d
>       return rc;
>   }
>
> -static int sched_credit_domain_set(int domid, libxl_sched_credit_domain *scinfo)
> +static int sched_credit_domain_set(int domid, libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> @@ -4407,7 +4408,7 @@ static int sched_credit_params_get(int p
>   static int sched_credit_domain_output(int domid)
>   {
>       char *domname;
> -    libxl_sched_credit_domain scinfo;
> +    libxl_domain_sched_params scinfo;
>       int rc;
>
>       if (domid<  0) {
> @@ -4424,7 +4425,7 @@ static int sched_credit_domain_output(in
>           scinfo.weight,
>           scinfo.cap);
>       free(domname);
> -    libxl_sched_credit_domain_dispose(&scinfo);
> +    libxl_domain_sched_params_dispose(&scinfo);
>       return 0;
>   }
>
> @@ -4450,7 +4451,7 @@ static int sched_credit_pool_output(uint
>   }
>
>   static int sched_credit2_domain_get(
> -    int domid, libxl_sched_credit2_domain *scinfo)
> +    int domid, libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> @@ -4462,7 +4463,7 @@ static int sched_credit2_domain_get(
>   }
>
>   static int sched_credit2_domain_set(
> -    int domid, libxl_sched_credit2_domain *scinfo)
> +    int domid, libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> @@ -4477,7 +4478,7 @@ static int sched_credit2_domain_output(
>       int domid)
>   {
>       char *domname;
> -    libxl_sched_credit2_domain scinfo;
> +    libxl_domain_sched_params scinfo;
>       int rc;
>
>       if (domid<  0) {
> @@ -4493,12 +4494,12 @@ static int sched_credit2_domain_output(
>           domid,
>           scinfo.weight);
>       free(domname);
> -    libxl_sched_credit2_domain_dispose(&scinfo);
> +    libxl_domain_sched_params_dispose(&scinfo);
>       return 0;
>   }
>
>   static int sched_sedf_domain_get(
> -    int domid, libxl_sched_sedf_domain *scinfo)
> +    int domid, libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> @@ -4510,7 +4511,7 @@ static int sched_sedf_domain_get(
>   }
>
>   static int sched_sedf_domain_set(
> -    int domid, libxl_sched_sedf_domain *scinfo)
> +    int domid, libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> @@ -4524,7 +4525,7 @@ static int sched_sedf_domain_output(
>       int domid)
>   {
>       char *domname;
> -    libxl_sched_sedf_domain scinfo;
> +    libxl_domain_sched_params scinfo;
>       int rc;
>
>       if (domid<  0) {
> @@ -4545,7 +4546,7 @@ static int sched_sedf_domain_output(
>           scinfo.extratime,
>           scinfo.weight);
>       free(domname);
> -    libxl_sched_sedf_domain_dispose(&scinfo);
> +    libxl_domain_sched_params_dispose(&scinfo);
>       return 0;
>   }
>
> @@ -4622,7 +4623,6 @@ static int sched_domain_output(libxl_sch
>    */
>   int main_sched_credit(int argc, char **argv)
>   {
> -    libxl_sched_credit_domain scinfo;
>       const char *dom = NULL;
>       const char *cpupool = NULL;
>       int weight = 256, cap = 0, opt_w = 0, opt_c = 0;
> @@ -4697,7 +4697,7 @@ int main_sched_credit(int argc, char **a
>       }
>
>       if (opt_s) {
> -        libxl_sched_credit_params  scparam;
> +        libxl_sched_credit_params scparam;
>           uint32_t poolid = 0;
>
>           if (cpupool) {
> @@ -4733,20 +4733,19 @@ int main_sched_credit(int argc, char **a
>       } else {
>           find_domain(dom);
>
> -        rc = sched_credit_domain_get(domid,&scinfo);
> -        if (rc)
> -            return -rc;
> -
>           if (!opt_w&&  !opt_c) { /* output credit scheduler info */
>               sched_credit_domain_output(-1);
>               return -sched_credit_domain_output(domid);
>           } else { /* set credit scheduler paramaters */
> +            libxl_domain_sched_params scinfo;
> +            libxl_domain_sched_params_init(&scinfo);
> +            scinfo.sched = LIBXL_SCHEDULER_CREDIT;
>               if (opt_w)
>                   scinfo.weight = weight;
>               if (opt_c)
>                   scinfo.cap = cap;
>               rc = sched_credit_domain_set(domid,&scinfo);
> -            libxl_sched_credit_domain_dispose(&scinfo);
> +            libxl_domain_sched_params_dispose(&scinfo);
>               if (rc)
>                   return -rc;
>           }
> @@ -4757,7 +4756,6 @@ int main_sched_credit(int argc, char **a
>
>   int main_sched_credit2(int argc, char **argv)
>   {
> -    libxl_sched_credit2_domain scinfo;
>       const char *dom = NULL;
>       const char *cpupool = NULL;
>       int weight = 256, opt_w = 0;
> @@ -4812,18 +4810,17 @@ int main_sched_credit2(int argc, char **
>       } else {
>           find_domain(dom);
>
> -        rc = sched_credit2_domain_get(domid,&scinfo);
> -        if (rc)
> -            return -rc;
> -
>           if (!opt_w) { /* output credit2 scheduler info */
>               sched_credit2_domain_output(-1);
>               return -sched_credit2_domain_output(domid);
>           } else { /* set credit2 scheduler paramaters */
> +            libxl_domain_sched_params scinfo;
> +            libxl_domain_sched_params_init(&scinfo);
> +            scinfo.sched = LIBXL_SCHEDULER_CREDIT2;
>               if (opt_w)
>                   scinfo.weight = weight;
>               rc = sched_credit2_domain_set(domid,&scinfo);
> -            libxl_sched_credit2_domain_dispose(&scinfo);
> +            libxl_domain_sched_params_dispose(&scinfo);
>               if (rc)
>                   return -rc;
>           }
> @@ -4834,7 +4831,6 @@ int main_sched_credit2(int argc, char **
>
>   int main_sched_sedf(int argc, char **argv)
>   {
> -    libxl_sched_sedf_domain scinfo;
>       const char *dom = NULL;
>       const char *cpupool = NULL;
>       int period = 0, opt_p = 0;
> @@ -4917,15 +4913,15 @@ int main_sched_sedf(int argc, char **arg
>       } else {
>           find_domain(dom);
>
> -        rc = sched_sedf_domain_get(domid,&scinfo);
> -        if (rc)
> -            return -rc;
> -
>           if (!opt_p&&  !opt_s&&  !opt_l&&  !opt_e&&  !opt_w) {
>               /* output sedf scheduler info */
>               sched_sedf_domain_output(-1);
>               return -sched_sedf_domain_output(domid);
>           } else { /* set sedf scheduler paramaters */
> +            libxl_domain_sched_params scinfo;
> +            libxl_domain_sched_params_init(&scinfo);
> +            scinfo.sched = LIBXL_SCHEDULER_SEDF;
> +
>               if (opt_p) {
>                   scinfo.period = period;
>                   scinfo.weight = 0;
> @@ -4944,7 +4940,7 @@ int main_sched_sedf(int argc, char **arg
>                   scinfo.slice = 0;
>               }
>               rc = sched_sedf_domain_set(domid,&scinfo);
> -            libxl_sched_sedf_domain_dispose(&scinfo);
> +            libxl_domain_sched_params_dispose(&scinfo);
>               if (rc)
>                   return -rc;
>           }

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c
  2012-05-29 13:57 ` [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c Ian Campbell
@ 2012-05-31 13:48   ` George Dunlap
  2012-05-31 15:14     ` Ian Jackson
  0 siblings, 1 reply; 30+ messages in thread
From: George Dunlap @ 2012-05-31 13:48 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Juergen Gross, Ian Jackson, Dario Faggioli, xen-devel

On 29/05/12 14:57, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1338299813 -3600
> # Node ID d89b5eeb94519fdc056f91663676cf012c40b654
> # Parent  274de8e1e0d116070d34731d93b53ce44530e5a0
> libxl: move libxl__sched_set_params into libxl.c
>
> All the other sched functions are here and I'm just about to make those static
> functions as I make libxl__sched_set_params the public function.
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
>
> diff -r 274de8e1e0d1 -r d89b5eeb9451 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Tue May 29 14:55:29 2012 +0100
> +++ b/tools/libxl/libxl.c	Tue May 29 14:56:53 2012 +0100
> @@ -3450,6 +3450,33 @@ int libxl_sched_sedf_domain_set(libxl_ct
>       return 0;
>   }
>
> +int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> +                            libxl_domain_sched_params *scparams)
> +{
> +    libxl_scheduler sched = scparams->sched;
> +    int ret;
> +
> +    if (sched == LIBXL_SCHEDULER_UNKNOWN)
> +        sched = libxl__domain_scheduler(gc, domid);
> +
> +    switch (sched) {
> +    case LIBXL_SCHEDULER_SEDF:
> +        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
> +        break;
> +    case LIBXL_SCHEDULER_CREDIT:
> +        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
> +        break;
> +    case LIBXL_SCHEDULER_CREDIT2:
> +        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
> +        break;
> +    default:
> +        LOG(ERROR, "Unknown scheduler");
> +        ret=ERROR_INVAL;
> +        break;
> +    }
> +    return ret;
> +}
> +
>   int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
>                          libxl_trigger trigger, uint32_t vcpuid)
>   {
> diff -r 274de8e1e0d1 -r d89b5eeb9451 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Tue May 29 14:55:29 2012 +0100
> +++ b/tools/libxl/libxl_dom.c	Tue May 29 14:56:53 2012 +0100
> @@ -42,33 +42,6 @@ libxl_domain_type libxl__domain_type(lib
>           return LIBXL_DOMAIN_TYPE_PV;
>   }
>
> -int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> -                            libxl_domain_sched_params *scparams)
> -{
> -    libxl_scheduler sched = scparams->sched;
> -    int ret;
> -
> -    if (sched == LIBXL_SCHEDULER_UNKNOWN)
> -        sched = libxl__domain_scheduler(gc, domid);
> -
> -    switch (sched) {
> -    case LIBXL_SCHEDULER_SEDF:
> -        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
> -        break;
> -    case LIBXL_SCHEDULER_CREDIT:
> -        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
> -        break;
> -    case LIBXL_SCHEDULER_CREDIT2:
> -        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
> -        break;
> -    default:
> -        LOG(ERROR, "Unknown scheduler");
> -        ret=ERROR_INVAL;
> -        break;
> -    }
> -    return ret;
> -}
> -
>   int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid)
>   {
>       libxl_ctx *ctx = libxl__gc_owner(gc);

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters
  2012-05-29 13:57 ` [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters Ian Campbell
@ 2012-05-31 13:51   ` George Dunlap
  2012-05-31 15:15     ` Ian Jackson
  0 siblings, 1 reply; 30+ messages in thread
From: George Dunlap @ 2012-05-31 13:51 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Juergen Gross, Ian Jackson, Dario Faggioli, xen-devel

On 29/05/12 14:57, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1338299813 -3600
> # Node ID 9094cf509df0e36d8c59ed131937b1ebc07cc2ad
> # Parent  d89b5eeb94519fdc056f91663676cf012c40b654
> libxl: expose a single get/setter for domain scheduler parameters.
>
> This is consistent with having a single struct for all parameters.
>
> Effectively renames and exports libxl__sched_set_params as
> libxl_domain_sched_params_set. libxl_domain_sched_params_get is new.
>
> Improve const correctness of the setters while I'm here.
>
> Use shorter LOG macros when touching a line anyway.
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

BTW, we obviously need to have "xl sched-credit" for xm compatibility; 
but do you think it makes sense to make a generic xl command that's a 
analog to the generic libxl function, that looks up the scheduler for 
you on get, and accepts all the parameters on the command-line?  
(Doesn't need to be part of this series, obviously.)
>
> diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c       Tue May 29 14:56:53 2012 +0100
> +++ b/tools/libxl/libxl.c       Tue May 29 14:56:53 2012 +0100
> @@ -3196,15 +3196,15 @@ libxl_scheduler libxl_get_scheduler(libx
>       return sched;
>   }
>
> -int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_domain_sched_params *scinfo)
> +static int sched_credit_domain_get(libxl__gc *gc, uint32_t domid,
> +                                   libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit sdom;
>       int rc;
>
> -    rc = xc_sched_credit_domain_get(ctx->xch, domid,&sdom);
> +    rc = xc_sched_credit_domain_get(CTX->xch, domid,&sdom);
>       if (rc != 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
> +        LOGE(ERROR, "getting domain sched credit");
>           return ERROR_FAIL;
>       }
>
> @@ -3216,32 +3216,31 @@ int libxl_sched_credit_domain_get(libxl_
>       return 0;
>   }
>
> -int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_domain_sched_params *scinfo)
> +static int sched_credit_domain_set(libxl__gc *gc, uint32_t domid,
> +                                   const libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit sdom;
>       xc_domaininfo_t domaininfo;
>       int rc;
>
> -    rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo);
> +    rc = xc_domain_getinfolist(CTX->xch, domid, 1,&domaininfo);
>       if (rc<  0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> +        LOGE(ERROR, "getting domain info list");
>           return ERROR_FAIL;
>       }
>       if (rc != 1 || domaininfo.domain != domid)
>           return ERROR_INVAL;
>
> -    rc = xc_sched_credit_domain_get(ctx->xch, domid,&sdom);
> +    rc = xc_sched_credit_domain_get(CTX->xch, domid,&sdom);
>       if (rc != 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit");
> +        LOGE(ERROR, "getting domain sched credit");
>           return ERROR_FAIL;
>       }
>
>       if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
>           if (scinfo->weight<  1 || scinfo->weight>  65535) {
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> -                       "Cpu weight out of range, "
> -                       "valid values are within range from 1 to 65535");
> +            LOG(ERROR, "Cpu weight out of range, "
> +                "valid values are within range from 1 to 65535");
>               return ERROR_INVAL;
>           }
>           sdom.weight = scinfo->weight;
> @@ -3250,18 +3249,17 @@ int libxl_sched_credit_domain_set(libxl_
>       if (scinfo->cap != LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT) {
>           if (scinfo->cap<  0
>               || scinfo->cap>  (domaininfo.max_vcpu_id + 1) * 100) {
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> -                "Cpu cap out of range, "
> +            LOG(ERROR, "Cpu cap out of range, "
>                   "valid range is from 0 to %d for specified number of vcpus",
> -                       ((domaininfo.max_vcpu_id + 1) * 100));
> +                ((domaininfo.max_vcpu_id + 1) * 100));
>               return ERROR_INVAL;
>           }
>           sdom.cap = scinfo->cap;
>       }
>
> -    rc = xc_sched_credit_domain_set(ctx->xch, domid,&sdom);
> +    rc = xc_sched_credit_domain_set(CTX->xch, domid,&sdom);
>       if ( rc<  0 ) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched credit");
> +        LOGE(ERROR, "setting domain sched credit");
>           return ERROR_FAIL;
>       }
>
> @@ -3329,16 +3327,15 @@ int libxl_sched_credit_params_set(libxl_
>       return 0;
>   }
>
> -int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_domain_sched_params *scinfo)
> +static int sched_credit2_domain_get(libxl__gc *gc, uint32_t domid,
> +                                    libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit2 sdom;
>       int rc;
>
> -    rc = xc_sched_credit2_domain_get(ctx->xch, domid,&sdom);
> +    rc = xc_sched_credit2_domain_get(CTX->xch, domid,&sdom);
>       if (rc != 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> -                         "getting domain sched credit2");
> +        LOGE(ERROR, "getting domain sched credit2");
>           return ERROR_FAIL;
>       }
>
> @@ -3349,42 +3346,38 @@ int libxl_sched_credit2_domain_get(libxl
>       return 0;
>   }
>
> -int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_domain_sched_params *scinfo)
> +static int sched_credit2_domain_set(libxl__gc *gc, uint32_t domid,
> +                                    const libxl_domain_sched_params *scinfo)
>   {
>       struct xen_domctl_sched_credit2 sdom;
>       int rc;
>
> -    rc = xc_sched_credit2_domain_get(ctx->xch, domid,&sdom);
> +    rc = xc_sched_credit2_domain_get(CTX->xch, domid,&sdom);
>       if (rc != 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> -                         "getting domain sched credit2");
> +        LOGE(ERROR, "getting domain sched credit2");
>           return ERROR_FAIL;
>       }
>
>       if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) {
>           if (scinfo->weight<  1 || scinfo->weight>  65535) {
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> -                       "Cpu weight out of range, "
> -                       "valid values are within range from "
> -                       "1 to 65535");
> +            LOG(ERROR, "Cpu weight out of range, "
> +                       "valid values are within range from 1 to 65535");
>               return ERROR_INVAL;
>           }
>           sdom.weight = scinfo->weight;
>       }
>
> -    rc = xc_sched_credit2_domain_set(ctx->xch, domid,&sdom);
> +    rc = xc_sched_credit2_domain_set(CTX->xch, domid,&sdom);
>       if ( rc<  0 ) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
> -                         "setting domain sched credit2");
> +        LOGE(ERROR, "setting domain sched credit2");
>           return ERROR_FAIL;
>       }
>
>       return 0;
>   }
>
> -int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_domain_sched_params *scinfo)
> +static int sched_sedf_domain_get(libxl__gc *gc, uint32_t domid,
> +                                 libxl_domain_sched_params *scinfo)
>   {
>       uint64_t period;
>       uint64_t slice;
> @@ -3393,10 +3386,10 @@ int libxl_sched_sedf_domain_get(libxl_ct
>       uint16_t weight;
>       int rc;
>
> -    rc = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency,
> +    rc = xc_sedf_domain_get(CTX->xch, domid,&period,&slice,&latency,
>                               &extratime,&weight);
>       if (rc != 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
> +        LOGE(ERROR, "getting domain sched sedf");
>           return ERROR_FAIL;
>       }
>
> @@ -3411,8 +3404,8 @@ int libxl_sched_sedf_domain_get(libxl_ct
>       return 0;
>   }
>
> -int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_domain_sched_params *scinfo)
> +static int sched_sedf_domain_set(libxl__gc *gc, uint32_t domid,
> +                                 const libxl_domain_sched_params *scinfo)
>   {
>       uint64_t period;
>       uint64_t slice;
> @@ -3422,10 +3415,10 @@ int libxl_sched_sedf_domain_set(libxl_ct
>
>       int ret;
>
> -    ret = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency,
> +    ret = xc_sedf_domain_get(CTX->xch, domid,&period,&slice,&latency,
>                               &extratime,&weight);
>       if (ret != 0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf");
> +        LOGE(ERROR, "getting domain sched sedf");
>           return ERROR_FAIL;
>       }
>
> @@ -3440,20 +3433,21 @@ int libxl_sched_sedf_domain_set(libxl_ct
>       if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT)
>           weight = scinfo->weight;
>
> -    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
> +    ret = xc_sedf_domain_set(CTX->xch, domid, period, slice, latency,
>                               extratime, weight);
>       if ( ret<  0 ) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf");
> +        LOGE(ERROR, "setting domain sched sedf");
>           return ERROR_FAIL;
>       }
>
>       return 0;
>   }
>
> -int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> -                            libxl_domain_sched_params *scparams)
> +int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
> +                                  const libxl_domain_sched_params *scinfo)
>   {
> -    libxl_scheduler sched = scparams->sched;
> +    GC_INIT(ctx);
> +    libxl_scheduler sched = scinfo->sched;
>       int ret;
>
>       if (sched == LIBXL_SCHEDULER_UNKNOWN)
> @@ -3461,19 +3455,51 @@ int libxl__sched_set_params(libxl__gc *g
>
>       switch (sched) {
>       case LIBXL_SCHEDULER_SEDF:
> -        ret=libxl_sched_sedf_domain_set(CTX, domid, scparams);
> +        ret=sched_sedf_domain_set(gc, domid, scinfo);
>           break;
>       case LIBXL_SCHEDULER_CREDIT:
> -        ret=libxl_sched_credit_domain_set(CTX, domid, scparams);
> +        ret=sched_credit_domain_set(gc, domid, scinfo);
>           break;
>       case LIBXL_SCHEDULER_CREDIT2:
> -        ret=libxl_sched_credit2_domain_set(CTX, domid, scparams);
> +        ret=sched_credit2_domain_set(gc, domid, scinfo);
>           break;
>       default:
>           LOG(ERROR, "Unknown scheduler");
>           ret=ERROR_INVAL;
>           break;
>       }
> +
> +    GC_FREE;
> +    return ret;
> +}
> +
> +int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
> +                                  libxl_domain_sched_params *scinfo)
> +{
> +    GC_INIT(ctx);
> +    int ret;
> +
> +    libxl_domain_sched_params_init(scinfo);
> +
> +    scinfo->sched = libxl__domain_scheduler(gc, domid);
> +
> +    switch (scinfo->sched) {
> +    case LIBXL_SCHEDULER_SEDF:
> +        ret=sched_sedf_domain_get(gc, domid, scinfo);
> +        break;
> +    case LIBXL_SCHEDULER_CREDIT:
> +        ret=sched_credit_domain_get(gc, domid, scinfo);
> +        break;
> +    case LIBXL_SCHEDULER_CREDIT2:
> +        ret=sched_credit2_domain_get(gc, domid, scinfo);
> +        break;
> +    default:
> +        LOG(ERROR, "Unknown scheduler");
> +        ret=ERROR_INVAL;
> +        break;
> +    }
> +
> +    GC_FREE;
>       return ret;
>   }
>
> diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h       Tue May 29 14:56:53 2012 +0100
> +++ b/tools/libxl/libxl.h       Tue May 29 14:56:53 2012 +0100
> @@ -790,18 +790,11 @@ int libxl_sched_credit_params_set(libxl_
>   #define LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT   -1
>   #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1
>
> -int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_domain_sched_params *scinfo);
> -int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                  libxl_domain_sched_params *scinfo);
> -int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_domain_sched_params *scinfo);
> -int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                   libxl_domain_sched_params *scinfo);
> -int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_domain_sched_params *scinfo);
> -int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> -                                libxl_domain_sched_params *scinfo);
> +int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid,
> +                                  libxl_domain_sched_params *params);
> +int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid,
> +                                  const libxl_domain_sched_params *params);
> +
>   int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
>                          libxl_trigger trigger, uint32_t vcpuid);
>   int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
> diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c   Tue May 29 14:56:53 2012 +0100
> +++ b/tools/libxl/libxl_dom.c   Tue May 29 14:56:53 2012 +0100
> @@ -174,7 +174,7 @@ int libxl__build_post(libxl__gc *gc, uin
>       char **ents, **hvm_ents;
>       int i;
>
> -    libxl__sched_set_params (gc, domid,&(info->sched_params));
> +    libxl_domain_sched_params_set(CTX, domid,&info->sched_params);
>
>       libxl_cpuid_apply_policy(ctx, domid);
>       if (info->cpuid != NULL)
> diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c  Tue May 29 14:56:53 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c  Tue May 29 14:56:53 2012 +0100
> @@ -4361,24 +4361,33 @@ int main_sharing(int argc, char **argv)
>       return 0;
>   }
>
> -static int sched_credit_domain_get(int domid, libxl_domain_sched_params *scinfo)
> +static int sched_domain_get(libxl_scheduler sched, int domid,
> +                            libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> -    rc = libxl_sched_credit_domain_get(ctx, domid, scinfo);
> -    if (rc)
> -        fprintf(stderr, "libxl_sched_credit_domain_get failed.\n");
> -
> -    return rc;
> -}
> -
> -static int sched_credit_domain_set(int domid, libxl_domain_sched_params *scinfo)
> +    rc = libxl_domain_sched_params_get(ctx, domid, scinfo);
> +    if (rc) {
> +        fprintf(stderr, "libxl_domain_sched_params_get failed.\n");
> +        return rc;
> +    }
> +    if (scinfo->sched != sched) {
> +        fprintf(stderr, "libxl_domain_sched_params_get returned %s not %s.\n",
> +                libxl_scheduler_to_string(scinfo->sched),
> +                libxl_scheduler_to_string(sched));
> +        return ERROR_INVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static int sched_domain_set(int domid, const libxl_domain_sched_params *scinfo)
>   {
>       int rc;
>
> -    rc = libxl_sched_credit_domain_set(ctx, domid, scinfo);
> +    rc = libxl_domain_sched_params_set(ctx, domid, scinfo);
>       if (rc)
> -        fprintf(stderr, "libxl_sched_credit_domain_set failed.\n");
> +        fprintf(stderr, "libxl_domain_sched_params_set failed.\n");
>
>       return rc;
>   }
> @@ -4415,7 +4424,7 @@ static int sched_credit_domain_output(in
>           printf("%-33s %4s %6s %4s\n", "Name", "ID", "Weight", "Cap");
>           return 0;
>       }
> -    rc = sched_credit_domain_get(domid,&scinfo);
> +    rc = sched_domain_get(LIBXL_SCHEDULER_CREDIT, domid,&scinfo);
>       if (rc)
>           return rc;
>       domname = libxl_domid_to_name(ctx, domid);
> @@ -4450,30 +4459,6 @@ static int sched_credit_pool_output(uint
>       return 0;
>   }
>
> -static int sched_credit2_domain_get(
> -    int domid, libxl_domain_sched_params *scinfo)
> -{
> -    int rc;
> -
> -    rc = libxl_sched_credit2_domain_get(ctx, domid, scinfo);
> -    if (rc)
> -        fprintf(stderr, "libxl_sched_credit2_domain_get failed.\n");
> -
> -    return rc;
> -}
> -
> -static int sched_credit2_domain_set(
> -    int domid, libxl_domain_sched_params *scinfo)
> -{
> -    int rc;
> -
> -    rc = libxl_sched_credit2_domain_set(ctx, domid, scinfo);
> -    if (rc)
> -        fprintf(stderr, "libxl_sched_credit2_domain_set failed.\n");
> -
> -    return rc;
> -}
> -
>   static int sched_credit2_domain_output(
>       int domid)
>   {
> @@ -4485,7 +4470,7 @@ static int sched_credit2_domain_output(
>           printf("%-33s %4s %6s\n", "Name", "ID", "Weight");
>           return 0;
>       }
> -    rc = sched_credit2_domain_get(domid,&scinfo);
> +    rc = sched_domain_get(LIBXL_SCHEDULER_CREDIT2, domid,&scinfo);
>       if (rc)
>           return rc;
>       domname = libxl_domid_to_name(ctx, domid);
> @@ -4498,29 +4483,6 @@ static int sched_credit2_domain_output(
>       return 0;
>   }
>
> -static int sched_sedf_domain_get(
> -    int domid, libxl_domain_sched_params *scinfo)
> -{
> -    int rc;
> -
> -    rc = libxl_sched_sedf_domain_get(ctx, domid, scinfo);
> -    if (rc)
> -        fprintf(stderr, "libxl_sched_sedf_domain_get failed.\n");
> -
> -    return rc;
> -}
> -
> -static int sched_sedf_domain_set(
> -    int domid, libxl_domain_sched_params *scinfo)
> -{
> -    int rc;
> -
> -    rc = libxl_sched_sedf_domain_set(ctx, domid, scinfo);
> -    if (rc)
> -        fprintf(stderr, "libxl_sched_sedf_domain_set failed.\n");
> -    return rc;
> -}
> -
>   static int sched_sedf_domain_output(
>       int domid)
>   {
> @@ -4533,7 +4495,7 @@ static int sched_sedf_domain_output(
>                  "Slice", "Latency", "Extra", "Weight");
>           return 0;
>       }
> -    rc = sched_sedf_domain_get(domid,&scinfo);
> +    rc = sched_domain_get(LIBXL_SCHEDULER_SEDF, domid,&scinfo);
>       if (rc)
>           return rc;
>       domname = libxl_domid_to_name(ctx, domid);
> @@ -4744,7 +4706,7 @@ int main_sched_credit(int argc, char **a
>                   scinfo.weight = weight;
>               if (opt_c)
>                   scinfo.cap = cap;
> -            rc = sched_credit_domain_set(domid,&scinfo);
> +            rc = sched_domain_set(domid,&scinfo);
>               libxl_domain_sched_params_dispose(&scinfo);
>               if (rc)
>                   return -rc;
> @@ -4819,7 +4781,7 @@ int main_sched_credit2(int argc, char **
>               scinfo.sched = LIBXL_SCHEDULER_CREDIT2;
>               if (opt_w)
>                   scinfo.weight = weight;
> -            rc = sched_credit2_domain_set(domid,&scinfo);
> +            rc = sched_domain_set(domid,&scinfo);
>               libxl_domain_sched_params_dispose(&scinfo);
>               if (rc)
>                   return -rc;
> @@ -4939,7 +4901,7 @@ int main_sched_sedf(int argc, char **arg
>                   scinfo.period = 0;
>                   scinfo.slice = 0;
>               }
> -            rc = sched_sedf_domain_set(domid,&scinfo);
> +            rc = sched_domain_set(domid,&scinfo);
>               libxl_domain_sched_params_dispose(&scinfo);
>               if (rc)
>                   return -rc;

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler
  2012-05-31 13:19   ` George Dunlap
@ 2012-05-31 15:11     ` Ian Jackson
  2012-06-01  9:51     ` Ian Campbell
  1 sibling, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-05-31 15:11 UTC (permalink / raw)
  To: George Dunlap; +Cc: Dario Faggioli, Juergen Gross, Ian Campbell, xen-devel

George Dunlap writes ("Re: [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler"):
> On 29/05/12 14:56, Ian Campbell wrote:
> > libxl: add internal function to get a domain's scheduler.
...
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

It's fine by me too.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params
  2012-05-31 13:21   ` George Dunlap
@ 2012-05-31 15:12     ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-05-31 15:12 UTC (permalink / raw)
  To: George Dunlap; +Cc: Dario Faggioli, Juergen Gross, Ian Campbell, xen-devel

George Dunlap writes ("Re: [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params"):
> On 29/05/12 14:57, Ian Campbell wrote:
> > libxl: rename libxl_sched_params to libxl_domain_sched_params
...
> > Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

On that basis

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-31 13:47   ` George Dunlap
@ 2012-05-31 15:13     ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-05-31 15:13 UTC (permalink / raw)
  To: George Dunlap; +Cc: Dario Faggioli, Juergen Gross, Ian Campbell, xen-devel

George Dunlap writes ("Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params"):
> On 29/05/12 14:57, Ian Campbell wrote:
> > libxl: make it possible to explicitly specify default sched params
...
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c
  2012-05-31 13:48   ` George Dunlap
@ 2012-05-31 15:14     ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-05-31 15:14 UTC (permalink / raw)
  To: George Dunlap; +Cc: Dario Faggioli, Juergen Gross, Ian Campbell, xen-devel

George Dunlap writes ("Re: [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c"):
> On 29/05/12 14:57, Ian Campbell wrote:
> > libxl: move libxl__sched_set_params into libxl.c

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters
  2012-05-31 13:51   ` George Dunlap
@ 2012-05-31 15:15     ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-05-31 15:15 UTC (permalink / raw)
  To: George Dunlap; +Cc: Dario Faggioli, Juergen Gross, Ian Campbell, xen-devel

George Dunlap writes ("Re: [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters"):
> On 29/05/12 14:57, Ian Campbell wrote:
> > libxl: expose a single get/setter for domain scheduler parameters.
...
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

> BTW, we obviously need to have "xl sched-credit" for xm compatibility;
> but do you think it makes sense to make a generic xl command that's a
> analog to the generic libxl function, that looks up the scheduler for
> you on get, and accepts all the parameters on the command-line?
> (Doesn't need to be part of this series, obviously.)

I think that would be a very good idea.

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler
  2012-05-31 13:19   ` George Dunlap
  2012-05-31 15:11     ` Ian Jackson
@ 2012-06-01  9:51     ` Ian Campbell
  1 sibling, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2012-06-01  9:51 UTC (permalink / raw)
  To: George Dunlap; +Cc: Juergen Gross, Ian Jackson, Dario Faggioli, xen-devel

On Thu, 2012-05-31 at 14:19 +0100, George Dunlap wrote:
> On 29/05/12 14:56, Ian Campbell wrote:
> > # HG changeset patch
> > # User Ian Campbell<ian.campbell@citrix.com>
> > # Date 1338299619 -3600
> > # Node ID 980a25d6ad12ba0f10fa616255b9382cc14ce69e
> > # Parent  12537c670e9ea9e7f73747e203ca318107b82cd9
> > libxl: add internal function to get a domain's scheduler.
> >
> > This takes into account cpupools.
> >
> > Add a helper to get the info for a single cpu pool, refactor libxl_list_cpupool
> > t use this. While there fix the leaks due to not disposing the partial list on
> > realloc failure in that function.
> >
> > Fix the failure of sched_domain_output to free the poolinfo list.
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> As an aside, I would prefer that the clean-up happen in separate 
> patches; having a single patch do several orthogonal things makes it 
> hard for me to tell what goes with what, both for review, and in case 
> someone in the future needs to do archaeology and figure out what's 
> going on.  Not really worth a respin just for that, though.

Agreed in general, and I should know better.

> >
> > Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
> > ---
> > v2: add libxl_cpupoolinfo_list_free, use it in libxl_cpupool_list error path.
> >      Also use it in libxl_cpupool_cpuremove_node (which fixes a leak) and in
> >      libxl_name_to_cpupoolid, sched_domain_output and main_cpupoollist (which
> >      also fixes a leak).
> >
> >      Also in libxl_cpupool_cpuremove_node use libxl_cputopology_list_free
> >      instead of open coding
> >
> > diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl.c
> > --- a/tools/libxl/libxl.c	Tue May 29 12:56:57 2012 +0100
> > +++ b/tools/libxl/libxl.c	Tue May 29 14:53:39 2012 +0100
> > @@ -552,41 +552,70 @@ int libxl_domain_info(libxl_ctx *ctx, li
> >       return 0;
> >   }
> >
> > +static int cpupool_info(libxl__gc *gc,
> > +                        libxl_cpupoolinfo *info,
> > +                        uint32_t poolid,
> > +                        bool exact /* exactly poolid or>= poolid */)
> > +{
> > +    xc_cpupoolinfo_t *xcinfo;
> > +    int rc = ERROR_FAIL;
> > +
> > +    xcinfo = xc_cpupool_getinfo(CTX->xch, poolid);
> > +    if (xcinfo == NULL)
> > +        return ERROR_FAIL;
> > +
> > +    if (exact&&  xcinfo->cpupool_id != poolid)
> > +        goto out;
> > +
> > +    info->poolid = xcinfo->cpupool_id;
> > +    info->sched = xcinfo->sched_id;
> > +    info->n_dom = xcinfo->n_dom;
> > +    if (libxl_cpumap_alloc(CTX,&info->cpumap))
> > +        goto out;
> > +    memcpy(info->cpumap.map, xcinfo->cpumap, info->cpumap.size);
> > +
> > +    rc = 0;
> > +out:
> > +    xc_cpupool_infofree(CTX->xch, xcinfo);
> > +    return rc;
> > +}
> > +
> > +int libxl_cpupool_info(libxl_ctx *ctx,
> > +                       libxl_cpupoolinfo *info, uint32_t poolid)
> > +{
> > +    GC_INIT(ctx);
> > +    int rc = cpupool_info(gc, info, poolid, true);
> > +    GC_FREE;
> > +    return rc;
> > +}
> > +
> >   libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx *ctx, int *nb_pool)
> >   {
> > -    libxl_cpupoolinfo *ptr, *tmp;
> > +    GC_INIT(ctx);
> > +    libxl_cpupoolinfo info, *ptr, *tmp;
> >       int i;
> > -    xc_cpupoolinfo_t *info;
> >       uint32_t poolid;
> >
> >       ptr = NULL;
> >
> >       poolid = 0;
> >       for (i = 0;; i++) {
> > -        info = xc_cpupool_getinfo(ctx->xch, poolid);
> > -        if (info == NULL)
> > +        if (cpupool_info(gc,&info, poolid, false))
> >               break;
> >           tmp = realloc(ptr, (i + 1) * sizeof(libxl_cpupoolinfo));
> >           if (!tmp) {
> >               LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating cpupool info");
> > -            free(ptr);
> > -            xc_cpupool_infofree(ctx->xch, info);
> > -            return NULL;
> > +            libxl_cpupoolinfo_list_free(ptr, i);
> > +            goto out;
> >           }
> >           ptr = tmp;
> > -        ptr[i].poolid = info->cpupool_id;
> > -        ptr[i].sched = info->sched_id;
> > -        ptr[i].n_dom = info->n_dom;
> > -        if (libxl_cpumap_alloc(ctx,&ptr[i].cpumap)) {
> > -            xc_cpupool_infofree(ctx->xch, info);
> > -            break;
> > -        }
> > -        memcpy(ptr[i].cpumap.map, info->cpumap, ptr[i].cpumap.size);
> > -        poolid = info->cpupool_id + 1;
> > -        xc_cpupool_infofree(ctx->xch, info);
> > +        ptr[i] = info;
> > +        poolid = info.poolid + 1;
> >       }
> >
> >       *nb_pool = i;
> > +out:
> > +    GC_FREE;
> >       return ptr;
> >   }
> >
> > @@ -3932,14 +3961,10 @@ int libxl_cpupool_cpuremove_node(libxl_c
> >           }
> >       }
> >
> > -    for (cpu = 0; cpu<  nr_cpus; cpu++)
> > -        libxl_cputopology_dispose(&topology[cpu]);
> > -    free(topology);
> > +    libxl_cputopology_list_free(topology, nr_cpus);
> >
> >   out:
> > -    for (p = 0; p<  n_pools; p++) {
> > -        libxl_cpupoolinfo_dispose(poolinfo + p);
> > -    }
> > +    libxl_cpupoolinfo_list_free(poolinfo, n_pools);
> >
> >       return ret;
> >   }
> > diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl.h
> > --- a/tools/libxl/libxl.h	Tue May 29 12:56:57 2012 +0100
> > +++ b/tools/libxl/libxl.h	Tue May 29 14:53:39 2012 +0100
> > @@ -576,6 +576,7 @@ int libxl_domain_info(libxl_ctx*, libxl_
> >   libxl_dominfo * libxl_list_domain(libxl_ctx*, int *nb_domain);
> >   void libxl_dominfo_list_free(libxl_dominfo *list, int nr);
> >   libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
> > +void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nr);
> >   libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm);
> >   void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
> >
> > @@ -829,6 +830,7 @@ int libxl_cpupool_cpuadd_node(libxl_ctx
> >   int libxl_cpupool_cpuremove(libxl_ctx *ctx, uint32_t poolid, int cpu);
> >   int libxl_cpupool_cpuremove_node(libxl_ctx *ctx, uint32_t poolid, int node, int *cpus);
> >   int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid);
> > +int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t poolid);
> >
> >   int libxl_domid_valid_guest(uint32_t domid);
> >
> > diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_dom.c
> > --- a/tools/libxl/libxl_dom.c	Tue May 29 12:56:57 2012 +0100
> > +++ b/tools/libxl/libxl_dom.c	Tue May 29 14:53:39 2012 +0100
> > @@ -93,6 +93,41 @@ int libxl__domain_shutdown_reason(libxl_
> >       return (info.flags>>  XEN_DOMINF_shutdownshift)&  XEN_DOMINF_shutdownmask;
> >   }
> >
> > +int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
> > +{
> > +    xc_domaininfo_t info;
> > +    int ret;
> > +
> > +    ret = xc_domain_getinfolist(CTX->xch, domid, 1,&info);
> > +    if (ret != 1)
> > +        return ERROR_FAIL;
> > +    if (info.domain != domid)
> > +        return ERROR_FAIL;
> > +
> > +    return info.cpupool;
> > +}
> > +
> > +libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid)
> > +{
> > +    uint32_t cpupool = libxl__domain_cpupool(gc, domid);
> > +    libxl_cpupoolinfo poolinfo;
> > +    libxl_scheduler sched = LIBXL_SCHEDULER_UNKNOWN;
> > +    int rc;
> > +
> > +    if (cpupool<  0)
> > +        return sched;
> > +
> > +    rc = libxl_cpupool_info(CTX,&poolinfo, cpupool);
> > +    if (rc<  0)
> > +        goto out;
> > +
> > +    sched = poolinfo.sched;
> > +
> > +out:
> > +    libxl_cpupoolinfo_dispose(&poolinfo);
> > +    return sched;
> > +}
> > +
> >   int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> >                 libxl_domain_build_info *info, libxl__domain_build_state *state)
> >   {
> > diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_internal.h
> > --- a/tools/libxl/libxl_internal.h	Tue May 29 12:56:57 2012 +0100
> > +++ b/tools/libxl/libxl_internal.h	Tue May 29 14:53:39 2012 +0100
> > @@ -738,6 +738,8 @@ _hidden int libxl__file_reference_unmap(
> >   /* from xl_dom */
> >   _hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
> >   _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
> > +_hidden int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid);
> > +_hidden libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid);
> >   _hidden int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, libxl_sched_params *scparams);
> >   #define LIBXL__DOMAIN_IS_TYPE(gc, domid, type) \
> >       libxl__domain_type((gc), (domid)) == LIBXL_DOMAIN_TYPE_##type
> > diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_types.idl
> > --- a/tools/libxl/libxl_types.idl	Tue May 29 12:56:57 2012 +0100
> > +++ b/tools/libxl/libxl_types.idl	Tue May 29 14:53:39 2012 +0100
> > @@ -107,7 +107,9 @@ libxl_bios_type = Enumeration("bios_type
> >       ])
> >
> >   # Consistent with values defined in domctl.h
> > +# Except unknown which we have made up
> >   libxl_scheduler = Enumeration("scheduler", [
> > +    (0, "unknown"),
> >       (4, "sedf"),
> >       (5, "credit"),
> >       (6, "credit2"),
> > diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/libxl_utils.c
> > --- a/tools/libxl/libxl_utils.c	Tue May 29 12:56:57 2012 +0100
> > +++ b/tools/libxl/libxl_utils.c	Tue May 29 14:53:39 2012 +0100
> > @@ -133,9 +133,8 @@ int libxl_name_to_cpupoolid(libxl_ctx *c
> >               }
> >               free(poolname);
> >           }
> > -        libxl_cpupoolinfo_dispose(poolinfo + i);
> >       }
> > -    free(poolinfo);
> > +    libxl_cpupoolinfo_list_free(poolinfo, nb_pools);
> >       return ret;
> >   }
> >
> > @@ -686,6 +685,14 @@ void libxl_vminfo_list_free(libxl_vminfo
> >       free(list);
> >   }
> >
> > +void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nr)
> > +{
> > +    int i;
> > +    for (i = 0; i<  nr; i++)
> > +        libxl_cpupoolinfo_dispose(&list[i]);
> > +    free(list);
> > +}
> > +
> >   int libxl_domid_valid_guest(uint32_t domid)
> >   {
> >       /* returns 1 if the value _could_ be a valid guest domid, 0 otherwise
> > diff -r 12537c670e9e -r 980a25d6ad12 tools/libxl/xl_cmdimpl.c
> > --- a/tools/libxl/xl_cmdimpl.c	Tue May 29 12:56:57 2012 +0100
> > +++ b/tools/libxl/xl_cmdimpl.c	Tue May 29 14:53:39 2012 +0100
> > @@ -4608,11 +4608,8 @@ static int sched_domain_output(libxl_sch
> >                   break;
> >           }
> >       }
> > -    if (poolinfo) {
> > -        for (p = 0; p<  n_pools; p++) {
> > -            libxl_cpupoolinfo_dispose(poolinfo + p);
> > -        }
> > -    }
> > +    if (poolinfo)
> > +        libxl_cpupoolinfo_list_free(poolinfo, n_pools);
> >       return 0;
> >   }
> >
> > @@ -6119,8 +6116,9 @@ int main_cpupoollist(int argc, char **ar
> >                   printf("\n");
> >               }
> >           }
> > -        libxl_cpupoolinfo_dispose(poolinfo + p);
> > -    }
> > +    }
> > +
> > +    libxl_cpupoolinfo_list_free(poolinfo, n_pools);
> >
> >       return ret;
> >   }
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-30  8:00       ` Dario Faggioli
@ 2012-06-01 10:14         ` Ian Campbell
  2012-06-01 10:26           ` Dario Faggioli
  0 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-06-01 10:14 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Juergen Gross, Ian Jackson, xen-devel

On Wed, 2012-05-30 at 09:00 +0100, Dario Faggioli wrote:
> On Wed, 2012-05-30 at 08:35 +0100, Ian Campbell wrote: 
> > > That being said, I'm not sure at what level we want to enforce something
> > > like the above. The lowest level toolstack seems fine to me, which would
> > > mean putting something like the code above in the config file parsing...
> > > If you agree, I'll try that and let you know whether or not it works.
> > 
> > If you could provide an incremental patch that would be much
> > appreciated.
> > 
> I sure can. :-)
> 
> > IMHO it would be fine (and a good idea) for libxl to return ERROR_INVAL
> > if the conditions aren't met too. 
> >
> Ok, sounds reasonable.
> 
> > If you want to also check it in xl's
> > config file parsing and either fix it up like the above or error out
> > then please do.
> >
> Let's see what fits better...

Thanks. I'm going to commit this as is and await the fixup, if that's
ok. I think this isn't a regression from the PoV of the configuration
file since this would also have happened before.

> 
> Dario
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-01 10:14         ` Ian Campbell
@ 2012-06-01 10:26           ` Dario Faggioli
  0 siblings, 0 replies; 30+ messages in thread
From: Dario Faggioli @ 2012-06-01 10:26 UTC (permalink / raw)
  To: Ian Campbell; +Cc: George Dunlap, Juergen Gross, Ian Jackson, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 712 bytes --]

On Fri, 2012-06-01 at 11:14 +0100, Ian Campbell wrote: 
> Thanks. I'm going to commit this as is and await the fixup, if that's
> ok. 
>
That's fine, I'm working on it today, as I have been busier that
expected with other things. I'll submit it separately on top of your
commits.

> I think this isn't a regression from the PoV of the configuration
> file since this would also have happened before.
> 
It is, indeed.

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-05-29 13:56 [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
                   ` (4 preceding siblings ...)
  2012-05-29 13:57 ` [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters Ian Campbell
@ 2012-06-01 11:07 ` Ian Campbell
  2012-06-01 18:08   ` Ian Jackson
  5 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-06-01 11:07 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Dario Faggioli, Ian Jackson, Juergen Gross

On Tue, 2012-05-29 at 14:56 +0100, Ian Campbell wrote:
> This series defines a descriminating value for each scheduler paramter
> and uses it to fix a warning when starting a guest:
>   "Cpu weight out of range, valid values are within range from 1 to 65535"

I applied this with George and Ian J's acks.

Thanks.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-01 11:07 ` [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
@ 2012-06-01 18:08   ` Ian Jackson
  2012-06-01 18:47     ` Ian Campbell
  0 siblings, 1 reply; 30+ messages in thread
From: Ian Jackson @ 2012-06-01 18:08 UTC (permalink / raw)
  To: Ian Campbell; +Cc: George Dunlap, Dario Faggioli, Juergen Gross, xen-devel

Ian Campbell writes ("Re: [Xen-devel] [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params"):
> On Tue, 2012-05-29 at 14:56 +0100, Ian Campbell wrote:
> > This series defines a descriminating value for each scheduler paramter
> > and uses it to fix a warning when starting a guest:
> >   "Cpu weight out of range, valid values are within range from 1 to 65535"
> 
> I applied this with George and Ian J's acks.

It seems to have produced a build failure in the ocaml bindings.  See
below.

Ian.

make[7]: Entering directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs/xl'
 MLC      xenlight.cmo
 MLA      xenlight.cma
 CC       xenlight_stubs.o
xenlight_stubs.c: In function 'stub_xl_sched_credit_domain_get':
xenlight_stubs.c:503: error: 'libxl_sched_credit_domain' undeclared (first use in this function)
xenlight_stubs.c:503: error: (Each undeclared identifier is reported only once
xenlight_stubs.c:503: error: for each function it appears in.)
xenlight_stubs.c:503: error: expected ';' before 'c_scinfo'
cc1: warnings being treated as errors
xenlight_stubs.c:504: error: ISO C90 forbids mixed declarations and code
xenlight_stubs.c:508: error: implicit declaration of function 'libxl_sched_credit_domain_get'
xenlight_stubs.c:508: error: 'c_scinfo' undeclared (first use in this function)
xenlight_stubs.c:513: error: implicit declaration of function 'Val_sched_credit_domain'
xenlight_stubs.c: In function 'stub_xl_sched_credit_domain_set':
xenlight_stubs.c:520: error: 'libxl_sched_credit_domain' undeclared (first use in this function)
xenlight_stubs.c:520: error: expected ';' before 'c_scinfo'
xenlight_stubs.c:521: error: ISO C90 forbids mixed declarations and code
xenlight_stubs.c:524: error: implicit declaration of function 'sched_credit_domain_val'
xenlight_stubs.c:524: error: 'c_scinfo' undeclared (first use in this function)
xenlight_stubs.c:527: error: implicit declaration of function 'libxl_sched_credit_domain_set'
make[7]: *** [xenlight_stubs.o] Error 1
make[7]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs/xl'
make[6]: *** [subdir-install-xl] Error 2
make[6]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs'
make[5]: *** [subdirs-install] Error 2
make[5]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs'
make[4]: *** [subdir-install-libs] Error 2
make[4]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml'
make[3]: *** [subdirs-install] Error 2
make[3]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml'
make[2]: *** [subdir-install-ocaml] Error 2
make[2]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools'
make[1]: *** [subdirs-install] Error 2
make[1]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools'
make: *** [install-tools] Error 2

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-01 18:08   ` Ian Jackson
@ 2012-06-01 18:47     ` Ian Campbell
  2012-06-01 19:14       ` Ian Campbell
  2012-06-02  6:37       ` Dario Faggioli
  0 siblings, 2 replies; 30+ messages in thread
From: Ian Campbell @ 2012-06-01 18:47 UTC (permalink / raw)
  To: Ian Jackson; +Cc: George Dunlap, Dario Faggioli, Juergen Gross, xen-devel

On Fri, 2012-06-01 at 19:08 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params"):
> > On Tue, 2012-05-29 at 14:56 +0100, Ian Campbell wrote:
> > > This series defines a descriminating value for each scheduler paramter
> > > and uses it to fix a warning when starting a guest:
> > >   "Cpu weight out of range, valid values are within range from 1 to 65535"
> > 
> > I applied this with George and Ian J's acks.
> 
> It seems to have produced a build failure in the ocaml bindings.

Damn, sorry about that. I swear I compiled this both before sending it
and as part of the commit procedure, and I just tried built it again and
didn't see it.

However I've now done a make clean and can now repro. Patch to follow
shortly.

>   See
> below.
> 
> Ian.
> 
> make[7]: Entering directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs/xl'
>  MLC      xenlight.cmo
>  MLA      xenlight.cma
>  CC       xenlight_stubs.o
> xenlight_stubs.c: In function 'stub_xl_sched_credit_domain_get':
> xenlight_stubs.c:503: error: 'libxl_sched_credit_domain' undeclared (first use in this function)
> xenlight_stubs.c:503: error: (Each undeclared identifier is reported only once
> xenlight_stubs.c:503: error: for each function it appears in.)
> xenlight_stubs.c:503: error: expected ';' before 'c_scinfo'
> cc1: warnings being treated as errors
> xenlight_stubs.c:504: error: ISO C90 forbids mixed declarations and code
> xenlight_stubs.c:508: error: implicit declaration of function 'libxl_sched_credit_domain_get'
> xenlight_stubs.c:508: error: 'c_scinfo' undeclared (first use in this function)
> xenlight_stubs.c:513: error: implicit declaration of function 'Val_sched_credit_domain'
> xenlight_stubs.c: In function 'stub_xl_sched_credit_domain_set':
> xenlight_stubs.c:520: error: 'libxl_sched_credit_domain' undeclared (first use in this function)
> xenlight_stubs.c:520: error: expected ';' before 'c_scinfo'
> xenlight_stubs.c:521: error: ISO C90 forbids mixed declarations and code
> xenlight_stubs.c:524: error: implicit declaration of function 'sched_credit_domain_val'
> xenlight_stubs.c:524: error: 'c_scinfo' undeclared (first use in this function)
> xenlight_stubs.c:527: error: implicit declaration of function 'libxl_sched_credit_domain_set'
> make[7]: *** [xenlight_stubs.o] Error 1
> make[7]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs/xl'
> make[6]: *** [subdir-install-xl] Error 2
> make[6]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs'
> make[5]: *** [subdirs-install] Error 2
> make[5]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml/libs'
> make[4]: *** [subdir-install-libs] Error 2
> make[4]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml'
> make[3]: *** [subdirs-install] Error 2
> make[3]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools/ocaml'
> make[2]: *** [subdir-install-ocaml] Error 2
> make[2]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools'
> make[1]: *** [subdirs-install] Error 2
> make[1]: Leaving directory `/u/iwj/work/xen-unstable-tools.hg/tools'
> make: *** [install-tools] Error 2

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-01 18:47     ` Ian Campbell
@ 2012-06-01 19:14       ` Ian Campbell
  2012-06-02  7:40         ` Ian Campbell
  2012-06-02  6:37       ` Dario Faggioli
  1 sibling, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-06-01 19:14 UTC (permalink / raw)
  To: Ian Jackson; +Cc: George Dunlap, Dario Faggioli, Juergen Gross, xen-devel

On Fri, 2012-06-01 at 19:47 +0100, Ian Campbell wrote:
> Patch to follow shortly.

Here we go. Since this fixes the build I'll apply it in the morning
unless someone has objected by then (rather than leave the build broken
over the 4 day UK bank holiday weekend...), or sooner if someone acks
it.

I'll also add "make distclean" to my commit process, possibly even a git
clean based rune too.

8<--------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1338578033 -3600
# Node ID bfe1d242bab30451b55e0bc12624a8e823d9a32a
# Parent  0197a9b4fd81dfd03f9df81a1c1eac64b5babdad
ocaml: fix build after 25446:648508ee27a2, 25449:68d46c5ea0a3 et al.

These renamed a type and the associated functions and the ocaml bindings were
not updated to suit.

This also highlighted that libxl_domain_sched_params should not be just DIR_IN
since it is also use as an output struct.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 0197a9b4fd81 -r bfe1d242bab3 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Fri Jun 01 19:59:24 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Fri Jun 01 20:13:53 2012 +0100
@@ -232,7 +232,7 @@ libxl_domain_sched_params = Struct("doma
     ("slice",        integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),
     ("latency",      integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
     ("extratime",    integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
-    ], dir=DIR_IN)
+    ])
 
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
diff -r 0197a9b4fd81 -r bfe1d242bab3 tools/ocaml/libs/xl/genwrap.py
--- a/tools/ocaml/libs/xl/genwrap.py	Fri Jun 01 19:59:24 2012 +0100
+++ b/tools/ocaml/libs/xl/genwrap.py	Fri Jun 01 20:13:53 2012 +0100
@@ -32,8 +32,9 @@ functions = { # ( name , [type1,type2,..
                       ],
     "cputopology":    [ ("get",            ["unit", "t array"]),
                       ],
-    "sched_credit":   [ ("domain_get",     ["domid", "t"]),
-                        ("domain_set",     ["domid", "t", "unit"]),
+    "domain_sched_params":
+                      [ ("get",            ["domid", "t"]),
+                        ("set",            ["domid", "t", "unit"]),
                       ],
 }
 def stub_fn_name(ty, name):
diff -r 0197a9b4fd81 -r bfe1d242bab3 tools/ocaml/libs/xl/xenlight_stubs.c
--- a/tools/ocaml/libs/xl/xenlight_stubs.c	Fri Jun 01 19:59:24 2012 +0100
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c	Fri Jun 01 20:13:53 2012 +0100
@@ -496,37 +496,37 @@ value stub_xl_cputopology_get(value unit
 	CAMLreturn(topology);
 }
 
-value stub_xl_sched_credit_domain_get(value domid)
+value stub_xl_domain_sched_params_get(value domid)
 {
 	CAMLparam1(domid);
 	CAMLlocal1(scinfo);
-	libxl_sched_credit_domain c_scinfo;
+	libxl_domain_sched_params c_scinfo;
 	int ret;
 	INIT_STRUCT();
 
 	INIT_CTX();
-	ret = libxl_sched_credit_domain_get(ctx, Int_val(domid), &c_scinfo);
+	ret = libxl_domain_sched_params_get(ctx, Int_val(domid), &c_scinfo);
 	if (ret != 0)
-		failwith_xl("sched_credit_domain_get", &lg);
+		failwith_xl("domain_sched_params_get", &lg);
 	FREE_CTX();
 
-	scinfo = Val_sched_credit_domain(&gc, &lg, &c_scinfo);
+	scinfo = Val_domain_sched_params(&gc, &lg, &c_scinfo);
 	CAMLreturn(scinfo);
 }
 
-value stub_xl_sched_credit_domain_set(value domid, value scinfo)
+value stub_xl_domain_sched_params_set(value domid, value scinfo)
 {
 	CAMLparam2(domid, scinfo);
-	libxl_sched_credit_domain c_scinfo;
+	libxl_domain_sched_params c_scinfo;
 	int ret;
 	INIT_STRUCT();
 
-	sched_credit_domain_val(&gc, &lg, &c_scinfo, scinfo);
+	domain_sched_params_val(&gc, &lg, &c_scinfo, scinfo);
 
 	INIT_CTX();
-	ret = libxl_sched_credit_domain_set(ctx, Int_val(domid), &c_scinfo);
+	ret = libxl_domain_sched_params_set(ctx, Int_val(domid), &c_scinfo);
 	if (ret != 0)
-		failwith_xl("sched_credit_domain_set", &lg);
+		failwith_xl("domain_sched_params_set", &lg);
 	FREE_CTX();
 
 	CAMLreturn(Val_unit);

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-01 18:47     ` Ian Campbell
  2012-06-01 19:14       ` Ian Campbell
@ 2012-06-02  6:37       ` Dario Faggioli
  2012-06-02  7:41         ` Ian Campbell
  1 sibling, 1 reply; 30+ messages in thread
From: Dario Faggioli @ 2012-06-02  6:37 UTC (permalink / raw)
  To: Ian Campbell; +Cc: George Dunlap, Juergen Gross, Ian Jackson, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 686 bytes --]

On Fri, 2012-06-01 at 19:47 +0100, Ian Campbell wrote: 
> Damn, sorry about that. I swear I compiled this both before sending it
> and as part of the commit procedure, and I just tried built it again and
> didn't see it.
> 
Uh? I can build it too...

> However I've now done a make clean and can now repro. Patch to follow
> shortly.
> 
Ah, perhaps it is this (I'm not sure I 'make clean'-ed)

Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-01 19:14       ` Ian Campbell
@ 2012-06-02  7:40         ` Ian Campbell
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Campbell @ 2012-06-02  7:40 UTC (permalink / raw)
  To: Ian Jackson; +Cc: George Dunlap, Dario Faggioli, Juergen Gross, xen-devel

On Fri, 2012-06-01 at 20:14 +0100, Ian Campbell wrote:
> On Fri, 2012-06-01 at 19:47 +0100, Ian Campbell wrote:
> > Patch to follow shortly.
> 
> Here we go. Since this fixes the build I'll apply it in the morning

Done.

[...]

> 8<--------------------------------------
> 
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1338578033 -3600
> # Node ID bfe1d242bab30451b55e0bc12624a8e823d9a32a
> # Parent  0197a9b4fd81dfd03f9df81a1c1eac64b5babdad
> ocaml: fix build after 25446:648508ee27a2, 25449:68d46c5ea0a3 et al.
> 
> These renamed a type and the associated functions and the ocaml bindings were
> not updated to suit.
> 
> This also highlighted that libxl_domain_sched_params should not be just DIR_IN
> since it is also use as an output struct.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> diff -r 0197a9b4fd81 -r bfe1d242bab3 tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Fri Jun 01 19:59:24 2012 +0100
> +++ b/tools/libxl/libxl_types.idl	Fri Jun 01 20:13:53 2012 +0100
> @@ -232,7 +232,7 @@ libxl_domain_sched_params = Struct("doma
>      ("slice",        integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),
>      ("latency",      integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
>      ("extratime",    integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
> -    ], dir=DIR_IN)
> +    ])
>  
>  libxl_domain_build_info = Struct("domain_build_info",[
>      ("max_vcpus",       integer),
> diff -r 0197a9b4fd81 -r bfe1d242bab3 tools/ocaml/libs/xl/genwrap.py
> --- a/tools/ocaml/libs/xl/genwrap.py	Fri Jun 01 19:59:24 2012 +0100
> +++ b/tools/ocaml/libs/xl/genwrap.py	Fri Jun 01 20:13:53 2012 +0100
> @@ -32,8 +32,9 @@ functions = { # ( name , [type1,type2,..
>                        ],
>      "cputopology":    [ ("get",            ["unit", "t array"]),
>                        ],
> -    "sched_credit":   [ ("domain_get",     ["domid", "t"]),
> -                        ("domain_set",     ["domid", "t", "unit"]),
> +    "domain_sched_params":
> +                      [ ("get",            ["domid", "t"]),
> +                        ("set",            ["domid", "t", "unit"]),
>                        ],
>  }
>  def stub_fn_name(ty, name):
> diff -r 0197a9b4fd81 -r bfe1d242bab3 tools/ocaml/libs/xl/xenlight_stubs.c
> --- a/tools/ocaml/libs/xl/xenlight_stubs.c	Fri Jun 01 19:59:24 2012 +0100
> +++ b/tools/ocaml/libs/xl/xenlight_stubs.c	Fri Jun 01 20:13:53 2012 +0100
> @@ -496,37 +496,37 @@ value stub_xl_cputopology_get(value unit
>  	CAMLreturn(topology);
>  }
>  
> -value stub_xl_sched_credit_domain_get(value domid)
> +value stub_xl_domain_sched_params_get(value domid)
>  {
>  	CAMLparam1(domid);
>  	CAMLlocal1(scinfo);
> -	libxl_sched_credit_domain c_scinfo;
> +	libxl_domain_sched_params c_scinfo;
>  	int ret;
>  	INIT_STRUCT();
>  
>  	INIT_CTX();
> -	ret = libxl_sched_credit_domain_get(ctx, Int_val(domid), &c_scinfo);
> +	ret = libxl_domain_sched_params_get(ctx, Int_val(domid), &c_scinfo);
>  	if (ret != 0)
> -		failwith_xl("sched_credit_domain_get", &lg);
> +		failwith_xl("domain_sched_params_get", &lg);
>  	FREE_CTX();
>  
> -	scinfo = Val_sched_credit_domain(&gc, &lg, &c_scinfo);
> +	scinfo = Val_domain_sched_params(&gc, &lg, &c_scinfo);
>  	CAMLreturn(scinfo);
>  }
>  
> -value stub_xl_sched_credit_domain_set(value domid, value scinfo)
> +value stub_xl_domain_sched_params_set(value domid, value scinfo)
>  {
>  	CAMLparam2(domid, scinfo);
> -	libxl_sched_credit_domain c_scinfo;
> +	libxl_domain_sched_params c_scinfo;
>  	int ret;
>  	INIT_STRUCT();
>  
> -	sched_credit_domain_val(&gc, &lg, &c_scinfo, scinfo);
> +	domain_sched_params_val(&gc, &lg, &c_scinfo, scinfo);
>  
>  	INIT_CTX();
> -	ret = libxl_sched_credit_domain_set(ctx, Int_val(domid), &c_scinfo);
> +	ret = libxl_domain_sched_params_set(ctx, Int_val(domid), &c_scinfo);
>  	if (ret != 0)
> -		failwith_xl("sched_credit_domain_set", &lg);
> +		failwith_xl("domain_sched_params_set", &lg);
>  	FREE_CTX();
>  
>  	CAMLreturn(Val_unit);
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-02  6:37       ` Dario Faggioli
@ 2012-06-02  7:41         ` Ian Campbell
  2012-06-06 10:38           ` Ian Jackson
  0 siblings, 1 reply; 30+ messages in thread
From: Ian Campbell @ 2012-06-02  7:41 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: George Dunlap, Juergen Gross, Ian Jackson, xen-devel

On Sat, 2012-06-02 at 07:37 +0100, Dario Faggioli wrote:
> On Fri, 2012-06-01 at 19:47 +0100, Ian Campbell wrote: 
> > Damn, sorry about that. I swear I compiled this both before sending it
> > and as part of the commit procedure, and I just tried built it again and
> > didn't see it.
> > 
> Uh? I can build it too...

So, apparently, could test flight 13006, which is odd because I thought
the test system was doing everything totally from scratch.

I've pushed the fix anyway.

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params
  2012-06-02  7:41         ` Ian Campbell
@ 2012-06-06 10:38           ` Ian Jackson
  0 siblings, 0 replies; 30+ messages in thread
From: Ian Jackson @ 2012-06-06 10:38 UTC (permalink / raw)
  To: Ian Campbell; +Cc: George Dunlap, Juergen Gross, Dario Faggioli, xen-devel

Ian Campbell writes ("Re: [Xen-devel] [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params"):
> On Sat, 2012-06-02 at 07:37 +0100, Dario Faggioli wrote:
> > Uh? I can build it too...
> 
> So, apparently, could test flight 13006, which is odd because I thought
> the test system was doing everything totally from scratch.

The test system doesn't have ocaml installed because when it did it
exposed what seemed to be a bug in the ocaml xenstored, which I
haven't yet had time to investigate.

Ian.

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2012-06-06 10:38 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-29 13:56 [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
2012-05-29 13:56 ` [PATCH 1 of 5 V2] libxl: add internal function to get a domain's scheduler Ian Campbell
2012-05-31 13:19   ` George Dunlap
2012-05-31 15:11     ` Ian Jackson
2012-06-01  9:51     ` Ian Campbell
2012-05-29 13:57 ` [PATCH 2 of 5 V2] libxl: rename libxl_sched_params to libxl_domain_sched_params Ian Campbell
2012-05-31 13:21   ` George Dunlap
2012-05-31 15:12     ` Ian Jackson
2012-05-29 13:57 ` [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
2012-05-30  7:26   ` Dario Faggioli
2012-05-30  7:35     ` Ian Campbell
2012-05-30  8:00       ` Dario Faggioli
2012-06-01 10:14         ` Ian Campbell
2012-06-01 10:26           ` Dario Faggioli
2012-05-31 13:47   ` George Dunlap
2012-05-31 15:13     ` Ian Jackson
2012-05-29 13:57 ` [PATCH 4 of 5 V2] libxl: move libxl__sched_set_params into libxl.c Ian Campbell
2012-05-31 13:48   ` George Dunlap
2012-05-31 15:14     ` Ian Jackson
2012-05-29 13:57 ` [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters Ian Campbell
2012-05-31 13:51   ` George Dunlap
2012-05-31 15:15     ` Ian Jackson
2012-06-01 11:07 ` [PATCH 0 of 5 V2] libxl: make it possible to explicitly specify default sched params Ian Campbell
2012-06-01 18:08   ` Ian Jackson
2012-06-01 18:47     ` Ian Campbell
2012-06-01 19:14       ` Ian Campbell
2012-06-02  7:40         ` Ian Campbell
2012-06-02  6:37       ` Dario Faggioli
2012-06-02  7:41         ` Ian Campbell
2012-06-06 10:38           ` Ian Jackson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.