Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Introduce use of a virDomainDefPtr in the domain scheduler
> APIs to simplify introduction of ACL security checks.
> The virDomainPtr cannot be safely used, since the app
> may have supplied mis-matching name/uuid/id fields. eg
> the name points to domain X, while the uuid points to
> domain Y. Resolving the virDomainPtr to a virDomainDefPtr
> ensures a consistent name/uuid/id set.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> src/xen/xen_driver.c | 45 ++++++++++++++++++++++++++++++++++++---------
> src/xen/xen_hypervisor.c | 19 +++++++++++--------
> src/xen/xen_hypervisor.h | 16 +++++++++-------
> src/xen/xend_internal.c | 27 +++++++++++++++------------
> src/xen/xend_internal.h | 9 ++++++---
> 5 files changed, 77 insertions(+), 39 deletions(-)
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 43b3020..5ab1a52 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -1851,17 +1851,26 @@ static char *
> xenUnifiedDomainGetSchedulerType(virDomainPtr dom, int *nparams)
> {
> xenUnifiedPrivatePtr priv = dom->conn->privateData;
> + virDomainDefPtr def = NULL;
> + char *ret = NULL;
> +
> + if (!(def = xenGetDomainDefForDom(dom)))
> + goto cleanup;
>
> if (dom->id < 0) {
> if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot change scheduler parameters"));
> - return NULL;
> + goto cleanup;
> }
> - return xenDaemonGetSchedulerType(dom, nparams);
> + ret = xenDaemonGetSchedulerType(dom->conn, nparams);
> } else {
> - return xenHypervisorGetSchedulerType(dom, nparams);
> + ret = xenHypervisorGetSchedulerType(dom->conn, nparams);
> }
> +
> +cleanup:
> + virDomainDefFree(def);
> + return ret;
> }
>
> static int
> @@ -1871,19 +1880,28 @@ xenUnifiedDomainGetSchedulerParametersFlags(virDomainPtr dom,
> unsigned int flags)
> {
> xenUnifiedPrivatePtr priv = dom->conn->privateData;
> + virDomainDefPtr def = NULL;
> + int ret = -1;
>
> virCheckFlags(0, -1);
>
> + if (!(def = xenGetDomainDefForDom(dom)))
> + goto cleanup;
> +
> if (dom->id < 0) {
> if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot change scheduler parameters"));
> - return -1;
> + goto cleanup;
> }
> - return xenDaemonGetSchedulerParameters(dom, params, nparams);
> + ret = xenDaemonGetSchedulerParameters(dom->conn, def, params, nparams);
> } else {
> - return xenHypervisorGetSchedulerParameters(dom, params, nparams);
> + ret = xenHypervisorGetSchedulerParameters(dom->conn, def, params, nparams);
> }
> +
> +cleanup:
> + virDomainDefFree(def);
> + return ret;
> }
>
> static int
> @@ -1902,19 +1920,28 @@ xenUnifiedDomainSetSchedulerParametersFlags(virDomainPtr dom,
> unsigned int flags)
> {
> xenUnifiedPrivatePtr priv = dom->conn->privateData;
> + virDomainDefPtr def = NULL;
> + int ret = -1;
>
> virCheckFlags(0, -1);
>
> + if (!(def = xenGetDomainDefForDom(dom)))
> + goto cleanup;
> +
> if (dom->id < 0) {
> if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot change scheduler parameters"));
> - return -1;
> + goto cleanup;
> }
> - return xenDaemonSetSchedulerParameters(dom, params, nparams);
> + ret = xenDaemonSetSchedulerParameters(dom->conn, def, params, nparams);
> } else {
> - return xenHypervisorSetSchedulerParameters(dom, params, nparams);
> + ret = xenHypervisorSetSchedulerParameters(dom->conn, def, params, nparams);
> }
> +
> +cleanup:
> + virDomainDefFree(def);
> + return ret;
> }
>
> static int
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index b97b329..2525566 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -1113,10 +1113,11 @@ virXen_getdomaininfo(int handle, int first_domain, xen_getdomaininfo *dominfo)
> * Returns scheduler name or NULL in case of failure
> */
> char *
> -xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
> +xenHypervisorGetSchedulerType(virConnectPtr conn,
> + int *nparams)
>
Still fits on one line. And function comments need updated here and
where similar changes are made below.
> {
> char *schedulertype = NULL;
> - xenUnifiedPrivatePtr priv = domain->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
>
> /*
> * Support only hv_versions.dom_interface >=5
> @@ -1176,11 +1177,12 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
> * Returns 0 or -1 in case of failure
> */
> int
> -xenHypervisorGetSchedulerParameters(virDomainPtr domain,
> +xenHypervisorGetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> virTypedParameterPtr params,
> int *nparams)
> {
> - xenUnifiedPrivatePtr priv = domain->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
>
> /*
> * Support only hv_versions.dom_interface >=5
> @@ -1218,7 +1220,7 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
> case XEN_SCHEDULER_CREDIT:
> memset(&op_dom, 0, sizeof(op_dom));
> op_dom.cmd = XEN_V2_OP_SCHEDULER;
> - op_dom.domain = (domid_t) domain->id;
> + op_dom.domain = (domid_t) def->id;
> op_dom.u.getschedinfo.sched_id = XEN_SCHEDULER_CREDIT;
> op_dom.u.getschedinfo.cmd = XEN_DOMCTL_SCHEDOP_getinfo;
> ret = xenHypervisorDoV2Dom(priv->handle, &op_dom);
> @@ -1262,13 +1264,14 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
> * Returns 0 or -1 in case of failure
> */
> int
> -xenHypervisorSetSchedulerParameters(virDomainPtr domain,
> +xenHypervisorSetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> virTypedParameterPtr params,
> int nparams)
> {
> int i;
> unsigned int val;
> - xenUnifiedPrivatePtr priv = domain->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
> char buf[256];
>
> if (nparams == 0) {
> @@ -1313,7 +1316,7 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
> case XEN_SCHEDULER_CREDIT: {
> memset(&op_dom, 0, sizeof(op_dom));
> op_dom.cmd = XEN_V2_OP_SCHEDULER;
> - op_dom.domain = (domid_t) domain->id;
> + op_dom.domain = (domid_t) def->id;
> op_dom.u.getschedinfo.sched_id = XEN_SCHEDULER_CREDIT;
> op_dom.u.getschedinfo.cmd = XEN_DOMCTL_SCHEDOP_putinfo;
>
> diff --git a/src/xen/xen_hypervisor.h b/src/xen/xen_hypervisor.h
> index 1cf1e14..1e5bb67 100644
> --- a/src/xen/xen_hypervisor.h
> +++ b/src/xen/xen_hypervisor.h
> @@ -106,18 +106,20 @@ int xenHypervisorGetVcpuMax (virConnectPtr conn,
> virDomainDefPtr def)
> ATTRIBUTE_NONNULL (1);
>
> -char * xenHypervisorGetSchedulerType (virDomainPtr domain,
> +char * xenHypervisorGetSchedulerType (virConnectPtr conn,
> int *nparams)
>
Is it better to fix this non-standard whitespace as the code is touched,
or with one cleanup patch? This code rarely gets touched, so maybe the
latter.
> ATTRIBUTE_NONNULL (1);
>
> -int xenHypervisorGetSchedulerParameters(virDomainPtr domain,
> - virTypedParameterPtr params,
> - int *nparams)
> +int xenHypervisorGetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> + virTypedParameterPtr params,
> + int *nparams)
> ATTRIBUTE_NONNULL (1);
>
> -int xenHypervisorSetSchedulerParameters(virDomainPtr domain,
> - virTypedParameterPtr params,
> - int nparams)
> +int xenHypervisorSetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> + virTypedParameterPtr params,
> + int nparams)
> ATTRIBUTE_NONNULL (1);
>
> int xenHypervisorDomainBlockStats (virDomainPtr domain,
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 3d852d2..3bcd19b 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -2972,9 +2972,10 @@ error:
> * caller or NULL in case of failure
> */
> char *
> -xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams)
> +xenDaemonGetSchedulerType(virConnectPtr conn,
> + int *nparams)
>
Fits on one line, and comment update here and below.
ACK.
Regards,
Jim
> {
> - xenUnifiedPrivatePtr priv = domain->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
> struct sexpr *root;
> const char *ret = NULL;
> char *schedulertype = NULL;
> @@ -2986,7 +2987,7 @@ xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams)
> return NULL;
> }
>
> - root = sexpr_get(domain->conn, "/xend/node/");
> + root = sexpr_get(conn, "/xend/node/");
> if (root == NULL)
> return NULL;
>
> @@ -3037,11 +3038,12 @@ error:
> * Returns 0 or -1 in case of failure
> */
> int
> -xenDaemonGetSchedulerParameters(virDomainPtr domain,
> +xenDaemonGetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> virTypedParameterPtr params,
> int *nparams)
> {
> - xenUnifiedPrivatePtr priv = domain->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
> struct sexpr *root;
> char *sched_type = NULL;
> int sched_nparam = 0;
> @@ -3055,12 +3057,12 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain,
> }
>
> /* look up the information by domain name */
> - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name);
> + root = sexpr_get(conn, "/xend/domain/%s?detail=1", def->name);
> if (root == NULL)
> return -1;
>
> /* get the scheduler type */
> - sched_type = xenDaemonGetSchedulerType(domain, &sched_nparam);
> + sched_type = xenDaemonGetSchedulerType(conn, &sched_nparam);
> if (sched_type == NULL) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("Failed to get a scheduler name"));
> @@ -3139,11 +3141,12 @@ error:
> * Returns 0 or -1 in case of failure
> */
> int
> -xenDaemonSetSchedulerParameters(virDomainPtr domain,
> +xenDaemonSetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> virTypedParameterPtr params,
> int nparams)
> {
> - xenUnifiedPrivatePtr priv = domain->conn->privateData;
> + xenUnifiedPrivatePtr priv = conn->privateData;
> struct sexpr *root;
> char *sched_type = NULL;
> int i;
> @@ -3158,12 +3161,12 @@ xenDaemonSetSchedulerParameters(virDomainPtr domain,
> }
>
> /* look up the information by domain name */
> - root = sexpr_get(domain->conn, "/xend/domain/%s?detail=1", domain->name);
> + root = sexpr_get(conn, "/xend/domain/%s?detail=1", def->name);
> if (root == NULL)
> return -1;
>
> /* get the scheduler type */
> - sched_type = xenDaemonGetSchedulerType(domain, &sched_nparam);
> + sched_type = xenDaemonGetSchedulerType(conn, &sched_nparam);
> if (sched_type == NULL) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("Failed to get a scheduler name"));
> @@ -3217,7 +3220,7 @@ xenDaemonSetSchedulerParameters(virDomainPtr domain,
> snprintf(buf_cap, sizeof(buf_cap), "%s", cap);
> }
>
> - ret = xend_op(domain->conn, domain->name, "op",
> + ret = xend_op(conn, def->name, "op",
> "domain_sched_credit_set", "weight", buf_weight,
> "cap", buf_cap, NULL);
> break;
> diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
> index 3a7c0ac..cef7da4 100644
> --- a/src/xen/xend_internal.h
> +++ b/src/xen/xend_internal.h
> @@ -189,11 +189,14 @@ int xenDaemonDomainMigratePerform (virConnectPtr conn,
>
> int xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer);
>
> -char * xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams);
> -int xenDaemonGetSchedulerParameters(virDomainPtr domain,
> +char * xenDaemonGetSchedulerType(virConnectPtr conn,
> + int *nparams);
> +int xenDaemonGetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> virTypedParameterPtr params,
> int *nparams);
> -int xenDaemonSetSchedulerParameters(virDomainPtr domain,
> +int xenDaemonSetSchedulerParameters(virConnectPtr conn,
> + virDomainDefPtr def,
> virTypedParameterPtr params,
> int nparams);
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list