Clarify the semantic of virDomainGetSchedulerParameters arguments

params and nparams are essential and cannot be NULL. Check this in
libvirt.c and remove redundant checks from the drivers (e.g. xend).

Instead of enforcing that nparams must point to exact same value as
returned by virDomainGetSchedulerType relax this to a lower bound
check. This is what some drivers (e.g. xen hypervisor and esx)
already did. Other drivers (e.g. xend) didn't check nparams at all
and assumed that there is enough space in params.

Unify the behavior in all drivers to a lower bound check and update
nparams to the number of valid values in params on success.

diff --git a/src/libvirt.c b/src/libvirt.c
index 45bc9b072ea8155c5deeccbfabb1c9e604073f1e..79a196a293f3487759d1d8412ce3b505cccfe013 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5015,14 +5015,15 @@ error:
 /**
  * virDomainGetSchedulerParameters:
  * @domain: pointer to domain object
- * @params: pointer to scheduler parameter object
+ * @params: pointer to scheduler parameter objects
  *          (return value)
- * @nparams: pointer to number of scheduler parameter
- *          (this value should be same than the returned value
+ * @nparams: pointer to number of scheduler parameter objects
+ *          (this value must be at least as large as the returned value
  *           nparams of virDomainGetSchedulerType)
  *
- * Get the scheduler parameters, the @params array will be filled with the
- * values.
+ * Get all scheduler parameters, the @params array will be filled with the
+ * values and @nparams will be updated to the number of valid elements in
+ * @params.
  *
  * Returns -1 in case of error, 0 in case of success.
  */
@@ -5041,6 +5042,12 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
         virDispatchError(NULL);
         return -1;
     }
+
+    if (params == NULL || nparams == NULL || *nparams <= 0) {
+        virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+        goto error;
+    }
+
     conn = domain->conn;
 
     if (conn->driver->domainGetSchedulerParameters) {

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6b2d806b012ca9e6c843524738a88c9b938b176c..b85c743eaf1fa8cd8a32764c51c96084277bf792 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2466,7 +2466,7 @@ libxlDomainGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr params,
         goto cleanup;
     }
 
-    if (*nparams != XEN_SCHED_CREDIT_NPARAM) {
+    if (*nparams < XEN_SCHED_CREDIT_NPARAM) {
         libxlError(VIR_ERR_INVALID_ARG, "%s", _("Invalid parameter count"));
         goto cleanup;
     }
@@ -2494,6 +2494,7 @@ libxlDomainGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr params,
         goto cleanup;
     }
 
+    *nparams = XEN_SCHED_CREDIT_NPARAM;
     ret = 0;
 
 cleanup:

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 2bb592dbbb43ce03f564fd23663ca51217d4a65f..a65299bc17e20c6a779373f718baecd1c158048e 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2234,7 +2234,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain,
     if (driver->cgroup == NULL)
         return -1;
 
