Re: [libvirt] [PATCH 2/4] qemu: Check QEMU_CAPS_OBJECT_IOTHREAD for qemuDomainPinIOThread
On 10/14/2015 04:38 AM, Peter Krempa wrote:
On Tue, Oct 13, 2015 at 18:33:26 -0400, John Ferlan wrote:
> On 10/13/2015 12:10 PM, Peter Krempa wrote:
>> On Tue, Oct 13, 2015 at 11:47:08 -0400, John Ferlan wrote:
...
>> Here this function should return that the iothread could not be found as
>> such VM shouldn't allow any iothreads if we don't support them.
>>
>>
> I think it's preferable to have consistent error messages. See
> qemuDomainGetIOThreadsLive and qemuDomainChgIOThread.
>
> Giving a message such as iothread could not be found is I believe
> misleading.
It is not misleading. It's true. In the configuration described you
can't have any iothreads so a message that you don't have any is spot
on.
And there are no iothreads because they are not supported in the
environment - it's the root cause. If someone looked at their XML and
saw "<iothreads>1</iothreads>" and/or "<iothreadids>
<iothread id="1"/>
</iothreadids>, they could claim they should have iothreads. Thus the
problem isn't because there are no iothreads defined, it's because
they're not supported.
Fortunately if we fix what was pointed out in patch 1 to not allow
startup if iothreads are requested, but the environment doesn't support
them, then this one won't matter anyway. Oh and IMO the right message
would then be displayed...
Looking back through history - the qemuBuildCommandLine check was put in
when there were *just* iothreads and we iothreadids weren't considered.
But still, thinking about it now - the check shouldn't have been
conjunctive. Although that may have been a conscious decision at the
time since the capability check was made when a disk requested using an
IOThread (and we couldn't dynamically add the iothread object when first
supported).
I'll make adjustments and post a v2 of the series...
John