Re: [libvirt] [PATCH 1/2] Parallels: add domainGetVcpus(). OpenStack Nova requires this function to start VM instance. Cpumask info is obtained via prlctl utility. Unlike KVM, Parallels Cloud Server is unable to set cpu affinity mask for every VCpu. Mask is unique for all VCpu. You can set it using 'prlctl set <vm_id|vm_name> --cpumask <{n[, n, n1-n2]|all}>' command. For example, 'prlctl set SomeDomain --cpumask 0, 1, 5-7' would set this mask to yy---yyy.

2 Jun 2014


      On 06/02/2014 07:40 AM, Alexander Burluka wrote:
...
From: A.Burluka <aburluka@parallels.com>
Subject line is WAAAY too long.  You missed a blank line in between "add
domainGetVcpus()." and the rest of your commit message.
...
---
 src/parallels/parallels_driver.c |  169 +++++++++++++++++++++++++++++++++++++-
 src/parallels/parallels_utils.h  |    1 +
 2 files changed, 167 insertions(+), 3 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c

...
+static int
+parallelsNodeGetCpuMask(parallelsDomObjPtr privatedomdata,
+                        virBitmapPtr *cpumask,
+                        int hostcpus)
+{
+    int ret = -1;
+    int cpunum = -1;
+    int prevcpunum = -1;
+    int offset = 0;
+    const char *it = privatedomdata->cpumask;
+    bool isrange = false;
+    size_t i;
+    int cpunums[512] = { 0 };
Why a magic number?
...
+    size_t count = 0;
+
+    if (STREQ(it, "all")) {
+        if (!(*cpumask = virBitmapNew(hostcpus)))
+            goto cleanup;
+        virBitmapSetAll(*cpumask);
+    } else {
+        while (sscanf(it, "%d%n", &cpunum, &offset)) {
sscanf(%d) is evil. It has undefined behavior on integer overflow, and
we have to assume that the input file that we are parsing could possibly
come from untrusted sources.  Please use virstring.h virStrToLong_i()
instead.
...
+                case ',':
+                    isrange = false;
+                    break;
+                case '-':
+                    isrange = true;
+                    prevcpunum = cpunum;
+                    break;
Instead of open-coding your own bitmap parser, can you see if
virBitmapParse() can do the job?  If it can't, can it at least be
refactored into a common helper function with an additional parameter,
so that you can reuse that code?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org