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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


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

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux