Re: [Qemu-devel] [PATCH RFC V2 8/9] machine-opts: replace qemu_opt_get by QOM QemuMachine queries

[Paolo Bonzini]

Il 02/03/2014 14:07, Marcel Apfelbaum ha scritto:
> @@ -1182,10 +1183,17 @@ ram_addr_t last_ram_offset(void)
>  static void qemu_ram_setup_dump(void *addr, ram_addr_t size)
>  {
>      int ret;
> +    bool dump_guest_core = true;
> +
> +    if (object_property_is_set(OBJECT(current_machine),
> +                               "dump-guest-core", &error_abort)) {
> +        dump_guest_core = object_property_get_bool(OBJECT(current_machine),
> +                                                   "dump-guest-core",
> +                                                   &error_abort);
> +    }

You do not need object_property_is_set.  You can just initialize the 
field to true in the instance_init function.  Same for mem_merge.

> @@ -563,11 +565,14 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, 
> MemoryRegion *ccsr,
>      mpic = g_new(qemu_irq, 256);
>
>      if (kvm_enabled()) {
> -        QemuOpts *machine_opts = qemu_get_machine_opts();
> -        bool irqchip_allowed = qemu_opt_get_bool(machine_opts,
> -                                                "kernel_irqchip", true);
> -        bool irqchip_required = qemu_opt_get_bool(machine_opts,
> -                                                  "kernel_irqchip", false);
> +        bool irqchip_allowed = true;
> +        bool irqchip_required = false;
> +        if (object_property_is_set(OBJECT(current_machine),
> +                                   "kernel_irqchip", &error_abort)) {
> +            irqchip_allowed = object_property_get_bool(OBJECT(current_machine),
> +                                                       "kernel_irqchip", 
> &error_abort);
> +            irqchip_required = irqchip_allowed;
> +        }

Alternatively, you could have three properties: kernel_irqchip, 
write-only (no getter); kernel-irqchip-allowed; kernel-irqchip-required. 
 Setting irqchip writes to both kernel-irqchip-allowed and 
kernel-irqchip-required.  kernel-irqchip-allowed and 
kernel-irqchip-required are initialized to true and false respectively 
in the instance_init.


> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index bdb057e..8651167 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -145,7 +145,8 @@ static int xilinx_load_device_tree(hwaddr addr,
>      int r;
>      const char *dtb_filename;
>
> -    dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
> +    dtb_filename = object_property_get_str(OBJECT(current_machine),
> +                                           "dtb", &error_abort);
>      if (dtb_filename) {
>          fdt = load_device_tree(dtb_filename, &fdt_size);
>          if (!fdt) {
> diff --git a/kvm-all.c b/kvm-all.c
> index 2ca9143..e483c3c 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -22,7 +22,7 @@
>
>  #include "qemu-common.h"
>  #include "qemu/atomic.h"
> -#include "qemu/option.h"
> +#include "hw/boards.h"
>  #include "qemu/config-file.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/hw.h"
> @@ -1292,8 +1292,15 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, 
> EventNotifier *n, int virq)
>  static int kvm_irqchip_create(KVMState *s)
>  {
>      int ret;
> +    bool irqchip_allowed = true;
>
> -    if (!qemu_opt_get_bool(qemu_get_machine_opts(), "kernel_irqchip", true) ||
> +    if (object_property_is_set(OBJECT(current_machine),
> +                               "kernel_irqchip", &error_abort)) {
> +        irqchip_allowed = object_property_get_bool(OBJECT(current_machine),
> +                                                   "kernel_irqchip", 
> &error_abort);
> +    }
> +
> +    if (!irqchip_allowed ||
>          !kvm_check_extension(s, KVM_CAP_IRQCHIP)) {
>          return 0;
>      }
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index e555040..ca7e6bc 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -33,6 +33,7 @@
>  #include "exec/ioport.h"
>  #include <asm/hyperv.h>
>  #include "hw/pci/pci.h"
> +#include "hw/boards.h"
>
>  //#define DEBUG_KVM
>
> @@ -853,8 +854,13 @@ int kvm_arch_init(KVMState *s)
>      }
>      qemu_register_reset(kvm_unpoison_all, NULL);
>
> -    shadow_mem = qemu_opt_get_size(qemu_get_machine_opts(),
> -                                   "kvm_shadow_mem", -1);
> +    shadow_mem = -1;
> +    if (object_property_is_set(OBJECT(current_machine),
> +                               "kvm_shadow_mem", &error_abort)) {
> +        shadow_mem = object_property_get_int(OBJECT(current_machine),
> +                                             "kvm_shadow_mem", &error_abort);
> +    }

This can also be simply initialized in the instance_init function.

Paolo

> +
>      if (shadow_mem != -1) {
>          shadow_mem /= 4096;
>          ret = kvm_vm_ioctl(s, KVM_SET_NR_MMU_PAGES, shadow_mem);
> diff --git a/vl.c b/vl.c
> index dc206e1..007342c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2624,7 +2624,8 @@ static int configure_accelerator(void)
>      bool accel_initialised = false;
>      bool init_failed = false;
>
> -    p = qemu_opt_get(qemu_get_machine_opts(), "accel");
> +    p = object_property_get_str(OBJECT(current_machine),
> +                                "accel", &error_abort);
>      if (p == NULL) {
>          /* Use the default "accelerator", tcg */
>          p = "tcg";
> @@ -4077,6 +4078,12 @@ int main(int argc, char **argv, char **envp)
>          exit(0);
>      }
>
> +    machine_opts = qemu_get_machine_opts();
> +    if (qemu_opt_foreach(machine_opts, object_set_property, current_machine, 1) 
> < 0) {
> +        object_unref(OBJECT(current_machine));
> +        exit(1);
> +    }
> +
>      configure_accelerator();
>
>      if (qtest_chrdev) {
> @@ -4089,12 +4096,14 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>
> -    machine_opts = qemu_get_machine_opts();
> -    kernel_filename = qemu_opt_get(machine_opts, "kernel");
> -    initrd_filename = qemu_opt_get(machine_opts, "initrd");
> -    kernel_cmdline = qemu_opt_get(machine_opts, "append");
> -    bios_name = qemu_opt_get(machine_opts, "firmware");
> -
> +    kernel_filename = object_property_get_str(OBJECT(current_machine),
> +                                              "kernel", &error_abort);
> +    initrd_filename = object_property_get_str(OBJECT(current_machine),
> +                                              "initrd", &error_abort);
> +    kernel_cmdline = object_property_get_str(OBJECT(current_machine),
> +                                             "append", &error_abort);
> +    bios_name = object_property_get_str(OBJECT(current_machine),
> +                                        "firmware", &error_abort);
>      boot_order = machine->default_boot_order;
>      opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
>      if (opts) {
> @@ -4135,7 +4144,8 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>
> -    if (!linux_boot && qemu_opt_get(machine_opts, "dtb")) {
> +    if (!linux_boot && object_property_get_str(OBJECT(current_machine),
> +                                               "dtb", &error_abort)) {
>          fprintf(stderr, "-dtb only allowed with -kernel option\n");
>          exit(1);
>      }