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@xxxxxxxxxx> > > --- > > 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list