Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 5307 bytes --]

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]