[libvirt] [PATCH v4 12/14] nodedev: Convert virNodeDeviceObj to use virObjectLockable

[John Ferlan]

On 07/11/2017 10:38 AM, Erik Skultety wrote:
> On Mon, Jul 03, 2017 at 05:25:28PM -0400, John Ferlan wrote:
>> Now that we have a bit more control, let's convert our object into
>> a lockable object and let that magic handle the create and lock/unlock.
>>
>> This also involves creating a virNodeDeviceEndAPI in order to handle
>> the object cleaup for API's that use the Add or Find API's in order
> 
> s/cleaup/cleanup
> 

eagle eyes

> 
> [...]
>>
>>   cleanup:
>> -    virNodeDeviceObjUnlock(obj);
>> +    virNodeDeviceObjEndAPI(&obj);
>>      if (ret == -1) {
>>          --ncaps;
>>          while (--ncaps >= 0)
>> @@ -613,8 +613,7 @@ nodeDeviceDestroy(virNodeDevicePtr device)
>>       * to be taken, so grab the object def which will have the various
>>       * fields used to search (name, parent, parent_wwnn, parent_wwpn,
>>       * or parent_fabric_wwn) and drop the object lock. */
> 
> ^This commentary got me thinking. There's a deadlock because of how the locks
> are acquired earlier in this function. Patch 14 solves the deadlock in exchange
> for a race, so see my comment in patch 14.
> 
> [...]

I'll respond there. Not sure where the deadlock would be here, but maybe
the answer there will clear things up...

>>   cleanup:
>> -    if (obj)
>> -        virNodeDeviceObjUnlock(obj);
>> +    virNodeDeviceObjEndAPI(&obj);
>>      testDriverUnlock(driver);
>>      virNodeDeviceDefFree(def);
>>      virObjectUnref(dev);
>> @@ -5596,13 +5595,13 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
>>       * taken, so we have to dup the parent's name and drop the lock
>>       * before calling it.  We don't need the reference to the object
>>       * any more once we have the parent's name.  */
> 
> Not that this patch would be touching it directly, but ^this commentary is
> pretty much useless now, since @parent_name is useless in this function,
> nodeDeviceDestroy doesn't work that way anymore.
> 
> Erik
> 

Hmmm, I see said the blind man.  Looks like commit id '7ad479d0b' should
have removed @parent_name as well.

That should be a separate patch - I can create and add it to the end.

John