[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple po
From: |
Gerd Hoffmann |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication |
Date: |
Wed, 23 Sep 2009 13:20:33 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.1) Gecko/20090814 Fedora/3.0-2.6.b3.fc11 Lightning/1.0pre Thunderbird/3.0b3 |
+struct VirtIOConsolePort {
+ DeviceState dev;
+
+ VirtIOConsole *vcon;
+ CharDriverState *hd;
This looks wrong.
We shouldn't have a charstate at all?
Not here. More below ...
+ char *name;
+
+ QTAILQ_HEAD(, VirtIOConsolePortBuffer) unflushed_buffer_head;
+
+ bool guest_connected;
+ bool host_connected;
+};
Sticking a pointer to VirtConPortDeviceInfo here is probably handy.
More consistent naming please.
Consistent naming for what?
The structs. Pick a prefix (say VirtCon) and stick with that. Then for
the bus implementation:
VirtConBus (fine)
VirtConPort (VirtIOConsolePort now)
VirtConPortInfo (VirtConPortDeviceInfo now)
The console port driver could name its state info this way:
VirtConPortConsole (doesn't exist right now it seems ...).
I've only put the new bus stuff in the new file and this file has
largely remained as-is, with a few functions changed to accomodate the
new init methods.
This is about more than just the init ...
+static bool has_complete_data(VirtIOConsolePort *port)
+{
+ VirtIOConsolePortBuffer *buf;
+ size_t len, size;
+
+ len = 0;
+ size = 0;
+ QTAILQ_FOREACH(buf,&port->unflushed_buffer_head, next) {
+ if (!buf->size&& buf == QTAILQ_FIRST(&port->unflushed_buffer_head)) {
+ /* We have a buffer that's lost its way; just flush it */
Can this happen? If not, assert() instead?
Shouldn't happen; but it's not a serious thing to error out instead.
It indicates a bug somewhere though, doesn't it? I'd suggest to not
paper over it.
+static size_t flush_buf(VirtIOConsolePort *port, const uint8_t *buf, size_t
len)
+{
+ if (!port->hd) {
+ return 0;
+ }
+ return qemu_chr_write(port->hd, buf, len);
port->info->data_for_you(port, buf, len);
OK; haven't yet seen how the charstate can be made transparent.
chardev should go into VirtConPortConsole.
If it shoudn't happen, then use assert(). If it triggers, find the bug.
The bug could actually be in the guest and not in qemu. Would be wrong
to penalise in that case, I guess.
Ah, ok. Fine then. Aborting qemu on guest bugs would be insane.
+/* Guest wants to notify us of some event */
+static void handle_control_message(VirtIOConsolePort *port,
+ struct virtio_console_control *cpkt)
+{
+ uint8_t *buffer;
+ size_t buffer_len;
+
+ switch(cpkt->event) {
+ case VIRTIO_CONSOLE_PORT_OPEN:
+ port->guest_connected = cpkt->value;
port->info->guest_open() notify callback?
You mean handle it in an async path?
No, have a way to notify the port driver about the state change.
Stick in more function pointers here. guest_open(), data_for_you(), ...
Well. The whole thing is still *way* to mixed up. It should be cleanly
separated.
Yeah; I've only got it working with -device so far. So I guess I'll have
to bug you more to get this further into shape :-)
You should be able to move the port driver(s) to a separate source file
without much trouble. Only the port driver should deal with a chardev.
Oh OK; maybe I understand what you're saying about the chardevs now.
The virtio-console core should not care at all how the data is piped to
the (host side) users. It just drives the ring, forwards events,
accepts data for the guest (via helper function), passes on data from
the guest (via callback in VirtConPortDeviceInfo).
Hm, let me think over this.
A port driver should look roughly like the attached one. That one does
something completely different: Implement a watchdog ;) Warning:
didn't even compile it.
The console port driver would have the chardev instead of the timer in
the driver state struct and would basically forward the data between the
port and the chardev.
HTH,
Gerd
wdt_vmchannel.c
Description: Text Data
- [Qemu-devel] [PATCH 5/5] vnc: Send / receive guest clipboard if virtio-console connected to clipboard port, (continued)
- [Qemu-devel] Multiple port support for virtio-console, Amit Shah, 2009/09/22
- [Qemu-devel] [PATCH 1/4] char: Emit 'OPENED' events on char device open, Amit Shah, 2009/09/22
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication, Gerd Hoffmann, 2009/09/23
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication, Amit Shah, 2009/09/23
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication,
Gerd Hoffmann <=
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication, Amit Shah, 2009/09/23
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication, Gerd Hoffmann, 2009/09/23
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication, Amit Shah, 2009/09/23
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication, Gerd Hoffmann, 2009/09/23
- Re: [Qemu-devel] [PATCH 3/4] virtio-console: Add support for multiple ports for generic guest-host communication, Amit Shah, 2009/09/23