qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars
@ 2015-11-11 19:09 Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 01/12] vl: Add DT_COCOA DisplayType value Eduardo Habkost
                   ` (12 more replies)
  0 siblings, 13 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

* Clean up the graphics initialization code to reduce the
  number of #ifdefs;
* Remove the display_type == DT_NOGRAPHIC checks from hardware
  emulation code;
* Make the display_type global variable a local variable on
  main();
* Make the display_remote static variable a local variable on
  main().

Eduardo Habkost (12):
  vl: Add DT_COCOA DisplayType value
  stubs: Add VNC initialization stubs
  stubs: curses_display_init() stub
  stubs: SDL initialization stubs
  stubs: cocoa_display_init() stub
  stubs: gtk_display_init() stub
  stubs: spice initialization stubs
  milkymist: Move DT_NOGRAPHIC check outside milkymist_tmu2_create()
  vl: Replace DT_NOGRAPHIC with MachineState field
  vl: Make display_type a local variable
  vl: Move DisplayType typedef to vl.c
  vl: Make display_remote a local variable

 hw/lm32/milkymist-hw.h  |  4 ----
 hw/lm32/milkymist.c     |  4 +++-
 hw/nvram/fw_cfg.c       |  6 +++--
 hw/sparc/sun4m.c        |  2 +-
 include/hw/boards.h     |  1 +
 include/sysemu/sysemu.h | 11 ---------
 include/ui/console.h    |  4 ++--
 stubs/Makefile.objs     |  5 ++++
 stubs/cocoa.c           | 10 ++++++++
 stubs/curses.c          | 10 ++++++++
 stubs/gtk.c             | 10 ++++++++
 stubs/sdl.c             | 17 +++++++++++++
 stubs/spice.c           | 13 ++++++++++
 stubs/vnc.c             | 22 +++++++++++++++++
 vl.c                    | 63 +++++++++++++++++++------------------------------
 15 files changed, 122 insertions(+), 60 deletions(-)
 create mode 100644 stubs/cocoa.c
 create mode 100644 stubs/curses.c
 create mode 100644 stubs/gtk.c
 create mode 100644 stubs/sdl.c
 create mode 100644 stubs/spice.c
 create mode 100644 stubs/vnc.c

-- 
2.1.0

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 01/12] vl: Add DT_COCOA DisplayType value
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 02/12] stubs: Add VNC initialization stubs Eduardo Habkost
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Andreas Färber, Peter Maydell

Instead of reusing DT_SDL for Cocoa, use DT_COCOA to indicate
that a Cocoa display was requested.

configure already ensures CONFIG_COCOA and CONFIG_SDL are never
set at the same time. The only case where DT_SDL is used outside
a #ifdef CONFIG_SDL block is in the no_frame/alt_grab/ctrl_grab
check. That means the only user-visible change is that we will
start printing a warning if the SDL-specific options are used in
Cocoa mode. This is a bugfix, because no_frame/alt_grab/ctrl_grab
are not used by Cocoa code.

Cc: Andreas Färber <andreas.faerber@web.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/sysemu/sysemu.h | 1 +
 vl.c                    | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index f992494..0f4e520 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -137,6 +137,7 @@ typedef enum DisplayType
     DT_DEFAULT,
     DT_CURSES,
     DT_SDL,
+    DT_COCOA,
     DT_GTK,
     DT_NOGRAPHIC,
     DT_NONE,
diff --git a/vl.c b/vl.c
index 7d993a5..bba1b0a 100644
--- a/vl.c
+++ b/vl.c
@@ -4247,8 +4247,10 @@ int main(int argc, char **argv, char **envp)
     if (display_type == DT_DEFAULT && !display_remote) {
 #if defined(CONFIG_GTK)
         display_type = DT_GTK;
-#elif defined(CONFIG_SDL) || defined(CONFIG_COCOA)
+#elif defined(CONFIG_SDL)
         display_type = DT_SDL;
+#elif defined(CONFIG_COCOA)
+        display_type = DT_COCOA;
 #elif defined(CONFIG_VNC)
         vnc_parse("localhost:0,to=99,id=default", &error_abort);
         show_vnc_port = 1;
@@ -4588,7 +4590,7 @@ int main(int argc, char **argv, char **envp)
         sdl_display_init(ds, full_screen, no_frame);
         break;
 #elif defined(CONFIG_COCOA)
-    case DT_SDL:
+    case DT_COCOA:
         cocoa_display_init(ds, full_screen);
         break;
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 02/12] stubs: Add VNC initialization stubs
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 01/12] vl: Add DT_COCOA DisplayType value Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 03/12] stubs: curses_display_init() stub Eduardo Habkost
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

This reduces the number of CONFIG_VNC #ifdefs in the vl.c code.

The only user-visible difference is that this will make QEMU
complain about syntax when using "-display vnc" ("VNC requires a
display argument vnc=<display>") even if CONFIG_VNC is disabled.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/ui/console.h |  4 ++--
 stubs/Makefile.objs  |  1 +
 stubs/vnc.c          | 22 ++++++++++++++++++++++
 vl.c                 | 15 +--------------
 4 files changed, 26 insertions(+), 16 deletions(-)
 create mode 100644 stubs/vnc.c

diff --git a/include/ui/console.h b/include/ui/console.h
index c249db4..30e5305 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -413,11 +413,11 @@ void vnc_display_init(const char *id);
 void vnc_display_open(const char *id, Error **errp);
 void vnc_display_add_client(const char *id, int csock, bool skipauth);
 char *vnc_display_local_addr(const char *id);
+QemuOpts *vnc_parse(const char *str, Error **errp);
+int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
 #ifdef CONFIG_VNC
 int vnc_display_password(const char *id, const char *password);
 int vnc_display_pw_expire(const char *id, time_t expires);
-QemuOpts *vnc_parse(const char *str, Error **errp);
-int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
 #else
 static inline int vnc_display_password(const char *id, const char *password)
 {
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 330e1a4..f69ab8d 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -37,3 +37,4 @@ stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += target-monitor-defs.o
 stub-obj-y += vhost.o
+stub-obj-y += vnc.o
diff --git a/stubs/vnc.c b/stubs/vnc.c
new file mode 100644
index 0000000..de89858
--- /dev/null
+++ b/stubs/vnc.c
@@ -0,0 +1,22 @@
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "qemu/error-report.h"
+
+QemuOpts *vnc_parse(const char *str, Error **errp)
+{
+    error_setg(errp, "VNC support is disabled");
+    return NULL;
+}
+
+int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+    error_setg(errp, "VNC support is disabled");
+    return -1;
+}
+
+char *vnc_display_local_addr(const char *id)
+{
+    /* This must never be called if CONFIG_VNC is disabled */
+    error_report("VNC support is disabled");
+    abort();
+}
diff --git a/vl.c b/vl.c
index bba1b0a..0558e87 100644
--- a/vl.c
+++ b/vl.c
@@ -2145,7 +2145,6 @@ static DisplayType select_display(const char *p)
         exit(1);
 #endif
     } else if (strstart(p, "vnc", &opts)) {
-#ifdef CONFIG_VNC
         if (*opts == '=') {
             Error *err = NULL;
             if (vnc_parse(opts + 1, &err) == NULL) {
@@ -2156,10 +2155,6 @@ static DisplayType select_display(const char *p)
             error_report("VNC requires a display argument vnc=<display>");
             exit(1);
         }
-#else
-        error_report("VNC support is disabled");
-        exit(1);
-#endif
     } else if (strstart(p, "curses", &opts)) {
 #ifdef CONFIG_CURSES
         display = DT_CURSES;
@@ -2984,9 +2979,7 @@ int main(int argc, char **argv, char **envp)
     const char *qtest_log = NULL;
     const char *pid_file = NULL;
     const char *incoming = NULL;
-#ifdef CONFIG_VNC
     int show_vnc_port = 0;
-#endif
     bool defconfig = true;
     bool userconfig = true;
     const char *log_mask = NULL;
@@ -3726,17 +3719,12 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_vnc:
             {
-#ifdef CONFIG_VNC
                 Error *local_err = NULL;
 
                 if (vnc_parse(optarg, &local_err) == NULL) {
                     error_report_err(local_err);
                     exit(1);
                 }
-#else
-                error_report("VNC support is disabled");
-                exit(1);
-#endif
                 break;
             }
             case QEMU_OPTION_no_acpi:
@@ -4606,7 +4594,6 @@ int main(int argc, char **argv, char **envp)
     /* must be after terminal init, SDL library changes signal handlers */
     os_setup_signal_handling();
 
-#ifdef CONFIG_VNC
     /* init remote displays */
     qemu_opts_foreach(qemu_find_opts("vnc"),
                       vnc_init_func, NULL, NULL);
@@ -4615,7 +4602,7 @@ int main(int argc, char **argv, char **envp)
         printf("VNC server running on '%s'\n", ret);
         g_free(ret);
     }
-#endif
+
 #ifdef CONFIG_SPICE
     if (using_spice) {
         qemu_spice_display_init();
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 03/12] stubs: curses_display_init() stub
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 01/12] vl: Add DT_COCOA DisplayType value Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 02/12] stubs: Add VNC initialization stubs Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 04/12] stubs: SDL initialization stubs Eduardo Habkost
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

One less #ifdef in vl.c.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 stubs/Makefile.objs |  1 +
 stubs/curses.c      | 10 ++++++++++
 vl.c                |  2 --
 3 files changed, 11 insertions(+), 2 deletions(-)
 create mode 100644 stubs/curses.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index f69ab8d..df86bc6 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -38,3 +38,4 @@ stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += target-monitor-defs.o
 stub-obj-y += vhost.o
 stub-obj-y += vnc.o
+stub-obj-y += curses.o
diff --git a/stubs/curses.c b/stubs/curses.c
new file mode 100644
index 0000000..2115c77
--- /dev/null
+++ b/stubs/curses.c
@@ -0,0 +1,10 @@
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "qemu/error-report.h"
+
+void curses_display_init(DisplayState *ds, int full_screen)
+{
+    /* This must never be called if CONFIG_CURSES is disabled */
+    error_report("curses support is disabled");
+    abort();
+}
diff --git a/vl.c b/vl.c
index 0558e87..3ca1c11 100644
--- a/vl.c
+++ b/vl.c
@@ -4568,11 +4568,9 @@ int main(int argc, char **argv, char **envp)
     case DT_NOGRAPHIC:
         (void)ds;	/* avoid warning if no display is configured */
         break;
-#if defined(CONFIG_CURSES)
     case DT_CURSES:
         curses_display_init(ds, full_screen);
         break;
-#endif
 #if defined(CONFIG_SDL)
     case DT_SDL:
         sdl_display_init(ds, full_screen, no_frame);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 04/12] stubs: SDL initialization stubs
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (2 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 03/12] stubs: curses_display_init() stub Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 05/12] stubs: cocoa_display_init() stub Eduardo Habkost
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

This reduces the number of CONFIG_SDL #ifdefs in vl.c.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 stubs/Makefile.objs |  1 +
 stubs/sdl.c         | 17 +++++++++++++++++
 vl.c                |  6 ++----
 3 files changed, 20 insertions(+), 4 deletions(-)
 create mode 100644 stubs/sdl.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index df86bc6..3a6cd37 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@ stub-obj-y += target-monitor-defs.o
 stub-obj-y += vhost.o
 stub-obj-y += vnc.o
 stub-obj-y += curses.o
+stub-obj-y += sdl.o
diff --git a/stubs/sdl.c b/stubs/sdl.c
new file mode 100644
index 0000000..f5ab668
--- /dev/null
+++ b/stubs/sdl.c
@@ -0,0 +1,17 @@
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "qemu/error-report.h"
+
+void sdl_display_early_init(int opengl)
+{
+    /* This must never be called if CONFIG_SDL is disabled */
+    error_report("SDL support is disabled");
+    abort();
+}
+
+void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
+{
+    /* This must never be called if CONFIG_SDL is disabled */
+    error_report("SDL support is disabled");
+    abort();
+}
diff --git a/vl.c b/vl.c
index 3ca1c11..5292648 100644
--- a/vl.c
+++ b/vl.c
@@ -4261,11 +4261,10 @@ int main(int argc, char **argv, char **envp)
         early_gtk_display_init(request_opengl);
     }
 #endif
-#if defined(CONFIG_SDL)
     if (display_type == DT_SDL) {
         sdl_display_early_init(request_opengl);
     }
-#endif
+
     if (request_opengl == 1 && display_opengl == 0) {
 #if defined(CONFIG_OPENGL)
         error_report("OpenGL is not supported by the display");
@@ -4571,11 +4570,10 @@ int main(int argc, char **argv, char **envp)
     case DT_CURSES:
         curses_display_init(ds, full_screen);
         break;
-#if defined(CONFIG_SDL)
     case DT_SDL:
         sdl_display_init(ds, full_screen, no_frame);
         break;
-#elif defined(CONFIG_COCOA)
+#if defined(CONFIG_COCOA)
     case DT_COCOA:
         cocoa_display_init(ds, full_screen);
         break;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 05/12] stubs: cocoa_display_init() stub
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (3 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 04/12] stubs: SDL initialization stubs Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 06/12] stubs: gtk_display_init() stub Eduardo Habkost
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

One less #ifdef in vl.c.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 stubs/Makefile.objs |  1 +
 stubs/cocoa.c       | 10 ++++++++++
 vl.c                |  2 --
 3 files changed, 11 insertions(+), 2 deletions(-)
 create mode 100644 stubs/cocoa.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 3a6cd37..1c2d1e0 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -40,3 +40,4 @@ stub-obj-y += vhost.o
 stub-obj-y += vnc.o
 stub-obj-y += curses.o
 stub-obj-y += sdl.o
+stub-obj-y += cocoa.o
diff --git a/stubs/cocoa.c b/stubs/cocoa.c
new file mode 100644
index 0000000..ef07a8a
--- /dev/null
+++ b/stubs/cocoa.c
@@ -0,0 +1,10 @@
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "qemu/error-report.h"
+
+void cocoa_display_init(DisplayState *ds, int full_screen)
+{
+    /* This must never be called if CONFIG_COCA is disabled */
+    error_report("Cocoa support is disabled");
+    abort();
+}
diff --git a/vl.c b/vl.c
index 5292648..ea83e17 100644
--- a/vl.c
+++ b/vl.c
@@ -4573,11 +4573,9 @@ int main(int argc, char **argv, char **envp)
     case DT_SDL:
         sdl_display_init(ds, full_screen, no_frame);
         break;
-#if defined(CONFIG_COCOA)
     case DT_COCOA:
         cocoa_display_init(ds, full_screen);
         break;
-#endif
 #if defined(CONFIG_GTK)
     case DT_GTK:
         gtk_display_init(ds, full_screen, grab_on_hover);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 06/12] stubs: gtk_display_init() stub
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (4 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 05/12] stubs: cocoa_display_init() stub Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 07/12] stubs: spice initialization stubs Eduardo Habkost
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

This reduces the number of CONFIG_GTK #ifdefs in vl.c.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 stubs/Makefile.objs |  1 +
 stubs/gtk.c         | 10 ++++++++++
 vl.c                |  4 ----
 3 files changed, 11 insertions(+), 4 deletions(-)
 create mode 100644 stubs/gtk.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 1c2d1e0..46cfb39 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -41,3 +41,4 @@ stub-obj-y += vnc.o
 stub-obj-y += curses.o
 stub-obj-y += sdl.o
 stub-obj-y += cocoa.o
+stub-obj-y += gtk.o
diff --git a/stubs/gtk.c b/stubs/gtk.c
new file mode 100644
index 0000000..a46ef0c
--- /dev/null
+++ b/stubs/gtk.c
@@ -0,0 +1,10 @@
+#include "qemu-common.h"
+#include "ui/console.h"
+#include "qemu/error-report.h"
+
+void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
+{
+    /* This must never be called if CONFIG_GTK is disabled */
+    error_report("GTK support is disabled");
+    abort();
+}
diff --git a/vl.c b/vl.c
index ea83e17..d4191d6 100644
--- a/vl.c
+++ b/vl.c
@@ -151,9 +151,7 @@ int vga_interface_type = VGA_NONE;
 static int full_screen = 0;
 static int no_frame = 0;
 int no_quit = 0;
-#ifdef CONFIG_GTK
 static bool grab_on_hover;
-#endif
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
@@ -4576,11 +4574,9 @@ int main(int argc, char **argv, char **envp)
     case DT_COCOA:
         cocoa_display_init(ds, full_screen);
         break;
-#if defined(CONFIG_GTK)
     case DT_GTK:
         gtk_display_init(ds, full_screen, grab_on_hover);
         break;
-#endif
     default:
         break;
     }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 07/12] stubs: spice initialization stubs
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (5 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 06/12] stubs: gtk_display_init() stub Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 08/12] milkymist: Move DT_NOGRAPHIC check outside milkymist_tmu2_create() Eduardo Habkost
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Gerd Hoffmann

This reduces the number of CONFIG_SPICE #ifdefs in vl.c.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 stubs/spice.c | 13 +++++++++++++
 vl.c          |  4 ----
 2 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 stubs/spice.c

diff --git a/stubs/spice.c b/stubs/spice.c
new file mode 100644
index 0000000..49994a5
--- /dev/null
+++ b/stubs/spice.c
@@ -0,0 +1,13 @@
+#include "qemu-common.h"
+#include "ui/console.h"
+
+void qemu_spice_display_init(void)
+{
+    /* This must never be called if CONFIG_SPICE is disabled */
+    error_report("spice support is disabled");
+    abort();
+}
+
+void qemu_spice_init(void)
+{
+}
diff --git a/vl.c b/vl.c
index d4191d6..57064ea 100644
--- a/vl.c
+++ b/vl.c
@@ -4386,10 +4386,8 @@ int main(int argc, char **argv, char **envp)
 
     os_set_line_buffering();
 
-#ifdef CONFIG_SPICE
     /* spice needs the timers to be initialized by this point */
     qemu_spice_init();
-#endif
 
     cpu_ticks_init();
     if (icount_opts) {
@@ -4593,11 +4591,9 @@ int main(int argc, char **argv, char **envp)
         g_free(ret);
     }
 
-#ifdef CONFIG_SPICE
     if (using_spice) {
         qemu_spice_display_init();
     }
-#endif
 
     if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
         exit(1);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 08/12] milkymist: Move DT_NOGRAPHIC check outside milkymist_tmu2_create()
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (6 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 07/12] stubs: spice initialization stubs Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field Eduardo Habkost
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Michael Walle

DT_NOGRAPHIC handling will be moved to a MachineState field, and
it will be easier to change milkymist_init() to check that field.

Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/lm32/milkymist-hw.h | 4 ----
 hw/lm32/milkymist.c    | 4 +++-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/lm32/milkymist-hw.h b/hw/lm32/milkymist-hw.h
index c8dfb4d..f857d28 100644
--- a/hw/lm32/milkymist-hw.h
+++ b/hw/lm32/milkymist-hw.h
@@ -108,10 +108,6 @@ static inline DeviceState *milkymist_tmu2_create(hwaddr base,
     int nelements;
     int ver_major, ver_minor;
 
-    if (display_type == DT_NOGRAPHIC) {
-        return NULL;
-    }
-
     /* check that GLX will work */
     d = XOpenDisplay(NULL);
     if (d == NULL) {
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 13976b3..e46283a 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -163,7 +163,9 @@ 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]);
-    milkymist_tmu2_create(0x60007000, irq[9]);
+    if (display_type != DT_NOGRAPHIC) {
+        milkymist_tmu2_create(0x60007000, irq[9]);
+    }
     milkymist_minimac2_create(0x60008000, 0x30000000, irq[10], irq[11]);
     milkymist_softusb_create(0x6000f000, irq[15],
             0x20000000, 0x1000, 0x20020000, 0x2000);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (7 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 08/12] milkymist: Move DT_NOGRAPHIC check outside milkymist_tmu2_create() Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-12  9:48   ` Paolo Bonzini
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 10/12] vl: Make display_type a local variable Eduardo Habkost
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Blue Swirl, Paolo Bonzini, Michael Walle, Mark Cave-Ayland

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 <michael@walle.cc>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 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;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 10/12] vl: Make display_type a local variable
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (8 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 11/12] vl: Move DisplayType typedef to vl.c Eduardo Habkost
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

