| Message ID | 1258450699-24445-2-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
Gerd Hoffmann <kraxel@redhat.com> writes: > Add a greeting string to CharDriverState which is printed after > initialization. Used to have the qemu vc consoles labeled. This > way we can avoid walking all the chardevs a second time after > initialization just to print the greeting. I doubt this would be worthwhile on its own, but it enables getting rid of serial_devices[] and parallel_devices[] later in the series, which I like.
On Tuesday 17 November 2009, Gerd Hoffmann wrote: > Add a greeting string to CharDriverState which is printed after > initialization. Used to have the qemu vc consoles labeled. This > way we can avoid walking all the chardevs a second time after > initialization just to print the greeting. I think "greeting" is propagating a bad idea into new code. Much better would be some form of ID and/or human readable description that can also be used elsewhere. Paul
On 11/20/09 18:41, Paul Brook wrote: > On Tuesday 17 November 2009, Gerd Hoffmann wrote: >> Add a greeting string to CharDriverState which is printed after >> initialization. Used to have the qemu vc consoles labeled. This >> way we can avoid walking all the chardevs a second time after >> initialization just to print the greeting. > > I think "greeting" is propagating a bad idea into new code. Much better would > be some form of ID and/or human readable description that can also be used > elsewhere. The naming is only one part of the problem. The second part is that the greeting is printed only for the 'vc' backend (where you really need it because there is no other way to figure what chardev you are looking at when switching screens via Ctrl-Alt-<nr>). There already is a 'label' field. So we could add a flag instead of a string, then do: if (chr->want_greeting) qemu_chr_printf(chr, "%s console\n", chr->label); How about this? cheers, Gerd
On Monday 23 November 2009, Gerd Hoffmann wrote: > On 11/20/09 18:41, Paul Brook wrote: > > On Tuesday 17 November 2009, Gerd Hoffmann wrote: > >> Add a greeting string to CharDriverState which is printed after > >> initialization. Used to have the qemu vc consoles labeled. This > >> way we can avoid walking all the chardevs a second time after > >> initialization just to print the greeting. > > > > I think "greeting" is propagating a bad idea into new code. Much better > > would be some form of ID and/or human readable description that can also > > be used elsewhere. > > The naming is only one part of the problem. The second part is that the > greeting is printed only for the 'vc' backend (where you really need it > because there is no other way to figure what chardev you are looking at > when switching screens via Ctrl-Alt-<nr>). > > There already is a 'label' field. So we could add a flag instead of a > string, then do: > > if (chr->want_greeting) > qemu_chr_printf(chr, "%s console\n", chr->label); > > How about this? I thinking more that this should be done by the character backend itself. For example, the "graphical" consoles should probably be putting this as part of the window title rather than having the interface layer randomly send extra characters in connect. Paul
On 11/23/09 14:26, Paul Brook wrote: > I thinking more that this should be done by the character backend itself. For > example, the "graphical" consoles should probably be putting this as part of > the window title Doesn't work with vnc. cheers, Gerd
On Monday 23 November 2009, Gerd Hoffmann wrote: > On 11/23/09 14:26, Paul Brook wrote: > > I thinking more that this should be done by the character backend itself. > > For example, the "graphical" consoles should probably be putting this as > > part of the window title > > Doesn't work with vnc. That's why it needs to be a backend decision. The VNC devices should display his information however is most appropriate. Paul
On Mon, Nov 23, 2009 at 04:10:48PM +0100, Gerd Hoffmann wrote: > On 11/23/09 14:26, Paul Brook wrote: > >I thinking more that this should be done by the character backend itself. > >For > >example, the "graphical" consoles should probably be putting this as part > >of > >the window title > > Doesn't work with vnc. It could be made to actually, there's a VNC extension for sending updates to the desktop name http://www.tigervnc.org/cgi-bin/rfbproto#desktopname-pseudo-encoding I'd happily implement that in GTK-VNC if QEMU had the server side Regards, Daniel
Gerd Hoffmann wrote: > On 11/23/09 14:26, Paul Brook wrote: >> I thinking more that this should be done by the character backend >> itself. For >> example, the "graphical" consoles should probably be putting this as >> part of >> the window title > > Doesn't work with vnc. A vc is what renders the for VNC so if the "vc" did this, it would just work for VNC and SDL. > > cheers, > Gerd > >
On 11/23/09 16:20, Anthony Liguori wrote: > Gerd Hoffmann wrote: >> On 11/23/09 14:26, Paul Brook wrote: >>> I thinking more that this should be done by the character backend >>> itself. For >>> example, the "graphical" consoles should probably be putting this as >>> part of >>> the window title >> >> Doesn't work with vnc. > > A vc is what renders the for VNC so if the "vc" did this, it would just > work for VNC and SDL. Yea, right, as vc is the only user we can easily make this local to console.c. Good idea. We can even colorize it then ;) What I was referring to is that updating the window title of the vnc client isn't going to work. Well, at least not that easy. As Daniel points out a extension for that exists, but I'd tend to not depend on a optional vnc client feature for this. cheers, Gerd
Gerd Hoffmann wrote: > On 11/23/09 16:20, Anthony Liguori wrote: >> Gerd Hoffmann wrote: >>> On 11/23/09 14:26, Paul Brook wrote: >>>> I thinking more that this should be done by the character backend >>>> itself. For >>>> example, the "graphical" consoles should probably be putting this as >>>> part of >>>> the window title >>> >>> Doesn't work with vnc. >> >> A vc is what renders the for VNC so if the "vc" did this, it would just >> work for VNC and SDL. > > Yea, right, as vc is the only user we can easily make this local to > console.c. Good idea. We can even colorize it then ;) > > What I was referring to is that updating the window title of the vnc > client isn't going to work. Well, at least not that easy. As Daniel > points out a extension for that exists, but I'd tend to not depend on > a optional vnc client feature for this. It will at least be updated on reconnect. Regards, Anthony Liguori
diff --git a/qemu-char.c b/qemu-char.c index 40bd7e8..19be58f 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -132,6 +132,8 @@ void qemu_chr_initial_reset(void) QTAILQ_FOREACH(chr, &chardevs, next) { qemu_chr_reset(chr); + if (chr->greeting) + qemu_chr_printf(chr, "%s", chr->greeting); } } diff --git a/qemu-char.h b/qemu-char.h index 05fe15d..e50a4f3 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -66,6 +66,7 @@ struct CharDriverState { QEMUBH *bh; char *label; char *filename; + char *greeting; QTAILQ_ENTRY(CharDriverState) next; }; diff --git a/vl.c b/vl.c index fff8e8d..095aff6 100644 --- a/vl.c +++ b/vl.c @@ -5657,6 +5657,10 @@ int main(int argc, char **argv, char **envp) devname, strerror(errno)); exit(1); } + if (strstart(devname, "vc", 0)) { + snprintf(label, sizeof(label), "serial%d console\r\n", i); + serial_hds[i]->greeting = qemu_strdup(label); + } } } @@ -5671,6 +5675,10 @@ int main(int argc, char **argv, char **envp) devname, strerror(errno)); exit(1); } + if (strstart(devname, "vc", 0)) { + snprintf(label, sizeof(label), "parallel%d console\r\n", i); + parallel_hds[i]->greeting = qemu_strdup(label); + } } } @@ -5685,6 +5693,10 @@ int main(int argc, char **argv, char **envp) devname, strerror(errno)); exit(1); } + if (strstart(devname, "vc", 0)) { + snprintf(label, sizeof(label), "virtio console%d\r\n", i); + virtcon_hds[i]->greeting = qemu_strdup(label); + } } } @@ -5800,30 +5812,6 @@ int main(int argc, char **argv, char **envp) } } - for(i = 0; i < MAX_SERIAL_PORTS; i++) { - const char *devname = serial_devices[i]; - if (devname && strcmp(devname, "none")) { - if (strstart(devname, "vc", 0)) - qemu_chr_printf(serial_hds[i], "serial%d console\r\n", i); - } - } - - for(i = 0; i < MAX_PARALLEL_PORTS; i++) { - const char *devname = parallel_devices[i]; - if (devname && strcmp(devname, "none")) { - if (strstart(devname, "vc", 0)) - qemu_chr_printf(parallel_hds[i], "parallel%d console\r\n", i); - } - } - - for(i = 0; i < MAX_VIRTIO_CONSOLES; i++) { - const char *devname = virtio_consoles[i]; - if (virtcon_hds[i] && devname) { - if (strstart(devname, "vc", 0)) - qemu_chr_printf(virtcon_hds[i], "virtio console%d\r\n", i); - } - } - if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) { fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n", gdbstub_dev);
Add a greeting string to CharDriverState which is printed after initialization. Used to have the qemu vc consoles labeled. This way we can avoid walking all the chardevs a second time after initialization just to print the greeting. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- qemu-char.c | 2 ++ qemu-char.h | 1 + vl.c | 36 ++++++++++++------------------------ 3 files changed, 15 insertions(+), 24 deletions(-)