[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RESEND 02/12] xl: allow for node-wise specification of vcpu pinning
On mar, 2013-11-05 at 14:50 +0000, George Dunlap wrote: > On 11/05/2013 02:34 PM, Dario Faggioli wrote: > > Making it possible to use something like the following: > > * "nodes:0-3": all pCPUs of nodes 0,1,2,3; > > * "nodes:0-3,^node:2": all pCPUS of nodes 0,1,3; > > * "1,nodes:1-2,^6": pCPU 1 plus all pCPUs of nodes 1,2 > > but not pCPU 6; > > * ... > > > > In both domain config file and `xl vcpu-pin'. > > > > Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx> > > Overall looks like a pretty clean patch; just a few comments. > Ok. > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > > > > -To allow all the vcpus of the guest to run on cpus 0,2,3,5. > > +To allow all the vcpus of the guest to run on cpus 0,2,3,5. Combining > > +this with "all" is possible, meaning "all,^7" results in all the vcpus > > +of the guest running on all the cpus on the host except cpu 7. > > + > > +=item "nodes:0-3,node:^2" > > Here you use both "nodes" and "node", while the code seems to only check > for "nodes". > Right... This was a leftover from a previous version where I was actually checking for both, as well as trying to enforce singular/plural consistency (which, I agree with you) is overkill. > I was originally going to say we should just check one; > but on the other hand, it's just an extra string compare -- I feel like > we might as well accept either "node" or "nodes". (No need to enforce > plurality: "nodes:2" and "node:1-3" should both be fine with me.) > Right, I'll add the support for both then. > > +static int update_cpumap_range(const char *str, libxl_bitmap *cpumap) > > { > > - libxl_bitmap exclude_cpumap; > > - uint32_t cpuida, cpuidb; > > - char *endptr, *toka, *tokb, *saveptr = NULL; > > - int i, rc = 0, rmcpu; > > + unsigned long ida, idb; > > + libxl_bitmap node_cpumap; > > + bool is_not = false, is_nodes = false; > > + int rc = 0; > > + > > + libxl_bitmap_init(&node_cpumap); > > > > - if (!strcmp(cpu, "all")) { > > + rc = libxl_node_bitmap_alloc(ctx, &node_cpumap, 0); > > + if (rc) { > > + fprintf(stderr, "libxl_node_bitmap_alloc failed.\n"); > > + goto out; > > + } > > + > > + /* Are we adding or removing cpus/nodes? */ > > + if (STR_SKIP_PREFIX(str, "^")) { > > + is_not = true; > > + } > > + > > + /* Are we dealing with cpus or full nodes? */ > > + if (STR_SKIP_PREFIX(str, "nodes:")) { > > + is_nodes = true; > > + } > > + > > + if (STR_HAS_PREFIX(str, "all")) { > > Is there any reason not to keep this "strcmp"? As it is, this will > accept any string that *starts* with "all", which isn't exactly what you > want, I don't think. > Ah, true... I'll revert this to strcmp(). Thanks and Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) Attachment:
signature.asc _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |