> >> > >> @@ -594,6 +600,11 @@ nodeDeviceDestroy(virNodeDevicePtr device) > >> int ret = -1; > >> virNodeDeviceObjPtr obj = NULL; > >> virNodeDeviceDefPtr def; > >> + char *name = NULL; > >> + char *parent = NULL; > >> + char *parent_wwnn = NULL; > >> + char *parent_wwpn = NULL; > >> + char *parent_fabric_wwn = NULL; > >> char *wwnn = NULL, *wwpn = NULL; > >> int parent_host = -1; > >> > >> @@ -609,12 +620,24 @@ nodeDeviceDestroy(virNodeDevicePtr device) > >> if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) > >> goto cleanup; > >> > >> - /* virNodeDeviceGetParentHost will cause the device object's lock > >> - * 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. */ > >> + /* Because we're about to release the lock and thus run into a race > >> + * possibility (however improbably) with a udevAddOneDevice change > >> + * event which would essentially free the existing @def (obj->def) and > >> + * replace it with something new, we need to save off and use the > >> + * various fields that virNodeDeviceObjListGetParentHost will use */ > > > > And as I originally suggested I would allocate a new temporary @def structure, > > initialize it, pass it to the *GetParentHost method like nothing out of the > > ordinary happened and mentioned in the commentary why we've done it that way > > (which you already do in this patch). > > > > So you'd prefer some sort of virNodeDeviceDefCopy function be created? > Or use VIR_ALLOC(def) and copy in the 5 fields only to then > virNodeDeviceDefFree() it afterwards? Yeah, exactly, I'm aware of those unused extra field, I don't know it just looked more appealing and transparent to me, again, matter of preference, the way I see it, you store/pass it in a much more compact way with the added benefit of other, in this case currently unused, fields, should virNodeDeviceObjListGetParentHost ever need them. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