Now display_type is only used inside main(), and don't need to be a
global variable.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/sysemu/sysemu.h | 1 -
 vl.c                    | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index f92a53c..917f327 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -155,7 +155,6 @@ extern int vga_interface_type;
 extern int graphic_width;
 extern int graphic_height;
 extern int graphic_depth;
-extern DisplayType display_type;
 extern int display_opengl;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
diff --git a/vl.c b/vl.c
index 5d0228b..915fa29 100644
--- a/vl.c
+++ b/vl.c
@@ -132,7 +132,6 @@ static const char *data_dir[16];
 static int data_dir_idx;
 const char *bios_name = NULL;
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
-DisplayType display_type = DT_DEFAULT;
 int request_opengl = -1;
 int display_opengl;
 static int display_remote;
@@ -2981,6 +2980,7 @@ int main(int argc, char **argv, char **envp)
     bool defconfig = true;
     bool userconfig = true;
     bool nographic = false;
+    DisplayType display_type = DT_DEFAULT;
     const char *log_mask = NULL;
     const char *log_file = NULL;
     const char *trace_events = NULL;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 11/12] vl: Move DisplayType typedef to vl.c
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (9 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 10/12] vl: Make display_type a local variable Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 12/12] vl: Make display_remote a local variable Eduardo Habkost
  2015-11-12  9:46 ` [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Paolo Bonzini
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

Now the type is only used inside vl.c and doesn't need to be in a
header file.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/sysemu/sysemu.h | 10 ----------
 vl.c                    | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 917f327..3ed384f 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -132,16 +132,6 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
 
 int qemu_loadvm_state(QEMUFile *f);
 
-typedef enum DisplayType
-{
-    DT_DEFAULT,
-    DT_CURSES,
-    DT_SDL,
-    DT_COCOA,
-    DT_GTK,
-    DT_NONE,
-} DisplayType;
-
 extern int autostart;
 
 typedef enum {
diff --git a/vl.c b/vl.c
index 915fa29..0ea0934 100644
--- a/vl.c
+++ b/vl.c
@@ -2074,6 +2074,16 @@ static void select_vgahw (const char *p)
     }
 }
 
+typedef enum DisplayType
+{
+    DT_DEFAULT,
+    DT_CURSES,
+    DT_SDL,
+    DT_COCOA,
+    DT_GTK,
+    DT_NONE,
+} DisplayType;
+
 static DisplayType select_display(const char *p)
 {
     const char *opts;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PATCH 12/12] vl: Make display_remote a local variable
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (10 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 11/12] vl: Move DisplayType typedef to vl.c Eduardo Habkost
@ 2015-11-11 19:09 ` Eduardo Habkost
  2015-11-12  9:46 ` [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Paolo Bonzini
  12 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-11 19:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini

The variable is used only inside main(), so it can be local.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 0ea0934..8c8b02c 100644
--- a/vl.c
+++ b/vl.c
@@ -134,7 +134,6 @@ const char *bios_name = NULL;
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 int request_opengl = -1;
 int display_opengl;
-static int display_remote;
 const char* keyboard_layout = NULL;
 ram_addr_t ram_size;
 const char *mem_path = NULL;
@@ -2991,6 +2990,7 @@ int main(int argc, char **argv, char **envp)
     bool userconfig = true;
     bool nographic = false;
     DisplayType display_type = DT_DEFAULT;
+    int display_remote = 0;
     const char *log_mask = NULL;
     const char *log_file = NULL;
     const char *trace_events = NULL;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars
  2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
                   ` (11 preceding siblings ...)
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 12/12] vl: Make display_remote a local variable Eduardo Habkost
@ 2015-11-12  9:46 ` Paolo Bonzini
  12 siblings, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-11-12  9:46 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel



On 11/11/2015 20:09, Eduardo Habkost wrote:
> * Clean up the graphics initialization code to reduce the
>   number of #ifdefs;
> * Remove the display_type == DT_NOGRAPHIC checks from hardware
>   emulation code;
> * Make the display_type global variable a local variable on
>   main();
> * Make the display_remote static variable a local variable on
>   main().
> 
> Eduardo Habkost (12):
>   vl: Add DT_COCOA DisplayType value
>   stubs: Add VNC initialization stubs
>   stubs: curses_display_init() stub
>   stubs: SDL initialization stubs
>   stubs: cocoa_display_init() stub
>   stubs: gtk_display_init() stub
>   stubs: spice initialization stubs
>   milkymist: Move DT_NOGRAPHIC check outside milkymist_tmu2_create()
>   vl: Replace DT_NOGRAPHIC with MachineState field
>   vl: Make display_type a local variable
>   vl: Move DisplayType typedef to vl.c
>   vl: Make display_remote a local variable
> 
>  hw/lm32/milkymist-hw.h  |  4 ----
>  hw/lm32/milkymist.c     |  4 +++-
>  hw/nvram/fw_cfg.c       |  6 +++--
>  hw/sparc/sun4m.c        |  2 +-
>  include/hw/boards.h     |  1 +
>  include/sysemu/sysemu.h | 11 ---------
>  include/ui/console.h    |  4 ++--
>  stubs/Makefile.objs     |  5 ++++
>  stubs/cocoa.c           | 10 ++++++++
>  stubs/curses.c          | 10 ++++++++
>  stubs/gtk.c             | 10 ++++++++
>  stubs/sdl.c             | 17 +++++++++++++
>  stubs/spice.c           | 13 ++++++++++
>  stubs/vnc.c             | 22 +++++++++++++++++
>  vl.c                    | 63 +++++++++++++++++++------------------------------
>  15 files changed, 122 insertions(+), 60 deletions(-)
>  create mode 100644 stubs/cocoa.c
>  create mode 100644 stubs/curses.c
>  create mode 100644 stubs/gtk.c
>  create mode 100644 stubs/sdl.c
>  create mode 100644 stubs/spice.c
>  create mode 100644 stubs/vnc.c

Interesting.  This wasn't how stubs were meant to be used, but I cannot
formulate any objection that makes sense. :)

However, please move the new files to stubs/ui/.

I'll review the DT_NOGRAPHIC changes shortly.

Paolo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-11 19:09 ` [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field Eduardo Habkost
@ 2015-11-12  9:48   ` Paolo Bonzini
  2015-11-12 19:44     ` Eduardo Habkost
  2015-11-13 11:49     ` Peter Maydell
  0 siblings, 2 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-11-12  9:48 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel; +Cc: Blue Swirl, Michael Walle, Mark Cave-Ayland



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 <michael@walle.cc>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

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;
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-12  9:48   ` Paolo Bonzini
@ 2015-11-12 19:44     ` Eduardo Habkost
  2015-11-13  9:56       ` Paolo Bonzini
  2015-11-13 11:49     ` Peter Maydell
  1 sibling, 1 reply; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-12 19:44 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Blue Swirl, Michael Walle, Mark Cave-Ayland, qemu-devel

On Thu, Nov 12, 2015 at 10:48:12AM +0100, Paolo Bonzini wrote:
> 
> 
> 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 <michael@walle.cc>
> > Cc: Blue Swirl <blauwirbel@gmail.com>
> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Can you add a QOM property too, so that "-machine graphics=yes|no" can
> be used?

I can, but I would like to clarify the expected semantics. With
the -machine option, we would have:

* -display, which affects only the display UI.
* -nographic, which affects:
  * The display UI;
  * Hardware emulation;
  * serial/paralllel/virtioconsole output redirection.
* -machine graphics=no, which would affect only hardware
  emulation.

Is that correct?

-- 
Eduardo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-12 19:44     ` Eduardo Habkost
@ 2015-11-13  9:56       ` Paolo Bonzini
  2015-11-13 12:22         ` Markus Armbruster
  0 siblings, 1 reply; 21+ messages in thread
From: Paolo Bonzini @ 2015-11-13  9:56 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Blue Swirl, Michael Walle, Mark Cave-Ayland, qemu-devel

> > Can you add a QOM property too, so that "-machine graphics=yes|no" can
> > be used?
> 
> I can, but I would like to clarify the expected semantics. With
> the -machine option, we would have:
> 
> * -display, which affects only the display UI.
> * -nographic, which affects:
>   * The display UI;
>   * Hardware emulation;
>   * serial/paralllel/virtioconsole output redirection.
> * -machine graphics=no, which would affect only hardware
>   emulation.
> 
> Is that correct?

Yes.  In the case of PC, it might also add "-device sga" unless -nodefaults
(or "-device sga") is specified.  But that's left for later.

Paolo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-12  9:48   ` Paolo Bonzini
  2015-11-12 19:44     ` Eduardo Habkost
@ 2015-11-13 11:49     ` Peter Maydell
  2015-11-13 13:01       ` Paolo Bonzini
  2015-11-13 15:13       ` Eduardo Habkost
  1 sibling, 2 replies; 21+ messages in thread
From: Peter Maydell @ 2015-11-13 11:49 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Blue Swirl, Michael Walle, Mark Cave-Ayland, Eduardo Habkost,
	QEMU Developers

On 12 November 2015 at 09:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> 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 <michael@walle.cc>
>> Cc: Blue Swirl <blauwirbel@gmail.com>
>> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> Can you add a QOM property too, so that "-machine graphics=yes|no" can
> be used?

We already have both '-nographic' and '-display none'.
I think adding yet another way to turn off graphics which isn't
the same as either of our existing command line options would
worsen this confusion...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-13  9:56       ` Paolo Bonzini
@ 2015-11-13 12:22         ` Markus Armbruster
  0 siblings, 0 replies; 21+ messages in thread
From: Markus Armbruster @ 2015-11-13 12:22 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Blue Swirl, Michael Walle, Mark Cave-Ayland, Eduardo Habkost,
	qemu-devel

Paolo Bonzini <pbonzini@redhat.com> writes:

>> > Can you add a QOM property too, so that "-machine graphics=yes|no" can
>> > be used?
>> 
>> I can, but I would like to clarify the expected semantics. With
>> the -machine option, we would have:
>> 
>> * -display, which affects only the display UI.
>> * -nographic, which affects:
>>   * The display UI;
>>   * Hardware emulation;
>>   * serial/paralllel/virtioconsole output redirection.
>> * -machine graphics=no, which would affect only hardware
>>   emulation.

Like usb=, this is a request to board code to enable/disable an optional
component.

>> Is that correct?
>
> Yes.  In the case of PC, it might also add "-device sga" unless -nodefaults
> (or "-device sga") is specified.  But that's left for later.

I figure -nographic should then set -machine graphic=no to do its
hardware emulation part.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-13 11:49     ` Peter Maydell
@ 2015-11-13 13:01       ` Paolo Bonzini
  2015-11-13 15:13       ` Eduardo Habkost
  1 sibling, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2015-11-13 13:01 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Blue Swirl, Michael Walle, Mark Cave-Ayland, Eduardo Habkost,
	QEMU Developers



On 13/11/2015 12:49, Peter Maydell wrote:
> On 12 November 2015 at 09:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> 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 <michael@walle.cc>
>>> Cc: Blue Swirl <blauwirbel@gmail.com>
>>> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> Can you add a QOM property too, so that "-machine graphics=yes|no" can
>> be used?
> 
> We already have both '-nographic' and '-display none'.
> I think adding yet another way to turn off graphics which isn't
> the same as either of our existing command line options would
> worsen this confusion...

I proposed the property exactly so that -nographic becomes the same as
"-display none -machine graphics=no -serial mon:stdio".

Eduardo's patches achieve that at thecode level, but not at the command
line level.

Paolo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field
  2015-11-13 11:49     ` Peter Maydell
  2015-11-13 13:01       ` Paolo Bonzini
@ 2015-11-13 15:13       ` Eduardo Habkost
  1 sibling, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2015-11-13 15:13 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Blue Swirl, Paolo Bonzini, Michael Walle, Mark Cave-Ayland,
	QEMU Developers

On Fri, Nov 13, 2015 at 11:49:33AM +0000, Peter Maydell wrote:
> On 12 November 2015 at 09:48, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> >
> > 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 <michael@walle.cc>
> >> Cc: Blue Swirl <blauwirbel@gmail.com>
> >> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >
> > Can you add a QOM property too, so that "-machine graphics=yes|no" can
> > be used?
> 
> We already have both '-nographic' and '-display none'.

-display none is not a way to turn off graphics hardware
emulation, it is just a way to control QEMU's GUI.

> I think adding yet another way to turn off graphics which isn't
> the same as either of our existing command line options would
> worsen this confusion...

I blame the confusion on the fact that "-nographic" does too much
(it affects hardware emulation, QEMU's GUI, and device output
redirection at the same time).

-- 
Eduardo

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2015-11-13 15:13 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-11 19:09 [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 01/12] vl: Add DT_COCOA DisplayType value Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 02/12] stubs: Add VNC initialization stubs Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 03/12] stubs: curses_display_init() stub Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 04/12] stubs: SDL initialization stubs Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 05/12] stubs: cocoa_display_init() stub Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 06/12] stubs: gtk_display_init() stub Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 07/12] stubs: spice initialization stubs Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 08/12] milkymist: Move DT_NOGRAPHIC check outside milkymist_tmu2_create() Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 09/12] vl: Replace DT_NOGRAPHIC with MachineState field Eduardo Habkost
2015-11-12  9:48   ` Paolo Bonzini
2015-11-12 19:44     ` Eduardo Habkost
2015-11-13  9:56       ` Paolo Bonzini
2015-11-13 12:22         ` Markus Armbruster
2015-11-13 11:49     ` Peter Maydell
2015-11-13 13:01       ` Paolo Bonzini
2015-11-13 15:13       ` Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 10/12] vl: Make display_type a local variable Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 11/12] vl: Move DisplayType typedef to vl.c Eduardo Habkost
2015-11-11 19:09 ` [Qemu-devel] [PATCH 12/12] vl: Make display_remote a local variable Eduardo Habkost
2015-11-12  9:46 ` [Qemu-devel] [PATCH 00/12] vl: graphics stubs + #ifdef cleanup, eliminate some global vars Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).