From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH 5 of 5 V2] libxl: expose a single get/setter for domain scheduler parameters Date: Thu, 31 May 2012 14:51:08 +0100 Message-ID: <4FC7774C.1010801@eu.citrix.com> References: <9094cf509df0e36d8c59.1338299823@cosworth.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <9094cf509df0e36d8c59.1338299823@cosworth.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Juergen Gross , Ian Jackson , Dario Faggioli , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 29/05/12 14:57, Ian Campbell wrote: > # HG changeset patch > # User Ian Campbell > # Date 1338299813 -3600 > # Node ID 9094cf509df0e36d8c59ed131937b1ebc07cc2ad > # Parent d89b5eeb94519fdc056f91663676cf012c40b654 > libxl: expose a single get/setter for domain scheduler parameters. > > This is consistent with having a single struct for all parameters. > > Effectively renames and exports libxl__sched_set_params as > libxl_domain_sched_params_set. libxl_domain_sched_params_get is new. > > Improve const correctness of the setters while I'm here. > > Use shorter LOG macros when touching a line anyway. > > Signed-off-by: Ian Campbell Acked-by: George Dunlap BTW, we obviously need to have "xl sched-credit" for xm compatibility; but do you think it makes sense to make a generic xl command that's a analog to the generic libxl function, that looks up the scheduler for you on get, and accepts all the parameters on the command-line? (Doesn't need to be part of this series, obviously.) > > diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Tue May 29 14:56:53 2012 +0100 > +++ b/tools/libxl/libxl.c Tue May 29 14:56:53 2012 +0100 > @@ -3196,15 +3196,15 @@ libxl_scheduler libxl_get_scheduler(libx > return sched; > } > > -int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo) > +static int sched_credit_domain_get(libxl__gc *gc, uint32_t domid, > + libxl_domain_sched_params *scinfo) > { > struct xen_domctl_sched_credit sdom; > int rc; > > - rc = xc_sched_credit_domain_get(ctx->xch, domid,&sdom); > + rc = xc_sched_credit_domain_get(CTX->xch, domid,&sdom); > if (rc != 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit"); > + LOGE(ERROR, "getting domain sched credit"); > return ERROR_FAIL; > } > > @@ -3216,32 +3216,31 @@ int libxl_sched_credit_domain_get(libxl_ > return 0; > } > > -int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo) > +static int sched_credit_domain_set(libxl__gc *gc, uint32_t domid, > + const libxl_domain_sched_params *scinfo) > { > struct xen_domctl_sched_credit sdom; > xc_domaininfo_t domaininfo; > int rc; > > - rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo); > + rc = xc_domain_getinfolist(CTX->xch, domid, 1,&domaininfo); > if (rc< 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list"); > + LOGE(ERROR, "getting domain info list"); > return ERROR_FAIL; > } > if (rc != 1 || domaininfo.domain != domid) > return ERROR_INVAL; > > - rc = xc_sched_credit_domain_get(ctx->xch, domid,&sdom); > + rc = xc_sched_credit_domain_get(CTX->xch, domid,&sdom); > if (rc != 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched credit"); > + LOGE(ERROR, "getting domain sched credit"); > return ERROR_FAIL; > } > > if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) { > if (scinfo->weight< 1 || scinfo->weight> 65535) { > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, > - "Cpu weight out of range, " > - "valid values are within range from 1 to 65535"); > + LOG(ERROR, "Cpu weight out of range, " > + "valid values are within range from 1 to 65535"); > return ERROR_INVAL; > } > sdom.weight = scinfo->weight; > @@ -3250,18 +3249,17 @@ int libxl_sched_credit_domain_set(libxl_ > if (scinfo->cap != LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT) { > if (scinfo->cap< 0 > || scinfo->cap> (domaininfo.max_vcpu_id + 1) * 100) { > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, > - "Cpu cap out of range, " > + LOG(ERROR, "Cpu cap out of range, " > "valid range is from 0 to %d for specified number of vcpus", > - ((domaininfo.max_vcpu_id + 1) * 100)); > + ((domaininfo.max_vcpu_id + 1) * 100)); > return ERROR_INVAL; > } > sdom.cap = scinfo->cap; > } > > - rc = xc_sched_credit_domain_set(ctx->xch, domid,&sdom); > + rc = xc_sched_credit_domain_set(CTX->xch, domid,&sdom); > if ( rc< 0 ) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched credit"); > + LOGE(ERROR, "setting domain sched credit"); > return ERROR_FAIL; > } > > @@ -3329,16 +3327,15 @@ int libxl_sched_credit_params_set(libxl_ > return 0; > } > > -int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo) > +static int sched_credit2_domain_get(libxl__gc *gc, uint32_t domid, > + libxl_domain_sched_params *scinfo) > { > struct xen_domctl_sched_credit2 sdom; > int rc; > > - rc = xc_sched_credit2_domain_get(ctx->xch, domid,&sdom); > + rc = xc_sched_credit2_domain_get(CTX->xch, domid,&sdom); > if (rc != 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > - "getting domain sched credit2"); > + LOGE(ERROR, "getting domain sched credit2"); > return ERROR_FAIL; > } > > @@ -3349,42 +3346,38 @@ int libxl_sched_credit2_domain_get(libxl > return 0; > } > > -int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo) > +static int sched_credit2_domain_set(libxl__gc *gc, uint32_t domid, > + const libxl_domain_sched_params *scinfo) > { > struct xen_domctl_sched_credit2 sdom; > int rc; > > - rc = xc_sched_credit2_domain_get(ctx->xch, domid,&sdom); > + rc = xc_sched_credit2_domain_get(CTX->xch, domid,&sdom); > if (rc != 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > - "getting domain sched credit2"); > + LOGE(ERROR, "getting domain sched credit2"); > return ERROR_FAIL; > } > > if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) { > if (scinfo->weight< 1 || scinfo->weight> 65535) { > - LIBXL__LOG(ctx, LIBXL__LOG_ERROR, > - "Cpu weight out of range, " > - "valid values are within range from " > - "1 to 65535"); > + LOG(ERROR, "Cpu weight out of range, " > + "valid values are within range from 1 to 65535"); > return ERROR_INVAL; > } > sdom.weight = scinfo->weight; > } > > - rc = xc_sched_credit2_domain_set(ctx->xch, domid,&sdom); > + rc = xc_sched_credit2_domain_set(CTX->xch, domid,&sdom); > if ( rc< 0 ) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > - "setting domain sched credit2"); > + LOGE(ERROR, "setting domain sched credit2"); > return ERROR_FAIL; > } > > return 0; > } > > -int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo) > +static int sched_sedf_domain_get(libxl__gc *gc, uint32_t domid, > + libxl_domain_sched_params *scinfo) > { > uint64_t period; > uint64_t slice; > @@ -3393,10 +3386,10 @@ int libxl_sched_sedf_domain_get(libxl_ct > uint16_t weight; > int rc; > > - rc = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency, > + rc = xc_sedf_domain_get(CTX->xch, domid,&period,&slice,&latency, > &extratime,&weight); > if (rc != 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf"); > + LOGE(ERROR, "getting domain sched sedf"); > return ERROR_FAIL; > } > > @@ -3411,8 +3404,8 @@ int libxl_sched_sedf_domain_get(libxl_ct > return 0; > } > > -int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo) > +static int sched_sedf_domain_set(libxl__gc *gc, uint32_t domid, > + const libxl_domain_sched_params *scinfo) > { > uint64_t period; > uint64_t slice; > @@ -3422,10 +3415,10 @@ int libxl_sched_sedf_domain_set(libxl_ct > > int ret; > > - ret = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency, > + ret = xc_sedf_domain_get(CTX->xch, domid,&period,&slice,&latency, > &extratime,&weight); > if (ret != 0) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf"); > + LOGE(ERROR, "getting domain sched sedf"); > return ERROR_FAIL; > } > > @@ -3440,20 +3433,21 @@ int libxl_sched_sedf_domain_set(libxl_ct > if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT) > weight = scinfo->weight; > > - ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency, > + ret = xc_sedf_domain_set(CTX->xch, domid, period, slice, latency, > extratime, weight); > if ( ret< 0 ) { > - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf"); > + LOGE(ERROR, "setting domain sched sedf"); > return ERROR_FAIL; > } > > return 0; > } > > -int libxl__sched_set_params(libxl__gc *gc, uint32_t domid, > - libxl_domain_sched_params *scparams) > +int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid, > + const libxl_domain_sched_params *scinfo) > { > - libxl_scheduler sched = scparams->sched; > + GC_INIT(ctx); > + libxl_scheduler sched = scinfo->sched; > int ret; > > if (sched == LIBXL_SCHEDULER_UNKNOWN) > @@ -3461,19 +3455,51 @@ int libxl__sched_set_params(libxl__gc *g > > switch (sched) { > case LIBXL_SCHEDULER_SEDF: > - ret=libxl_sched_sedf_domain_set(CTX, domid, scparams); > + ret=sched_sedf_domain_set(gc, domid, scinfo); > break; > case LIBXL_SCHEDULER_CREDIT: > - ret=libxl_sched_credit_domain_set(CTX, domid, scparams); > + ret=sched_credit_domain_set(gc, domid, scinfo); > break; > case LIBXL_SCHEDULER_CREDIT2: > - ret=libxl_sched_credit2_domain_set(CTX, domid, scparams); > + ret=sched_credit2_domain_set(gc, domid, scinfo); > break; > default: > LOG(ERROR, "Unknown scheduler"); > ret=ERROR_INVAL; > break; > } > + > + GC_FREE; > + return ret; > +} > + > +int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid, > + libxl_domain_sched_params *scinfo) > +{ > + GC_INIT(ctx); > + int ret; > + > + libxl_domain_sched_params_init(scinfo); > + > + scinfo->sched = libxl__domain_scheduler(gc, domid); > + > + switch (scinfo->sched) { > + case LIBXL_SCHEDULER_SEDF: > + ret=sched_sedf_domain_get(gc, domid, scinfo); > + break; > + case LIBXL_SCHEDULER_CREDIT: > + ret=sched_credit_domain_get(gc, domid, scinfo); > + break; > + case LIBXL_SCHEDULER_CREDIT2: > + ret=sched_credit2_domain_get(gc, domid, scinfo); > + break; > + default: > + LOG(ERROR, "Unknown scheduler"); > + ret=ERROR_INVAL; > + break; > + } > + > + GC_FREE; > return ret; > } > > diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Tue May 29 14:56:53 2012 +0100 > +++ b/tools/libxl/libxl.h Tue May 29 14:56:53 2012 +0100 > @@ -790,18 +790,11 @@ int libxl_sched_credit_params_set(libxl_ > #define LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT -1 > #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1 > > -int libxl_sched_credit_domain_get(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo); > -int libxl_sched_credit_domain_set(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo); > -int libxl_sched_credit2_domain_get(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo); > -int libxl_sched_credit2_domain_set(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo); > -int libxl_sched_sedf_domain_get(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo); > -int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid, > - libxl_domain_sched_params *scinfo); > +int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid, > + libxl_domain_sched_params *params); > +int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid, > + const libxl_domain_sched_params *params); > + > int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid, > libxl_trigger trigger, uint32_t vcpuid); > int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq); > diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/libxl_dom.c > --- a/tools/libxl/libxl_dom.c Tue May 29 14:56:53 2012 +0100 > +++ b/tools/libxl/libxl_dom.c Tue May 29 14:56:53 2012 +0100 > @@ -174,7 +174,7 @@ int libxl__build_post(libxl__gc *gc, uin > char **ents, **hvm_ents; > int i; > > - libxl__sched_set_params (gc, domid,&(info->sched_params)); > + libxl_domain_sched_params_set(CTX, domid,&info->sched_params); > > libxl_cpuid_apply_policy(ctx, domid); > if (info->cpuid != NULL) > diff -r d89b5eeb9451 -r 9094cf509df0 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Tue May 29 14:56:53 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Tue May 29 14:56:53 2012 +0100 > @@ -4361,24 +4361,33 @@ int main_sharing(int argc, char **argv) > return 0; > } > > -static int sched_credit_domain_get(int domid, libxl_domain_sched_params *scinfo) > +static int sched_domain_get(libxl_scheduler sched, int domid, > + libxl_domain_sched_params *scinfo) > { > int rc; > > - rc = libxl_sched_credit_domain_get(ctx, domid, scinfo); > - if (rc) > - fprintf(stderr, "libxl_sched_credit_domain_get failed.\n"); > - > - return rc; > -} > - > -static int sched_credit_domain_set(int domid, libxl_domain_sched_params *scinfo) > + rc = libxl_domain_sched_params_get(ctx, domid, scinfo); > + if (rc) { > + fprintf(stderr, "libxl_domain_sched_params_get failed.\n"); > + return rc; > + } > + if (scinfo->sched != sched) { > + fprintf(stderr, "libxl_domain_sched_params_get returned %s not %s.\n", > + libxl_scheduler_to_string(scinfo->sched), > + libxl_scheduler_to_string(sched)); > + return ERROR_INVAL; > + } > + > + return 0; > +} > + > +static int sched_domain_set(int domid, const libxl_domain_sched_params *scinfo) > { > int rc; > > - rc = libxl_sched_credit_domain_set(ctx, domid, scinfo); > + rc = libxl_domain_sched_params_set(ctx, domid, scinfo); > if (rc) > - fprintf(stderr, "libxl_sched_credit_domain_set failed.\n"); > + fprintf(stderr, "libxl_domain_sched_params_set failed.\n"); > > return rc; > } > @@ -4415,7 +4424,7 @@ static int sched_credit_domain_output(in > printf("%-33s %4s %6s %4s\n", "Name", "ID", "Weight", "Cap"); > return 0; > } > - rc = sched_credit_domain_get(domid,&scinfo); > + rc = sched_domain_get(LIBXL_SCHEDULER_CREDIT, domid,&scinfo); > if (rc) > return rc; > domname = libxl_domid_to_name(ctx, domid); > @@ -4450,30 +4459,6 @@ static int sched_credit_pool_output(uint > return 0; > } > > -static int sched_credit2_domain_get( > - int domid, libxl_domain_sched_params *scinfo) > -{ > - int rc; > - > - rc = libxl_sched_credit2_domain_get(ctx, domid, scinfo); > - if (rc) > - fprintf(stderr, "libxl_sched_credit2_domain_get failed.\n"); > - > - return rc; > -} > - > -static int sched_credit2_domain_set( > - int domid, libxl_domain_sched_params *scinfo) > -{ > - int rc; > - > - rc = libxl_sched_credit2_domain_set(ctx, domid, scinfo); > - if (rc) > - fprintf(stderr, "libxl_sched_credit2_domain_set failed.\n"); > - > - return rc; > -} > - > static int sched_credit2_domain_output( > int domid) > { > @@ -4485,7 +4470,7 @@ static int sched_credit2_domain_output( > printf("%-33s %4s %6s\n", "Name", "ID", "Weight"); > return 0; > } > - rc = sched_credit2_domain_get(domid,&scinfo); > + rc = sched_domain_get(LIBXL_SCHEDULER_CREDIT2, domid,&scinfo); > if (rc) > return rc; > domname = libxl_domid_to_name(ctx, domid); > @@ -4498,29 +4483,6 @@ static int sched_credit2_domain_output( > return 0; > } > > -static int sched_sedf_domain_get( > - int domid, libxl_domain_sched_params *scinfo) > -{ > - int rc; > - > - rc = libxl_sched_sedf_domain_get(ctx, domid, scinfo); > - if (rc) > - fprintf(stderr, "libxl_sched_sedf_domain_get failed.\n"); > - > - return rc; > -} > - > -static int sched_sedf_domain_set( > - int domid, libxl_domain_sched_params *scinfo) > -{ > - int rc; > - > - rc = libxl_sched_sedf_domain_set(ctx, domid, scinfo); > - if (rc) > - fprintf(stderr, "libxl_sched_sedf_domain_set failed.\n"); > - return rc; > -} > - > static int sched_sedf_domain_output( > int domid) > { > @@ -4533,7 +4495,7 @@ static int sched_sedf_domain_output( > "Slice", "Latency", "Extra", "Weight"); > return 0; > } > - rc = sched_sedf_domain_get(domid,&scinfo); > + rc = sched_domain_get(LIBXL_SCHEDULER_SEDF, domid,&scinfo); > if (rc) > return rc; > domname = libxl_domid_to_name(ctx, domid); > @@ -4744,7 +4706,7 @@ int main_sched_credit(int argc, char **a > scinfo.weight = weight; > if (opt_c) > scinfo.cap = cap; > - rc = sched_credit_domain_set(domid,&scinfo); > + rc = sched_domain_set(domid,&scinfo); > libxl_domain_sched_params_dispose(&scinfo); > if (rc) > return -rc; > @@ -4819,7 +4781,7 @@ int main_sched_credit2(int argc, char ** > scinfo.sched = LIBXL_SCHEDULER_CREDIT2; > if (opt_w) > scinfo.weight = weight; > - rc = sched_credit2_domain_set(domid,&scinfo); > + rc = sched_domain_set(domid,&scinfo); > libxl_domain_sched_params_dispose(&scinfo); > if (rc) > return -rc; > @@ -4939,7 +4901,7 @@ int main_sched_sedf(int argc, char **arg > scinfo.period = 0; > scinfo.slice = 0; > } > - rc = sched_sedf_domain_set(domid,&scinfo); > + rc = sched_domain_set(domid,&scinfo); > libxl_domain_sched_params_dispose(&scinfo); > if (rc) > return -rc;