Re: [libvirt] [PATCH v3 01/12] nodedev: Alter virNodeDeviceObjRemove

On Thu, Jun 29, 2017 at 02:12:55PM +0200, Erik Skultety wrote: > On Sat, Jun 03, 2017 at 09:11:51AM -0400, John Ferlan wrote: > > Rather than passing the object to be removed by reference, pass by value > > and then let the caller decide whether or not the object should be free'd. > > This function should just handle the remove of the object from the list > > for which it was placed during virNodeDeviceObjAssignDef. > > > > One caller in node_device_hal would fail to go through the dev_create path > > since the @dev would have been NULL after returning from the Remove API. > > This is the main motivation for the patch I presume - in which case, I'm > wondering why do we actually have to remove the device from the list when > handling 'change'/'update' for hal instead of just replacing the ->def with a > new instance but it's perfectly fine to do that for udev...I don't see the > point in doing what we currently do for hal. It will basically be a historical artifact since few people have touched the HAL driver in years. We only keep it around for compat with FreeBSD IIRC. It is perfectly reasonable to update the code to follow our modern best practice.