From e430c0d0c689d3ca057b507f07bd12007d0055e0 Mon Sep 17 00:00:00 2001 From: Matthias Bolte Date: Wed, 18 May 2011 10:52:57 +0200 Subject: [PATCH] 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. --- 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 45bc9b072e..79a196a293 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 6b2d806b01..b85c743eaf 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 2bb592dbbb..a65299bc17 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 fdb3b30059..9a7286dcfa 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 e716c98f64..2d5f48906b 100644 --- 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 8fb58320f6..20eca6e90f 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 3ec6e2b635..8d579bc593 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 359d5f4972..0fa8042adc 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,