Re: [Qemu-devel] [PATCH 07/11] hw/arm/virt: pass VirtMachineState instead of VirtGuestInfo

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 15 Dec 2016 17:24:05 +0100
Andrew Jones <drjones@redhat.com> wrote:

> On Thu, Dec 15, 2016 at 03:45:55PM +0100, Igor Mammedov wrote:
> > On Tue, 13 Dec 2016 22:45:18 +0100
> > Andrew Jones <drjones@redhat.com> wrote:
> >   
> > > Only two functions take VirtGuestInfo parameters. Now that guest-info
> > > is part of VirtMachineState, and VirtMachineState is defined in the
> > > virt header, pass that instead.
> > > 
> > > Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> > > ---
> > >  hw/arm/virt-acpi-build.c | 3 ++-
> > >  hw/arm/virt.c            | 8 ++++----
> > >  include/hw/arm/virt.h    | 2 +-
> > >  3 files changed, 7 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > > index db44e2dd5f12..f7ce6aeed307 100644
> > > --- a/hw/arm/virt-acpi-build.c
> > > +++ b/hw/arm/virt-acpi-build.c
> > > @@ -822,8 +822,9 @@ static const VMStateDescription vmstate_virt_acpi_build = {
> > >      },
> > >  };
> > >  
> > > -void virt_acpi_setup(VirtGuestInfo *guest_info)
> > > +void virt_acpi_setup(VirtMachineState *vms)
> > >  {
> > > +    VirtGuestInfo *guest_info = &vms->acpi_guest_info;
> > >      AcpiBuildTables tables;
> > >      AcpiBuildState *build_state;
> > >  
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 7860fade52f1..bcf8a2954c16 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -1156,9 +1156,9 @@ static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
> > >      return board->fdt;
> > >  }
> > >  
> > > -static void virt_build_smbios(VirtGuestInfo *guest_info)
> > > +static void virt_build_smbios(VirtMachineState *vms)  
> > Looks like all it needs is 'fw_cfg'
> > If there aren't plans to use more data from VirtMachineState,
> > just passing fw_cfg should be enough.  
> 
> I prefer leaving it vms, as that's consistent with pretty much all
> the other functions in this file, and if we do need more state then
> it'll already be there. But I won't insist on this, it can always be
> changed later.
fw_Cfg seems cleaner but I don't mind it as it's now.


> 
> Thanks,
> drew
> 
> >   
> > >  {
> > > -    FWCfgState *fw_cfg = guest_info->fw_cfg;
> > > +    FWCfgState *fw_cfg = vms->acpi_guest_info.fw_cfg;
> > >      uint8_t *smbios_tables, *smbios_anchor;
> > >      size_t smbios_tables_len, smbios_anchor_len;
> > >      const char *product = "QEMU Virtual Machine";
> > > @@ -1191,8 +1191,8 @@ void virt_machine_done(Notifier *notifier, void *data)
> > >      VirtMachineState *vms = container_of(notifier, VirtMachineState,
> > >                                           machine_done);
> > >  
> > > -    virt_acpi_setup(&vms->acpi_guest_info);
> > > -    virt_build_smbios(&vms->acpi_guest_info);
> > > +    virt_acpi_setup(vms);
> > > +    virt_build_smbios(vms);
> > >  }
> > >  
> > >  static void machvirt_init(MachineState *machine)
> > > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> > > index 77d25dbddb62..cbedc682ae99 100644
> > > --- a/include/hw/arm/virt.h
> > > +++ b/include/hw/arm/virt.h
> > > @@ -128,6 +128,6 @@ typedef struct {
> > >      OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
> > >  
> > >  
> > > -void virt_acpi_setup(VirtGuestInfo *guest_info);
> > > +void virt_acpi_setup(VirtMachineState *vms);
> > >  
> > >  #endif /* QEMU_ARM_VIRT_H */  
> > 
> >   
>