7:45 a.m.
Peter Krempa (2):
qemu: process: Move emulator thread setting code into one function
qemu: Allow setting pinning of emulator/iohtread with automatic
placement
src/qemu/qemu_cgroup.c | 66 -----------------------------------------
src/qemu/qemu_cgroup.h | 1 -
src/qemu/qemu_driver.c | 14 ---------
src/qemu/qemu_process.c | 78 +++++++++++++++++++++++++++++++++++++++----------
4 files changed, 62 insertions(+), 97 deletions(-)
--
2.6.2
7:45 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: process: Move emulator thread setting code into one function
Similarly to the refactors to iothreads and vcpus, move the code that
initializes the emulator thread settings into single function.
---
src/qemu/qemu_cgroup.c | 66 -----------------------------------------
src/qemu/qemu_cgroup.h | 1 -
src/qemu/qemu_process.c | 78 +++++++++++++++++++++++++++++++++++++++----------
3 files changed, 62 insertions(+), 83 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index f3c5fbb..a294bb0 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1062,72 +1062,6 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup,
int
-qemuSetupCgroupForEmulator(virDomainObjPtr vm)
-{
- virBitmapPtr cpumask = NULL;
- virCgroupPtr cgroup_emulator = NULL;
- virDomainDefPtr def = vm->def;
- qemuDomainObjPrivatePtr priv = vm->privateData;
- unsigned long long period = vm->def->cputune.emulator_period;
- long long quota = vm->def->cputune.emulator_quota;
-
- if ((period || quota) &&
- !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("cgroup cpu is required for scheduler tuning"));
- return -1;
- }
-
- /*
- * If CPU cgroup controller is not initialized here, then we need
- * neither period nor quota settings. And if CPUSET controller is
- * not initialized either, then there's nothing to do anyway.
- */
- if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) &&
- !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
- return 0;
-
- if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
- true, &cgroup_emulator) < 0)
- goto cleanup;
-
- if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0)
- goto cleanup;
-
- if (def->cputune.emulatorpin)
- cpumask = def->cputune.emulatorpin;
- else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
- cpumask = priv->autoCpuset;
- else if (def->cpumask)
- cpumask = def->cpumask;
-
- if (cpumask) {
- if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)
&&
- qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0)
- goto cleanup;
- }
-
- if (period || quota) {
- if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)
&&
- qemuSetupCgroupVcpuBW(cgroup_emulator, period,
- quota) < 0)
- goto cleanup;
- }
-
- virCgroupFree(&cgroup_emulator);
- return 0;
-
- cleanup:
- if (cgroup_emulator) {
- virCgroupRemove(cgroup_emulator);
- virCgroupFree(&cgroup_emulator);
- }
-
- return -1;
-}
-
-
-int
qemuRemoveCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index a8b8e1b..7a9d10a 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -54,7 +54,6 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
unsigned long long period,
long long quota);
int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask);
-int qemuSetupCgroupForEmulator(virDomainObjPtr vm);
int qemuRemoveCgroup(virDomainObjPtr vm);
#endif /* __QEMU_CGROUP_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e760182..07b7d63 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2185,22 +2185,72 @@ qemuProcessSetLinkStates(virQEMUDriverPtr driver,
}
-/* Set CPU affinities for emulator threads. */
static int
-qemuProcessSetEmulatorAffinity(virDomainObjPtr vm)
+qemuProcessSetupEmulator(virDomainObjPtr vm)
{
- virBitmapPtr cpumask;
- virDomainDefPtr def = vm->def;
+ virBitmapPtr cpumask = NULL;
+ virCgroupPtr cgroup_emulator = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ unsigned long long period = vm->def->cputune.emulator_period;
+ long long quota = vm->def->cputune.emulator_quota;
int ret = -1;
- if (def->cputune.emulatorpin)
- cpumask = def->cputune.emulatorpin;
- else if (def->cpumask)
- cpumask = def->cpumask;
+ if ((period || quota) &&
+ !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cgroup cpu is required for scheduler tuning"));
+ return -1;
+ }
+
+ if (vm->def->cputune.emulatorpin)
+ cpumask = vm->def->cputune.emulatorpin;
+ else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO
&&
+ priv->autoCpuset)
+ cpumask = priv->autoCpuset;
else
- return 0;
+ cpumask = vm->def->cpumask;
+
+ /* If CPU cgroup controller is not initialized here, then we need
+ * neither period nor quota settings. And if CPUSET controller is
+ * not initialized either, then there's nothing to do anyway. */
+ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) ||
+ virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
+
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+ true, &cgroup_emulator) < 0)
+ goto cleanup;
+
+ if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0)
+ goto cleanup;
+
+
+ if (cpumask) {
+ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)
&&
+ qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0)
+ goto cleanup;
+ }
+
+ if (period || quota) {
+ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)
&&
+ qemuSetupCgroupVcpuBW(cgroup_emulator, period,
+ quota) < 0)
+ goto cleanup;
+ }
+ }
+
+ if (cpumask &&
+ virProcessSetAffinity(vm->pid, cpumask) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ if (cgroup_emulator) {
+ if (ret < 0)
+ virCgroupRemove(cgroup_emulator);
+ virCgroupFree(&cgroup_emulator);
+ }
- ret = virProcessSetAffinity(vm->pid, cpumask);
return ret;
}
@@ -5135,12 +5185,8 @@ qemuProcessLaunch(virConnectPtr conn,
if (rv == -1) /* The VM failed to start */
goto cleanup;
- VIR_DEBUG("Setting cgroup for emulator (if required)");
- if (qemuSetupCgroupForEmulator(vm) < 0)
- goto cleanup;
-
- VIR_DEBUG("Setting affinity of emulator threads");
- if (qemuProcessSetEmulatorAffinity(vm) < 0)
+ VIR_DEBUG("Setting emulator tuning/settings");
+ if (qemuProcessSetupEmulator(vm) < 0)
goto cleanup;
VIR_DEBUG("Waiting for monitor to show up");
--
2.6.2
5:18 p.m.
New subject: [libvirt] [PATCH 1/2] qemu: process: Move emulator thread setting code into one function
On 02/24/2016 08:45 AM, Peter Krempa wrote:
Similarly to the refactors to iothreads and vcpus, move the code
that
initializes the emulator thread settings into single function.
---
src/qemu/qemu_cgroup.c | 66 -----------------------------------------
src/qemu/qemu_cgroup.h | 1 -
src/qemu/qemu_process.c | 78 +++++++++++++++++++++++++++++++++++++++----------
3 files changed, 62 insertions(+), 83 deletions(-)
This is going to conflict with Henning Schild's series:
http://www.redhat.com/archives/libvir-list/2016-February/msg01157.html
In particular:
http://www.redhat.com/archives/libvir-list/2016-February/msg01166.html
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index f3c5fbb..a294bb0 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -1062,72 +1062,6 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup,
int
-qemuSetupCgroupForEmulator(virDomainObjPtr vm)
-{
- virBitmapPtr cpumask = NULL;
- virCgroupPtr cgroup_emulator = NULL;
- virDomainDefPtr def = vm->def;
- qemuDomainObjPrivatePtr priv = vm->privateData;
- unsigned long long period = vm->def->cputune.emulator_period;
- long long quota = vm->def->cputune.emulator_quota;
-
- if ((period || quota) &&
- !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("cgroup cpu is required for scheduler tuning"));
- return -1;
- }
-
- /*
- * If CPU cgroup controller is not initialized here, then we need
- * neither period nor quota settings. And if CPUSET controller is
- * not initialized either, then there's nothing to do anyway.
- */
- if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) &&
- !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
- return 0;
-
- if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
- true, &cgroup_emulator) < 0)
- goto cleanup;
-
- if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0)
- goto cleanup;
-
- if (def->cputune.emulatorpin)
- cpumask = def->cputune.emulatorpin;
- else if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
- cpumask = priv->autoCpuset;
- else if (def->cpumask)
- cpumask = def->cpumask;
-
- if (cpumask) {
- if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)
&&
- qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0)
- goto cleanup;
- }
-
- if (period || quota) {
- if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)
&&
- qemuSetupCgroupVcpuBW(cgroup_emulator, period,
- quota) < 0)
- goto cleanup;
- }
-
- virCgroupFree(&cgroup_emulator);
- return 0;
-
- cleanup:
- if (cgroup_emulator) {
- virCgroupRemove(cgroup_emulator);
- virCgroupFree(&cgroup_emulator);
- }
-
- return -1;
-}
-
-
-int
qemuRemoveCgroup(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index a8b8e1b..7a9d10a 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -54,7 +54,6 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
unsigned long long period,
long long quota);
int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask);
-int qemuSetupCgroupForEmulator(virDomainObjPtr vm);
int qemuRemoveCgroup(virDomainObjPtr vm);
#endif /* __QEMU_CGROUP_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e760182..07b7d63 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2185,22 +2185,72 @@ qemuProcessSetLinkStates(virQEMUDriverPtr driver,
}
-/* Set CPU affinities for emulator threads. */
static int
-qemuProcessSetEmulatorAffinity(virDomainObjPtr vm)
+qemuProcessSetupEmulator(virDomainObjPtr vm)
{
- virBitmapPtr cpumask;
- virDomainDefPtr def = vm->def;
+ virBitmapPtr cpumask = NULL;
+ virCgroupPtr cgroup_emulator = NULL;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ unsigned long long period = vm->def->cputune.emulator_period;
+ long long quota = vm->def->cputune.emulator_quota;
int ret = -1;
- if (def->cputune.emulatorpin)
- cpumask = def->cputune.emulatorpin;
- else if (def->cpumask)
- cpumask = def->cpumask;
+ if ((period || quota) &&
+ !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("cgroup cpu is required for scheduler tuning"));
+ return -1;
+ }
+
+ if (vm->def->cputune.emulatorpin)
+ cpumask = vm->def->cputune.emulatorpin;
+ else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO
&&
+ priv->autoCpuset)
+ cpumask = priv->autoCpuset;
else
- return 0;
+ cpumask = vm->def->cpumask;
+
+ /* If CPU cgroup controller is not initialized here, then we need
+ * neither period nor quota settings. And if CPUSET controller is
+ * not initialized either, then there's nothing to do anyway. */
+ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) ||
+ virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
+
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+ true, &cgroup_emulator) < 0)
+ goto cleanup;
+
+ if (virCgroupMoveTask(priv->cgroup, cgroup_emulator) < 0)
+ goto cleanup;
+
+
+ if (cpumask) {
+ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)
&&
+ qemuSetupCgroupCpusetCpus(cgroup_emulator, cpumask) < 0)
+ goto cleanup;
+ }
+
+ if (period || quota) {
+ if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)
&&
+ qemuSetupCgroupVcpuBW(cgroup_emulator, period,
+ quota) < 0)
+ goto cleanup;
+ }
+ }
+
+ if (cpumask &&
+ virProcessSetAffinity(vm->pid, cpumask) < 0)
+ goto cleanup;
+
+ ret = 0;
+
+ cleanup:
+ if (cgroup_emulator) {
+ if (ret < 0)
+ virCgroupRemove(cgroup_emulator);
+ virCgroupFree(&cgroup_emulator);
+ }
- ret = virProcessSetAffinity(vm->pid, cpumask);
return ret;
}
@@ -5135,12 +5185,8 @@ qemuProcessLaunch(virConnectPtr conn,
if (rv == -1) /* The VM failed to start */
goto cleanup;
- VIR_DEBUG("Setting cgroup for emulator (if required)");
- if (qemuSetupCgroupForEmulator(vm) < 0)
- goto cleanup;
-
- VIR_DEBUG("Setting affinity of emulator threads");
- if (qemuProcessSetEmulatorAffinity(vm) < 0)
+ VIR_DEBUG("Setting emulator tuning/settings");
+ if (qemuProcessSetupEmulator(vm) < 0)
Interesting to note in the other series the SetupCgroupForEmulator was
moved to right after qemuSetupCgroup - whether that's exactly right, I'm
not sure; however, is it reasonable to move the new call to below
qemuProcessInitCpuAffinity? Before the labeling starts?
There perhaps is even more overlap between those two since the
InitCpuAffinity will virProcessSetAffinity for vm->pid just like the new
qemuProcessSetupEmulator.
Hopefully you, Dan, and Henning can work all this out...
John
goto cleanup;
VIR_DEBUG("Waiting for monitor to show up");
5:42 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: process: Move emulator thread setting code into one function
On Thu, Feb 25, 2016 at 18:18:09 -0500, John Ferlan wrote:
On 02/24/2016 08:45 AM, Peter Krempa wrote:
> Similarly to the refactors to iothreads and vcpus, move the code that
> initializes the emulator thread settings into single function.
> ---
> src/qemu/qemu_cgroup.c | 66 -----------------------------------------
> src/qemu/qemu_cgroup.h | 1 -
> src/qemu/qemu_process.c | 78 +++++++++++++++++++++++++++++++++++++++----------
> 3 files changed, 62 insertions(+), 83 deletions(-)
>
This is going to conflict with Henning Schild's series:
http://www.redhat.com/archives/libvir-list/2016-February/msg01157.html
In particular:
http://www.redhat.com/archives/libvir-list/2016-February/msg01166.html
Well, that one is trivial enough, but I'll redo this change after that
patch goes in, so that the bug gets fixed and then I'll refactor it.
Peter
6:27 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: process: Move emulator thread setting code into one function
On 02/24/2016 08:45 AM, Peter Krempa wrote:
Similarly to the refactors to iothreads and vcpus, move the code
that
initializes the emulator thread settings into single function.
---
src/qemu/qemu_cgroup.c | 66 -----------------------------------------
src/qemu/qemu_cgroup.h | 1 -
src/qemu/qemu_process.c | 78 +++++++++++++++++++++++++++++++++++++++----------
3 files changed, 62 insertions(+), 83 deletions(-)
Since it seems Henning's series is waiting for this... At the time I
wasn't sure which would go first, but wanted to be sure we had the right
adjustments. My main hangup was when the call would be made (after the
exec or in between fork/exec as was done in the other series). Since the
other patch will move the call into the fork/exec window, this is fine...
ACK
John
7:45 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Allow setting pinning of emulator/iohtread with automatic placement
We honour the placement bitmaps when starting up, so there's no point in
having this check. Additionally the check was buggy since it checked
vm->def all the time even if the user requested to modify the persistent
definition which had different configuration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1308317
---
src/qemu/qemu_driver.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 45ff3c0..ae75694 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5240,13 +5240,6 @@ qemuDomainPinEmulator(virDomainPtr dom,
if (virDomainPinEmulatorEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Changing affinity for emulator thread dynamically "
- "is not allowed when CPU placement is
'auto'"));
- goto cleanup;
- }
-
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
@@ -5714,13 +5707,6 @@ qemuDomainPinIOThread(virDomainPtr dom,
if (virDomainPinIOThreadEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Changing affinity for IOThread dynamically is "
- "not allowed when CPU placement is 'auto'"));
- goto cleanup;
- }
-
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
--
2.6.2
5:27 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: Allow setting pinning of emulator/iohtread with automatic placement
$SUBJ
"emulator/iothread"
On 02/24/2016 08:45 AM, Peter Krempa wrote:
We honour the placement bitmaps when starting up, so there's no
point in
having this check. Additionally the check was buggy since it checked
vm->def all the time even if the user requested to modify the persistent
definition which had different configuration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1308317
---
src/qemu/qemu_driver.c | 14 --------------
1 file changed, 14 deletions(-)
Seem reasonable
ACK
John
3188
days inactive
3194
days old
6 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa