diff mbox

[PULL,7/8] gtk: Fix -serial vc

Message ID 1398764774-22595-8-git-send-email-kraxel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann April 29, 2014, 9:46 a.m. UTC 
From: Cole Robinson <crobinso@redhat.com>

Try kicking off a rhel5 text install over serial, the text menu navigation
is all messed up, and some of the kernel boot messages are randomly
corrupted.

Drop use of a pty and just use vte infrastructure for reading and writing.
This fixes the above corruption, and is simpler to boot.

(I don't know what was wrong with the original code though. FWIW this is
what virt-manager has done for years).

Signed-off-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 41 +++++++++--------------------------------
 1 file changed, 9 insertions(+), 32 deletions(-)

Comments

Jan Kiszka May 4, 2014, 8:35 a.m. UTC | #1 
On 2014-04-29 11:46, Gerd Hoffmann wrote:
> From: Cole Robinson <crobinso@redhat.com>
> 
> Try kicking off a rhel5 text install over serial, the text menu navigation
> is all messed up, and some of the kernel boot messages are randomly
> corrupted.
> 
> Drop use of a pty and just use vte infrastructure for reading and writing.
> This fixes the above corruption, and is simpler to boot.
> 
> (I don't know what was wrong with the original code though. FWIW this is
> what virt-manager has done for years).
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/gtk.c | 41 +++++++++--------------------------------
>  1 file changed, 9 insertions(+), 32 deletions(-)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index c85aea3..1465a38 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -115,7 +115,6 @@ typedef struct VirtualConsole
>      GtkWidget *scrolled_window;
>      CharDriverState *chr;
>  #endif
> -    int fd;
>  } VirtualConsole;
>  
>  typedef struct GtkDisplayState
> @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget,
>  
>  static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>  {
> +#if defined(CONFIG_VTE)
>      VirtualConsole *vc = chr->opaque;
>  
> -    return vc ? write(vc->fd, buf, len) : len;
> +    vte_terminal_feed(VTE_TERMINAL(vc->terminal), (const char *)buf, len);
> +#endif
> +    return len;
>  }
>  
>  static int nb_vcs;
> @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void)
>  }
>  
>  #if defined(CONFIG_VTE)
> -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
> +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
> +                         gpointer user_data)
>  {
> -    VirtualConsole *vc = opaque;
> -    uint8_t buffer[1024];
> -    ssize_t len;
> -
> -    len = read(vc->fd, buffer, sizeof(buffer));
> -    if (len <= 0) {
> -        return FALSE;
> -    }
> -
> -    qemu_chr_be_write(vc->chr, buffer, len);
> +    VirtualConsole *vc = user_data;
>  
> +    qemu_chr_be_write(vc->chr, (uint8_t  *)text, (unsigned int)size);
>      return TRUE;
>  }
>  #endif
> @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>      const char *label;
>      char buffer[32];
>      char path[32];
> -#if VTE_CHECK_VERSION(0, 26, 0)
> -    VtePty *pty;
> -#endif
> -    GIOChannel *chan;
>      GtkWidget *scrolled_window;
>      GtkAdjustment *vadjustment;
> -    int master_fd, slave_fd;
>  
>      snprintf(buffer, sizeof(buffer), "vc%d", index);
>      snprintf(path, sizeof(path), "<QEMU>/View/VC%d", index);
> @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>      gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
>  
>      vc->terminal = vte_terminal_new();
> -
> -    master_fd = qemu_openpty_raw(&slave_fd, NULL);
> -    g_assert(master_fd != -1);
> -
> -#if VTE_CHECK_VERSION(0, 26, 0)
> -    pty = vte_pty_new_foreign(master_fd, NULL);
> -    vte_terminal_set_pty_object(VTE_TERMINAL(vc->terminal), pty);
> -#else
> -    vte_terminal_set_pty(VTE_TERMINAL(vc->terminal), master_fd);
> -#endif
> +    g_signal_connect(vc->terminal, "commit", G_CALLBACK(gd_vc_in), vc);
>  
>      vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->terminal), -1);
>  
> @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>  
>      vte_terminal_set_size(VTE_TERMINAL(vc->terminal), 80, 25);
>  
> -    vc->fd = slave_fd;
>      vc->chr->opaque = vc;
>      vc->scrolled_window = scrolled_window;
>  
> @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>          vc->chr->init(vc->chr);
>      }
>  
> -    chan = g_io_channel_unix_new(vc->fd);
> -    g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
> -
>  #endif /* CONFIG_VTE */
>      return group;
>  }
> 

