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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
