[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, s
From: |
Gerd Hoffmann |
Subject: |
Re: [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports |
Date: |
Wed, 30 Sep 2009 10:59:47 +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 |
virtio-serial-bus (and the corresponding -device virtio-serial-pci)
virtio-console
virtio-serial-port
virtio-serial-vnc
Is that OK with all?
Looks fine to me.
+static VirtIOConsole *virtio_console;
Why do you need this?
Just to keep the current behaviour of restricting to one virtio-console
device. This can be later reworked to allow multiple devices.
I'd drop the limit right from the start.
+static void flush_queue(VirtConPort *port)
+{
+ VirtConPortBuffer *buf, *buf2;
+ uint8_t *outbuf;
+ size_t outlen, outsize;
+
+ /*
+ * If the app isn't interested in buffering packets till it's
+ * opened, just drop the data guest sends us till a connection is
+ * established.
+ */
+ if (!port->host_connected&& !port->flush_buffers)
+ return;
Hmm. Who needs that buffering? Only user seems to be the port driver
(patch 4/6). So move the buffering (and the host_connected state
tracking) from the core to that driver?
The buffering could be used by other devices too I guess. The other two
users that we currently have, vnc (which is just a demo patch) and
console, don't need the buffering, but it's a general facility.
If more users need the buffering, the code could get duplicated so I
thought of keeping it here.
I'd suggest to look what do do when another user which wants buffering
comes along. Might be the requirements are different, so it wouldn't
work anyway.
Also when you pass around pointers to VirtConPortBuffer the port drivers
can implement buffering very easily.
+static VirtConBus *virtcon_bus;
Why do you need this?
I could put this in the VirtIOConsole struct.
Yes, please. You'll need that for multiple devices anyway.
+static int virtcon_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
+{
+ if (port->chr) {
+ qemu_chr_add_handlers(port->chr, vcon_can_read, vcon_read, vcon_event,
+ port);
Should be handled by the VirtConPort drivers.
There are two reasons why I didn't put this there: virtio-console as
well as virtio-serial-port will need char drivers. There could be others
as well.
They are handled differently though. I think console will not do any
buffering at all, whereas serial-port provides the option to do buffering.
There is also the option to stick both console and serial-port
implementations into the same source file and make them share the
functions which handle the chardev interaction.
Also, some virtio-specific checks are done in vcon_can_read. So it's
better this is put in the common code.
Having a helper function for the port drivers which fill the role
vcon_can_read() fills right now certainly makes sense.
The naming is bad though. vcon_can_read() is named from the chardevs
point of view. It actually checks how many bytes can be written to the
virtio ring. It should be named accordingly.
Also the parameters should make sense from a port drivers point of view,
not tweaked for chardev. Port drivers which don't need a chardev at all
might want to use this too. Port drivers which link to a chardev can
have a trivial one-liner wrapper function to map chardev callbacks into
virtio-serial helper functions.
+ /* Spawn a new virtio-console bus on which the ports will ride as devices
*/
+ virtcon_bus_new(dev);
s->bus = virtcon_bus_new(dev);
port0 = qdev_create(s->bus, "virtconsole"); /* console @ port0 for
backward compat */
qdev_prop_set_*(port0, ...);
qdev_init(port0);
Or does that happen somewhere else?
I'm nowhere assuming now any port number - function mapping and so
spawning a virtio-console on port#0 is not necessary.
How old kernels which expect a single console are supposed to work if
you don't create one?
cheers,
Gerd
- [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports, (continued)
- [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports, Amit Shah, 2009/09/29
- [Qemu-devel] [PATCH 4/6] virtio-console-port: Add a new device on the virtio-console-bus for generic host-guest communication, Amit Shah, 2009/09/29
- [Qemu-devel] [PATCH 5/6] vnc: add a is_vnc_active() helper, Amit Shah, 2009/09/29
- [Qemu-devel] [PATCH 6/6] vnc: Add a virtio-console-bus device to send / receive guest clipboard, Amit Shah, 2009/09/29
- Re: [Qemu-devel] [PATCH 6/6] vnc: Add a virtio-console-bus device to send / receive guest clipboard, Gerd Hoffmann, 2009/09/29
- Re: [Qemu-devel] [PATCH 6/6] vnc: Add a virtio-console-bus device to send / receive guest clipboard, Amit Shah, 2009/09/30
- Re: [Qemu-devel] [PATCH 4/6] virtio-console-port: Add a new device on the virtio-console-bus for generic host-guest communication, Gerd Hoffmann, 2009/09/29
- Re: [Qemu-devel] [PATCH 4/6] virtio-console-port: Add a new device on the virtio-console-bus for generic host-guest communication, Nathan Baum, 2009/09/30
- Re: [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports, Gerd Hoffmann, 2009/09/29
- Re: [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports, Amit Shah, 2009/09/30
- Re: [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports,
Gerd Hoffmann <=
- Re: [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports, Amit Shah, 2009/09/30
- Re: [Qemu-devel] [PATCH 3/6] virtio-console: Add a virtio-console bus, support for multiple ports, Gerd Hoffmann, 2009/09/30
Re: [Qemu-devel] [PATCH 1/6] char: Emit 'OPENED' events on char device open, Anthony Liguori, 2009/09/30
Re: [Qemu-devel] virtio-console-bus, multiport, virtio-console-port, Amit Shah, 2009/09/29