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