libxl: validate scheduler parameters

This was previously done by xl itself however the domain was not
created at that point so there was no domid to check. This happened to
work on first boot because xl's global domid was initialised to zero
so we would (incorrectly) validate the new domain to be against
domain0. On reboot though we would try to use the old domain's id and
fail.

sched_params_valid is moved and gains a gc+domid parameters and
s/ctx/CTX/. The call is placed after
libxl__domain_build_info_setdefault in the create path, because
set_defaults doesn't have access to the domid and there are other
callers which don't even have a domid to give it.

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

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4456ae80128e8427eaf2f54525841b7c4985a606..9cb7a5e47d156f35d6314c13dea25bd943aabc5d 100644 (file)
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -72,6 +72,49 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
     return 0;
 }
 
+static int sched_params_valid(libxl__gc *gc,
+                              uint32_t domid, libxl_domain_sched_params *scp)
+{
+    int has_weight = scp->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT;
+    int has_period = scp->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT;
+    int has_slice = scp->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT;
+    int has_extratime =
+                scp->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT;
+    libxl_domain_sched_params sci;
+
+    libxl_domain_sched_params_get(CTX, domid, &sci);
+
+    /* The sedf scheduler needs some more consistency checking */
+    if (sci.sched == LIBXL_SCHEDULER_SEDF) {
+        if (has_weight && (has_period || has_slice))
+            return 0;
+        if (has_period != has_slice)
+            return 0;
+
+        /*
+         * Idea is, if we specify a weight, then both period and
+         * slice has to be zero. OTOH, if we do not specify a weight,
+         * that means we want a pure best effort domain or an actual
+         * real-time one. In the former case, it is period that needs
+         * to be zero, in the latter, weight should be.
+         */
+        if (has_weight) {
+            scp->slice = 0;
+            scp->period = 0;
+        }
+        else if (!has_period) {
+            /* We can setup a proper best effort domain (extra time only)
+             * iff we either already have or are asking for some extra time. */
+            scp->weight = has_extratime ? scp->extratime : sci.extratime;
+            scp->period = 0;
+        }
+        if (has_period && has_slice)
+            scp->weight = 0;
+    }
+
+    return 1;
+}
+
 int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                         libxl_domain_build_info *b_info)
 {
@@ -622,6 +665,12 @@ static void initiate_domain_create(libxl__egc *egc,
     ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
     if (ret) goto error_out;
 
+    if (!sched_params_valid(gc, domid, &d_config->b_info.sched_params)) {
+        LOG(ERROR, "Invalid scheduling parameters\n");
+        ret = ERROR_INVAL;
+        goto error_out;
+    }
+
     for (i = 0; i < d_config->num_disks; i++) {
         ret = libxl__device_disk_setdefault(gc, &d_config->disks[i]);
         if (ret) goto error_out;

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 63dc564cb28f733a70aa8287a6e8a94de5ce8a8b..afa0af679f8bcc05e81a3d89c3459ed763e0dfdf 100644 (file)
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -550,48 +550,6 @@ vcpp_out:
     return rc;
 }
 
-static int sched_params_valid(libxl_domain_sched_params *scp)
-{
-    int has_weight = scp->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT;
-    int has_period = scp->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT;
-    int has_slice = scp->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT;
-    int has_extratime =
-                scp->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT;
-    libxl_domain_sched_params sci;
-
-    libxl_domain_sched_params_get(ctx, domid, &sci);
-
-    /* The sedf scheduler needs some more consistency checking */
-    if (sci.sched == LIBXL_SCHEDULER_SEDF) {
-        if (has_weight && (has_period || has_slice))
-            return 0;
-        if (has_period != has_slice)
-            return 0;
-
-        /*
-         * Idea is, if we specify a weight, then both period and
-         * slice has to be zero. OTOH, if we do not specify a weight,
-         * that means we want a pure best effort domain or an actual
-         * real-time one. In the former case, it is period that needs
-         * to be zero, in the latter, weight should be.
-         */
-        if (has_weight) {
-            scp->slice = 0;
-            scp->period = 0;
-        }
-        else if (!has_period) {
-            /* We can setup a proper best effort domain (extra time only)
-             * iff we either already have or are asking for some extra time. */
-            scp->weight = has_extratime ? scp->extratime : sci.extratime;
-            scp->period = 0;
-        }
-        if (has_period && has_slice)
-            scp->weight = 0;
-    }
-
-    return 1;
-}
-
 static void parse_config_data(const char *config_source,
                               const char *config_data,
                               int config_len,
@@ -686,10 +644,6 @@ static void parse_config_data(const char *config_source,
         b_info->sched_params.latency = l;
     if (!xlu_cfg_get_long (config, "extratime", &l, 0))
         b_info->sched_params.extratime = l;
-    if (!sched_params_valid(&b_info->sched_params)) {
-        fprintf(stderr, "Invalid scheduling parameters\n");
-        exit(1);
-    }
 
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         b_info->max_vcpus = l;