This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64
and switch to console 2 (monitor). You'll find it formatted as if the
console was only ~10 chars wide during printout of the monitor
greetings. When typing, everything is fine again. Maybe an ordering
issue that was only revealed by this commit, dunno yet.

Jan
Cole Robinson May 4, 2014, 5:07 p.m. UTC | #2 
On 05/04/2014 04:35 AM, Jan Kiszka wrote:
> On 2014-04-29 11:46, Gerd Hoffmann wrote:
>> From: Cole Robinson <crobinso@redhat.com>
>>
>> Try kicking off a rhel5 text install over serial, the text menu navigation
>> is all messed up, and some of the kernel boot messages are randomly
>> corrupted.
>>
>> Drop use of a pty and just use vte infrastructure for reading and writing.
>> This fixes the above corruption, and is simpler to boot.
>>
>> (I don't know what was wrong with the original code though. FWIW this is
>> what virt-manager has done for years).
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  ui/gtk.c | 41 +++++++++--------------------------------
>>  1 file changed, 9 insertions(+), 32 deletions(-)
>>
>> diff --git a/ui/gtk.c b/ui/gtk.c
>> index c85aea3..1465a38 100644
>> --- a/ui/gtk.c
>> +++ b/ui/gtk.c
>> @@ -115,7 +115,6 @@ typedef struct VirtualConsole
>>      GtkWidget *scrolled_window;
>>      CharDriverState *chr;
>>  #endif
>> -    int fd;
>>  } VirtualConsole;
>>  
>>  typedef struct GtkDisplayState
>> @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget,
>>  
>>  static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>>  {
>> +#if defined(CONFIG_VTE)
>>      VirtualConsole *vc = chr->opaque;
>>  
>> -    return vc ? write(vc->fd, buf, len) : len;
>> +    vte_terminal_feed(VTE_TERMINAL(vc->terminal), (const char *)buf, len);
>> +#endif
>> +    return len;
>>  }
>>  
>>  static int nb_vcs;
>> @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void)
>>  }
>>  
>>  #if defined(CONFIG_VTE)
>> -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
>> +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
>> +                         gpointer user_data)
>>  {
>> -    VirtualConsole *vc = opaque;
>> -    uint8_t buffer[1024];
>> -    ssize_t len;
>> -
>> -    len = read(vc->fd, buffer, sizeof(buffer));
>> -    if (len <= 0) {
>> -        return FALSE;
>> -    }
>> -
>> -    qemu_chr_be_write(vc->chr, buffer, len);
>> +    VirtualConsole *vc = user_data;
>>  
>> +    qemu_chr_be_write(vc->chr, (uint8_t  *)text, (unsigned int)size);
>>      return TRUE;
>>  }
>>  #endif
>> @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>      const char *label;
>>      char buffer[32];
>>      char path[32];
>> -#if VTE_CHECK_VERSION(0, 26, 0)
>> -    VtePty *pty;
>> -#endif
>> -    GIOChannel *chan;
>>      GtkWidget *scrolled_window;
>>      GtkAdjustment *vadjustment;
>> -    int master_fd, slave_fd;
>>  
>>      snprintf(buffer, sizeof(buffer), "vc%d", index);
>>      snprintf(path, sizeof(path), "<QEMU>/View/VC%d", index);
>> @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>      gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
>>  
>>      vc->terminal = vte_terminal_new();
>> -
>> -    master_fd = qemu_openpty_raw(&slave_fd, NULL);
>> -    g_assert(master_fd != -1);
>> -
>> -#if VTE_CHECK_VERSION(0, 26, 0)
>> -    pty = vte_pty_new_foreign(master_fd, NULL);
>> -    vte_terminal_set_pty_object(VTE_TERMINAL(vc->terminal), pty);
>> -#else
>> -    vte_terminal_set_pty(VTE_TERMINAL(vc->terminal), master_fd);
>> -#endif
>> +    g_signal_connect(vc->terminal, "commit", G_CALLBACK(gd_vc_in), vc);
>>  
>>      vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->terminal), -1);
>>  
>> @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>  
>>      vte_terminal_set_size(VTE_TERMINAL(vc->terminal), 80, 25);
>>  
>> -    vc->fd = slave_fd;
>>      vc->chr->opaque = vc;
>>      vc->scrolled_window = scrolled_window;
>>  
>> @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>          vc->chr->init(vc->chr);
>>      }
>>  
>> -    chan = g_io_channel_unix_new(vc->fd);
>> -    g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
>> -
>>  #endif /* CONFIG_VTE */
>>      return group;
>>  }
>>
> 
> This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64
> and switch to console 2 (monitor). You'll find it formatted as if the
> console was only ~10 chars wide during printout of the monitor
> greetings. When typing, everything is fine again. Maybe an ordering
> issue that was only revealed by this commit, dunno yet.
> 

