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

[Thomas Huth <thuth@redhat.com>]

On Tue, 26 May 2015 12:22:56 +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         |  6 ++--
>  hw/intc/xics.c              | 20 ++++++-------
>  hw/intc/xics_kvm.c          |  2 +-
>  hw/net/spapr_llan.c         | 12 ++++----
>  hw/nvram/spapr_nvram.c      |  4 +--
>  hw/ppc/spapr.c              | 73 +++++++++++++++++++--------------------------
>  hw/ppc/spapr_events.c       | 13 +++++---
>  hw/ppc/spapr_hcall.c        | 36 +++++++++++-----------
>  hw/ppc/spapr_iommu.c        |  8 ++---
>  hw/ppc/spapr_pci.c          | 37 ++++++++++++-----------
>  hw/ppc/spapr_rtas.c         | 38 +++++++++++------------
>  hw/ppc/spapr_rtc.c          |  4 +--
>  hw/ppc/spapr_vio.c          | 15 +++++-----
>  include/hw/pci-host/spapr.h | 10 ++++---
>  include/hw/ppc/spapr.h      | 33 ++++++++++++++------
>  include/hw/ppc/spapr_vio.h  |  4 ++-
>  16 files changed, 167 insertions(+), 148 deletions(-)
[...]
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f069609..0016f25 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
[...]
> @@ -1756,17 +1743,17 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus,
>  
>  static char *spapr_get_kvm_type(Object *obj, Error **errp)
>  {
> -    sPAPRMachineState *sm = SPAPR_MACHINE(obj);
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>  
> -    return g_strdup(sm->kvm_type);
> +    return g_strdup(spapr->kvm_type);
>  }
>  
>  static void spapr_set_kvm_type(Object *obj, const char *value, Error **errp)
>  {
> -    sPAPRMachineState *sm = SPAPR_MACHINE(obj);
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
>  
> -    g_free(sm->kvm_type);
> -    sm->kvm_type = g_strdup(value);
> +    g_free(spapr->kvm_type);
> +    spapr->kvm_type = g_strdup(value);
>  }

I think you could keep the "sm" variable name in these two functions.

Anyway, patch looks good to me now, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>