+ unsigned long long int ul;Is "long long" part of ANSI C? I thought it was a gcc extension. I think you should use <stdint.h> to give you integers of fixed sizes, which seems to be what you want.
+/** + * virDomainGetSchedulerType: + * @domain: pointer to domain object + * @nparams: number of scheduler parameters(return value) + * + * Get the scheduler type. + * + * Returns NULL in case of error. + */It's good that virDomainGetSchedulerType now returns an allocated string, but you need to add to the documentation to tell callers that they must free up the string.
+ switch (op.u.getschedulerid.sched_id){
+ case XEN_SCHEDULER_SEDF:
+ schedulertype = strdup("sedf");
+ *nparams = 6;
+ break;
+ case XEN_SCHEDULER_CREDIT:
+ schedulertype = strdup("credit");
+ *nparams = 2;
+ break;
+ default:
+ break;
+ }
+ }
+
+ return(schedulertype);
What happens if strdup fails?
+ if (hypervisor_version > 1) {
+ xen_op_v2_sys op_sys;
+ xen_op_v2_dom op_dom;
+ char *str_weight =strdup("weight");
+ char *str_cap =strdup("cap");
[...]
+ strncpy(params[0].field,str_weight,strlen(str_weight));
In this code, it wasn't really clear to me why you are 'strdup'-ing
these fields. It seems like a memory leak?
+ case XEN_SCHEDULER_SEDF:
+ /* TODO: Implement for Xen/SEDF */
+ break;
This should return an error (or be implemented!)
+ for(i = 0;i < *nparams; i++ ){
+ if(!strncmp(params[i].field,str_weight,strlen(str_weight)))
+ op_dom.u.getschedinfo.u.credit.weight =
+ params[i].value.ui;
+ if(!strncmp(params[i].field,str_cap,strlen(str_cap)))
+ op_dom.u.getschedinfo.u.credit.cap =
+ params[i].value.ui;
+ }
It's a good idea to check that the .type field is set to
VIR_DOMAIN_SCHED_FIELD_UINT.
+static char *
+xenUnifiedDomainGetSchedulerType (virDomainPtr dom, int *nparams)
+{
+ int i;
+ char *schedulertype = NULL;
+ for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; i++) {
+ if (drivers[i]->domainGetSchedulerType) {
+ schedulertype = drivers[i]->domainGetSchedulerType (dom,
nparams);
+ if (schedulertype != NULL) + return(schedulertype); + } + } + return(schedulertype); +}There's a couple of things wrong with this (and the other functions in xen_unified.c):
(1) You need to check priv->opened[i]. See how the other calls are done.(2) Since only the xen_internal driver implements the calls, you might consider just calling that driver directly instead of having a loop (but still check priv->opened[hypervisor_offset]).
I'll let others comment on the more general aspects of this patch. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature