Re: [libvirt] [PATCH 1/4] qemu: During startup need more QEMU_CAPS_OBJECT_IOTHREAD checks

On 10/13/2015 12:08 PM, Peter Krempa wrote: > 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(a)redhat.com&gt; >> --- >> 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. > Upon further reflection... During qemuBuildCommandLine there is a check for 'iothreads > 0 and have the capability', then create 'niothreadids' objects. Then during qemuCheckIOThreads called from qemuBuildDriveDevStr, we will fail if there's a disk with an "iothread='#'" property. Thus, if just defined (eg, <iothreads> and <iothreadids>), we ignore them if the emulator doesn't support it. However, if used, then yes we

fail. Naturally this is one of those only one downstream platform really cares - just happens to be one that does a lot of QA for Red Hat. Although a fairly useless configuration, I think because it seems it's possible to have 'niothreadids' with empty fields other than 'iothread_id' that checking for the capability is more correct in these instances than checking for niothreadids. Unless of course you feel we should fail in qemuBuildCommandLine if iothreads are defined.

John >> >> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c