Re: [Qemu-devel] [PATCH 1/4] spapr: Merge sPAPREnvironment into sPAPRMachineState

[Thomas Huth <thuth@redhat.com>]

On Thu,  7 May 2015 13:57:01 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> The code for -machine pseries maintains a global sPAPREnvironment structure
> which keeps track of general state information about the guest platform.
> This predates the existence of the MachineState structure, but performs
> basically the same function.
> 
> Now that we have the generic MachineState, fold sPAPREnvironment into
> sPAPRMachineState, the pseries specific subclass of MachineState.
> 
> This is mostly a matter of search and replace, although a few places which
> relied on the global spapr variable are changed to find the structure via
> qdev_get_machine().
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/char/spapr_vty.c         |  14 +-
>  hw/intc/xics.c              |  42 +++---
>  hw/intc/xics_kvm.c          |   2 +-
>  hw/net/spapr_llan.c         |  20 +--
>  hw/nvram/spapr_nvram.c      |   8 +-
>  hw/ppc/spapr.c              | 339 +++++++++++++++++++++-----------------------
>  hw/ppc/spapr_events.c       |  37 ++---
>  hw/ppc/spapr_hcall.c        |  44 +++---
>  hw/ppc/spapr_iommu.c        |   8 +-
>  hw/ppc/spapr_pci.c          |  91 ++++++------
>  hw/ppc/spapr_rtas.c         |  66 ++++-----
>  hw/ppc/spapr_rtc.c          |  12 +-
>  hw/ppc/spapr_vio.c          |  31 ++--
>  include/hw/pci-host/spapr.h |  12 +-
>  include/hw/ppc/spapr.h      |  33 +++--
>  include/hw/ppc/spapr_vio.h  |   6 +-
>  16 files changed, 391 insertions(+), 374 deletions(-)
> 
> diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
> index 4e464bd..5a27740 100644
> --- a/hw/char/spapr_vty.c
> +++ b/hw/char/spapr_vty.c
> @@ -74,7 +74,7 @@ static void spapr_vty_realize(VIOsPAPRDevice *sdev, Error **errp)
>  }
>  
>  /* Forward declaration */
> -static target_ulong h_put_term_char(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> +static target_ulong h_put_term_char(PowerPCCPU *cpu, sPAPRMachineState *sm,
>                                      target_ulong opcode, target_ulong *args)

Phew, this patch is really hugh, ... mainly just because of the
renaming. Could you maybe keep the "spapr" as parameter name everywhere
instead of "sm"? (i.e. use "sPAPRMachineState *spapr" as parameter?)
That should IMHO be fine, too, and it would decrease the size of the
patch drastically. (And it likely does not break other patches so badly
which are currently pending)

 Thomas