[libvirt] [PATCH 2/8] tests: Add createVHBAByStoragePool-by-parent to fchosttest
Michal Privoznik
mprivozn at redhat.com
Tue Feb 28 08:06:52 UTC 2017
On 02/27/2017 11:05 PM, John Ferlan wrote:
> [...]
>
>>> Anyway, so as an adjustment at least here... I could move the hunk below
>>> the pool->active = 1 and before the event. Then drop the lock entirely
>>> before call the testCreateVport. Would also need to add a 'isLocked' so
>>> that the unlock isn't called unnecessarily. Of course that's almost as
>>> equally as ugly.
>>>
>>> Unless you had another methodology in mind...
>>
>> What about these lines (in testNodeDeviceMockCreateVport):
>>
>> if (!(objcopy = virNodeDeviceFindByName(&driver->devs, "scsi_host11")) ||
>> !(xml = virNodeDeviceDefFormat(objcopy->def)) ||
>> !(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL)))
>> goto cleanup;
>>
>> I haven't even compile-tested them, but in general - they call the important parts of the public APIs without us needing to lock()/unlock() the driver meanwhile. There might be some additional unlock(objcopy) required, unref() or free(), but I'm willing to do that if it saves us from weird driver lock()/unlock() calls.
>>
>> If you have this ^^ patch before this one, you can just drop test driver lock()/unlock() here.
>>
>> Michal
>>
>
> The above doesn't work as cleanly as one would hope as eventtest hangs,
> but after a bit of finagling, the following works:
Ah, now I see why it hangs. The problem lies elsewhere:
testNodeDeviceCreateXML() calls public APIs thus it unlocks the driver
again. I mean, testNodeDeviceMockCreateVport() is not the only function
it calls with unlocked driver. Le sigh.
>
>
> if (!(objcpy = virNodeDeviceFindByName(&driver->devs, "scsi_host11")))
> goto cleanup;
>
> xml = virNodeDeviceDefFormat(objcpy->def);
> virNodeDeviceObjUnlock(objcpy);
> if (!xml)
> goto cleanup;
>
> if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL)))
> goto cleanup;
Yup, I've expected that we will need to unlock the object returned by
virNodeDeviceFindByName(). This looks much nicer IMO. But we still need
to fix testNodeDeviceCreateXML(). Working on the fix now.
>
> Going this route also removes the need for the existing caller to do
> unlock/lock game as well.
>
> John
>
> FWIW: The lock gets easier with RFC series and of course that's in the
> back of my mind every time I touch this code... All the fun I'll have
> merging changes...
>
Ah, which patches are that? I want to review them.
Michal
More information about the libvir-list
mailing list