-    if ((*nparams) != 1) {
+    if (*nparams < 1) {
         lxcError(VIR_ERR_INVALID_ARG,
                  "%s", _("Invalid parameter count"));
         return -1;
@@ -2264,6 +2264,7 @@ static int lxcGetSchedulerParameters(virDomainPtr domain,
     }
     params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG;
 
+    *nparams = 1;
     ret = 0;
 
 cleanup:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fdb3b30059bb921db9fcabe5383510718c5f7fab..9a7286dcfa4d532280f777fc18a407f24c8416b2 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5181,7 +5181,7 @@ static int qemuGetSchedulerParameters(virDomainPtr dom,
         goto cleanup;
     }
 
-    if ((*nparams) != 1) {
+    if (*nparams < 1) {
         qemuReportError(VIR_ERR_INVALID_ARG,
                         "%s", _("Invalid parameter count"));
         goto cleanup;
@@ -5221,6 +5221,7 @@ out:
         goto cleanup;
     }
 
+    *nparams = 1;
     ret = 0;
 
 cleanup:

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e716c98f64cbf0a0fcc6404cae9430ec3e7726aa..2d5f48906b1632c1e1d7cabc2886e2e267ab5869 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2652,8 +2652,8 @@ static int testDomainGetSchedulerParams(virDomainPtr domain,
         goto cleanup;
     }
 
-    if (*nparams != 1) {
-        testError(VIR_ERR_INVALID_ARG, "nparams");
+    if (*nparams < 1) {
+        testError(VIR_ERR_INVALID_ARG, "%s", _("Invalid parameter count"));
         goto cleanup;
     }
     strcpy(params[0].field, "weight");
@@ -2661,6 +2661,8 @@ static int testDomainGetSchedulerParams(virDomainPtr domain,
     /* XXX */
     /*params[0].value.ui = privdom->weight;*/
     params[0].value.ui = 50;
+
+    *nparams = 1;
     ret = 0;
 
 cleanup:

diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index 8fb58320f6928740b0884b6dc76ca3a4dab38a35..20eca6e90f530eae72648a3ed775d7748c2a5276 100644 (file)
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -60,6 +60,9 @@ extern int xenRegister (void);
 
 # define XEND_DOMAINS_DIR "/var/lib/xend/domains"
 
+# define XEN_SCHED_SEDF_NPARAM   6
+# define XEN_SCHED_CRED_NPARAM   2
+
 /* _xenUnifiedDriver:
  *
  * Entry points into the underlying Xen drivers.  This structure

diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 3ec6e2b6353733d37176d1cd333e69f4540415af..8d579bc593d9f9f4a1d0d4bf3ddf197c901a04a8 100644 (file)
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1207,14 +1207,14 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
                 if (schedulertype == NULL)
                     virReportOOMError();
                 if (nparams)
-                    *nparams = 6;
+                    *nparams = XEN_SCHED_SEDF_NPARAM;
                 break;
             case XEN_SCHEDULER_CREDIT:
                 schedulertype = strdup("credit");
                 if (schedulertype == NULL)
                     virReportOOMError();
                 if (nparams)
-                    *nparams = 2;
+                    *nparams = XEN_SCHED_CRED_NPARAM;
                 break;
             default:
                 break;
@@ -1232,8 +1232,8 @@ static const char *str_cap = "cap";
  * @domain: pointer to the Xen Hypervisor block
  * @params: pointer to scheduler parameters.
  *     This memory area should be allocated before calling.
- * @nparams:this parameter should be same as
- *     a given number of scheduler parameters.
+ * @nparams: this parameter must be at least as large as
+ *     the given number of scheduler parameters.
  *     from xenHypervisorGetSchedulerType().
  *
  * Do a low level hypercall to get scheduler parameters
@@ -1288,12 +1288,21 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
 
         switch (op_sys.u.getschedulerid.sched_id){
             case XEN_SCHEDULER_SEDF:
+                if (*nparams < XEN_SCHED_SEDF_NPARAM) {
+                    virXenError(VIR_ERR_INVALID_ARG,
+                                "%s", _("Invalid parameter count"));
+                    return -1;
+                }
+
                 /* TODO: Implement for Xen/SEDF */
                 TODO
                 return(-1);
             case XEN_SCHEDULER_CREDIT:
-                if (*nparams < 2)
-                    return(-1);
+                if (*nparams < XEN_SCHED_CRED_NPARAM) {
+                    virXenError(VIR_ERR_INVALID_ARG,
+                                "%s", _("Invalid parameter count"));
+                    return -1;
+                }
                 memset(&op_dom, 0, sizeof(op_dom));
                 op_dom.cmd = XEN_V2_OP_SCHEDULER;
                 op_dom.domain = (domid_t) domain->id;
@@ -1319,7 +1328,7 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
                 params[1].type = VIR_DOMAIN_SCHED_FIELD_UINT;
                 params[1].value.ui = op_dom.u.getschedinfo.u.credit.cap;
 
-                *nparams = 2;
+                *nparams = XEN_SCHED_CRED_NPARAM;
                 break;
             default:
                 virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__,

diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 359d5f497216511fec3ea10ac149e5eb155a307a..0fa8042adc8014faa4e51597d9b05a22dfefcba0 100644 (file)
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -55,8 +55,6 @@
 /*
  * The number of Xen scheduler parameters
  */
-#define XEN_SCHED_SEDF_NPARAM   6
-#define XEN_SCHED_CRED_NPARAM   2
 
 #define XEND_RCV_BUF_MAX_LEN 65536
 
@@ -3607,8 +3605,7 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain,
     int sched_nparam = 0;
     int ret = -1;
 
-    if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)
-        || (params == NULL) || (nparams == NULL)) {
+    if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
         virXendError(VIR_ERR_INVALID_ARG, __FUNCTION__);
         return (-1);
     }
@@ -3636,10 +3633,22 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain,
 
     switch (sched_nparam){
         case XEN_SCHED_SEDF_NPARAM:
+            if (*nparams < XEN_SCHED_SEDF_NPARAM) {
+                virXendError(VIR_ERR_INVALID_ARG,
+                             "%s", _("Invalid parameter count"));
+                goto error;
+            }
+
             /* TODO: Implement for Xen/SEDF */
             TODO
             goto error;
         case XEN_SCHED_CRED_NPARAM:
+            if (*nparams < XEN_SCHED_CRED_NPARAM) {
+                virXendError(VIR_ERR_INVALID_ARG,
+                             "%s", _("Invalid parameter count"));
+                goto error;
+            }
+
             /* get cpu_weight/cpu_cap from xend/domain */
             if (sexpr_node(root, "domain/cpu_weight") == NULL) {
                 virXendError(VIR_ERR_INTERNAL_ERROR,