At 06/20/2011 02:46 PM, Daniel Veillard Write:
> 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
s/sequencially/sequentially/
>> 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.
I have pushed it with this(add extra documentation in the virsh.pod):
>From bab581dce13850509b38beef0de329ef35e4c6d1 Mon Sep 17 00:00:00 2001
From: Taku Izumi <izumi.taku@xxxxxxxxxxxxxx>
Date: Fri, 10 Jun 2011 15:38:55 +0900
Subject: [PATCH] vcpupin: improve vcpupin definition of virsh vcpupin
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.
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".
Signed-off-by: Taku Izumi <izumi.taku@xxxxxxxxxxxxxx>
---
tools/virsh.c | 125 +++++++++++++++++++++++++++++++------------------------
tools/virsh.pod | 7 +++-
2 files changed, 76 insertions(+), 56 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index b7b9552..92faffc 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2998,8 +2998,9 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
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");
@@ -3055,66 +3056,74 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
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);
- 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;
- }
- 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;
- }
+ /* Parse cpulist */
+ cur = cpulist;
+ if (*cur == 0)
+ goto parse_error;
+
+ while (*cur != 0) {
+
+ /* the char '^' denotes exclusive */
+ if (*cur == '^') {
+ cur++;
+ unuse = true;
}
- }
- 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);
+ /* 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);
- do {
- unsigned int cpu = atoi(cpulist);
+ if ((*cur == ',') || (*cur == 0)) {
+ if (unuse) {
+ VIR_UNUSE_CPU(cpumap, cpu);
+ } else {
+ VIR_USE_CPU(cpumap, cpu);
+ }
+ } 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 (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) {
@@ -3126,9 +3135,15 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd)
}
}
+cleanup:
VIR_FREE(cpumap);
virDomainFree(dom);
return ret;
+
+parse_error:
+ vshError(ctl, "%s", _("cpulist: Invalid format."));
+ ret = false;
+ goto cleanup;
}
/*
diff --git a/tools/virsh.pod b/tools/virsh.pod
index fa37442..b90f37e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -833,13 +833,18 @@ vCPUs, the running time, the affinity to physical processors.
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.
Both I<--live> and I<--config> flags may be given, but I<--current> is exclusive.
If no flag is specified, behavior is different depending on hypervisor.
+B<Note>: The expression is sequentially evaluated, so "0-15,^8" is not identical
+to "^8,0-15".
+
=item B<vncdisplay> I<domain-id>
Output the IP address and port number for the VNC display. If the information
--
1.7.1
>
> Daniel
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list