Open Stack

On 02/12/2015 01:37 PM, Daniel P. Berrange wrote:
> On Thu, Feb 12, 2015 at 01:17:55PM -0500, Sean Dague wrote:
>> On 02/12/2015 01:09 PM, Daniel P. Berrange wrote:
>>> On Thu, Feb 12, 2015 at 12:32:10PM -0500, Sean Dague wrote:
>>>> Looking recently at the following failure -
>>>> http://logs.openstack.org/04/154804/1/gate/gate-nova-python27/1fe94bf/console.html#_2015-02-12_15_02_19_593
>>>>
>>>> It appears that the fakelibvirt fixture is potentially causing races in
>>>> tests because after the first test in a worker starts a libvirt
>>>> connection, the libvirt python library spawns a thread which keeps
>>>> running in a loop for the duration of the tests. This is happening
>>>> regardless of whether or not the test in question is using libvirt (as
>>>> in this case). Having threads thumping around in the background means
>>>> that doing things like testing for when sleep is called can fail because
>>>> libvirt's thread is getting in the way.
>>>
>>> libvirt-python shouldn't be spawning any threads itself - any threads
>>> will have been spawned by Nova.
>>>
>>>>
>>>> What's the proper method of completely tearing down all the libvirt
>>>> resources so that when this fixture exits it will actually do that
>>>> correctly -
>>>> https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/fakelibvirt.py#L1181-L1202
>>>> and not impact unrelated tests?
>>>
>>> Most likely the thread will have been created when the libvirt driver
>>> is setup in the tests.
>>>
>>> eg nova.virt.libvirt.driver.LibvirtDriver.init_host() method will
>>> call nova.virt.ibvirt.host.Host.initialize() which in turn spawns
>>> a background *native* thread to receive event notifications from
>>> libvirt.
>>>
>>> Assuming this is indeed the root cause of the thread you see, I'd
>>> say we want to arrange for the nova.virt.libvirt.host.Host._init_events
>>> method to be a no-op for the tests. This async events thread is not
>>> something any of the tests should need to have around in general.
>>
>> Yeh, we just got to a similar place after mriedem's email.
>>
>> I'll propose patching that out in the fakelibvirt fixture once I get
>> some lunch, and make sure there is no other fall out from that.
>>
>> Thanks for diving in.
> 
> It is probably worth making fakelibvirt.virEvenRunDefaultImpl
> raise an exception by default too, so we clearly see if anything
> in the test suite mistakenly runs it in the future too.

Here's an initial patch - https://review.openstack.org/#/c/155729/ - I'm
not in love with all parts of it, but it is effective, and the Poisoning
ensures that nothing can touch that part of the code.

Comments welcomed.

	-Sean

-- 
Sean Dague
http://dague.net