[libvirt] parallels: add 2 functions needed by OpenStack
Hello, I would like to contribute in Libvirt development. Parallels team continues working on driver to make it compatible with OpenStack. So, this patches are part of our goal. I would gracefully accept your comments and remarks. Thank you! -- Regards, Alexander Burluka
From: A.Burluka <aburluka@parallels.com> --- 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 index ab59599..f1d5ecc 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -108,6 +108,7 @@ parallelsDomObjFreePrivate(void *p) if (!pdom) return; + VIR_FREE(pdom->cpumask); VIR_FREE(pdom->uuid); VIR_FREE(pdom->home); VIR_FREE(p); @@ -654,6 +655,9 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) if (VIR_ALLOC(def) < 0) goto cleanup; + if (VIR_ALLOC(pdom) < 0) + goto cleanup; + def->virtType = VIR_DOMAIN_VIRT_PARALLELS; def->id = -1; @@ -716,6 +720,17 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; } + if (!(tmp = virJSONValueObjectGetString(jobj3, "mask"))) { + /* Absence of this field means that all domains cpus are available */ + if (VIR_STRDUP(pdom->cpumask, "all") < 0) { + goto cleanup; + } + } else { + if (VIR_STRDUP(pdom->cpumask, tmp) < 0) { + goto cleanup; + } + } + if (!(jobj3 = virJSONValueObjectGet(jobj2, "memory"))) { parallelsParseError(); goto cleanup; @@ -757,9 +772,6 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) def->os.arch = VIR_ARCH_X86_64; - if (VIR_ALLOC(pdom) < 0) - goto cleanup; - if (virJSONValueObjectGetNumberUint(jobj, "EnvID", &x) < 0) goto cleanup; pdom->id = x; @@ -2300,6 +2312,156 @@ static int parallelsConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) } +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 }; + size_t count = 0; + + if (STREQ(it, "all")) { + if (!(*cpumask = virBitmapNew(hostcpus))) + goto cleanup; + virBitmapSetAll(*cpumask); + } else { + while (sscanf(it, "%d%n", &cpunum, &offset)) { + char delim = 0; + if (isrange) { + for (i = prevcpunum + 1; i <= cpunum; ++i) { + cpunums[count++] = i; + } + } else { + cpunums[count++] = cpunum; + } + + it += offset; + + if (sscanf(it, "%c%n", &delim, &offset) == EOF) { + break; + } else { + it += offset; + switch (delim) { + case ',': + isrange = false; + break; + case '-': + isrange = true; + prevcpunum = cpunum; + break; + default: + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid cpumask format '%s'"), + privatedomdata->cpumask); + goto cleanup; + break; + } + } + } + if (!(*cpumask = virBitmapNew(cpunums[count-1] + 1))) + goto cleanup; + virBitmapClearAll(*cpumask); + for (i = 0; i < count; ++i) { + if (virBitmapSetBit(*cpumask, cpunums[i]) == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot set %d bit in cpumask"), + cpunums[i]); + goto cleanup; + } + } + } + + return 0; + cleanup: + virBitmapFree(*cpumask); + return ret; +} + +static int +parallelsDomainGetVcpus(virDomainPtr domain, + virVcpuInfoPtr info, + int maxinfo, + unsigned char *cpumaps, + int maplen) +{ + parallelsConnPtr privconn = domain->conn->privateData; + parallelsDomObjPtr privdomdata = NULL; + virDomainObjPtr privdom = NULL; + size_t i; + int v, maxcpu, hostcpus; + int ret = -1; + + parallelsDriverLock(privconn); + privdom = virDomainObjListFindByUUID(privconn->domains, domain->uuid); + parallelsDriverUnlock(privconn); + + if (privdom == NULL) { + parallelsDomNotFoundError(domain); + goto cleanup; + } + + if (!virDomainObjIsActive(privdom)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", + _("cannot list vcpu pinning for an inactive domain")); + goto cleanup; + } + + privdomdata = privdom->privateData; + if ((hostcpus = nodeGetCPUCount()) < 0) + goto cleanup; + + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + if (maxinfo >= 1) { + if (info != NULL) { + memset(info, 0, sizeof(*info) * maxinfo); + for (i = 0; i < maxinfo; i++) { + info[i].number = i; + info[i].state = VIR_VCPU_RUNNING; + } + } + if (cpumaps != NULL) { + unsigned char *tmpmap = NULL; + int tmpmapLen = 0; + virBitmapPtr map = NULL; + + memset(cpumaps, 0, maplen * maxinfo); + if (parallelsNodeGetCpuMask(privdomdata, &map, hostcpus) == -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get cpu affinity mask ")); + goto cleanup; + } + virBitmapToData(map, &tmpmap, &tmpmapLen); + if (tmpmapLen > maplen) + tmpmapLen = maplen; + + for (v = 0; v < maxinfo; v++) { + unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); + memcpy(cpumap, tmpmap, tmpmapLen); + } + VIR_FREE(tmpmap); + virBitmapFree(map); + } + } + ret = maxinfo; + + cleanup: + if (privdom) + virObjectUnlock(privdom); + return ret; +} + + static virDriver parallelsDriver = { .no = VIR_DRV_PARALLELS, .name = "Parallels", @@ -2323,6 +2485,7 @@ static virDriver parallelsDriver = { .domainGetXMLDesc = parallelsDomainGetXMLDesc, /* 0.10.0 */ .domainIsPersistent = parallelsDomainIsPersistent, /* 0.10.0 */ .domainGetAutostart = parallelsDomainGetAutostart, /* 0.10.0 */ + .domainGetVcpus = parallelsDomainGetVcpus, /* 1.2.6 */ .domainSuspend = parallelsDomainSuspend, /* 0.10.0 */ .domainResume = parallelsDomainResume, /* 0.10.0 */ .domainDestroy = parallelsDomainDestroy, /* 0.10.0 */ diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 6215553..e88af1c 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -54,6 +54,7 @@ struct parallelsDomObj { int id; char *uuid; char *home; + char *cpumask; }; typedef struct parallelsDomObj *parallelsDomObjPtr; -- 1.7.1
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
From: A.Burluka <aburluka@parallels.com> --- src/parallels/parallels_driver.c | 34 ++++++++++++++++++++++++++++++++-- 1 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index f1d5ecc..3aa73f0 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -51,6 +51,7 @@ #include "nodeinfo.h" #include "c-ctype.h" #include "virstring.h" +#include "cpu/cpu.h" #include "parallels_driver.h" #include "parallels_utils.h" @@ -117,7 +118,9 @@ parallelsDomObjFreePrivate(void *p) static virCapsPtr parallelsBuildCapabilities(void) { - virCapsPtr caps; + virCapsPtr caps = NULL; + virCPUDefPtr cpu = NULL; + virCPUDataPtr data = NULL; virCapsGuestPtr guest; if ((caps = virCapabilitiesNew(virArchFromHost(), @@ -147,11 +150,25 @@ parallelsBuildCapabilities(void) "parallels", NULL, NULL, 0, NULL) == NULL) goto error; + if (VIR_ALLOC(cpu) < 0) + goto error; + + cpu->arch = caps->host.arch; + cpu->type = VIR_CPU_TYPE_HOST; + caps->host.cpu = cpu; + + if (!(data = cpuNodeData(cpu->arch)) + || cpuDecode(cpu, data, NULL, 0, NULL) < 0) { + goto error; + } + + cleanup: + cpuDataFree(data); return caps; error: virObjectUnref(caps); - return NULL; + goto cleanup; } static char * @@ -2312,6 +2329,18 @@ static int parallelsConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) } +static char * +parallelsConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED, + const char **xmlCPUs, + unsigned int ncpus, + unsigned int flags) +{ + virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL); + + return cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags); +} + + static int parallelsNodeGetCpuMask(parallelsDomObjPtr privatedomdata, virBitmapPtr *cpumask, @@ -2471,6 +2500,7 @@ static virDriver parallelsDriver = { .connectGetHostname = parallelsConnectGetHostname, /* 0.10.0 */ .nodeGetInfo = parallelsNodeGetInfo, /* 0.10.0 */ .connectGetCapabilities = parallelsConnectGetCapabilities, /* 0.10.0 */ + .connectBaselineCPU = parallelsConnectBaselineCPU, /* 1.2.6 */ .connectListDomains = parallelsConnectListDomains, /* 0.10.0 */ .connectNumOfDomains = parallelsConnectNumOfDomains, /* 0.10.0 */ .connectListDefinedDomains = parallelsConnectListDefinedDomains, /* 0.10.0 */ -- 1.7.1
participants (2)
-
Alexander Burluka -
Eric Blake