On Tue, Oct 13, 2015 at 11:47:07 -0400, John Ferlan wrote:
> During qemu process startup (qemuProcessStart), the call to
> qemuProcessDetectIOThreadPIDs will not attempt to fill in the
> ->thread_id values if the binary doesn't support IOThreads.
> However, subsequent calls to setup the IOThread cgroups, affinity,
> and scheduler parameters had no such check, thus could attempt
> to set thread_id = 0, which would not be a good idea.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/qemu/qemu_cgroup.c | 3 +++
> src/qemu/qemu_process.c | 18 ++++++++++++------
> 2 files changed, 15 insertions(+), 6 deletions(-)
Do we even allow to start a VM that has IOthreads enabled but qemu does
not support them? In that case that's the point to fix so that we have a
central point where it's checked.
>
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 570dab5..66318ea 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -1163,6 +1163,9 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
> char *mem_mask = NULL;
> virDomainNumatuneMemMode mem_mode;
>
> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD))
> + return 0;
Spreading checks like this through the code usually ends up to be
fragile.
I think this should be if (def->niothreads == 0) return 0;
> +
> if ((period || quota) &&
> !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8aa9efc..67e7cbc 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2514,10 +2514,14 @@ qemuProcessSetEmulatorAffinity(virDomainObjPtr vm)
> static int
> qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
> {
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> virDomainDefPtr def = vm->def;
> size_t i;
> int ret = -1;
>
> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD))
> + return 0;
> +
> for (i = 0; i < def->niothreadids; i++) {
Here this function is doing the right thing.
> /* set affinity only for existing iothreads */
> if (!def->iothreadids[i]->cpumask)
> @@ -2574,12 +2578,14 @@ qemuProcessSetSchedulers(virDomainObjPtr vm)
> return -1;
> }
>
> - for (i = 0; i < vm->def->niothreadids; i++) {
> - if (qemuProcessSetSchedParams(vm->def->iothreadids[i]->iothread_id,
> - vm->def->iothreadids[i]->thread_id,
> - vm->def->cputune.niothreadsched,
> - vm->def->cputune.iothreadsched) < 0)
> - return -1;
> + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_IOTHREAD)) {
> + for (i = 0; i < vm->def->niothreadids; i++) {
As well as here.
> + if (qemuProcessSetSchedParams(vm->def->iothreadids[i]->iothread_id,
> + vm->def->iothreadids[i]->thread_id,
> + vm->def->cputune.niothreadsched,
> + vm->def->cputune.iothreadsched) < 0)
> + return -1;
> + }
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list