On Fri, Jun 10, 2011 at 03:38:55PM +0900, Taku Izumi wrote:
>
> When using vcpupin command, we have to speficy comma-separated list as cpulist,
> but this is tedious in case the number of phsycal cpus is large.
> This patch improves this by introducing special markup "-" and "^" which are
> similar to XML schema of "cpuset" attribute.
>
> That is:
The example
> # virsh vcpupin Guest 0 0-15,^8
>
> is identical to
>
> # virsh vcpupin Guest 0 0,1,2,3,4,5,6,7,9,10,11,12,13,14,15
>
> NOTE: The expression is sequencially evaluated, so "0-15,^8" is not identical
> to "^8,0-15".
and this limitation should also be added to the virsh command doc I think
> Signed-off-by: Taku Izumi <izumi.taku@xxxxxxxxxxxxxx>
> ---
> tools/virsh.c | 125 +++++++++++++++++++++++++++++++-------------------------
> tools/virsh.pod | 4 +
> 2 files changed, 73 insertions(+), 56 deletions(-)
>
> Index: libvirt/tools/virsh.c
> ===================================================================
> --- libvirt.orig/tools/virsh.c
> +++ libvirt/tools/virsh.c
> @@ -2946,8 +2946,9 @@ cmdVcpupin(vshControl *ctl, const vshCmd
> bool ret = true;
> unsigned char *cpumap;
> int cpumaplen;
> - int i;
> - enum { expect_num, expect_num_or_comma } state;
> + int i, cpu, lastcpu, maxcpu;
> + bool unuse = false;
> + char *cur;
> int config = vshCommandOptBool(cmd, "config");
> int live = vshCommandOptBool(cmd, "live");
> int current = vshCommandOptBool(cmd, "current");
> @@ -3003,66 +3004,74 @@ cmdVcpupin(vshControl *ctl, const vshCmd
> return false;
> }
>
> - /* Check that the cpulist parameter is a comma-separated list of
> - * numbers and give an intelligent error message if not.
> - */
> - if (cpulist[0] == '\0') {
> - vshError(ctl, "%s", _("cpulist: Invalid format. Empty string."));
> - virDomainFree (dom);
> - return false;
> - }
> + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo);
> + cpumaplen = VIR_CPU_MAPLEN(maxcpu);
> + cpumap = vshCalloc(ctl, 0, cpumaplen);
> +
> + /* Parse cpulist */
> + cur = cpulist;
> + if (*cur == 0)
> + goto parse_error;
> +
> + while (*cur != 0) {
> +
> + /* the char '^' denotes exclusive */
> + if (*cur == '^') {
> + cur++;
> + unuse = true;
> + }
> +
> + /* parse physical CPU number */
> + if (!c_isdigit(*cur))
> + goto parse_error;
> + cpu = virParseNumber(&cur);
> + if (cpu < 0) {
> + goto parse_error;
> + }
> + if (cpu >= maxcpu) {
> + vshError(ctl, _("Physical CPU %d doesn't exist."), cpu);
> + goto parse_error;
> + }
> + virSkipSpaces(&cur);
>
> - state = expect_num;
> - for (i = 0; cpulist[i]; i++) {
> - switch (state) {
> - case expect_num:
> - if (!c_isdigit (cpulist[i])) {
> - vshError(ctl, _("cpulist: %s: Invalid format. Expecting "
> - "digit at position %d (near '%c')."),
> - cpulist, i, cpulist[i]);
> - virDomainFree (dom);
> - return false;
> + if ((*cur == ',') || (*cur == 0)) {
> + if (unuse) {
> + VIR_UNUSE_CPU(cpumap, cpu);
> + } else {
> + VIR_USE_CPU(cpumap, cpu);
> }
> - state = expect_num_or_comma;
> - break;
> - case expect_num_or_comma:
> - if (cpulist[i] == ',')
> - state = expect_num;
> - else if (!c_isdigit (cpulist[i])) {
> - vshError(ctl, _("cpulist: %s: Invalid format. Expecting "
> - "digit or comma at position %d (near '%c')."),
> - cpulist, i, cpulist[i]);
> - virDomainFree (dom);
> - return false;
> + } else if (*cur == '-') {
> + /* the char '-' denotes range */
> + if (unuse) {
> + goto parse_error;
> + }
> + cur++;
> + virSkipSpaces(&cur);
> + /* parse the end of range */
> + lastcpu = virParseNumber(&cur);
> + if (lastcpu < cpu) {
> + goto parse_error;
> + }
> + if (lastcpu >= maxcpu) {
> + vshError(ctl, _("Physical CPU %d doesn't exist."), maxcpu);
> + goto parse_error;
> + }
> + for (i = cpu; i <= lastcpu; i++) {
> + VIR_USE_CPU(cpumap, i);
> }
> + virSkipSpaces(&cur);
> }
> - }
> - if (state == expect_num) {
> - vshError(ctl, _("cpulist: %s: Invalid format. Trailing comma "
> - "at position %d."),
> - cpulist, i);
> - virDomainFree (dom);
> - return false;
> - }
>
> - cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo));
> - cpumap = vshCalloc(ctl, 1, cpumaplen);
> -
> - do {
> - unsigned int cpu = atoi(cpulist);
> -
> - if (cpu < VIR_NODEINFO_MAXCPUS(nodeinfo)) {
> - VIR_USE_CPU(cpumap, cpu);
> + if (*cur == ',') {
> + cur++;
> + virSkipSpaces(&cur);
> + unuse = false;
> + } else if (*cur == 0) {
> + break;
> } else {
> - vshError(ctl, _("Physical CPU %d doesn't exist."), cpu);
> - VIR_FREE(cpumap);
> - virDomainFree(dom);
> - return false;
> + goto parse_error;
> }
> - cpulist = strchr(cpulist, ',');
> - if (cpulist)
> - cpulist++;
> - } while (cpulist);
> + }
>
> if (flags == -1) {
> if (virDomainPinVcpu(dom, vcpu, cpumap, cpumaplen) != 0) {
> @@ -3074,9 +3083,15 @@ cmdVcpupin(vshControl *ctl, const vshCmd
> }
> }
>
> +cleanup:
> VIR_FREE(cpumap);
> virDomainFree(dom);
> return ret;
> +
> +parse_error:
> + vshError(ctl, "%s", _("cpulist: Invalid format."));
> + ret = false;
> + goto cleanup;
> }
>
> /*
> Index: libvirt/tools/virsh.pod
> ===================================================================
> --- libvirt.orig/tools/virsh.pod
> +++ libvirt/tools/virsh.pod
> @@ -794,7 +794,9 @@ vCPUs, the running time, the affinity to
> I<--current>
>
> Pin domain VCPUs to host physical CPUs. The I<vcpu> number must be provided
> -and I<cpulist> is a comma separated list of physical CPU numbers.
> +and I<cpulist> is a list of physical CPU numbers. Its syntax is a comma
> +separated list and a special markup using '-' and '^' (ex. '0-4', '0-3,^2') can
> +also be allowed. The '-' denotes the range and the '^' denotes exclusive.
> If I<--live> is specified, affect a running guest.
> If I<--config> is specified, affect the next boot of a persistent guest.
> If I<--current> is specified, affect the current guest state.
Okay, ACK, the point about having to generate the long comma list is
right and this seems like the right solution.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list