[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 3/5] xl: allow for node-wise specification of vcpu pinning
Dario Faggioli writes ("[PATCH v2 3/5] xl: allow for node-wise specification of vcpu pinning"): > 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; > * ... ... > * code rearranged in order to look more simple to follow > and understand, as requested during review; This is much better now. Thank you! > +static int parse_range(const char *str, unsigned long *a, unsigned long *b) > { ... > + if (endptr == str) > + return EINVAL; > + if (*a == ULONG_MAX) > + return ERANGE; So parse_range returns errno value or 0. This isn't mentioned anywhere and is a bit unusual. > +static int update_cpumap_range(const char *str, libxl_bitmap *cpumap) > +{ ... > + rc = libxl_node_bitmap_alloc(ctx, &node_cpumap, 0); > + if (rc) { > + fprintf(stderr, "libxl_node_bitmap_alloc failed.\n"); > + return rc; So update_cpumap_range returns a libxl error code. > + rc = parse_range(str, &ida, &idb); > + if (rc) { But here you assign the errno value to an `rc' variable which holds a libxl error code. I think it would be better to make parse_range return a libxl rc value. I think also that parse_range doesn't notice if the specified range contains junk between the second number and the comma ? > +static int vcpupin_parse(char *cpu, libxl_bitmap *cpumap) > +{ ... > + if (STR_HAS_PREFIX(ptr, "all") || > + STR_HAS_PREFIX(ptr, "nodes:all")) { > + libxl_bitmap_set_any(cpumap); Why not deal with all in parse_range ? You'd avoid the second special-case of "nodes:", and constructions like all,^3 would work. > -vcpp_out: > - libxl_bitmap_dispose(&exclude_cpumap); > + rc = update_cpumap_range(ptr, cpumap); > + if (rc) { > + /* If failing, reset the cpumap and exit */ > + libxl_bitmap_set_none(cpumap); Surely the caller who gets a error should expect the cpumap to contain arbitrary contents ? Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |