From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40451) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a60yE-0002ha-AI for qemu-devel@nongnu.org; Mon, 07 Dec 2015 13:57:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a60yB-0004wY-0M for qemu-devel@nongnu.org; Mon, 07 Dec 2015 13:57:10 -0500 Received: from mail-wm0-x235.google.com ([2a00:1450:400c:c09::235]:37624) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a60yA-0004wU-Q8 for qemu-devel@nongnu.org; Mon, 07 Dec 2015 13:57:06 -0500 Received: by wmww144 with SMTP id w144so162889623wmw.0 for ; Mon, 07 Dec 2015 10:57:06 -0800 (PST) References: <1449020831-8414-1-git-send-email-ehabkost@redhat.com> From: Marcel Apfelbaum Message-ID: <5665D67F.2000001@gmail.com> Date: Mon, 7 Dec 2015 20:57:03 +0200 MIME-Version: 1.0 In-Reply-To: <1449020831-8414-1-git-send-email-ehabkost@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/16] pc: Eliminate struct PcGuestInfo Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , qemu-devel@nongnu.org Cc: Igor Mammedov , "Michael S. Tsirkin" , Marcel Apfelbaum 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. 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 :) Thanks, Marcel > > hw/i386/acpi-build.c | 75 ++++++++++++++++++++++++----------------------- > hw/i386/acpi-build.h | 2 +- > hw/i386/pc.c | 71 ++++++++++++++++++-------------------------- > hw/i386/pc_piix.c | 14 ++------- > hw/i386/pc_q35.c | 15 ++-------- > include/hw/i386/pc.h | 30 +++++++------------ > include/hw/pci-host/q35.h | 1 - > 7 files changed, 82 insertions(+), 126 deletions(-) >