On Fri, Apr 10, 2015 at 16:32:33 +0200, Ján Tomko wrote:
> Use virBitmap helpers that were added after this function.
>
> Change cpumaplen to int and fill it out by this function.
> ---
> tools/virsh-domain.c | 112 +++++++++------------------------------------------
> 1 file changed, 19 insertions(+), 93 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index d5352d7..90e23aa 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -6335,95 +6335,23 @@ vshPrintPinInfo(unsigned char *cpumap, size_t cpumaplen)
> }
>
> static unsigned char *
> -vshParseCPUList(vshControl *ctl, const char *cpulist,
> - int maxcpu, size_t cpumaplen)
> +vshParseCPUList(int *cpumaplen, const char *cpulist, int maxcpu)
Hmm virBitmapToData could be refactored to take a size_t rather than
using int here
> {
> unsigned char *cpumap = NULL;
> - const char *cur;
> - bool unuse = false;
> - int cpu, lastcpu;
> - size_t i;
> -
> - cpumap = vshCalloc(ctl, cpumaplen, sizeof(*cpumap));
> -
> - /* Parse cpulist */
> - cur = cpulist;
> - if (*cur == 'r') {
> - for (cpu = 0; cpu < maxcpu; cpu++)
> - VIR_USE_CPU(cpumap, cpu);
> - return cpumap;
> - } else if (*cur == 0) {
> - goto error;
> - }
> -
> - while (*cur != 0) {
> - /* The char '^' denotes exclusive */
> - if (*cur == '^') {
> - cur++;
> - unuse = true;
> - }
> -
> - /* Parse physical CPU number */
> - if (!c_isdigit(*cur))
> - goto error;
> -
> - if ((cpu = virParseNumber(&cur)) < 0)
> - goto error;
> + virBitmapPtr map = NULL;
>
> - if (cpu >= maxcpu) {
> - vshError(ctl, _("Physical CPU %d doesn't exist."), cpu);
> - goto cleanup;
> - }
> -
> - virSkipSpaces(&cur);
> -
> - 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 error;
> - cur++;
> - virSkipSpaces(&cur);
> -
> - /* Parse the end of range */
> - lastcpu = virParseNumber(&cur);
> -
> - if (lastcpu < cpu)
> - goto error;
> -
> - if (lastcpu >= maxcpu) {
> - vshError(ctl, _("Physical CPU %d doesn't exist."), lastcpu);
> - goto cleanup;
> - }
> -
> - for (i = cpu; i <= lastcpu; i++)
> - VIR_USE_CPU(cpumap, i);
> -
> - virSkipSpaces(&cur);
> - }
> -
> - if (*cur == ',') {
> - cur++;
> - virSkipSpaces(&cur);
> - unuse = false;
> - } else if (*cur == 0) {
> - break;
> - } else {
> - goto error;
> - }
> + if (cpulist[0] == 'r') {
> + if (!(map = virBitmapNew(maxcpu)))
> + return NULL;
> + virBitmapSetAll(map);
> + } else {
> + if (virBitmapParse(cpulist, '\0', &map, maxcpu) < 0)
> + return NULL;
> }
>
> + if (virBitmapToData(map, &cpumap, cpumaplen) < 0)
> + virBitmapFree(map);
@map needs to be freed on success too.
> return cpumap;
> -
> - error:
> - vshError(ctl, "%s", _("cpulist: Invalid format."));
> - cleanup:
> - VIR_FREE(cpumap);
> - return NULL;
> }
>
> static bool
> @@ -6434,7 +6362,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> const char *cpulist = NULL;
> bool ret = false;
> unsigned char *cpumap = NULL;
> - size_t cpumaplen;
> + int cpumaplen;
And changing the types here on ...
> int maxcpu, ncpus;
> size_t i;
> bool config = vshCommandOptBool(cmd, "config");
> @@ -6473,7 +6401,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
>
> if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0)
> return false;
> - cpumaplen = VIR_CPU_MAPLEN(maxcpu);
>
> if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> return false;
> @@ -6495,6 +6422,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> + cpumaplen = VIR_CPU_MAPLEN(maxcpu);
> cpumap = vshMalloc(ctl, ncpus * cpumaplen);
> if ((ncpus = virDomainGetVcpuPinInfo(dom, ncpus, cpumap,
> cpumaplen, flags)) >= 0) {
> @@ -6514,7 +6442,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
> }
> } else {
> /* Pin mode: pinning specified vcpu to specified physical cpus*/
> - if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen)))
> + if (!(cpumap = vshParseCPUList(&cpumaplen, cpulist, maxcpu)))
> goto cleanup;
>
> if (flags == -1) {
> @@ -6580,7 +6508,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
> bool ret = false;
> unsigned char *cpumap = NULL;
> unsigned char *cpumaps = NULL;
> - size_t cpumaplen;
> + int cpumaplen;
> int maxcpu;
> bool config = vshCommandOptBool(cmd, "config");
> bool live = vshCommandOptBool(cmd, "live");
> @@ -6613,8 +6541,6 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
> return false;
> }
>
> - cpumaplen = VIR_CPU_MAPLEN(maxcpu);
> -
> /* Query mode: show CPU affinity information then exit.*/
> if (query) {
> /* When query mode and neither "live", "config" nor "current"
> @@ -6622,6 +6548,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
> if (flags == -1)
> flags = VIR_DOMAIN_AFFECT_CURRENT;
>
> + cpumaplen = VIR_CPU_MAPLEN(maxcpu);
> cpumaps = vshMalloc(ctl, cpumaplen);
> if (virDomainGetEmulatorPinInfo(dom, cpumap,
> cpumaplen, flags) >= 0) {
> @@ -6636,7 +6563,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
> }
>
> /* Pin mode: pinning emulator threads to specified physical cpus*/
> - if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen)))
> + if (!(cpumap = vshParseCPUList(&cpumaplen, cpulist, maxcpu)))
> goto cleanup;
>
> if (flags == -1)
> @@ -6912,7 +6839,7 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd)
> int maxcpu;
> bool ret = false;
> unsigned char *cpumap = NULL;
> - size_t cpumaplen;
> + int cpumaplen;
> unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
>
> VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
> @@ -6938,9 +6865,8 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd)
>
> if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0)
> goto cleanup;
> - cpumaplen = VIR_CPU_MAPLEN(maxcpu);
>
> - if (!(cpumap = vshParseCPUList(ctl, cpulist, maxcpu, cpumaplen)))
> + if (!(cpumap = vshParseCPUList(&cpumaplen, cpulist, maxcpu)))
> goto cleanup;
>
> if (virDomainPinIOThread(dom, iothread_id,
ACK with the memleak fixed. Refactoring virBitmapToData is optional.
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list