Re: [Qemu-devel] [PATCH 1/4] machine: conversion of QEMUMachineInitArgs to MachineState

[Laszlo Ersek <lersek@redhat.com>]

On 05/07/14 16:42, Marcel Apfelbaum wrote:
> Total removal of QEMUMachineInitArgs struct. QEMUMachineInitArgs's fields
> are copied into MachineState. Removed duplicated fields from MachineState.
> 
> All the other changes are only mechanical refactoring, no semantic changes.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>    - I am perfectly aware that patches touching a lot of files
>      are not desirable, but this one is a very simple replacement
>      patch:
>        QEMUMachineInitArgs -> MachineState
>        args -> ms
>    - This is the simplest way to get rid of QEMUMachineInitArgs fast.

I think I only ever touched the following two files, from the files
changed by this patch:

>  hw/i386/pc_piix.c                        | 108 +++++++++++++++----------------
>  vl.c                                     |  19 +++---


> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index ea72502..173c875 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -69,7 +69,7 @@ static bool smbios_legacy_mode;
>  static bool gigabyte_align = true;
>  
>  /* PC hardware initialisation */
> -static void pc_init1(QEMUMachineInitArgs *args,
> +static void pc_init1(MachineState *ms,
>                       int pci_enabled,
>                       int kvmclock_enabled)
>  {
> @@ -106,7 +106,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
>  
> -    pc_cpus_init(args->cpu_model, icc_bridge);
> +    pc_cpus_init(ms->cpu_model, icc_bridge);
>  
>      if (kvm_enabled() && kvmclock_enabled) {
>          kvmclock_create();
> @@ -119,13 +119,13 @@ static void pc_init1(QEMUMachineInitArgs *args,
>       * For old machine types, use whatever split we used historically to avoid
>       * breaking migration.
>       */
> -    if (args->ram_size >= 0xe0000000) {
> +    if (ms->ram_size >= 0xe0000000) {
>          ram_addr_t lowmem = gigabyte_align ? 0xc0000000 : 0xe0000000;
> -        above_4g_mem_size = args->ram_size - lowmem;
> +        above_4g_mem_size = ms->ram_size - lowmem;
>          below_4g_mem_size = lowmem;
>      } else {
>          above_4g_mem_size = 0;
> -        below_4g_mem_size = args->ram_size;
> +        below_4g_mem_size = ms->ram_size;
>      }
>  
>      if (pci_enabled) {
> @@ -147,14 +147,14 @@ static void pc_init1(QEMUMachineInitArgs *args,
>      if (smbios_defaults) {
>          /* These values are guest ABI, do not change */
>          smbios_set_defaults("QEMU", "Standard PC (i440FX + PIIX, 1996)",
> -                            args->machine->name, smbios_legacy_mode);
> +                            ms->machine->name, smbios_legacy_mode);
>      }
>  
>      /* allocate ram and load rom/bios */
>      if (!xen_enabled()) {
>          fw_cfg = pc_memory_init(system_memory,
> -                       args->kernel_filename, args->kernel_cmdline,
> -                       args->initrd_filename,
> +                       ms->kernel_filename, ms->kernel_cmdline,
> +                       ms->initrd_filename,
>                         below_4g_mem_size, above_4g_mem_size,
>                         rom_memory, &ram_memory, guest_info);
>      }
> @@ -170,7 +170,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>  
>      if (pci_enabled) {
>          pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
> -                              system_memory, system_io, args->ram_size,
> +                              system_memory, system_io, ms->ram_size,
>                                below_4g_mem_size,
>                                above_4g_mem_size,
>                                pci_memory, ram_memory);
> @@ -235,7 +235,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>          }
>      }
>  
> -    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, args->boot_order,
> +    pc_cmos_init(below_4g_mem_size, above_4g_mem_size, ms->boot_order,
>                   floppy, idebus[0], idebus[1], rtc_state);
>  
>      if (pci_enabled && usb_enabled(false)) {
> @@ -258,131 +258,131 @@ static void pc_init1(QEMUMachineInitArgs *args,
>      }
>  }
>  
> -static void pc_init_pci(QEMUMachineInitArgs *args)
> +static void pc_init_pci(MachineState *ms)
>  {
> -    pc_init1(args, 1, 1);
> +    pc_init1(ms, 1, 1);
>  }
>  
> -static void pc_compat_2_0(QEMUMachineInitArgs *args)
> +static void pc_compat_2_0(MachineState *ms)
>  {
>      smbios_legacy_mode = true;
>  }
>  
> -static void pc_compat_1_7(QEMUMachineInitArgs *args)
> +static void pc_compat_1_7(MachineState *ms)
>  {
> -    pc_compat_2_0(args);
> +    pc_compat_2_0(ms);
>      smbios_defaults = false;
>      gigabyte_align = false;
>      option_rom_has_mr = true;
>      x86_cpu_compat_disable_kvm_features(FEAT_1_ECX, CPUID_EXT_X2APIC);
>  }
>  
> -static void pc_compat_1_6(QEMUMachineInitArgs *args)
> +static void pc_compat_1_6(MachineState *ms)
>  {
> -    pc_compat_1_7(args);
> +    pc_compat_1_7(ms);
>      has_pci_info = false;
>      rom_file_has_mr = false;
>      has_acpi_build = false;
>  }
>  
> -static void pc_compat_1_5(QEMUMachineInitArgs *args)
> +static void pc_compat_1_5(MachineState *ms)
>  {
> -    pc_compat_1_6(args);
> +    pc_compat_1_6(ms);
>  }
>  
> -static void pc_compat_1_4(QEMUMachineInitArgs *args)
> +static void pc_compat_1_4(MachineState *ms)
>  {
> -    pc_compat_1_5(args);
> +    pc_compat_1_5(ms);
>      x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>      x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, CPUID_EXT_PCLMULQDQ);
>  }
>  
> -static void pc_compat_1_3(QEMUMachineInitArgs *args)
> +static void pc_compat_1_3(MachineState *ms)
>  {
> -    pc_compat_1_4(args);
> +    pc_compat_1_4(ms);
>      enable_compat_apic_id_mode();
>  }
>  
>  /* PC compat function for pc-0.14 to pc-1.2 */
> -static void pc_compat_1_2(QEMUMachineInitArgs *args)
> +static void pc_compat_1_2(MachineState *ms)
>  {
> -    pc_compat_1_3(args);
> +    pc_compat_1_3(ms);
>      x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
>  }
>  
> -static void pc_init_pci_2_0(QEMUMachineInitArgs *args)
> +static void pc_init_pci_2_0(MachineState *ms)
>  {
> -    pc_compat_2_0(args);
> -    pc_init_pci(args);
> +    pc_compat_2_0(ms);
> +    pc_init_pci(ms);
>  }
>  
> -static void pc_init_pci_1_7(QEMUMachineInitArgs *args)
> +static void pc_init_pci_1_7(MachineState *ms)
>  {
> -    pc_compat_1_7(args);
> -    pc_init_pci(args);
> +    pc_compat_1_7(ms);
> +    pc_init_pci(ms);
>  }
>  
> -static void pc_init_pci_1_6(QEMUMachineInitArgs *args)
> +static void pc_init_pci_1_6(MachineState *ms)
>  {
> -    pc_compat_1_6(args);
> -    pc_init_pci(args);
> +    pc_compat_1_6(ms);
> +    pc_init_pci(ms);
>  }
>  
> -static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
> +static void pc_init_pci_1_5(MachineState *ms)
>  {
> -    pc_compat_1_5(args);
> -    pc_init_pci(args);
> +    pc_compat_1_5(ms);
> +    pc_init_pci(ms);
>  }
>  
> -static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
> +static void pc_init_pci_1_4(MachineState *ms)
>  {
> -    pc_compat_1_4(args);
> -    pc_init_pci(args);
> +    pc_compat_1_4(ms);
> +    pc_init_pci(ms);
>  }
>  
> -static void pc_init_pci_1_3(QEMUMachineInitArgs *args)
> +static void pc_init_pci_1_3(MachineState *ms)
>  {
> -    pc_compat_1_3(args);
> -    pc_init_pci(args);
> +    pc_compat_1_3(ms);
> +    pc_init_pci(ms);
>  }
>  
>  /* PC machine init function for pc-0.14 to pc-1.2 */
> -static void pc_init_pci_1_2(QEMUMachineInitArgs *args)
> +static void pc_init_pci_1_2(MachineState *ms)
>  {
> -    pc_compat_1_2(args);
> -    pc_init_pci(args);
> +    pc_compat_1_2(ms);
> +    pc_init_pci(ms);
>  }
>  
>  /* PC init function for pc-0.10 to pc-0.13, and reused by xenfv */
> -static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args)
> +static void pc_init_pci_no_kvmclock(MachineState *ms)
>  {
>      has_pci_info = false;
>      has_acpi_build = false;
>      smbios_defaults = false;
>      x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
>      enable_compat_apic_id_mode();
> -    pc_init1(args, 1, 0);
> +    pc_init1(ms, 1, 0);
>  }
>  
> -static void pc_init_isa(QEMUMachineInitArgs *args)
> +static void pc_init_isa(MachineState *ms)
>  {
>      has_pci_info = false;
>      has_acpi_build = false;
>      smbios_defaults = false;
> -    if (!args->cpu_model) {
> -        args->cpu_model = "486";
> +    if (!ms->cpu_model) {
> +        ms->cpu_model = "486";
>      }
>      x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_PV_EOI);
>      enable_compat_apic_id_mode();
> -    pc_init1(args, 0, 1);
> +    pc_init1(ms, 0, 1);
>  }
>  
>  #ifdef CONFIG_XEN
> -static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
> +static void pc_xen_hvm_init(MachineState *ms)
>  {
>      PCIBus *bus;
>  
> -    pc_init_pci(args);
> +    pc_init_pci(ms);
>  
>      bus = pci_find_primary_bus();
>      if (bus != NULL) {

> diff --git a/vl.c b/vl.c
> index c4505dc..58673bd 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4422,16 +4422,15 @@ int main(int argc, char **argv, char **envp)
>  
>      qdev_machine_init();
>  
> -    current_machine->init_args = (QEMUMachineInitArgs) {
> -        .machine = machine_class,
> -        .ram_size = ram_size,
> -        .boot_order = boot_order,
> -        .kernel_filename = kernel_filename,
> -        .kernel_cmdline = kernel_cmdline,
> -        .initrd_filename = initrd_filename,
> -        .cpu_model = cpu_model };
> -
> -    machine_class->init(&current_machine->init_args);
> +    current_machine->machine = machine_class;
> +    current_machine->ram_size = ram_size;
> +    current_machine->boot_order = boot_order;
> +    current_machine->kernel_filename = kernel_filename;
> +    current_machine->kernel_cmdline = kernel_cmdline;
> +    current_machine->initrd_filename = initrd_filename;
> +    current_machine->cpu_model = cpu_model;
> +
> +    machine_class->init(current_machine);
>  
>      audio_init();
>  
> 

These look OK to me. I also checked the "include/hw/boards.h" hunks (I
already snipped them from the reply and I'm too lazy to restore them),
they should be fine too.

Laszlo