Check out gerd's ui-gtk-next branch, there's a few extra patches related to
vte sizing that might fix it.

- Cole
Jan Kiszka May 4, 2014, 6:25 p.m. UTC | #3 
On 2014-05-04 19:07, Cole Robinson wrote:
> On 05/04/2014 04:35 AM, Jan Kiszka wrote:
>> On 2014-04-29 11:46, Gerd Hoffmann wrote:
>>> From: Cole Robinson <crobinso@redhat.com>
>>>
>>> Try kicking off a rhel5 text install over serial, the text menu navigation
>>> is all messed up, and some of the kernel boot messages are randomly
>>> corrupted.
>>>
>>> Drop use of a pty and just use vte infrastructure for reading and writing.
>>> This fixes the above corruption, and is simpler to boot.
>>>
>>> (I don't know what was wrong with the original code though. FWIW this is
>>> what virt-manager has done for years).
>>>
>>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>>  ui/gtk.c | 41 +++++++++--------------------------------
>>>  1 file changed, 9 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>> index c85aea3..1465a38 100644
>>> --- a/ui/gtk.c
>>> +++ b/ui/gtk.c
>>> @@ -115,7 +115,6 @@ typedef struct VirtualConsole
>>>      GtkWidget *scrolled_window;
>>>      CharDriverState *chr;
>>>  #endif
>>> -    int fd;
>>>  } VirtualConsole;
>>>  
>>>  typedef struct GtkDisplayState
>>> @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget,
>>>  
>>>  static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>>>  {
>>> +#if defined(CONFIG_VTE)
>>>      VirtualConsole *vc = chr->opaque;
>>>  
>>> -    return vc ? write(vc->fd, buf, len) : len;
>>> +    vte_terminal_feed(VTE_TERMINAL(vc->terminal), (const char *)buf, len);
>>> +#endif
>>> +    return len;
>>>  }
>>>  
>>>  static int nb_vcs;
>>> @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void)
>>>  }
>>>  
>>>  #if defined(CONFIG_VTE)
>>> -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
>>> +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
>>> +                         gpointer user_data)
>>>  {
>>> -    VirtualConsole *vc = opaque;
>>> -    uint8_t buffer[1024];
>>> -    ssize_t len;
>>> -
>>> -    len = read(vc->fd, buffer, sizeof(buffer));
>>> -    if (len <= 0) {
>>> -        return FALSE;
>>> -    }
>>> -
>>> -    qemu_chr_be_write(vc->chr, buffer, len);
>>> +    VirtualConsole *vc = user_data;
>>>  
>>> +    qemu_chr_be_write(vc->chr, (uint8_t  *)text, (unsigned int)size);
>>>      return TRUE;
>>>  }
>>>  #endif
>>> @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>      const char *label;
>>>      char buffer[32];
>>>      char path[32];
>>> -#if VTE_CHECK_VERSION(0, 26, 0)
>>> -    VtePty *pty;
>>> -#endif
>>> -    GIOChannel *chan;
>>>      GtkWidget *scrolled_window;
>>>      GtkAdjustment *vadjustment;
>>> -    int master_fd, slave_fd;
>>>  
>>>      snprintf(buffer, sizeof(buffer), "vc%d", index);
>>>      snprintf(path, sizeof(path), "<QEMU>/View/VC%d", index);
>>> @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>      gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
>>>  
>>>      vc->terminal = vte_terminal_new();
>>> -
>>> -    master_fd = qemu_openpty_raw(&slave_fd, NULL);
>>> -    g_assert(master_fd != -1);
>>> -
>>> -#if VTE_CHECK_VERSION(0, 26, 0)
>>> -    pty = vte_pty_new_foreign(master_fd, NULL);
>>> -    vte_terminal_set_pty_object(VTE_TERMINAL(vc->terminal), pty);
>>> -#else
>>> -    vte_terminal_set_pty(VTE_TERMINAL(vc->terminal), master_fd);
>>> -#endif
>>> +    g_signal_connect(vc->terminal, "commit", G_CALLBACK(gd_vc_in), vc);
>>>  
>>>      vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->terminal), -1);
>>>  
>>> @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>  
>>>      vte_terminal_set_size(VTE_TERMINAL(vc->terminal), 80, 25);
>>>  
>>> -    vc->fd = slave_fd;
>>>      vc->chr->opaque = vc;
>>>      vc->scrolled_window = scrolled_window;
>>>  
>>> @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>          vc->chr->init(vc->chr);
>>>      }
>>>  
>>> -    chan = g_io_channel_unix_new(vc->fd);
>>> -    g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
>>> -
>>>  #endif /* CONFIG_VTE */
>>>      return group;
>>>  }
>>>
>>
>> This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64
>> and switch to console 2 (monitor). You'll find it formatted as if the
>> console was only ~10 chars wide during printout of the monitor
>> greetings. When typing, everything is fine again. Maybe an ordering
>> issue that was only revealed by this commit, dunno yet.
>>
> 
> Check out gerd's ui-gtk-next branch, there's a few extra patches related to
> vte sizing that might fix it.

