Re: [libvirt] [PATCH 04/13] nodedev: Move node_device_linux_sysfs from node_driver to conf

On 05/23/2017 04:13 AM, Bjoern Walk wrote: > John Ferlan <jferlan(a)redhat.com&gt; [2017-05-19, 09:08AM -0400]: >> Move the whole file from src/node_device into src/conf and rename the >> API's to have the "virNodeDevice" prefix. >> >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> src/Makefile.am | 8 ++++---- >> .../node_device_linux_sysfs.c | 24 >> ++++++++++------------ >> .../node_device_linux_sysfs.h | 9 +++++--- >> src/libvirt_private.syms | 5 +++++ >> src/node_device/node_device_driver.c | 8 ++++---- >> src/node_device/node_device_hal.c | 4 ++-- >> src/node_device/node_device_udev.c | 4 ++-- >> 7 files changed, 34 insertions(+), 28 deletions(-) >> rename src/{node_device => conf}/node_device_linux_sysfs.c (89%) >> rename src/{node_device => conf}/node_device_linux_sysfs.h (82%) >> > > What's the reasoning for this move? It somehow doesn't feel right and it > will make backports harder. "Eventually" code that is currently in node_device_driver to peruse the node device object list is going to move to virnodedevobj.c and there was some "boundary" thing about calling back into the driver that I don't quite recall the exact compile/link error which caused me to move the code. Although the commit date is relatively recent - it's more or less a copy of work done in Dec 2016.

One could also make the argument that there's nothing "driver specific" in the code, so why should it have ever been put in the src/node_device directory? My other option would be to essentially move the guts of the code into virnodedevobj and call it from src/node_device/... IDC which way this happens, but I just found this easier. Using gitk I've never had issues in the past following history of the code when a module moves from one directory to another. FWIW: My first instinct was move the code to src/util like other code that has linux (or arch specific) pieces, but I couldn't do that because node_device_linux_sysfs.h references "node_device_conf.h". Inclusion of conf/*.h has been disallowed from src/util.