On Wed, May 24, 2017 at 09:45:09AM -0400, John Ferlan wrote:
>
>
> On 05/23/2017 04:13 AM, Bjoern Walk wrote:
> > John Ferlan <jferlan@xxxxxxxxxx> [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@xxxxxxxxxx>
> >> ---
> >> 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.
This still doesn't make sense really. The src/conf directory should
only contain code that's related to the XML parsing/formatting, and
generic object management. It should never contain any functional
driver code, and certainly none that pokes in sysfs.
> 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.
To get around that simply change the APIs so that instead of taking
the virNodeDevXXX structs as a single parameter, you just pass in
an exploded list of parameters, populated from the individual struct
fields that are needed for the methods in question.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
