Re: [libvirt] [PATCH v4 12/14] nodedev: Convert virNodeDeviceObj to use virObjectLockable
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