Looks better thanks to "gtk: zap scrolled_window" (monitor is properly
formatted again). But the whole queue spits this out on the terminal:

(<unknown>:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width > 0' failed

(<unknown>:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion `GDK_IS_WINDOW (window)' failed

The last two patches seem to be responsible for this.

Jan
Jan Kiszka May 4, 2014, 6:32 p.m. UTC | #4 
On 2014-05-04 20:25, Jan Kiszka wrote:
> On 2014-05-04 19:07, Cole Robinson wrote:
>> On 05/04/2014 04:35 AM, Jan Kiszka wrote:
>>> On 2014-04-29 11:46, Gerd Hoffmann wrote:
>>>> From: Cole Robinson <crobinso@redhat.com>
>>>>
>>>> Try kicking off a rhel5 text install over serial, the text menu navigation
>>>> is all messed up, and some of the kernel boot messages are randomly
>>>> corrupted.
>>>>
>>>> Drop use of a pty and just use vte infrastructure for reading and writing.
>>>> This fixes the above corruption, and is simpler to boot.
>>>>
>>>> (I don't know what was wrong with the original code though. FWIW this is
>>>> what virt-manager has done for years).
>>>>
>>>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>>> ---
>>>>  ui/gtk.c | 41 +++++++++--------------------------------
>>>>  1 file changed, 9 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/ui/gtk.c b/ui/gtk.c
>>>> index c85aea3..1465a38 100644
>>>> --- a/ui/gtk.c
>>>> +++ b/ui/gtk.c
>>>> @@ -115,7 +115,6 @@ typedef struct VirtualConsole
>>>>      GtkWidget *scrolled_window;
>>>>      CharDriverState *chr;
>>>>  #endif
>>>> -    int fd;
>>>>  } VirtualConsole;
>>>>  
>>>>  typedef struct GtkDisplayState
>>>> @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget,
>>>>  
>>>>  static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>>>>  {
>>>> +#if defined(CONFIG_VTE)
>>>>      VirtualConsole *vc = chr->opaque;
>>>>  
>>>> -    return vc ? write(vc->fd, buf, len) : len;
>>>> +    vte_terminal_feed(VTE_TERMINAL(vc->terminal), (const char *)buf, len);
>>>> +#endif
>>>> +    return len;
>>>>  }
>>>>  
>>>>  static int nb_vcs;
>>>> @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void)
>>>>  }
>>>>  
>>>>  #if defined(CONFIG_VTE)
>>>> -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
>>>> +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
>>>> +                         gpointer user_data)
>>>>  {
>>>> -    VirtualConsole *vc = opaque;
>>>> -    uint8_t buffer[1024];
>>>> -    ssize_t len;
>>>> -
>>>> -    len = read(vc->fd, buffer, sizeof(buffer));
>>>> -    if (len <= 0) {
>>>> -        return FALSE;
>>>> -    }
>>>> -
>>>> -    qemu_chr_be_write(vc->chr, buffer, len);
>>>> +    VirtualConsole *vc = user_data;
>>>>  
>>>> +    qemu_chr_be_write(vc->chr, (uint8_t  *)text, (unsigned int)size);
>>>>      return TRUE;
>>>>  }
>>>>  #endif
>>>> @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>>      const char *label;
>>>>      char buffer[32];
>>>>      char path[32];
>>>> -#if VTE_CHECK_VERSION(0, 26, 0)
>>>> -    VtePty *pty;
>>>> -#endif
>>>> -    GIOChannel *chan;
>>>>      GtkWidget *scrolled_window;
>>>>      GtkAdjustment *vadjustment;
>>>> -    int master_fd, slave_fd;
>>>>  
>>>>      snprintf(buffer, sizeof(buffer), "vc%d", index);
>>>>      snprintf(path, sizeof(path), "<QEMU>/View/VC%d", index);
>>>> @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>>      gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
>>>>  
>>>>      vc->terminal = vte_terminal_new();
>>>> -
>>>> -    master_fd = qemu_openpty_raw(&slave_fd, NULL);
>>>> -    g_assert(master_fd != -1);
>>>> -
>>>> -#if VTE_CHECK_VERSION(0, 26, 0)
>>>> -    pty = vte_pty_new_foreign(master_fd, NULL);
>>>> -    vte_terminal_set_pty_object(VTE_TERMINAL(vc->terminal), pty);
>>>> -#else
>>>> -    vte_terminal_set_pty(VTE_TERMINAL(vc->terminal), master_fd);
>>>> -#endif
>>>> +    g_signal_connect(vc->terminal, "commit", G_CALLBACK(gd_vc_in), vc);
>>>>  
>>>>      vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->terminal), -1);
>>>>  
>>>> @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>>  
>>>>      vte_terminal_set_size(VTE_TERMINAL(vc->terminal), 80, 25);
>>>>  
>>>> -    vc->fd = slave_fd;
>>>>      vc->chr->opaque = vc;
>>>>      vc->scrolled_window = scrolled_window;
>>>>  
>>>> @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
>>>>          vc->chr->init(vc->chr);
>>>>      }
>>>>  
>>>> -    chan = g_io_channel_unix_new(vc->fd);
>>>> -    g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
>>>> -
>>>>  #endif /* CONFIG_VTE */
>>>>      return group;
>>>>  }
>>>>
>>>
>>> This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64
>>> and switch to console 2 (monitor). You'll find it formatted as if the
>>> console was only ~10 chars wide during printout of the monitor
>>> greetings. When typing, everything is fine again. Maybe an ordering
>>> issue that was only revealed by this commit, dunno yet.
>>>
>>
>> Check out gerd's ui-gtk-next branch, there's a few extra patches related to
>> vte sizing that might fix it.
> 
> Looks better thanks to "gtk: zap scrolled_window" (monitor is properly
> formatted again).

...err - no. The price of this patch is that the window is no longer
properly resized on all guest mode changes. There is still more broken.

Jan
Gerd Hoffmann May 5, 2014, 11:55 a.m. UTC | #5 
Hi,

> Looks better thanks to "gtk: zap scrolled_window" (monitor is properly
> formatted again). But the whole queue spits this out on the terminal:
> 
> (<unknown>:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width > 0' failed
> 
> (<unknown>:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion `GDK_IS_WINDOW (window)' failed

Yes, there is some work-in-progress stuff in that branch causing this.

cheers,
  Gerd
Gerd Hoffmann May 5, 2014, 11:58 a.m. UTC | #6 
Hi,

> >> Check out gerd's ui-gtk-next branch, there's a few extra patches related to
> >> vte sizing that might fix it.
> > 
> > Looks better thanks to "gtk: zap scrolled_window" (monitor is properly
> > formatted again).
> 
> ...err - no. The price of this patch is that the window is no longer
> properly resized on all guest mode changes. There is still more broken.

Indeed.  Switching to higher resolutions make the window larger, but
switching back to lower res doesn't make it smaller.  I'll have a look.

cheers,
  Gerd
Gerd Hoffmann May 5, 2014, 2:46 p.m. UTC | #7 
On Mo, 2014-05-05 at 13:55 +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > Looks better thanks to "gtk: zap scrolled_window" (monitor is properly
> > formatted again). But the whole queue spits this out on the terminal:
> > 
> > (<unknown>:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width > 0' failed
> > 
> > (<unknown>:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion `GDK_IS_WINDOW (window)' failed
> 
> Yes, there is some work-in-progress stuff in that branch causing this.

Feel free to check out the branch again.  Warnings are fixed, resizing
is fixed, and you can move tabs to windows (so you can see vga + monitor
at the same time).

comments welcome,

  Gerd
Jan Kiszka May 6, 2014, 11:14 a.m. UTC | #8 
On 2014-05-05 16:46, Gerd Hoffmann wrote:
> On Mo, 2014-05-05 at 13:55 +0200, Gerd Hoffmann wrote:
>>   Hi,
>>
>>> Looks better thanks to "gtk: zap scrolled_window" (monitor is properly
>>> formatted again). But the whole queue spits this out on the terminal:
>>>
>>> (<unknown>:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width > 0' failed
>>>
>>> (<unknown>:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion `GDK_IS_WINDOW (window)' failed
>>
>> Yes, there is some work-in-progress stuff in that branch causing this.
> 
> Feel free to check out the branch again.  Warnings are fixed, resizing
> is fixed, and you can move tabs to windows (so you can see vga + monitor
> at the same time).

Still issues remaining. Scenario: Fired up a Linux guest up to graphical
mode, issued hard reset from monitor console, switched to guest display
-> window is larger than boot loader screen.

Jan
diff mbox

Patch

diff --git a/ui/gtk.c b/ui/gtk.c
index c85aea3..1465a38 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -115,7 +115,6 @@  typedef struct VirtualConsole
     GtkWidget *scrolled_window;
     CharDriverState *chr;
 #endif
-    int fd;
 } VirtualConsole;
 
 typedef struct GtkDisplayState
@@ -1162,9 +1161,12 @@  static gboolean gd_focus_out_event(GtkWidget *widget,
 
 static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
 {
+#if defined(CONFIG_VTE)
     VirtualConsole *vc = chr->opaque;
 
-    return vc ? write(vc->fd, buf, len) : len;
+    vte_terminal_feed(VTE_TERMINAL(vc->terminal), (const char *)buf, len);
+#endif
+    return len;
 }
 
 static int nb_vcs;
@@ -1190,19 +1192,12 @@  void early_gtk_display_init(void)
 }
 
 #if defined(CONFIG_VTE)
-static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
+static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
+                         gpointer user_data)
 {
-    VirtualConsole *vc = opaque;
-    uint8_t buffer[1024];
-    ssize_t len;
-
-    len = read(vc->fd, buffer, sizeof(buffer));
-    if (len <= 0) {
-        return FALSE;
-    }
-
-    qemu_chr_be_write(vc->chr, buffer, len);
+    VirtualConsole *vc = user_data;
 
+    qemu_chr_be_write(vc->chr, (uint8_t  *)text, (unsigned int)size);
     return TRUE;
 }
 #endif
@@ -1214,13 +1209,8 @@  static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
     const char *label;
     char buffer[32];
     char path[32];
-#if VTE_CHECK_VERSION(0, 26, 0)
-    VtePty *pty;
-#endif
-    GIOChannel *chan;
     GtkWidget *scrolled_window;
     GtkAdjustment *vadjustment;
-    int master_fd, slave_fd;
 
     snprintf(buffer, sizeof(buffer), "vc%d", index);
     snprintf(path, sizeof(path), "<QEMU>/View/VC%d", index);
@@ -1239,16 +1229,7 @@  static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
     gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
 
     vc->terminal = vte_terminal_new();
-
-    master_fd = qemu_openpty_raw(&slave_fd, NULL);
-    g_assert(master_fd != -1);
-
-#if VTE_CHECK_VERSION(0, 26, 0)
-    pty = vte_pty_new_foreign(master_fd, NULL);
-    vte_terminal_set_pty_object(VTE_TERMINAL(vc->terminal), pty);
-#else
-    vte_terminal_set_pty(VTE_TERMINAL(vc->terminal), master_fd);
-#endif
+    g_signal_connect(vc->terminal, "commit", G_CALLBACK(gd_vc_in), vc);
 
     vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc->terminal), -1);
 
@@ -1263,7 +1244,6 @@  static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
 
     vte_terminal_set_size(VTE_TERMINAL(vc->terminal), 80, 25);
 
-    vc->fd = slave_fd;
     vc->chr->opaque = vc;
     vc->scrolled_window = scrolled_window;
 
@@ -1281,9 +1261,6 @@  static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL
         vc->chr->init(vc->chr);
     }
 
-    chan = g_io_channel_unix_new(vc->fd);
-    g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
-
 #endif /* CONFIG_VTE */
     return group;
 }