9:01 a.m.
Hi,
Take two, with the comments from Jim on v1 fixed. Basically, that means I've
added a few comments and removed some non necessary safety checking.
Series available here:
git://xenbits.xen.org/people/dariof/libvirt.git libxl/VcpuPinX-v2
Regards,
Dario
---
Dario Faggioli (3):
libxl: implement virDomainGetVcpuPinInfo
libxl: implement virDomainPinVcpuFlags
libxl: correctly handle affinity reset in virDomainPinVcpu[Flags]
src/libxl/libxl_driver.c | 166 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 145 insertions(+), 21 deletions(-)
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
9:01 a.m.
New subject: [libvirt] [PATCH v2 1/3] libxl: implement virDomainGetVcpuPinInfo
So that it is possible to query vcpu related information of
a persistent but not running domain, like it is for the QEMU
driver.
In fact, before this patch, we have:
# virsh list --all
Id Name State
----------------------------------------------------
5 debian_32 running
- fedora20_64 shut off
# virsh vcpuinfo fedora20_64
error: this function is not supported by the connection driver: virDomainGetVcpuPinInfo
After (same situation as above, i.e., fedora20_64 not running):
# virsh vcpuinfo fedora20_64
VCPU: 0
CPU: N/A
State: N/A
CPU time N/A
CPU Affinity: yyyyyyyy
VCPU: 1
CPU: N/A
State: N/A
CPU time N/A
CPU Affinity: yyyyyyyy
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
Cc: Jim Fehlig <jfehlig(a)suse.com>
Cc: Ian Jackson <Ian.Jackson(a)eu.citrix.com>
---
src/libxl/libxl_driver.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 29aa6c7..3855509 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2417,6 +2417,83 @@ cleanup:
return ret;
}
+static int
+libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
+ unsigned char *cpumaps, int maplen,
+ unsigned int flags)
+{
+ libxlDriverPrivatePtr driver = dom->conn->privateData;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+ virDomainObjPtr vm = NULL;
+ virDomainDefPtr targetDef = NULL;
+ virDomainVcpuPinDefPtr *vcpupin_list;
+ virBitmapPtr cpumask = NULL;
+ int maxcpu, hostcpus, vcpu, pcpu, n, ret = -1;
+ unsigned char *cpumap;
+ bool pinned;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+ if (!(vm = libxlDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainGetVcpuPinInfoEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
+ &flags, &targetDef) < 0)
+ goto cleanup;
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ targetDef = vm->def;
+ }
+
+ /* Make sure coverity knows targetDef is valid at this point. */
+ sa_assert(targetDef);
+
+ /* Clamp to actual number of vcpus */
+ if (ncpumaps > targetDef->vcpus)
+ ncpumaps = targetDef->vcpus;
+
+ /* we use cfg->ctx, as vm->privateData->ctx may be NULL if VM is down. */
+ if ((hostcpus = libxl_get_max_cpus(cfg->ctx)) < 0)
+ goto cleanup;
+
+ maxcpu = maplen * 8;
+ if (maxcpu > hostcpus)
+ maxcpu = hostcpus;
+
+ /* initialize cpumaps */
+ memset(cpumaps, 0xff, maplen * ncpumaps);
+ if (maxcpu % 8) {
+ for (vcpu = 0; vcpu < ncpumaps; vcpu++) {
+ cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu);
+ cpumap[maplen - 1] &= (1 << maxcpu % 8) - 1;
+ }
+ }
+
+ /* if vcpupin setting exists, there may be unused pcpus */
+ for (n = 0; n < targetDef->cputune.nvcpupin; n++) {
+ vcpupin_list = targetDef->cputune.vcpupin;
+ vcpu = vcpupin_list[n]->vcpuid;
+ cpumask = vcpupin_list[n]->cpumask;
+ cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu);
+ for (pcpu = 0; pcpu < maxcpu; pcpu++) {
+ if (virBitmapGetBit(cpumask, pcpu, &pinned) < 0)
+ goto cleanup;
+ if (!pinned)
+ VIR_UNUSE_CPU(cpumap, pcpu);
+ }
+ }
+ ret = ncpumaps;
+
+cleanup:
+ if (vm)
+ virObjectUnlock(vm);
+ virObjectUnref(cfg);
+ return ret;
+}
static int
libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo,
@@ -4244,6 +4321,7 @@ static virDriver libxlDriver = {
.domainGetVcpusFlags = libxlDomainGetVcpusFlags, /* 0.9.0 */
.domainPinVcpu = libxlDomainPinVcpu, /* 0.9.0 */
.domainGetVcpus = libxlDomainGetVcpus, /* 0.9.0 */
+ .domainGetVcpuPinInfo = libxlDomainGetVcpuPinInfo, /* 1.2.1 */
.domainGetXMLDesc = libxlDomainGetXMLDesc, /* 0.9.0 */
.connectDomainXMLFromNative = libxlConnectDomainXMLFromNative, /* 0.9.0 */
.connectDomainXMLToNative = libxlConnectDomainXMLToNative, /* 0.9.0 */
9:01 a.m.
New subject: [libvirt] [PATCH v2 2/3] libxl: implement virDomainPinVcpuFlags
And use it to implement libxlDomainPinVcpu(), similarly to what
happens in the QEMU driver. This way, it is possible to both
query and change the vcpu affinity of a persistent but not
running domain.
In face, before this patch, we have:
# virsh list --all
Id Name State
----------------------------------------------------
5 debian_32 running
- fedora20_64 shut off
# virsh vcpupin fedora20_64 0 2-4 --current
error: this function is not supported by the connection driver: virDomainPinVcpuFlags
After (same situation as above):
# virsh vcpupin fedora20_64 0 2-4 --current
# virsh vcpupin fedora20_64 0
VCPU: CPU Affinity
----------------------------------
0: 2-4
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
Cc: Jim Fehlig <jfehlig(a)suse.com>
Cc: Ian Jackson <Ian.Jackson(a)eu.citrix.com>
---
src/libxl/libxl_driver.c | 73 ++++++++++++++++++++++++++++++++--------------
1 file changed, 51 insertions(+), 22 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3855509..e3da646 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2358,45 +2358,62 @@ cleanup:
}
static int
-libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
- int maplen)
+libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
+ unsigned char *cpumap, int maplen,
+ unsigned int flags)
{
libxlDriverPrivatePtr driver = dom->conn->privateData;
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
- libxlDomainObjPrivatePtr priv;
+ virDomainDefPtr targetDef = NULL;
virDomainObjPtr vm;
int ret = -1;
- libxl_bitmap map;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
if (!(vm = libxlDomObjFromDomain(dom)))
goto cleanup;
- if (virDomainPinVcpuEnsureACL(dom->conn, vm->def) < 0)
+ if (virDomainPinVcpuFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
+ if ((flags & VIR_DOMAIN_AFFECT_LIVE) && !virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot pin vcpus on an inactive domain"));
+ _("domain is inactive"));
goto cleanup;
}
- priv = vm->privateData;
-
- map.size = maplen;
- map.map = cpumap;
- if (libxl_set_vcpuaffinity(priv->ctx, dom->id, vcpu, &map) != 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to pin vcpu '%d' with libxenlight"),
vcpu);
+ if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm,
+ &flags, &targetDef) < 0)
goto cleanup;
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ targetDef = vm->def;
+ }
+
+ /* Make sure coverity knows targetDef is valid at this point. */
+ sa_assert(targetDef);
+
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ libxl_bitmap map = { .size = maplen, .map = cpumap };
+ libxlDomainObjPrivatePtr priv;
+
+ priv = vm->privateData;
+ if (libxl_set_vcpuaffinity(priv->ctx, dom->id, vcpu, &map) != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to pin vcpu '%d' with
libxenlight"),
+ vcpu);
+ goto cleanup;
+ }
}
- if (!vm->def->cputune.vcpupin) {
- if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0)
+ if (!targetDef->cputune.vcpupin) {
+ if (VIR_ALLOC(targetDef->cputune.vcpupin) < 0)
goto cleanup;
- vm->def->cputune.nvcpupin = 0;
+ targetDef->cputune.nvcpupin = 0;
}
- if (virDomainVcpuPinAdd(&vm->def->cputune.vcpupin,
- &vm->def->cputune.nvcpupin,
+ if (virDomainVcpuPinAdd(&targetDef->cputune.vcpupin,
+ &targetDef->cputune.nvcpupin,
cpumap,
maplen,
vcpu) < 0) {
@@ -2405,11 +2422,14 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned
char *cpumap,
goto cleanup;
}
- if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
- goto cleanup;
-
ret = 0;
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ ret = virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm);
+ } else if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ ret = virDomainSaveConfig(cfg->configDir, targetDef);
+ }
+
cleanup:
if (vm)
virObjectUnlock(vm);
@@ -2418,6 +2438,14 @@ cleanup:
}
static int
+libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap,
+ int maplen)
+{
+ return libxlDomainPinVcpuFlags(dom, vcpu, cpumap, maplen,
+ VIR_DOMAIN_AFFECT_LIVE);
+}
+
+static int
libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
unsigned char *cpumaps, int maplen,
unsigned int flags)
@@ -4320,6 +4348,7 @@ static virDriver libxlDriver = {
.domainSetVcpusFlags = libxlDomainSetVcpusFlags, /* 0.9.0 */
.domainGetVcpusFlags = libxlDomainGetVcpusFlags, /* 0.9.0 */
.domainPinVcpu = libxlDomainPinVcpu, /* 0.9.0 */
+ .domainPinVcpuFlags = libxlDomainPinVcpuFlags, /* 1.2.1 */
.domainGetVcpus = libxlDomainGetVcpus, /* 0.9.0 */
.domainGetVcpuPinInfo = libxlDomainGetVcpuPinInfo, /* 1.2.1 */
.domainGetXMLDesc = libxlDomainGetXMLDesc, /* 0.9.0 */
9:01 a.m.
New subject: [libvirt] [PATCH v2 3/3] libxl: correctly handle affinity reset in virDomainPinVcpu[Flags]
By actually removing the <vcpupin> element (from within the
<cputune> section) from the XML, rather than jus update it with
a fully set vcpu affinity mask.
Signed-off-by: Dario Faggioli <dario.faggioli(a)citrix.com>
Cc: Jim Fehlig <jfehlig(a)suse.com>
Cc: Ian Jackson <Ian.Jackson(a)eu.citrix.com>
---
src/libxl/libxl_driver.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8528106..4705870 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2365,6 +2365,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
libxlDriverPrivatePtr driver = dom->conn->privateData;
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainDefPtr targetDef = NULL;
+ virBitmapPtr pcpumap = NULL;
virDomainObjPtr vm;
int ret = -1;
@@ -2393,6 +2394,10 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
sa_assert(targetDef);
+ pcpumap = virBitmapNewData(cpumap, maplen);
+ if (!pcpumap)
+ goto cleanup;
+
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
libxl_bitmap map = { .size = maplen, .map = cpumap };
libxlDomainObjPrivatePtr priv;
@@ -2406,6 +2411,17 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
}
}
+ /* full bitmap means reset the settings (if any). */
+ if (virBitmapIsAllSet(pcpumap)) {
+ if (virDomainVcpuPinDel(targetDef, vcpu) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to delete vcpupin xml for vcpu
'%d'"),
+ vcpu);
+ goto cleanup;
+ }
+ goto out;
+ }
+
if (!targetDef->cputune.vcpupin) {
if (VIR_ALLOC(targetDef->cputune.vcpupin) < 0)
goto cleanup;
@@ -2421,6 +2437,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
goto cleanup;
}
+out:
ret = 0;
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
@@ -2432,6 +2449,7 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
cleanup:
if (vm)
virObjectUnlock(vm);
+ virBitmapFree(pcpumap);
virObjectUnref(cfg);
return ret;
}
11:49 p.m.
Dario Faggioli wrote:
Hi,
Take two, with the comments from Jim on v1 fixed. Basically, that means I've
added a few comments and removed some non necessary safety checking.
Series available here:
git://xenbits.xen.org/people/dariof/libvirt.git libxl/VcpuPinX-v2
Regards,
Dario
---
Dario Faggioli (3):
libxl: implement virDomainGetVcpuPinInfo
libxl: implement virDomainPinVcpuFlags
libxl: correctly handle affinity reset in virDomainPinVcpu[Flags]
src/libxl/libxl_driver.c | 166 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 145 insertions(+), 21 deletions(-)
Thanks for addressing my previous comments. Patch 3 needed a trivial
rebase after you added the Coverity comments in patch 2.
ACK series and now pushed. Thanks Dario!
Regards,
Jim
3958
days inactive
3962
days old
4 comments
2 participants
participants (2)
-
Dario Faggioli -
Jim Fehlig