Peter Maydell <peter.mayd...@linaro.org> writes: > On 7 January 2013 19:11, Anthony Liguori <aligu...@us.ibm.com> wrote: >> I appreciate the thoroughness here but 66 patches is too much all at >> once. Can this be done in chunks or more reasonable patch sizes? It >> would make review and committing a lot easier. > > So do you have any suggestions? The patchset is long but it > is actually in a fairly easily separable set of sections: >
Sounds like you have already identified how to break things up... > I: Initial class definitions and transport level refactoring: > >>> qdev : add a maximum device allowed field for the bus. >>> virtio-bus : introduce virtio-bus >>> virtio-pci-bus : introduce virtio-pci-bus. >>> virtio-pci : refactor virtio-pci device. >>> virtio-device : refactor virtio-device. >>> virtio-s390-bus : add virtio-s390-bus. >>> virtio-s390-device : create a virtio-s390-bus during init. I would start with the above. > > II: Update virtio-blk: > >>> virtio-blk : show VirtIOBlock structure. >>> virtio-blk : don't use pointer for configuration. >>> virtio-blk : add the virtio-blk device. >>> virtio-blk-pci : switch to new API. >>> virtio-blk-s390 : switch to the new API. >>> virtio-blk : cleanup : use QOM cast. >>> virtio-blk : cleanup : remove qdev field. > > III: Update virtio-net: > >>> virtio-net : show the VirtIONet structure. >>> virtio-net : add the virtio-net device. >>> virtio-net-pci : switch to the new API. >>> virtio-net-s390 : switch to the new API. >>> virtio-net : cleanup : use QOM cast. >>> virtio-net : cleanup : init and exit function. >>> virtio-net : cleanup : remove qdev field. > > [etc; all the backends are handled in basically the same way] I would do all of the devices at once. > > N: Final cleanup now all backends are converted: > >>> virtio : remove the function pointer. >>> virtio-pci : cleanup : init, exit and reset functions. >>> s390-virtio-bus : cleanup >>> virtio : remove virtiobindings. >>> virtio : cleanup : init and exit function. I would do this independently. Regards, Anthony Liguori > > where I guess the interesting bits to review in particular > would be phases I and N and a randomly selected backend. > > Perhaps you could squash some of the patches together, > for instance the "virtio-foo: show the VirtIOFoo structure" > (which is just moving a struct from a private .c file to the .h) > could go in with the following "virtio-net : add the virtio-foo > device." patch, which would reduce the patch count by > seven or so -- is that worth doing? (obviously it's the same > amount of actual code just in fewer patches...) > > thanks > -- PMM
