Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field

[Paolo Bonzini]

On 11/11/2015 20:09, Eduardo Habkost wrote:
> All DisplayType values are just UI options that don't affect any
> hardware emulation code, except for DT_NOGRAPHIC. Replace
> DT_NOGRAPHIC with DT_NONE plus a new MachineState.nographic
> field, so hardware emulation code don't need to use the
> display_type variable.
> 
> Cc: Michael Walle <address@hidden>
> Cc: Blue Swirl <address@hidden>
> Cc: Mark Cave-Ayland <address@hidden>
> Signed-off-by: Eduardo Habkost <address@hidden>

Can you add a QOM property too, so that "-machine graphics=yes|no" can
be used?

Paolo

> ---
>  hw/lm32/milkymist.c     |  2 +-
>  hw/nvram/fw_cfg.c       |  6 ++++--
>  hw/sparc/sun4m.c        |  2 +-
>  include/hw/boards.h     |  1 +
>  include/sysemu/sysemu.h |  1 -
>  vl.c                    | 12 ++++++------
>  6 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
> index e46283a..947c7db 100644
> --- a/hw/lm32/milkymist.c
> +++ b/hw/lm32/milkymist.c
> @@ -163,7 +163,7 @@ milkymist_init(MachineState *machine)
>      milkymist_memcard_create(0x60004000);
>      milkymist_ac97_create(0x60005000, irq[4], irq[5], irq[6], irq[7]);
>      milkymist_pfpu_create(0x60006000, irq[8]);
> -    if (display_type != DT_NOGRAPHIC) {
> +    if (!machine->nographic) {
>          milkymist_tmu2_create(0x60007000, irq[9]);
>      }
>      milkymist_minimac2_create(0x60008000, 0x30000000, irq[10], irq[11]);
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 73b0a81..e42b198 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -24,6 +24,7 @@
>  #include "hw/hw.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/dma.h"
> +#include "hw/boards.h"
>  #include "hw/isa/isa.h"
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/sysbus.h"
> @@ -755,16 +756,17 @@ static void fw_cfg_machine_ready(struct Notifier *n, 
> void *data)
>  static void fw_cfg_init1(DeviceState *dev)
>  {
>      FWCfgState *s = FW_CFG(dev);
> +    MachineState *machine = MACHINE(qdev_get_machine());
>  
>      assert(!object_resolve_path(FW_CFG_PATH, NULL));
>  
> -    object_property_add_child(qdev_get_machine(), FW_CFG_NAME, OBJECT(s), 
> NULL);
> +    object_property_add_child(OBJECT(machine), FW_CFG_NAME, OBJECT(s), NULL);
>  
>      qdev_init_nofail(dev);
>  
>      fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
>      fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
> -    fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == 
> DT_NOGRAPHIC));
> +    fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)machine->nographic);
>      fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
>      fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
>      fw_cfg_bootsplash(s);
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 230dac9..d47f06a 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -1017,7 +1017,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef 
> *hwdef,
>      slavio_timer_init_all(hwdef->counter_base, slavio_irq[19], 
> slavio_cpu_irq, smp_cpus);
>  
>      slavio_serial_ms_kbd_init(hwdef->ms_kb_base, slavio_irq[14],
> -                              display_type == DT_NOGRAPHIC, ESCC_CLOCK, 1);
> +                              machine->nographic, ESCC_CLOCK, 1);
>      /* Slavio TTYA (base+4, Linux ttyS0) is the first QEMU serial device
>         Slavio TTYB (base+0, Linux ttyS1) is the second QEMU serial device */
>      escc_init(hwdef->serial_base, slavio_irq[15], slavio_irq[15],
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3e9a92c..1353f8a 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -120,6 +120,7 @@ struct MachineState {
>      char *firmware;
>      bool iommu;
>      bool suppress_vmdesc;
> +    bool nographic;
>  
>      ram_addr_t ram_size;
>      ram_addr_t maxram_size;
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 0f4e520..f92a53c 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -139,7 +139,6 @@ typedef enum DisplayType
>      DT_SDL,
>      DT_COCOA,
>      DT_GTK,
> -    DT_NOGRAPHIC,
>      DT_NONE,
>  } DisplayType;
>  
> diff --git a/vl.c b/vl.c
> index 57064ea..5d0228b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2980,6 +2980,7 @@ int main(int argc, char **argv, char **envp)
>      int show_vnc_port = 0;
>      bool defconfig = true;
>      bool userconfig = true;
> +    bool nographic = false;
>      const char *log_mask = NULL;
>      const char *log_file = NULL;
>      const char *trace_events = NULL;
> @@ -3226,7 +3227,8 @@ int main(int argc, char **argv, char **envp)
>                  display_type = select_display(optarg);
>                  break;
>              case QEMU_OPTION_nographic:
> -                display_type = DT_NOGRAPHIC;
> +                nographic = true;
> +                display_type = DT_NONE;
>                  break;
>              case QEMU_OPTION_curses:
>  #ifdef CONFIG_CURSES
> @@ -4177,7 +4179,7 @@ int main(int argc, char **argv, char **envp)
>           * -nographic _and_ redirects all ports explicitly - this is valid
>           * usage, -nographic is just a no-op in this case.
>           */
> -        if (display_type == DT_NOGRAPHIC
> +        if (nographic
>              && (default_parallel || default_serial
>                  || default_monitor || default_virtcon)) {
>              error_report("-nographic cannot be used with -daemonize");
> @@ -4191,7 +4193,7 @@ int main(int argc, char **argv, char **envp)
>  #endif
>      }
>  
> -    if (display_type == DT_NOGRAPHIC) {
> +    if (nographic) {
>          if (default_parallel)
>              add_device_config(DEV_PARALLEL, "null");
>          if (default_serial && default_monitor) {
> @@ -4510,6 +4512,7 @@ int main(int argc, char **argv, char **envp)
>      current_machine->ram_slots = ram_slots;
>      current_machine->boot_order = boot_order;
>      current_machine->cpu_model = cpu_model;
> +    current_machine->nographic = nographic;
>  
>      machine_class->init(current_machine);
>  
> @@ -4560,9 +4563,6 @@ int main(int argc, char **argv, char **envp)
>  
>      /* init local displays */
>      switch (display_type) {
> -    case DT_NOGRAPHIC:
> -        (void)ds;    /* avoid warning if no display is configured */
> -        break;
>      case DT_CURSES:
>          curses_display_init(ds, full_screen);
>          break;
>