[libvirt] [PATCH 2/6] Clarify the semantic of virDomainGetSchedulerParameters arguments

Matthias Bolte matthias.bolte at googlemail.com
Wed May 18 10:21:09 UTC 2011


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.
---
 src/libvirt.c            |   17 ++++++++++++-----
 src/libxl/libxl_driver.c |    3 ++-
 src/lxc/lxc_driver.c     |    3 ++-
 src/qemu/qemu_driver.c   |    3 ++-
 src/test/test_driver.c   |    6 ++++--
 src/xen/xen_driver.h     |    3 +++
 src/xen/xen_hypervisor.c |   23 ++++++++++++++++-------
 src/xen/xend_internal.c  |   17 +++++++++++++----
 8 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 45bc9b0..e8b5b80 100644
--- 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 6b2d806..b85c743 100644
--- 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 2bb592d..a65299b 100644
--- 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 fdb3b30..9a7286d 100644
--- 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 d9e9cb9..4897081 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2645,8 +2645,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");
@@ -2654,6 +2654,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 8fb5832..20eca6e 100644
--- 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 3ec6e2b..8d579bc 100644
--- 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 359d5f4..0fa8042 100644
--- 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,
-- 
1.7.0.4




More information about the libvir-list mailing list