All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.