[libvirt] [PATCH 0/3] Parallels: patch set

From: A.Burluka <aburluka@parallels.com> I've fixed my previous patches after Libvirt community reviews (thanks to Eric Blake and Daniel P. Berrange). Parallels team continues working on Libvirt Parallels driver integration with OpenStack. A.Burluka (3): Parallels: add domainGetVcpus(). Parallels: add connectBaselineCPU(). Parallels: Include CPU info in the capabilities XML

From: A.Burluka <aburluka@parallels.com> 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. --- src/parallels/parallels_driver.c | 93 ++++++++++++++++++++++++++++++++++++- src/parallels/parallels_utils.h | 1 + 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index ab59599..f6e701d 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -108,6 +108,7 @@ parallelsDomObjFreePrivate(void *p) if (!pdom) return; + virBitmapFree(pdom->cpumask); VIR_FREE(pdom->uuid); VIR_FREE(pdom->home); VIR_FREE(p); @@ -650,10 +651,14 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) unsigned int x; const char *autostart; const char *state; + int hostcpus; 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 +721,19 @@ parallelsLoadDomain(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; } + if ((hostcpus = nodeGetCPUCount()) < 0) + goto cleanup; + + if (!(tmp = virJSONValueObjectGetString(jobj3, "mask"))) { + /* Absence of this field means that all domains cpus are available */ + if (!(pdom->cpumask = virBitmapNew(hostcpus))) + goto cleanup; + virBitmapSetAll(pdom->cpumask); + } else { + if (virBitmapParse(tmp, 0, &pdom->cpumask, hostcpus) < 0) + goto cleanup; + } + if (!(jobj3 = virJSONValueObjectGet(jobj2, "memory"))) { parallelsParseError(); goto cleanup; @@ -757,9 +775,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 +2315,77 @@ static int parallelsConnectIsAlive(virConnectPtr conn ATTRIBUTE_UNUSED) } +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; + + memset(cpumaps, 0, maplen * maxinfo); + virBitmapToData(privdomdata->cpumask, &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); + } + } + ret = maxinfo; + + cleanup: + if (privdom) + virObjectUnlock(privdom); + return ret; +} + + static virDriver parallelsDriver = { .no = VIR_DRV_PARALLELS, .name = "Parallels", @@ -2323,6 +2409,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..599e2c5 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; + virBitmapPtr cpumask; }; typedef struct parallelsDomObj *parallelsDomObjPtr; -- 1.7.1

On Thu, Jun 05, 2014 at 09:50:04AM +0400, Alexander Burluka wrote:
From: A.Burluka <aburluka@parallels.com>
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. --- src/parallels/parallels_driver.c | 93 ++++++++++++++++++++++++++++++++++++- src/parallels/parallels_utils.h | 1 + 2 files changed, 91 insertions(+), 3 deletions(-)
ACK looks much nicer with this virBitmap usage Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: A.Burluka <aburluka@parallels.com> Openstack Nova (starting at Icehouse release) requires this function to start VM. --- src/parallels/parallels_driver.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index f6e701d..96d2e45 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" @@ -2315,6 +2316,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 parallelsDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, @@ -2395,6 +2408,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

On Thu, Jun 05, 2014 at 09:50:05AM +0400, Alexander Burluka wrote:
From: A.Burluka <aburluka@parallels.com>
Openstack Nova (starting at Icehouse release) requires this function to start VM. --- src/parallels/parallels_driver.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: A.Burluka <aburluka@parallels.com> Openstack uses (or will start to using) CPU info from the capabilities XML. So this section is expanded, added CPU info about arch, type and info about number of cores, sockets and threads. --- src/parallels/parallels_driver.c | 28 ++++++++++++++++++++++++++-- 1 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 96d2e45..87e540e 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -118,8 +118,11 @@ parallelsDomObjFreePrivate(void *p) static virCapsPtr parallelsBuildCapabilities(void) { - virCapsPtr caps; + virCapsPtr caps = NULL; + virCPUDefPtr cpu = NULL; + virCPUDataPtr data = NULL; virCapsGuestPtr guest; + virNodeInfo nodeinfo; if ((caps = virCapabilitiesNew(virArchFromHost(), 0, 0)) == NULL) @@ -148,11 +151,32 @@ parallelsBuildCapabilities(void) "parallels", NULL, NULL, 0, NULL) == NULL) goto error; + if (VIR_ALLOC(cpu) < 0) + goto error; + + if (nodeGetInfo(&nodeinfo)) + goto error; + + cpu->arch = caps->host.arch; + cpu->type = VIR_CPU_TYPE_HOST; + cpu->sockets = nodeinfo.sockets; + cpu->cores = nodeinfo.cores; + cpu->threads = nodeinfo.threads; + + caps->host.cpu = cpu; + + if (!(data = cpuNodeData(cpu->arch)) + || cpuDecode(cpu, data, NULL, 0, NULL) < 0) { + goto cleanup; + } + + cleanup: + cpuDataFree(data); return caps; error: virObjectUnref(caps); - return NULL; + goto cleanup; } static char * -- 1.7.1

On Thu, Jun 05, 2014 at 09:50:06AM +0400, Alexander Burluka wrote:
From: A.Burluka <aburluka@parallels.com>
Openstack uses (or will start to using) CPU info from the capabilities XML. So this section is expanded, added CPU info about arch, type and info about number of cores, sockets and threads. --- src/parallels/parallels_driver.c | 28 ++++++++++++++++++++++++++-- 1 files changed, 26 insertions(+), 2 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 06/05/2014 07:58 AM, Daniel P. Berrange wrote:
On Thu, Jun 05, 2014 at 09:50:06AM +0400, Alexander Burluka wrote:
From: A.Burluka <aburluka@parallels.com>
Openstack uses (or will start to using) CPU info from the capabilities XML. So this section is expanded, added CPU info about arch, type and info about number of cores, sockets and threads. --- src/parallels/parallels_driver.c | 28 ++++++++++++++++++++++++++-- 1 files changed, 26 insertions(+), 2 deletions(-)
ACK
I'm about to push the series, but have a minor question: Your email lists "Alexander Burluka" as your name, but your commits list "A.Burluka" as author. Am I okay to adjust your commits to go with your full name before I push? Congrats on your first libvirt patch! -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Thank you, Eric and Daniel! Yes, you can correct it, I would fix this git setting in my repo copy. On 06/05/2014 07:24 PM, Eric Blake wrote:
On 06/05/2014 07:58 AM, Daniel P. Berrange wrote:
On Thu, Jun 05, 2014 at 09:50:06AM +0400, Alexander Burluka wrote:
From: A.Burluka <aburluka@parallels.com>
Openstack uses (or will start to using) CPU info from the capabilities XML. So this section is expanded, added CPU info about arch, type and info about number of cores, sockets and threads. --- src/parallels/parallels_driver.c | 28 ++++++++++++++++++++++++++-- 1 files changed, 26 insertions(+), 2 deletions(-) ACK I'm about to push the series, but have a minor question:
Your email lists "Alexander Burluka" as your name, but your commits list "A.Burluka" as author. Am I okay to adjust your commits to go with your full name before I push?
Congrats on your first libvirt patch!

On 06/06/2014 02:53 AM, aburluka wrote:
Thank you, Eric and Daniel! Yes, you can correct it, I would fix this git setting in my repo copy.
git config --global user.name 'Alexander Burluka' should do it (or hand-modify ~/.gitconfig); you may also need to tweak sendemail.from to ensure From: lines are correct. At any rate, your patch is now pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
aburluka
-
Alexander Burluka
-
Daniel P. Berrange
-
Eric Blake