On Wed, Oct 22, 2014 at 05:00:55PM -0600, Eric Blake wrote:
On 10/22/2014 11:14 AM, Daniel P. Berrange wrote:
With the large number of APIs in libvirt the driver.h file, it is easy to get lost looking for things. Split each driver into a separate header file based on the functional driver groups.
Someday, I'd also like to see the public .h headers get split, where libvirt.h merely pulls in all the subsidiary public .h headers. But that's a separate patch series, and this one is nice to have now.
Guess what patches I will be posting once this is reviewed :-)
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/driver-hypervisor.h | 1396 ++++++++++++++++++++++++++++++ src/driver-interface.h | 131 +++ src/driver-network.h | 166 ++++ src/driver-nodedev.h | 112 +++ src/driver-nwfilter.h | 94 ++ src/driver-secret.h | 114 +++ src/driver-state.h | 58 ++ src/driver-storage.h | 265 ++++++ src/driver-stream.h | 72 ++ src/driver.h | 2170 +----------------------------------------------
Big. I reviewed this for sanity that the bulk of it is code motion from driver.h to driver-hypervisor.h:
$ diff -u <(sed -n 's/^-//p' patch) \ <(sed -n 's/^\+//p' patch)
but because the other files are also part of the same diff, but the sorting of the filenames is different than the order of the sections in driver.h, even the filtered patch got pretty ugly to read.
If I were doing it from scratch, it might have been easier to submit multiple patches, one per .h creation (because the above formula shows a MUCH smaller diff when it is not intermixing files). But it's not worth you respinning the series just for that.
That said, I'm a huge fan of this split, and since it still compiles, you appear to have done it correctly. ACK, and let's get it in sooner rather than later to minimize the time you have to carry it around in rebases.
Ok, thanks. As you see in the next patch series, for the more complex libvirt.c I did indeed use multiple patchess because it got too big. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|