Re: [Qemu-devel] [PATCH v5 00/24] ACPI reorganization for hardware-reduced API addition

[Michael S. Tsirkin]

On Mon, Nov 19, 2018 at 04:31:10PM +0100, Igor Mammedov wrote:
> On Fri, 16 Nov 2018 17:37:54 +0100
> Paolo Bonzini <address@hidden> wrote:
> 
> > On 16/11/18 17:29, Igor Mammedov wrote:
> > > General suggestions for this series:
> > >   1. Preferably don't do multiple changes within a patch
> > >      neither post huge patches (unless it's pure code movement).
> > >      (it's easy to squash patches later it necessary)
> > >   2. Start small, pick a table generalize it and send as
> > >      one small patchset. Tables are often independent
> > >      and it's much easier on both author/reviewer to agree upon
> > >      changes and rewrite it if necessary.  
> > 
> > How would that be done?  This series is on the bigger side, agreed, but
> > most of it is really just code movement.  It's a starting point, having
> > a generic ACPI library is way beyond what this is trying to do.
> I've tried to give suggestions how to restructure series
> on per patch basis. In my opinion it quite possible to split
> series in several smaller ones and it should really help with
> making series cleaner and easier/faster to review/amend/merge
> vs what we have in v5.
> (it's more frustrating to rework large series vs smaller one)
> 
> If something isn't clear, it's easy to reach out to me here
> or directly (email/irc/github) for clarification/feed back.

I assume the #1 goal is to add reduced HW support.  So another
option to speed up merging is to just go ahead and duplicate a
bunch of code e.g. in pc_virt.c acpi/reduced.c or in any other
file.
This way it might be easier to see what's common code and what isn't.
And I think offline Igor said he might prefer that way. Right Igor?

> > 
> > Paolo
> > 
> > >   3. when you think about refactoring acpi into a generic API
> > >      think about it as routines that go into a separate library
> > >      (pure acpi spec code) and qemu/acpi glue routines and
> > >       divide them correspondingly.  
> >