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