[libvirt] [PATCH 2/8] tests: Add createVHBAByStoragePool-by-parent to fchosttest

[John Ferlan]

[...]

>> 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:


    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;

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...