Re: [libvirt] [PATCH v5 1/3] nodedev: Fix usage of virNodeDeviceObjListGetParentHost

>> >> @@ -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?