Re: [Qemu-devel] [PATCH 00/16] pc: Eliminate struct PcGuestInfo

[Eduardo Habkost]

On Mon, Dec 07, 2015 at 08:57:03PM +0200, Marcel Apfelbaum wrote:
> On 12/02/2015 03:46 AM, Eduardo Habkost wrote:
> >This moves all data from PcGuestInfo to either PCMachineState or
> >PCMachineClass.
> >
> >This series depends on other two series:
> >* [PATCH v3 0/6] pc: Initialization and compat function cleanup
> >* [PATCH V3 0/3]  hw/pcie: Multi-root support for Q35
> >
> >For reference, there's a git tree containing this series plus all
> >the dependencies, at:
> >   git://github.com/ehabkost/qemu-hacks.git work/pcguestinfo-eliminate
> >
> >Eduardo Habkost (16):
> >   pc: Move PcGuestInfo declaration to top of file
> >   pc: Eliminate struct PcGuestInfoState
> >   pc: Remove guest_info parameter from pc_memory_init()
> >   acpi: Make acpi_setup() get PCMachineState as argument
> >   acpi: Remove unused build_facs() PcGuestInfo paramter
> >   acpi: Save PCMachineState on AcpiBuildState
> >   acpi: Make acpi_build() get PCMachineState as argument
> >   acpi: Make build_srat() get PCMachineState as argument
> >   acpi: Remove ram size fields fron PcGuestInfo
> >   pc: Move PcGuestInfo.fw_cfg field to PCMachineState
> >   pc: Simplify signature of xen_load_linux()
> >   pc: Remove PcGuestInfo.isapc_ram_fw field
> >   q35: Remove MCHPCIState.guest_info field
> >   acpi: Use PCMachineClass fields directly
> >   pc: Move PcGuestInfo.apic_xrupt_override field to PCMachineState
> >   pc: Move APIC and NUMA data from PcGuestInfo to PCMachineState
> 
> Hi,
> 
> I mainly agree with the removal of PcGuestInfo , I commented on some patches.
> 
> I do have a minor reservation, we kind of loose some information about the 
> fields.
> Until now it was pretty clear that the fields were related to guest because
> they were part of PcGuestInfo. Now this information is lost and the fields
> appear as yet other machine attributes.

But they really are just machine attributes, aren't they?

> 
> I suppose this can be addressed by:
> - a prefix for guest fields (e.g numa_nodes-> guest_numa_nodes),
> - a comment in the class /* guest fields */,
> - keeping the fields in PcGuestInfo struct but make the machine field short: 
> guest so we can call machine->guest.numa_nodes
> - or not be addressed at all :)

I don't see your point. Could you explain what you mean by
"related to the guest" and "guest fields"?

They are just machine attributes, and they happen to be used as
input when building ACPI tables (just like other machine
attributes are used as input for other guest-visible data, like
CPUID, SMBIOS, and other tables). What exactly make them "related
to guest"?

-- 
Eduardo