3:12 p.m.
Actually our checking for unsupported parameters is too strict about
cpumask, vcpupin and NUMA parameters and in general wrong.
First of all, we do support cpumask and consequently we don't have to prevent
domain definition in case cpumask is specified. Also we should allow vcpupin
parameters if they don't contradict with common cpumask. NUMA checking was also
wrong because def->numa is always allocated. Here we check that its content is
not specified.
Maxim Nestratov (4):
parallels: cpumask support
parallels: don't forget to unlock domain in
parallelsDomainHasManagedSaveImage
parallels: prevent domain define only if NUMA is really specified
parallels: prevent domain define only if vcpupin is specified
3:12 p.m.
New subject: [libvirt] [PATCH 1/4] parallels: cpumask support
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_driver.c | 4 +---
src/parallels/parallels_sdk.c | 31 ++++++++++++++++---------------
src/parallels/parallels_utils.h | 1 -
3 files changed, 17 insertions(+), 19 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 650b790..09d1cca 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -777,7 +777,6 @@ parallelsDomainGetVcpus(virDomainPtr domain,
int maplen)
{
parallelsConnPtr privconn = domain->conn->privateData;
- parallelsDomObjPtr privdomdata = NULL;
virDomainObjPtr privdom = NULL;
size_t i;
int v, maxcpu, hostcpus;
@@ -799,7 +798,6 @@ parallelsDomainGetVcpus(virDomainPtr domain,
goto cleanup;
}
- privdomdata = privdom->privateData;
if ((hostcpus = nodeGetCPUCount()) < 0)
goto cleanup;
@@ -820,7 +818,7 @@ parallelsDomainGetVcpus(virDomainPtr domain,
int tmpmapLen = 0;
memset(cpumaps, 0, maplen * maxinfo);
- virBitmapToData(privdomdata->cpumask, &tmpmap, &tmpmapLen);
+ virBitmapToData(privdom->def->cpumask, &tmpmap, &tmpmapLen);
if (tmpmapLen > maplen)
tmpmapLen = maplen;
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index fec145d..5a3969e 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -412,7 +412,6 @@ prlsdkDomObjFreePrivate(void *p)
return;
PrlHandle_Free(pdom->sdkdom);
- virBitmapFree(pdom->cpumask);
VIR_FREE(pdom->uuid);
VIR_FREE(pdom->home);
VIR_FREE(p);
@@ -1053,8 +1052,7 @@ prlsdkConvertDomainState(VIRTUAL_MACHINE_STATE domainState,
static int
prlsdkConvertCpuInfo(PRL_HANDLE sdkdom,
- virDomainDefPtr def,
- parallelsDomObjPtr pdom)
+ virDomainDefPtr def)
{
char *buf;
PRL_UINT32 buflen = 0;
@@ -1085,11 +1083,11 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom,
pret = PrlVmCfg_GetCpuMask(sdkdom, buf, &buflen);
if (strlen(buf) == 0) {
- if (!(pdom->cpumask = virBitmapNew(hostcpus)))
+ if (!(def->cpumask = virBitmapNew(hostcpus)))
goto cleanup;
- virBitmapSetAll(pdom->cpumask);
+ virBitmapSetAll(def->cpumask);
} else {
- if (virBitmapParse(buf, 0, &pdom->cpumask, hostcpus) < 0)
+ if (virBitmapParse(buf, 0, &def->cpumask, hostcpus) < 0)
goto cleanup;
}
@@ -1217,7 +1215,7 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
convert to Kbytes */
def->mem.cur_balloon = def->mem.max_balloon;
- if (prlsdkConvertCpuInfo(sdkdom, def, pdom) < 0)
+ if (prlsdkConvertCpuInfo(sdkdom, def) < 0)
goto error;
if (prlsdkConvertCpuMode(sdkdom, def) < 0)
@@ -1807,13 +1805,6 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr
def)
return -1;
}
- if (def->cpumask != NULL) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("changing cpu mask is not supported "
- "by parallels driver"));
- return -1;
- }
-
if (def->cputune.shares ||
def->cputune.sharesSpecified ||
def->cputune.period ||
@@ -2842,6 +2833,7 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
size_t i;
char uuidstr[VIR_UUID_STRING_BUFLEN + 2];
bool needBoot = true;
+ char *mask = NULL;
if (prlsdkCheckUnsupportedParams(sdkdom, def) < 0)
return -1;
@@ -2869,6 +2861,13 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
pret = PrlVmCfg_SetCpuCount(sdkdom, def->vcpus);
prlsdkCheckRetGoto(pret, error);
+ if (!(mask = virBitmapFormat(def->cpumask)))
+ goto error;
+
+ pret = PrlVmCfg_SetCpuMask(sdkdom, mask);
+ prlsdkCheckRetGoto(pret, error);
+ VIR_FREE(mask);
+
if (prlsdkClearDevices(sdkdom) < 0)
goto error;
@@ -2912,7 +2911,9 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
return 0;
error:
- return -1;
+ VIR_FREE(mask);
+
+ return -1;
}
int
diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
index ead5586..394548a 100644
--- a/src/parallels/parallels_utils.h
+++ b/src/parallels/parallels_utils.h
@@ -69,7 +69,6 @@ struct parallelsDomObj {
int id;
char *uuid;
char *home;
- virBitmapPtr cpumask;
PRL_HANDLE sdkdom;
};
--
1.7.1
2:54 a.m.
New subject: [libvirt] [PATCH 1/4] parallels: cpumask support
On Tue, Mar 10, 2015 at 23:12:19 +0300, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_driver.c | 4 +---
src/parallels/parallels_sdk.c | 31 ++++++++++++++++---------------
src/parallels/parallels_utils.h | 1 -
3 files changed, 17 insertions(+), 19 deletions(-)
ACK,
Peter
3:12 p.m.
New subject: [libvirt] [PATCH 2/4] parallels: don't forget to unlock domain in parallelsDomainHasManagedSaveImage
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_driver.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 09d1cca..bbe9450 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -973,6 +973,9 @@ parallelsDomainHasManagedSaveImage(virDomainPtr domain, unsigned int
flags)
return -1;
}
+ if (dom)
+ virObjectUnlock(dom);
+
return 0;
}
--
1.7.1
2:56 a.m.
New subject: [libvirt] [PATCH 2/4] parallels: don't forget to unlock domain in parallelsDomainHasManagedSaveImage
On Tue, Mar 10, 2015 at 23:12:20 +0300, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_driver.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 09d1cca..bbe9450 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -973,6 +973,9 @@ parallelsDomainHasManagedSaveImage(virDomainPtr domain, unsigned int
flags)
return -1;
}
+ if (dom)
No need to check dom here. It is guaranteed to be set at this point.
As it might set off our coverity checker I'll touch up the patch.
+ virObjectUnlock(dom);
+
return 0;
}
ACK,
Peter
3:12 p.m.
New subject: [libvirt] [PATCH 3/4] parallels: prevent domain define only if NUMA is really specified
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 5a3969e..4ec9161 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1816,7 +1816,15 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr
def)
return -1;
}
- if (def->numa) {
+
+ /*
+ * Though we don't support NUMA configuration at the moment
+ * virDomainDefPtr always contain non zero NUMA configuration
+ * So, just make sure this configuration does't differ from auto generated.
+ */
+ if ((virDomainNumatuneGetMode(def->numa, -1) !=
+ VIR_DOMAIN_NUMATUNE_MEM_STRICT) ||
+ virDomainNumatuneHasPerNodeBinding(def->numa)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("numa parameters are not supported "
"by parallels driver"));
--
1.7.1
3:06 a.m.
New subject: [libvirt] [PATCH 3/4] parallels: prevent domain define only if NUMA is really specified
On Tue, Mar 10, 2015 at 23:12:21 +0300, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 5a3969e..4ec9161 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1816,7 +1816,15 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr
def)
return -1;
}
- if (def->numa) {
ACK, and sorry for breaking this code with my changes.
Peter
3:12 p.m.
New subject: [libvirt] [PATCH 4/4] parallels: prevent domain define only if vcpupin is specified
and their settings differ from common cpumask
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 4ec9161..fde908b 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1808,14 +1808,24 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr
def)
if (def->cputune.shares ||
def->cputune.sharesSpecified ||
def->cputune.period ||
- def->cputune.quota ||
- def->cputune.nvcpupin) {
+ def->cputune.quota) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("cputune is not supported by parallels driver"));
return -1;
}
+ if (def->cputune.vcpupin) {
+ for (i = 0; i < def->vcpus; i++) {
+ if (!virBitmapEqual(def->cpumask,
+ def->cputune.vcpupin[i]->cpumask)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s", _("vcpupin cpumask differs from
default cpumask"));
+ return -1;
+ }
+ }
+ }
+
/*
* Though we don't support NUMA configuration at the moment
--
1.7.1
3:10 a.m.
New subject: [libvirt] [PATCH 4/4] parallels: prevent domain define only if vcpupin is specified
On Tue, Mar 10, 2015 at 23:12:22 +0300, Maxim Nestratov wrote:
and their settings differ from common cpumask
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 4ec9161..fde908b 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1808,14 +1808,24 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr
def)
if (def->cputune.shares ||
def->cputune.sharesSpecified ||
def->cputune.period ||
- def->cputune.quota ||
- def->cputune.nvcpupin) {
+ def->cputune.quota) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("cputune is not supported by parallels driver"));
return -1;
}
+ if (def->cputune.vcpupin) {
+ for (i = 0; i < def->vcpus; i++) {
While this is still correct as the rest of the vcpupin array is filled
by the default cpu pinning mask, the rest of the cpus can be skipped.
ACK regardless.
Peter
3522
days inactive
3523
days old
8 comments
2 participants
participants (2)
-
Maxim Nestratov -
Peter Krempa