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

[Erik Skultety]

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