* [Qemu-devel] [6352] Fix character devices after DisplayState refactoring
@ 2009-01-16 20:23 Anthony Liguori
2009-01-16 21:43 ` [Qemu-devel] " Jan Kiszka
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Anthony Liguori @ 2009-01-16 20:23 UTC (permalink / raw)
To: qemu-devel
Revision: 6352
http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6352
Author: aliguori
Date: 2009-01-16 20:23:27 +0000 (Fri, 16 Jan 2009)
Log Message:
-----------
Fix character devices after DisplayState refactoring
The DisplayState refactoring changed the machine init function to create a
DisplayState for each VGA device instead of being passed an existing
DisplayState. This change is critical to enable multiple graphics device
support.
Unfortunately, the serial/parallel/console code is structured today to run
before machine init to fill out the CharDriverState table which the machine
init function uses to determine whether to create the required devices.
Since a 'vc' is a type of CharDriverState, the CharDriverState code requires
that a DisplayState exist before it runs creating a circular dependency.
To fix this, this splits the creation of the initial CharDriverState from
the initialization of the text console. We can then in a second step associate
a DisplayState with all TextConsoles. This allows us to create the
CharDriverState's first, machine init, then associate the TextConsoles with
a DisplayState.
This code screams for more cleanup.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Modified Paths:
--------------
trunk/console.c
trunk/console.h
trunk/qemu-char.c
trunk/vl.c
Modified: trunk/console.c
===================================================================
--- trunk/console.c 2009-01-16 20:07:19 UTC (rev 6351)
+++ trunk/console.c 2009-01-16 20:23:27 UTC (rev 6352)
@@ -1285,17 +1285,17 @@
}
}
-CharDriverState *text_console_init(DisplayState *ds, const char *p)
+static int n_text_consoles;
+static CharDriverState *text_consoles[128];
+static char *text_console_strs[128];
+
+static void text_console_do_init(CharDriverState *chr, DisplayState *ds, const char *p)
{
- CharDriverState *chr;
TextConsole *s;
unsigned width;
unsigned height;
static int color_inited;
- chr = qemu_mallocz(sizeof(CharDriverState));
- if (!chr)
- return NULL;
s = new_console(ds, (p == 0) ? TEXT_CONSOLE : TEXT_CONSOLE_FIXED_SIZE);
if (!s) {
free(chr);
@@ -1352,7 +1352,6 @@
s->t_attrib_default.unvisible = 0;
s->t_attrib_default.fgcol = COLOR_WHITE;
s->t_attrib_default.bgcol = COLOR_BLACK;
-
/* set current text attributes to default */
s->t_attrib = s->t_attrib_default;
text_console_resize(s);
@@ -1362,6 +1361,37 @@
return chr;
}
+CharDriverState *text_console_init(const char *p)
+{
+ CharDriverState *chr;
+
+ chr = qemu_mallocz(sizeof(CharDriverState));
+ if (!chr)
+ return NULL;
+
+ if (n_text_consoles == 128) {
+ fprintf(stderr, "Too many text consoles\n");
+ exit(1);
+ }
+ text_consoles[n_text_consoles] = chr;
+ text_console_strs[n_text_consoles] = p ? qemu_strdup(p) : NULL;
+ n_text_consoles++;
+
+ return chr;
+}
+
+void text_consoles_set_display(DisplayState *ds)
+{
+ int i;
+
+ for (i = 0; i < n_text_consoles; i++) {
+ text_console_do_init(text_consoles[i], ds, text_console_strs[i]);
+ qemu_free(text_console_strs[i]);
+ }
+
+ n_text_consoles = 0;
+}
+
void qemu_console_resize(DisplayState *ds, int width, int height)
{
TextConsole *s = get_graphic_console();
Modified: trunk/console.h
===================================================================
--- trunk/console.h 2009-01-16 20:07:19 UTC (rev 6351)
+++ trunk/console.h 2009-01-16 20:23:27 UTC (rev 6352)
@@ -265,7 +265,8 @@
int is_graphic_console(void);
int is_fixedsize_console(void);
-CharDriverState *text_console_init(DisplayState *ds, const char *p);
+CharDriverState *text_console_init(const char *p);
+void text_consoles_set_display(DisplayState *ds);
void console_select(unsigned int index);
void console_color_init(DisplayState *ds);
void qemu_console_resize(DisplayState *ds, int width, int height);
Modified: trunk/qemu-char.c
===================================================================
--- trunk/qemu-char.c 2009-01-16 20:07:19 UTC (rev 6351)
+++ trunk/qemu-char.c 2009-01-16 20:23:27 UTC (rev 6352)
@@ -2128,10 +2128,10 @@
CharDriverState *chr;
if (!strcmp(filename, "vc")) {
- chr = text_console_init(get_displaystate(), 0);
+ chr = text_console_init(0);
} else
if (strstart(filename, "vc:", &p)) {
- chr = text_console_init(get_displaystate(), p);
+ chr = text_console_init(p);
} else
if (!strcmp(filename, "null")) {
chr = qemu_chr_open_null();
Modified: trunk/vl.c
===================================================================
--- trunk/vl.c 2009-01-16 20:07:19 UTC (rev 6351)
+++ trunk/vl.c 2009-01-16 20:23:27 UTC (rev 6352)
@@ -5461,6 +5461,48 @@
}
}
+ for(i = 0; i < MAX_SERIAL_PORTS; i++) {
+ const char *devname = serial_devices[i];
+ if (devname && strcmp(devname, "none")) {
+ char label[32];
+ snprintf(label, sizeof(label), "serial%d", i);
+ serial_hds[i] = qemu_chr_open(label, devname);
+ if (!serial_hds[i]) {
+ fprintf(stderr, "qemu: could not open serial device '%s'\n",
+ devname);
+ exit(1);
+ }
+ }
+ }
+
+ for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
+ const char *devname = parallel_devices[i];
+ if (devname && strcmp(devname, "none")) {
+ char label[32];
+ snprintf(label, sizeof(label), "parallel%d", i);
+ parallel_hds[i] = qemu_chr_open(label, devname);
+ if (!parallel_hds[i]) {
+ fprintf(stderr, "qemu: could not open parallel device '%s'\n",
+ devname);
+ exit(1);
+ }
+ }
+ }
+
+ for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
+ const char *devname = virtio_consoles[i];
+ if (devname && strcmp(devname, "none")) {
+ char label[32];
+ snprintf(label, sizeof(label), "virtcon%d", i);
+ virtcon_hds[i] = qemu_chr_open(label, devname);
+ if (!virtcon_hds[i]) {
+ fprintf(stderr, "qemu: could not open virtio console '%s'\n",
+ devname);
+ exit(1);
+ }
+ }
+ }
+
machine->init(ram_size, vga_ram_size, boot_devices,
kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
@@ -5529,6 +5571,8 @@
dcl = dcl->next;
}
+ text_consoles_set_display(display_state);
+
if (monitor_device) {
monitor_hd = qemu_chr_open("monitor", monitor_device);
if (!monitor_hd) {
@@ -5543,12 +5587,6 @@
if (devname && strcmp(devname, "none")) {
char label[32];
snprintf(label, sizeof(label), "serial%d", i);
- serial_hds[i] = qemu_chr_open(label, devname);
- if (!serial_hds[i]) {
- fprintf(stderr, "qemu: could not open serial device '%s'\n",
- devname);
- exit(1);
- }
if (strstart(devname, "vc", 0))
qemu_chr_printf(serial_hds[i], "serial%d console\r\n", i);
}
@@ -5559,12 +5597,6 @@
if (devname && strcmp(devname, "none")) {
char label[32];
snprintf(label, sizeof(label), "parallel%d", i);
- parallel_hds[i] = qemu_chr_open(label, devname);
- if (!parallel_hds[i]) {
- fprintf(stderr, "qemu: could not open parallel device '%s'\n",
- devname);
- exit(1);
- }
if (strstart(devname, "vc", 0))
qemu_chr_printf(parallel_hds[i], "parallel%d console\r\n", i);
}
@@ -5572,15 +5604,9 @@
for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) {
const char *devname = virtio_consoles[i];
- if (devname && strcmp(devname, "none")) {
+ if (virtcon_hds[i] && devname) {
char label[32];
snprintf(label, sizeof(label), "virtcon%d", i);
- virtcon_hds[i] = qemu_chr_open(label, devname);
- if (!virtcon_hds[i]) {
- fprintf(stderr, "qemu: could not open virtio console '%s'\n",
- devname);
- exit(1);
- }
if (strstart(devname, "vc", 0))
qemu_chr_printf(virtcon_hds[i], "virtio console%d\r\n", i);
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [6352] Fix character devices after DisplayState refactoring
2009-01-16 20:23 [Qemu-devel] [6352] Fix character devices after DisplayState refactoring Anthony Liguori
@ 2009-01-16 21:43 ` Jan Kiszka
2009-01-16 21:54 ` Anthony Liguori
2009-01-17 0:58 ` [Qemu-devel] " Stuart Brady
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2009-01-16 21:43 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]
Anthony Liguori wrote:
> Revision: 6352
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6352
> Author: aliguori
> Date: 2009-01-16 20:23:27 +0000 (Fri, 16 Jan 2009)
>
> Log Message:
> -----------
> Fix character devices after DisplayState refactoring
>
> The DisplayState refactoring changed the machine init function to create a
> DisplayState for each VGA device instead of being passed an existing
> DisplayState. This change is critical to enable multiple graphics device
> support.
>
> Unfortunately, the serial/parallel/console code is structured today to run
> before machine init to fill out the CharDriverState table which the machine
> init function uses to determine whether to create the required devices.
>
> Since a 'vc' is a type of CharDriverState, the CharDriverState code requires
> that a DisplayState exist before it runs creating a circular dependency.
>
> To fix this, this splits the creation of the initial CharDriverState from
> the initialization of the text console. We can then in a second step associate
> a DisplayState with all TextConsoles. This allows us to create the
> CharDriverState's first, machine init, then associate the TextConsoles with
> a DisplayState.
>
> This code screams for more cleanup.
...and a fix to make the monitor show up on the second vc again. Any
immediate idea what went wrong?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] Re: [6352] Fix character devices after DisplayState refactoring
2009-01-16 21:43 ` [Qemu-devel] " Jan Kiszka
@ 2009-01-16 21:54 ` Anthony Liguori
0 siblings, 0 replies; 8+ messages in thread
From: Anthony Liguori @ 2009-01-16 21:54 UTC (permalink / raw)
To: qemu-devel
Jan Kiszka wrote:
> Anthony Liguori wrote:
>
>> Revision: 6352
>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6352
>> Author: aliguori
>> Date: 2009-01-16 20:23:27 +0000 (Fri, 16 Jan 2009)
>>
>> Log Message:
>> -----------
>> Fix character devices after DisplayState refactoring
>>
>> The DisplayState refactoring changed the machine init function to create a
>> DisplayState for each VGA device instead of being passed an existing
>> DisplayState. This change is critical to enable multiple graphics device
>> support.
>>
>> Unfortunately, the serial/parallel/console code is structured today to run
>> before machine init to fill out the CharDriverState table which the machine
>> init function uses to determine whether to create the required devices.
>>
>> Since a 'vc' is a type of CharDriverState, the CharDriverState code requires
>> that a DisplayState exist before it runs creating a circular dependency.
>>
>> To fix this, this splits the creation of the initial CharDriverState from
>> the initialization of the text console. We can then in a second step associate
>> a DisplayState with all TextConsoles. This allows us to create the
>> CharDriverState's first, machine init, then associate the TextConsoles with
>> a DisplayState.
>>
>> This code screams for more cleanup.
>>
>
> ...and a fix to make the monitor show up on the second vc again.
Just got that one.
> Any immediate idea what went wrong?
>
Not enough testing.
Regards,
Anthony Liguori
> Jan
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [6352] Fix character devices after DisplayState refactoring
2009-01-16 20:23 [Qemu-devel] [6352] Fix character devices after DisplayState refactoring Anthony Liguori
2009-01-16 21:43 ` [Qemu-devel] " Jan Kiszka
@ 2009-01-17 0:58 ` Stuart Brady
2009-01-17 22:11 ` Stefan Weil
2009-01-19 11:26 ` Stefano Stabellini
3 siblings, 0 replies; 8+ messages in thread
From: Stuart Brady @ 2009-01-17 0:58 UTC (permalink / raw)
To: qemu-devel
On Fri, Jan 16, 2009 at 08:23:28PM +0000, Anthony Liguori wrote:
> Log Message:
> -----------
> Fix character devices after DisplayState refactoring
FWIW, I've noticed that malta_fpga_init segfaults when calling
qemu_chr_printf() right after calling qemu_chr_open().
Cheers,
--
Stuart Brady
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [6352] Fix character devices after DisplayState refactoring
2009-01-16 20:23 [Qemu-devel] [6352] Fix character devices after DisplayState refactoring Anthony Liguori
2009-01-16 21:43 ` [Qemu-devel] " Jan Kiszka
2009-01-17 0:58 ` [Qemu-devel] " Stuart Brady
@ 2009-01-17 22:11 ` Stefan Weil
2009-01-18 14:09 ` Aurelien Jarno
2009-01-19 11:26 ` Stefano Stabellini
3 siblings, 1 reply; 8+ messages in thread
From: Stefan Weil @ 2009-01-17 22:11 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Anthony Liguori schrieb:
> Revision: 6352
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6352
> Author: aliguori
> Date: 2009-01-16 20:23:27 +0000 (Fri, 16 Jan 2009)
>
> Log Message:
> -----------
> Fix character devices after DisplayState refactoring
>
> The DisplayState refactoring changed the machine init function to create a
> DisplayState for each VGA device instead of being passed an existing
> DisplayState. This change is critical to enable multiple graphics device
> support.
>
> Unfortunately, the serial/parallel/console code is structured today to run
> before machine init to fill out the CharDriverState table which the
> machine
> init function uses to determine whether to create the required devices.
>
> Since a 'vc' is a type of CharDriverState, the CharDriverState code
> requires
> that a DisplayState exist before it runs creating a circular dependency.
>
> To fix this, this splits the creation of the initial CharDriverState from
> the initialization of the text console. We can then in a second step
> associate
> a DisplayState with all TextConsoles. This allows us to create the
> CharDriverState's first, machine init, then associate the TextConsoles
> with
> a DisplayState.
>
> This code screams for more cleanup.
Yes, it screams for more cleanups.
MIPS Malta also no longer works, same reason as for the serial code:
Malta creates a character device during machine init and writes some
characters to it.
This is now impossible because qemu_chr_printf can no longer be called
before text_consoles_set_display was called.
Regards
Stefan Weil
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [6352] Fix character devices after DisplayState refactoring
2009-01-17 22:11 ` Stefan Weil
@ 2009-01-18 14:09 ` Aurelien Jarno
0 siblings, 0 replies; 8+ messages in thread
From: Aurelien Jarno @ 2009-01-18 14:09 UTC (permalink / raw)
To: qemu-devel
On Sat, Jan 17, 2009 at 11:11:26PM +0100, Stefan Weil wrote:
> Anthony Liguori schrieb:
> > Revision: 6352
> > http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6352
> > Author: aliguori
> > Date: 2009-01-16 20:23:27 +0000 (Fri, 16 Jan 2009)
> >
> > Log Message:
> > -----------
> > Fix character devices after DisplayState refactoring
> >
> > The DisplayState refactoring changed the machine init function to create a
> > DisplayState for each VGA device instead of being passed an existing
> > DisplayState. This change is critical to enable multiple graphics device
> > support.
> >
> > Unfortunately, the serial/parallel/console code is structured today to run
> > before machine init to fill out the CharDriverState table which the
> > machine
> > init function uses to determine whether to create the required devices.
> >
> > Since a 'vc' is a type of CharDriverState, the CharDriverState code
> > requires
> > that a DisplayState exist before it runs creating a circular dependency.
> >
> > To fix this, this splits the creation of the initial CharDriverState from
> > the initialization of the text console. We can then in a second step
> > associate
> > a DisplayState with all TextConsoles. This allows us to create the
> > CharDriverState's first, machine init, then associate the TextConsoles
> > with
> > a DisplayState.
> >
> > This code screams for more cleanup.
>
> Yes, it screams for more cleanups.
>
> MIPS Malta also no longer works, same reason as for the serial code:
>
> Malta creates a character device during machine init and writes some
> characters to it.
> This is now impossible because qemu_chr_printf can no longer be called
> before text_consoles_set_display was called.
>
I have committed a fix in revision 6365. It is a bit ugly, don't
hesistate to commit a better fix.
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [6352] Fix character devices after DisplayState refactoring
2009-01-16 20:23 [Qemu-devel] [6352] Fix character devices after DisplayState refactoring Anthony Liguori
` (2 preceding siblings ...)
2009-01-17 22:11 ` Stefan Weil
@ 2009-01-19 11:26 ` Stefano Stabellini
2009-01-19 22:11 ` Anthony Liguori
3 siblings, 1 reply; 8+ messages in thread
From: Stefano Stabellini @ 2009-01-19 11:26 UTC (permalink / raw)
To: qemu-devel
Anthony Liguori wrote:
> Revision: 6352
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6352
> Author: aliguori
> Date: 2009-01-16 20:23:27 +0000 (Fri, 16 Jan 2009)
>
> Log Message:
> -----------
> Fix character devices after DisplayState refactoring
>
> The DisplayState refactoring changed the machine init function to create a
> DisplayState for each VGA device instead of being passed an existing
> DisplayState. This change is critical to enable multiple graphics device
> support.
>
> Unfortunately, the serial/parallel/console code is structured today to run
> before machine init to fill out the CharDriverState table which the machine
> init function uses to determine whether to create the required devices.
>
> Since a 'vc' is a type of CharDriverState, the CharDriverState code requires
> that a DisplayState exist before it runs creating a circular dependency.
>
> To fix this, this splits the creation of the initial CharDriverState from
> the initialization of the text console. We can then in a second step associate
> a DisplayState with all TextConsoles. This allows us to create the
> CharDriverState's first, machine init, then associate the TextConsoles with
> a DisplayState.
>
> This code screams for more cleanup.
>
I am sorry for this.
While I was testing the patch series I was expecting many rendering
issues or console switching issues: once I saw that both the serial and
parallel port consoles were present I didn't check they were actually
working.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [6352] Fix character devices after DisplayState refactoring
2009-01-19 11:26 ` Stefano Stabellini
@ 2009-01-19 22:11 ` Anthony Liguori
0 siblings, 0 replies; 8+ messages in thread
From: Anthony Liguori @ 2009-01-19 22:11 UTC (permalink / raw)
To: qemu-devel
Stefano Stabellini wrote:
> Anthony Liguori wrote:
>
>
>> Revision: 6352
>> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6352
>> Author: aliguori
>> Date: 2009-01-16 20:23:27 +0000 (Fri, 16 Jan 2009)
>>
>> Log Message:
>> -----------
>> Fix character devices after DisplayState refactoring
>>
>> The DisplayState refactoring changed the machine init function to create a
>> DisplayState for each VGA device instead of being passed an existing
>> DisplayState. This change is critical to enable multiple graphics device
>> support.
>>
>> Unfortunately, the serial/parallel/console code is structured today to run
>> before machine init to fill out the CharDriverState table which the machine
>> init function uses to determine whether to create the required devices.
>>
>> Since a 'vc' is a type of CharDriverState, the CharDriverState code requires
>> that a DisplayState exist before it runs creating a circular dependency.
>>
>> To fix this, this splits the creation of the initial CharDriverState from
>> the initialization of the text console. We can then in a second step associate
>> a DisplayState with all TextConsoles. This allows us to create the
>> CharDriverState's first, machine init, then associate the TextConsoles with
>> a DisplayState.
>>
>> This code screams for more cleanup.
>>
>>
>
> I am sorry for this.
>
No worries. What I meant about more cleanup was that this patch series
demonstrated a fundamental flaw in the CharDriverState abstraction.
That is, we really ought to separate the creation of a CharDriverState
front-end from the creation of the back-end.
We should then attach a front-end to a back-end. This would make it
easy to dynamically change the back-end of any given CharDriverState
which is something I've liked to see for a while now.
Regards,
Anthony Liguori
> While I was testing the patch series I was expecting many rendering
> issues or console switching issues: once I saw that both the serial and
> parallel port consoles were present I didn't check they were actually
> working.
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-01-19 22:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-16 20:23 [Qemu-devel] [6352] Fix character devices after DisplayState refactoring Anthony Liguori
2009-01-16 21:43 ` [Qemu-devel] " Jan Kiszka
2009-01-16 21:54 ` Anthony Liguori
2009-01-17 0:58 ` [Qemu-devel] " Stuart Brady
2009-01-17 22:11 ` Stefan Weil
2009-01-18 14:09 ` Aurelien Jarno
2009-01-19 11:26 ` Stefano Stabellini
2009-01-19 22:11 ` Anthony Liguori
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).