[Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
We have to buffer data from guest as ports may not consume all the data
in one go or the guest could be fast in sending data and the apps may
not consume at the same rate.
We keep caching data the guest sends us till a port accepts it. However,
this could lead to an OOM condition where a rogue process on the guest
could continue pumping in data while the host continues to cache it.
We introduce a per-port byte-limit property to alleviate this condition.
When this limit is reached, we send a control message to the guest
indicating it to not send us any more data till further indication. When
the number of bytes cached go lesser than the limit specified, we tell
the guest to restart sending data.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 215 +++-
hw/virtio-serial.c |6 --
hw/virtio-serial.h | 30 ++-
3 files changed, 203 insertions(+), 48 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 2576140..1ec67d2 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -44,6 +44,18 @@ struct VirtIOSerial {
struct virtio_console_config config;
};
+/* This struct holds individual buffers received for each port */
+typedef struct VirtIOSerialPortBuffer {
+QTAILQ_ENTRY(VirtIOSerialPortBuffer) next;
+
+uint8_t *buf;
+
+size_t len; /* length of the buffer */
+size_t offset; /* offset from which to consume data in the buffer */
+
+bool previous_failure; /* Did sending out this buffer fail previously? */
+} VirtIOSerialPortBuffer;
+
static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
{
VirtIOSerialPort *port;
@@ -75,11 +87,9 @@ static size_t write_to_port(VirtIOSerialPort *port,
const uint8_t *buf, size_t size)
{
VirtQueueElement elem;
-struct virtio_console_header header;
VirtQueue *vq;
size_t offset = 0;
size_t len = 0;
-int header_len;
vq = port-ivq;
if (!virtio_queue_ready(vq)) {
@@ -88,8 +98,6 @@ static size_t write_to_port(VirtIOSerialPort *port,
if (!size) {
return 0;
}
-header.flags = 0;
-header_len = use_multiport(port-vser) ? sizeof(header) : 0;
while (offset size) {
int i;
@@ -97,26 +105,14 @@ static size_t write_to_port(VirtIOSerialPort *port,
if (!virtqueue_pop(vq, elem)) {
break;
}
-if (elem.in_sg[0].iov_len header_len) {
-/* We can't even store our port number in this buffer. Bug? */
-qemu_error(virtio-serial: size %zd less than expected\n,
-elem.in_sg[0].iov_len);
-exit(1);
-}
-if (header_len) {
-memcpy(elem.in_sg[0].iov_base, header, header_len);
-}
for (i = 0; offset size i elem.in_num; i++) {
-/* Copy the header only in the first sg. */
-len = MIN(elem.in_sg[i].iov_len - header_len, size - offset);
+len = MIN(elem.in_sg[i].iov_len, size - offset);
-memcpy(elem.in_sg[i].iov_base + header_len, buf + offset, len);
+memcpy(elem.in_sg[i].iov_base, buf + offset, len);
offset += len;
-header_len = 0;
}
-header_len = use_multiport(port-vser) ? sizeof(header) : 0;
-virtqueue_push(vq, elem, len + header_len);
+virtqueue_push(vq, elem, len);
}
virtio_notify(port-vser-vdev, vq);
@@ -157,6 +153,96 @@ static size_t send_control_event(VirtIOSerialPort *port,
uint16_t event,
return send_control_msg(port, cpkt, sizeof(cpkt));
}
+static void init_buf(VirtIOSerialPortBuffer *buf, uint8_t *buffer, size_t len)
+{
+buf-buf = buffer;
+buf-len = len;
+buf-offset = 0;
+buf-previous_failure = false;
+}
+
+static VirtIOSerialPortBuffer *alloc_buf(size_t len)
+{
+VirtIOSerialPortBuffer *buf;
+
+buf = qemu_malloc(sizeof(*buf));
+buf-buf = qemu_malloc(len);
+
+init_buf(buf, buf-buf, len);
+
+return buf;
+}
+
+static void free_buf(VirtIOSerialPortBuffer *buf)
+{
+qemu_free(buf-buf);
+qemu_free(buf);
+}
+
+static void flush_queue(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf;
+size_t out_size;
+ssize_t ret;
+
+while ((buf = QTAILQ_FIRST(port-unflushed_buffers))) {
+QTAILQ_REMOVE(port-unflushed_buffers, buf, next);
+
+out_size = buf-len - buf-offset;
+if (!port-host_connected) {
+port-nr_bytes -= buf-len + buf-offset;
+free_buf(buf);
+continue;
+}
+
+ret = port-info-have_data(port, buf-buf + buf-offset, out_size);
+if (ret out_size) {
+QTAILQ_INSERT_HEAD(port-unflushed_buffers, buf, next);
+}
+if (ret = 0) {
+/* We're not progressing at all */
+if (buf-previous_failure) {
+break;
+}
+buf-previous_failure =
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Anthony Liguori anth...@codemonkey.ws writes: On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes 4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. Yes. Either it's a datagram or a stream pipe. I always thought it would be a stream pipe, as the name serial suggests. As to the clipboard use case: same problem exists with any old stream pipe, including TCP, same solutions apply. If you told the peer I'm going to send you 12345 bytes now, and your stream pipe chokes after 7890 bytes, you retry until everything got through. If you want to be able to abort a partial transfer and start a new one, you layer a protocol suitable for that on top of your stream pipe.
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 01/13/2010 11:14 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes 4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. Yes. Either it's a datagram or a stream pipe. I always thought it would be a stream pipe, as the name serial suggests. And if it's a datagram, then we should accept that there will be a fixed max message size which is pretty common in all datagram protocols. That fixed size should be no larger than what the transport supports so in this case, it would be 4k. If a guest wants to send larger messages, it must build a continuation protocol on top of the datagram protocol. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes 4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. Regards, Anthony Liguori Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Tue) Jan 12 2010 [09:00:52], Anthony Liguori wrote: On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes 4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. For the vnc clipboard copy-paste case, I explained that in the couple of mails before in this thread. There might be other use-cases, I don't know about them though. Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 01/12/2010 09:13 AM, Amit Shah wrote: On (Tue) Jan 12 2010 [09:00:52], Anthony Liguori wrote: On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes 4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. For the vnc clipboard copy-paste case, I explained that in the couple of mails before in this thread. It didn't make sense to me. I think the assumption has to be that the client can send corrupt data and the host has to handle it. Regards, Anthony Liguori There might be other use-cases, I don't know about them though. Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Tue) Jan 12 2010 [09:46:55], Anthony Liguori wrote: On 01/12/2010 09:13 AM, Amit Shah wrote: On (Tue) Jan 12 2010 [09:00:52], Anthony Liguori wrote: On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes 4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. For the vnc clipboard copy-paste case, I explained that in the couple of mails before in this thread. It didn't make sense to me. I think the assumption has to be that the client can send corrupt data and the host has to handle it. You mean if the guest kernel sends the wrong flags? Or doesn't set the flags? Can you explain what scenario you're talking about? Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 01/12/2010 09:49 AM, Amit Shah wrote: On (Tue) Jan 12 2010 [09:46:55], Anthony Liguori wrote: On 01/12/2010 09:13 AM, Amit Shah wrote: On (Tue) Jan 12 2010 [09:00:52], Anthony Liguori wrote: On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. For the vnc clipboard copy-paste case, I explained that in the couple of mails before in this thread. It didn't make sense to me. I think the assumption has to be that the client can send corrupt data and the host has to handle it. You mean if the guest kernel sends the wrong flags? Or doesn't set the flags? Can you explain what scenario you're talking about? It's very likely that you'll have to implement some sort of protocol on top of virtio-serial. It won't always just be simple strings. If you have a simple datagram protocol, that contains two ints and a string, it's going to have to be encoded like int aint bint lenchar data[len]. You need to validate that len fits within the boundaries and deal with len being less than the boundary. If you've got a command protocol where the you send the guest something and then expect a response, you have to deal with the fact that the guest may never respond. Having well defined message boundaries does not help the general problem and it only helps in the most trivial cases. Basically, it boils down to a lot of complexity for something that isn't going to be helpful in most circumstances. Regards, Anthony Liguori Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Tue) Jan 12 2010 [09:55:41], Anthony Liguori wrote: On 01/12/2010 09:49 AM, Amit Shah wrote: On (Tue) Jan 12 2010 [09:46:55], Anthony Liguori wrote: On 01/12/2010 09:13 AM, Amit Shah wrote: On (Tue) Jan 12 2010 [09:00:52], Anthony Liguori wrote: On 01/12/2010 01:16 AM, Amit Shah wrote: BTW I don't really want this too, I can get rid of it if everyone agrees we won't support clipboard writes4k over vnc or if there's a better idea. Why bother trying to preserve message boundaries? I think that's the fundamental question. For the vnc clipboard copy-paste case, I explained that in the couple of mails before in this thread. It didn't make sense to me. I think the assumption has to be that the client can send corrupt data and the host has to handle it. You mean if the guest kernel sends the wrong flags? Or doesn't set the flags? Can you explain what scenario you're talking about? It's very likely that you'll have to implement some sort of protocol on top of virtio-serial. It won't always just be simple strings. Yes, virtio-serial is just meant to be a transport agnostic of whatever data or protocols that ride over it. If you have a simple datagram protocol, that contains two ints and a string, it's going to have to be encoded like int aint bint lenchar data[len]. You need to validate that len fits within the boundaries and deal with len being less than the boundary. If you've got a command protocol where the you send the guest something and then expect a response, you have to deal with the fact that the guest may never respond. Having well defined message boundaries does not help the general problem and it only helps in the most trivial cases. Basically, it boils down to a lot of complexity for something that isn't going to be helpful in most circumstances. I don't know why you're saying virtio-serial-bus does (or needs to) do anything of this. Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Fri) Jan 08 2010 [13:35:03], Jamie Lokier wrote: Since VNC is clearly designed to work over TCP, and is written by people who know this, I'm wondering why you think it needs to be different for virtio-serial. For vnc putting stuff from a guest clipboard into vnc client clipboard using the ServerCutText command, the entire buffer has to be provided after sending the command and the 'length' values. In this case, if the data from guest arrives in multiple packets, we really don't want to call into the write function multiple times. A single clipboard entry has to be created in the client with the entire contents, so a single write operation has to be invoked. For this to happen, there has to be some indication from the guest as to how much data was written in one write() operation, which will let us make a single write operation to the vnc client. Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Fri) Jan 08 2010 [10:26:59], Anthony Liguori wrote: On 01/08/2010 07:35 AM, Jamie Lokier wrote: Sometimes it looks like TCP is maintaining write boundaries, but it is just an artifact of its behaviour on many systems, and is not reliable even on those systems where it seems to happen most of the time. Even when connecting to localhost, you cannot rely on that. I have seen people write code assuming TCP keeps boundaries, and then some weeks later they are very confused debugging their code because it is not reliable... Since VNC is clearly designed to work over TCP, and is written by people who know this, I'm wondering why you think it needs to be different for virtio-serial. I'm confused about why the buffering is needed in the first place. I would think that any buffering should be pushed back to the guest. IOW, if there's available data from the char driver, but the guest doesn't have a buffer. Don't select on the char driver until the guest has a buffer available. If the guest attempts to write data but the char driver isn't ready to receive data, don't complete the request until the char driver can accept data. This is a different thing from what Jamie's talking about. A guest or a host might be interested in communicating data without waiting for the other end to come up. The other end can just start consuming the data (even the data that it missed while it wasn't connected) once it's up. (I can remove this option for now and add it later, if you prefer it that way.) Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Amit Shah wrote: On (Fri) Jan 08 2010 [13:35:03], Jamie Lokier wrote: Since VNC is clearly designed to work over TCP, and is written by people who know this, I'm wondering why you think it needs to be different for virtio-serial. For vnc putting stuff from a guest clipboard into vnc client clipboard using the ServerCutText command, the entire buffer has to be provided after sending the command and the 'length' values. Are you talking about a VNC protocol command between qemu's VNC server and the user's VNC client, or a private protocol between the guest and qemu's VNC server? In this case, if the data from guest arrives in multiple packets, we really don't want to call into the write function multiple times. A single clipboard entry has to be created in the client with the entire contents, so a single write operation has to be invoked. Same question again: *Why do you think the VNC server (in qemu) needs to see the entire clipboard in a aingle write from the guest?* You have already told it the total length to expect. There is no ambiguity about where it ends. There is no need to do any more, if the reciever (in qemu) is implemented correctly with a sane protocol. That's assuming the guest sends to qemu's VNC server which then sends it to the user's VNC client. For this to happen, there has to be some indication from the guest as to how much data was written in one write() operation, which will let us make a single write operation to the vnc client. When it is sent to the user's VNC client, it will be split into multiple packets by TCP. You *can't* send a single large write over TCP without getting it split at arbitrary places. It's *impossible*. TCP doesn't support that. It will split and merge your writes arbitrarily. So the only interesting part is how it's transmitted from the guest to qemu's VNC server first. Do you get to design that protocol yourself? -- Jamie
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Mon) Jan 11 2010 [10:45:53], Jamie Lokier wrote: Amit Shah wrote: On (Fri) Jan 08 2010 [13:35:03], Jamie Lokier wrote: Since VNC is clearly designed to work over TCP, and is written by people who know this, I'm wondering why you think it needs to be different for virtio-serial. For vnc putting stuff from a guest clipboard into vnc client clipboard using the ServerCutText command, the entire buffer has to be provided after sending the command and the 'length' values. Are you talking about a VNC protocol command between qemu's VNC server and the user's VNC client, or a private protocol between the guest and qemu's VNC server? What happens is: 1. Guest puts something on its clipboard 2. An agent on the guest gets notified of new clipboard contents 3. This agent sends over the entire clipboard contents to qemu via virtio-serial 4. virtio-serial sends off this data to the virtio-serial-vnc code 5. ServerCutText message from the vnc backend is sent to the vnc client 6. vnc client's clipboard gets updated 7. You can see guest's clipboard contents in your client's clipboard. I'm talking about steps 3, 4, 5 here. In this case, if the data from guest arrives in multiple packets, we really don't want to call into the write function multiple times. A single clipboard entry has to be created in the client with the entire contents, so a single write operation has to be invoked. Same question again: *Why do you think the VNC server (in qemu) needs to see the entire clipboard in a aingle write from the guest?* You have already told it the total length to expect. There is no ambiguity about where it ends. Where does the total length come from? It has to come from the guest. Otherwise, the vnc code will not know if a byte stream contains two separate clipboard entries or just one huge clipboard entry. Earlier, I used to send the length of one write as issued by a guest to qemu. I just changed that to send a START and END flag so that I don't have to send the length. If this doesn't explain it, then I think we're not understanding each other here. Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Amit Shah wrote:
Are you talking about a VNC protocol command between qemu's VNC server
and the user's VNC client, or a private protocol between the guest and
qemu's VNC server?
What happens is:
1. Guest puts something on its clipboard
2. An agent on the guest gets notified of new clipboard contents
3. This agent sends over the entire clipboard contents to qemu via
virtio-serial
4. virtio-serial sends off this data to the virtio-serial-vnc code
5. ServerCutText message from the vnc backend is sent to the vnc client
6. vnc client's clipboard gets updated
7. You can see guest's clipboard contents in your client's clipboard.
I'm talking about steps 3, 4, 5 here.
Ok. Let's not worry about 5; it doesn't seem relevant, only that the
guest clipboad is sent to the host somehow.
You have already told it the total length to expect. There is no
ambiguity about where it ends.
Where does the total length come from? It has to come from the guest.
Otherwise, the vnc code will not know if a byte stream contains two
separate clipboard entries or just one huge clipboard entry.
I see. So it's a *really simple* protocol where the clipboard entry
is sent by the guest agent with a single write() without any framing bytes?
Earlier, I used to send the length of one write as issued by a guest to
qemu. I just changed that to send a START and END flag so that I don't
have to send the length.
Why not just have the guest agent send a 4-byte header which is the
integer length of the clipboard blob to follow?
I.e. instead of
int guest_send_clipboard(const char *data, size_t length)
{
return write_full(virtio_fd, data, length);
}
do this:
int guest_send_clipboard(const char *data, size_t length)
{
u32 encoded_length = cpu_to_be32(length);
int err = write_full(virtio_serial_fd, encoded_length,
sizeof(encoded_length));
if (err == 0)
err = write_full(virtio_serial_fd, data, length);
return err;
}
If this doesn't explain it, then I think we're not understanding each
other here.
It does explain it very well, thanks. I think you're misguided about
the solution :-)
What confused me was you mentioned the VNC ServerCutText command
having to receive the whole data in one go. ServerCutText isn't
really relevant to this, and clearly is encoded with VNC protocol
framing. If it was RDP or the SDL client instead of VNC, it would be
something else. All that matters is getting the clipboard blob from
guest to qemu in one piece, right?
Having the guest agent send a few framing bytes seems very simple, and
would have the added bonus that the same guest agent protocol would
work on a real emulated serial port, guest-host TCP, etc. where
virtio-serial isn't available in the guest OS (e.g. older kernels).
I really can't see any merit in making virtio-serial not be a serial
port, being instead like a unix datagram socket, to support a specific
user of virtio-serial when a trivial 4-byte header in the guest agent
code would be easier for that user anyway.
If it did that, I think the name virtio-serial would have to change to
virtio-datagram, becuase it wouldn't behave like a serial port any
more. It would also be less useful for things that _do_ want
something like a pipe/serial port. But why bother?
-- Jamie
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 01/11/2010 05:33 PM, Jamie Lokier wrote:
Amit Shah wrote:
Are you talking about a VNC protocol command between qemu's VNC server
and the user's VNC client, or a private protocol between the guest and
qemu's VNC server?
What happens is:
1. Guest puts something on its clipboard
2. An agent on the guest gets notified of new clipboard contents
3. This agent sends over the entire clipboard contents to qemu via
virtio-serial
4. virtio-serial sends off this data to the virtio-serial-vnc code
5. ServerCutText message from the vnc backend is sent to the vnc client
6. vnc client's clipboard gets updated
7. You can see guest's clipboard contents in your client's clipboard.
I'm talking about steps 3, 4, 5 here.
Ok. Let's not worry about 5; it doesn't seem relevant, only that the
guest clipboad is sent to the host somehow.
You have already told it the total length to expect. There is no
ambiguity about where it ends.
Where does the total length come from? It has to come from the guest.
Otherwise, the vnc code will not know if a byte stream contains two
separate clipboard entries or just one huge clipboard entry.
I see. So it's a *really simple* protocol where the clipboard entry
is sent by the guest agent with a single write() without any framing bytes?
Earlier, I used to send the length of one write as issued by a guest to
qemu. I just changed that to send a START and END flag so that I don't
have to send the length.
Why not just have the guest agent send a 4-byte header which is the
integer length of the clipboard blob to follow?
I.e. instead of
int guest_send_clipboard(const char *data, size_t length)
{
return write_full(virtio_fd, data, length);
}
do this:
int guest_send_clipboard(const char *data, size_t length)
{
u32 encoded_length = cpu_to_be32(length);
int err = write_full(virtio_serial_fd,encoded_length,
sizeof(encoded_length));
if (err == 0)
err = write_full(virtio_serial_fd, data, length);
return err;
}
If this doesn't explain it, then I think we're not understanding each
other here.
It does explain it very well, thanks. I think you're misguided about
the solution :-)
What confused me was you mentioned the VNC ServerCutText command
having to receive the whole data in one go. ServerCutText isn't
really relevant to this, and clearly is encoded with VNC protocol
framing. If it was RDP or the SDL client instead of VNC, it would be
something else. All that matters is getting the clipboard blob from
guest to qemu in one piece, right?
Having the guest agent send a few framing bytes seems very simple, and
would have the added bonus that the same guest agent protocol would
work on a real emulated serial port, guest-host TCP, etc. where
virtio-serial isn't available in the guest OS (e.g. older kernels).
I really can't see any merit in making virtio-serial not be a serial
port, being instead like a unix datagram socket, to support a specific
user of virtio-serial when a trivial 4-byte header in the guest agent
code would be easier for that user anyway.
If it did that, I think the name virtio-serial would have to change to
virtio-datagram, becuase it wouldn't behave like a serial port any
more. It would also be less useful for things that _do_ want
something like a pipe/serial port. But why bother?
I agree wrt a streaming protocol verses a datagram protocol. The core
argument IMHO is that the userspace interface is a file descriptor.
Most programmers are used to assuming that boundaries aren't preserved
in read/write calls.
Regards,
Anthony Liguori
-- Jamie
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 01/11/2010 02:39 AM, Amit Shah wrote: On (Fri) Jan 08 2010 [10:26:59], Anthony Liguori wrote: On 01/08/2010 07:35 AM, Jamie Lokier wrote: Sometimes it looks like TCP is maintaining write boundaries, but it is just an artifact of its behaviour on many systems, and is not reliable even on those systems where it seems to happen most of the time. Even when connecting to localhost, you cannot rely on that. I have seen people write code assuming TCP keeps boundaries, and then some weeks later they are very confused debugging their code because it is not reliable... Since VNC is clearly designed to work over TCP, and is written by people who know this, I'm wondering why you think it needs to be different for virtio-serial. I'm confused about why the buffering is needed in the first place. I would think that any buffering should be pushed back to the guest. IOW, if there's available data from the char driver, but the guest doesn't have a buffer. Don't select on the char driver until the guest has a buffer available. If the guest attempts to write data but the char driver isn't ready to receive data, don't complete the request until the char driver can accept data. This is a different thing from what Jamie's talking about. A guest or a host might be interested in communicating data without waiting for the other end to come up. The other end can just start consuming the data (even the data that it missed while it wasn't connected) once it's up. (I can remove this option for now and add it later, if you prefer it that way.) If it's not needed by your use case, please remove it. Doing buffering gets tricky because you can't allow an infinite buffer for security reasons. All you end up doing is increasing the size of the buffer beyond what the guest and client are capable of doing. Since you still can lose data, apps have to be written to handle this. I think it adds complexity without a lot of benefit. Regards, Anthony Liguori Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Mon) Jan 11 2010 [18:28:52], Anthony Liguori wrote: I would think that any buffering should be pushed back to the guest. IOW, if there's available data from the char driver, but the guest doesn't have a buffer. Don't select on the char driver until the guest has a buffer available. If the guest attempts to write data but the char driver isn't ready to receive data, don't complete the request until the char driver can accept data. This is a different thing from what Jamie's talking about. A guest or a host might be interested in communicating data without waiting for the other end to come up. The other end can just start consuming the data (even the data that it missed while it wasn't connected) once it's up. (I can remove this option for now and add it later, if you prefer it that way.) If it's not needed by your use case, please remove it. Doing buffering gets tricky because you can't allow an infinite buffer for security reasons. All you end up doing is increasing the size of the buffer beyond what the guest and client are capable of doing. Since you still can lose data, apps have to be written to handle this. I think it adds complexity without a lot of benefit. The buffering has to remain anyway since we can't assume that the ports will consume the entire buffers we pass on to them. So we'll have to buffer the data till the entire buffer is consumed. That, or the buffer management should be passed off to individual ports. Which might result in a lot of code duplication since we can have a lot of these ports in different places in the qemu code. So I guess it's better to leave the buffer management in the bus itself. Which means we get the 'cache_buffers' functionality essentially for free. Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Mon) Jan 11 2010 [23:33:56], Jamie Lokier wrote:
Amit Shah wrote:
Are you talking about a VNC protocol command between qemu's VNC server
and the user's VNC client, or a private protocol between the guest and
qemu's VNC server?
What happens is:
1. Guest puts something on its clipboard
2. An agent on the guest gets notified of new clipboard contents
3. This agent sends over the entire clipboard contents to qemu via
virtio-serial
4. virtio-serial sends off this data to the virtio-serial-vnc code
5. ServerCutText message from the vnc backend is sent to the vnc client
6. vnc client's clipboard gets updated
7. You can see guest's clipboard contents in your client's clipboard.
I'm talking about steps 3, 4, 5 here.
Ok. Let's not worry about 5; it doesn't seem relevant, only that the
guest clipboad is sent to the host somehow.
Actually, it is important...
You have already told it the total length to expect. There is no
ambiguity about where it ends.
Where does the total length come from? It has to come from the guest.
Otherwise, the vnc code will not know if a byte stream contains two
separate clipboard entries or just one huge clipboard entry.
I see. So it's a *really simple* protocol where the clipboard entry
is sent by the guest agent with a single write() without any framing bytes?
Earlier, I used to send the length of one write as issued by a guest to
qemu. I just changed that to send a START and END flag so that I don't
have to send the length.
Why not just have the guest agent send a 4-byte header which is the
integer length of the clipboard blob to follow?
I.e. instead of
int guest_send_clipboard(const char *data, size_t length)
{
return write_full(virtio_fd, data, length);
}
do this:
int guest_send_clipboard(const char *data, size_t length)
{
u32 encoded_length = cpu_to_be32(length);
int err = write_full(virtio_serial_fd, encoded_length,
sizeof(encoded_length));
if (err == 0)
err = write_full(virtio_serial_fd, data, length);
return err;
}
If this doesn't explain it, then I think we're not understanding each
other here.
It does explain it very well, thanks. I think you're misguided about
the solution :-)
The above solution you specify works if it's assumed that we hold off
writes to the vnc client till we get a complete buffer according to the
header received.
Now, a header might contain the length 1, meaning 1 bytes are to
be expected.
What if the write() on the guest fails after writing 8000 bytes? There's
no way for us to signal that.
So this vnc port might just be waiting for all 1 bytes to be
received, and it may never receive anything more.
Or, it might receive the start of the next clipboard entry and it could
be interpreted as data from the previous copy.
What confused me was you mentioned the VNC ServerCutText command
having to receive the whole data in one go. ServerCutText isn't
really relevant to this,
It is relevant. You can't split up one ServerCutText command in multiple
buffers. You can also not execute any other commands while one command
is in progress, so you have to hold off on executing ServerCutText till
all the data is available. And you can't reliably do that from guest
userspace because of the previously-mentioned scenario.
I really can't see any merit in making virtio-serial not be a serial
port, being instead like a unix datagram socket, to support a specific
user of virtio-serial when a trivial 4-byte header in the guest agent
code would be easier for that user anyway.
BTW I don't really want this too, I can get rid of it if everyone agrees
we won't support clipboard writes 4k over vnc or if there's a better
idea.
Amit
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Amit Shah wrote: On (Fri) Jan 08 2010 [01:12:31], Jamie Lokier wrote: Amit Shah wrote: Guests send us one buffer at a time. Current guests send buffers sized 4K bytes. If guest userspace applications sent out 4K bytes in one write() syscall, the write request actually sends out multiple buffers, each of 4K in size. This usually isn't a problem but for some apps, like VNC, the entire data has to be sent in one go to make copy/paste work fine. So if an app on the guest sends out guest clipboard contents, it has to be sent to the vnc server in one go as the guest app sent it. For this to be done, we need the guest to send us START and END markers for each write request so that we can find out complete buffers and send them off to ports. That looks very dubious. TCP/IP doesn't maintain write boundaries; neither do pipes, unix domain sockets, pseudo-terminals, and almost every other modern byte-oriented transport. So how does VNC transmit the clipboard over TCP/IP to a VNC client, without those boundaries, and why is it different with virtserialport? TCP does this in its stack: it waits for the number of bytes written to be received and then notifies userspace of data availibility. In this case, consider the case where the guest writes 10k of data. The guest gives us those 10k in 3 chunks: the first containing 4k (- header size), the 2nd containing the next 4k (- header size) and the 3rd chunk the remaining data. I want to flush out this data only when I get all 10k. No, TCP does not do that. It does not maintain boundaries, or delay delivery until a full write is transmitted. Even if you use TCP_CORK (Linux specific), that is just a performance hint. If the sender writes 10k of data in a single write() over TCP, and it is split into packets size 4k/4k/2k (assume just over 4k MSS :-), the receiver will be notified of availability any time after the *first* packet is received, and the read() call may indeed return less than 10k. In fact it can be split at any byte position, depending on other activity. Applications handle this by using their own framing protocol on top of the TCP byte stream. For example a simple header saying expect N bytes followed by N bytes, or line delimiters or escape characters. Sometimes it looks like TCP is maintaining write boundaries, but it is just an artifact of its behaviour on many systems, and is not reliable even on those systems where it seems to happen most of the time. Even when connecting to localhost, you cannot rely on that. I have seen people write code assuming TCP keeps boundaries, and then some weeks later they are very confused debugging their code because it is not reliable... Since VNC is clearly designed to work over TCP, and is written by people who know this, I'm wondering why you think it needs to be different for virtio-serial. -- Jamie
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 01/08/2010 07:35 AM, Jamie Lokier wrote: Sometimes it looks like TCP is maintaining write boundaries, but it is just an artifact of its behaviour on many systems, and is not reliable even on those systems where it seems to happen most of the time. Even when connecting to localhost, you cannot rely on that. I have seen people write code assuming TCP keeps boundaries, and then some weeks later they are very confused debugging their code because it is not reliable... Since VNC is clearly designed to work over TCP, and is written by people who know this, I'm wondering why you think it needs to be different for virtio-serial. I'm confused about why the buffering is needed in the first place. I would think that any buffering should be pushed back to the guest. IOW, if there's available data from the char driver, but the guest doesn't have a buffer. Don't select on the char driver until the guest has a buffer available. If the guest attempts to write data but the char driver isn't ready to receive data, don't complete the request until the char driver can accept data. Where does buffering come in? Regards, Anthony Liguori -- Jamie
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Amit Shah wrote: Guests send us one buffer at a time. Current guests send buffers sized 4K bytes. If guest userspace applications sent out 4K bytes in one write() syscall, the write request actually sends out multiple buffers, each of 4K in size. This usually isn't a problem but for some apps, like VNC, the entire data has to be sent in one go to make copy/paste work fine. So if an app on the guest sends out guest clipboard contents, it has to be sent to the vnc server in one go as the guest app sent it. For this to be done, we need the guest to send us START and END markers for each write request so that we can find out complete buffers and send them off to ports. That looks very dubious. TCP/IP doesn't maintain write boundaries; neither do pipes, unix domain sockets, pseudo-terminals, and almost every other modern byte-oriented transport. So how does VNC transmit the clipboard over TCP/IP to a VNC client, without those boundaries, and why is it different with virtserialport? -- Jamie
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On (Fri) Jan 08 2010 [01:12:31], Jamie Lokier wrote: Amit Shah wrote: Guests send us one buffer at a time. Current guests send buffers sized 4K bytes. If guest userspace applications sent out 4K bytes in one write() syscall, the write request actually sends out multiple buffers, each of 4K in size. This usually isn't a problem but for some apps, like VNC, the entire data has to be sent in one go to make copy/paste work fine. So if an app on the guest sends out guest clipboard contents, it has to be sent to the vnc server in one go as the guest app sent it. For this to be done, we need the guest to send us START and END markers for each write request so that we can find out complete buffers and send them off to ports. That looks very dubious. TCP/IP doesn't maintain write boundaries; neither do pipes, unix domain sockets, pseudo-terminals, and almost every other modern byte-oriented transport. So how does VNC transmit the clipboard over TCP/IP to a VNC client, without those boundaries, and why is it different with virtserialport? TCP does this in its stack: it waits for the number of bytes written to be received and then notifies userspace of data availibility. In this case, consider the case where the guest writes 10k of data. The guest gives us those 10k in 3 chunks: the first containing 4k (- header size), the 2nd containing the next 4k (- header size) and the 3rd chunk the remaining data. I want to flush out this data only when I get all 10k. Amit
[Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Guests send us one buffer at a time. Current guests send buffers sized
4K bytes. If guest userspace applications sent out 4K bytes in one
write() syscall, the write request actually sends out multiple buffers,
each of 4K in size.
This usually isn't a problem but for some apps, like VNC, the entire
data has to be sent in one go to make copy/paste work fine. So if an app
on the guest sends out guest clipboard contents, it has to be sent to
the vnc server in one go as the guest app sent it.
For this to be done, we need the guest to send us START and END markers
for each write request so that we can find out complete buffers and send
them off to ports.
This needs us to buffer all the data that comes in from the guests, hold
it off till we see all the data corresponding to one write request,
merge it all in one buffer and then send it to the port the data was
destined for.
Also, we add support for caching of these buffers till a port indicates
it's ready to receive data.
We keep caching data the guest sends us till a port accepts it. However,
this could lead to an OOM condition where a rogue process on the guest
could continue pumping in data while the host continues to cache it.
We introduce a per-port byte-limit property to alleviate this condition.
When this limit is reached, we send a control message to the guest
indicating it to not send us any more data till further indication. When
the number of bytes cached go lesser than the limit specified, we open
tell the guest to restart sending data.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 309 +++-
hw/virtio-serial.c | 11 +-
hw/virtio-serial.h | 39 ++
3 files changed, 352 insertions(+), 7 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 20d9580..c947143 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -44,6 +44,20 @@ struct VirtIOSerial {
struct virtio_console_config config;
};
+/* This struct holds individual buffers received for each port */
+typedef struct VirtIOSerialPortBuffer {
+QTAILQ_ENTRY(VirtIOSerialPortBuffer) next;
+
+uint8_t *buf;
+
+size_t len; /* length of the buffer */
+size_t offset; /* offset from which to consume data in the buffer */
+
+uint32_t flags; /* Sent by guest (start of data stream, end of stream) */
+
+bool previous_failure; /* Did sending out this buffer fail previously? */
+} VirtIOSerialPortBuffer;
+
static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
{
VirtIOSerialPort *port;
@@ -157,6 +171,198 @@ static size_t send_control_event(VirtIOSerialPort *port,
uint16_t event,
return send_control_msg(port, cpkt, sizeof(cpkt));
}
+static void init_buf(VirtIOSerialPortBuffer *buf, uint8_t *buffer, size_t len)
+{
+buf-buf = buffer;
+buf-len = len;
+buf-offset = 0;
+buf-flags = 0;
+buf-previous_failure = false;
+}
+
+static VirtIOSerialPortBuffer *alloc_buf(size_t len)
+{
+VirtIOSerialPortBuffer *buf;
+
+buf = qemu_malloc(sizeof(*buf));
+buf-buf = qemu_malloc(len);
+
+init_buf(buf, buf-buf, len);
+
+return buf;
+}
+
+static void free_buf(VirtIOSerialPortBuffer *buf)
+{
+qemu_free(buf-buf);
+qemu_free(buf);
+}
+
+static size_t get_complete_data_size(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf;
+size_t size;
+bool is_complete, start_seen;
+
+size = 0;
+is_complete = false;
+start_seen = false;
+QTAILQ_FOREACH(buf, port-unflushed_buffers, next) {
+size += buf-len - buf-offset;
+
+if (buf-flags VIRTIO_CONSOLE_HDR_END_DATA) {
+is_complete = true;
+break;
+}
+if (buf == QTAILQ_FIRST(port-unflushed_buffers)
+ !(buf-flags VIRTIO_CONSOLE_HDR_START_DATA)) {
+
+/* There's some data that arrived without a START flag. Flush it.
*/
+is_complete = true;
+break;
+}
+
+if (buf-flags VIRTIO_CONSOLE_HDR_START_DATA) {
+if (start_seen) {
+/*
+ * There's some data that arrived without an END
+ * flag. Flush it.
+ */
+size -= buf-len + buf-offset;
+is_complete = true;
+break;
+}
+start_seen = true;
+}
+}
+return is_complete ? size : 0;
+}
+
+/*
+ * The guest could have sent the data corresponding to one write
+ * request split up in multiple buffers. The first buffer has the
+ * VIRTIO_CONSOLE_HDR_START_DATA flag set and the last buffer has the
+ * VIRTIO_CONSOLE_HDR_END_DATA flag set. Using this information, merge
+ * the parts into one buf here to process it for output.
+ */
+static VirtIOSerialPortBuffer *get_complete_buf(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf, *buf2;
+uint8_t *outbuf;
+size_t out_offset, out_size;
+
+out_size =
[Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Guests send us one buffer at a time. Current guests send buffers sized
4K bytes. If guest userspace applications sent out 4K bytes in one
write() syscall, the write request actually sends out multiple buffers,
each of 4K in size.
This usually isn't a problem but for some apps, like VNC, the entire
data has to be sent in one go to make copy/paste work fine. So if an app
on the guest sends out guest clipboard contents, it has to be sent to
the vnc server in one go as the guest app sent it.
For this to be done, we need the guest to send us START and END markers
for each write request so that we can find out complete buffers and send
them off to ports.
This needs us to buffer all the data that comes in from the guests, hold
it off till we see all the data corresponding to one write request,
merge it all in one buffer and then send it to the port the data was
destined for.
Also, we add support for caching of these buffers till a port indicates
it's ready to receive data.
We keep caching data the guest sends us till a port accepts it. However,
this could lead to an OOM condition where a rogue process on the guest
could continue pumping in data while the host continues to cache it.
We introduce a per-port byte-limit property to alleviate this condition.
When this limit is reached, we send a control message to the guest
indicating it to not send us any more data till further indication. When
the number of bytes cached go lesser than the limit specified, we open
tell the guest to restart sending data.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 305 +++-
hw/virtio-serial.c | 11 +-
hw/virtio-serial.h | 39 ++
3 files changed, 348 insertions(+), 7 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 0a953cd..bc47629 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -46,6 +46,20 @@ struct VirtIOSerial {
uint32_t guest_features;
};
+/* This struct holds individual buffers received for each port */
+typedef struct VirtIOSerialPortBuffer {
+QTAILQ_ENTRY(VirtIOSerialPortBuffer) next;
+
+uint8_t *buf;
+
+size_t len; /* length of the buffer */
+size_t offset; /* offset from which to consume data in the buffer */
+
+uint32_t flags; /* Sent by guest (start of data stream, end of stream) */
+
+bool previous_failure; /* Did sending out this buffer fail previously? */
+} VirtIOSerialPortBuffer;
+
static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
{
VirtIOSerialPort *port;
@@ -159,6 +173,197 @@ static size_t send_control_event(VirtIOSerialPort *port,
uint16_t event,
return send_control_msg(port, cpkt, sizeof(cpkt));
}
+static void init_buf(VirtIOSerialPortBuffer *buf, uint8_t *buffer, size_t len)
+{
+buf-buf = buffer;
+buf-len = len;
+buf-offset = 0;
+buf-flags = 0;
+buf-previous_failure = false;
+}
+
+static VirtIOSerialPortBuffer *alloc_buf(size_t len)
+{
+VirtIOSerialPortBuffer *buf;
+
+buf = qemu_malloc(sizeof(*buf));
+buf-buf = qemu_malloc(len);
+
+init_buf(buf, buf-buf, len);
+
+return buf;
+}
+
+static void free_buf(VirtIOSerialPortBuffer *buf)
+{
+qemu_free(buf-buf);
+qemu_free(buf);
+}
+
+static size_t get_complete_data_size(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf;
+size_t size;
+bool is_complete, start_seen;
+
+size = 0;
+is_complete = false;
+start_seen = false;
+QTAILQ_FOREACH(buf, port-unflushed_buffers, next) {
+size += buf-len - buf-offset;
+
+if (buf-flags VIRTIO_CONSOLE_HDR_END_DATA) {
+is_complete = true;
+break;
+}
+if (buf == QTAILQ_FIRST(port-unflushed_buffers)
+ !(buf-flags VIRTIO_CONSOLE_HDR_START_DATA)) {
+
+/* There's some data that arrived without a START flag. Flush it.
*/
+is_complete = true;
+break;
+}
+
+if (buf-flags VIRTIO_CONSOLE_HDR_START_DATA) {
+if (start_seen) {
+/*
+ * There's some data that arrived without an END
+ * flag. Flush it.
+ */
+size -= buf-len + buf-offset;
+is_complete = true;
+break;
+}
+start_seen = true;
+}
+}
+return is_complete ? size : 0;
+}
+
+/*
+ * The guest could have sent the data corresponding to one write
+ * request split up in multiple buffers. The first buffer has the
+ * VIRTIO_CONSOLE_HDR_START_DATA flag set and the last buffer has the
+ * VIRTIO_CONSOLE_HDR_END_DATA flag set. Using this information, merge
+ * the parts into one buf here to process it for output.
+ */
+static VirtIOSerialPortBuffer *get_complete_buf(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf, *buf2;
+uint8_t *outbuf;
+size_t out_offset, out_size;
+
+out_size =
[Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
Guests send us one buffer at a time. Current guests send buffers sized
4K bytes. If guest userspace applications sent out 4K bytes in one
write() syscall, the write request actually sends out multiple buffers,
each of 4K in size.
This usually isn't a problem but for some apps, like VNC, the entire
data has to be sent in one go to make copy/paste work fine. So if an app
on the guest sends out guest clipboard contents, it has to be sent to
the vnc server in one go as the guest app sent it.
For this to be done, we need the guest to send us START and END markers
for each write request so that we can find out complete buffers and send
them off to ports.
This needs us to buffer all the data that comes in from the guests, hold
it off till we see all the data corresponding to one write request,
merge it all in one buffer and then send it to the port the data was
destined for.
Also, we add support for caching of these buffers till a port indicates
it's ready to receive data.
We keep caching data the guest sends us till a port accepts it. However,
this could lead to an OOM condition where a rogue process on the guest
could continue pumping in data while the host continues to cache it.
We introduce a per-port byte-limit property to alleviate this condition.
When this limit is reached, we send a control message to the guest
indicating it to not send us any more data till further indication. When
the number of bytes cached go lesser than the limit specified, we open
tell the guest to restart sending data.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/virtio-serial-bus.c | 315 +++-
hw/virtio-serial.c |7 +
hw/virtio-serial.h | 44 +++
3 files changed, 364 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index b683109..12317ba 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -52,6 +52,20 @@ struct VirtIOSerial {
uint32_t guest_features;
};
+/* This struct holds individual buffers received for each port */
+typedef struct VirtIOSerialPortBuffer {
+QTAILQ_ENTRY(VirtIOSerialPortBuffer) next;
+
+uint8_t *buf;
+
+size_t len; /* length of the buffer */
+size_t offset; /* offset from which to consume data in the buffer */
+
+uint32_t flags; /* Sent by guest (start of data stream, end of stream) */
+
+bool previous_failure; /* Did sending out this buffer fail previously? */
+} VirtIOSerialPortBuffer;
+
static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
{
VirtIOSerialPort *port;
@@ -167,6 +181,201 @@ static size_t send_control_event(VirtIOSerialPort *port,
uint16_t event,
return send_control_msg(port, cpkt, sizeof(cpkt));
}
+static void init_buf(VirtIOSerialPortBuffer *buf, uint8_t *buffer, size_t len)
+{
+buf-buf = buffer;
+buf-len = len;
+buf-offset = 0;
+buf-flags = 0;
+buf-previous_failure = false;
+}
+
+static VirtIOSerialPortBuffer *alloc_buf(size_t len)
+{
+VirtIOSerialPortBuffer *buf;
+
+buf = qemu_malloc(sizeof(*buf));
+buf-buf = qemu_malloc(len);
+
+init_buf(buf, buf-buf, len);
+
+return buf;
+}
+
+static void free_buf(VirtIOSerialPortBuffer *buf)
+{
+qemu_free(buf-buf);
+qemu_free(buf);
+}
+
+static size_t get_complete_data_size(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf;
+size_t size;
+bool is_complete, start_seen;
+
+size = 0;
+is_complete = false;
+start_seen = false;
+QTAILQ_FOREACH(buf, port-unflushed_buffers, next) {
+size += buf-len - buf-offset;
+
+if (buf-flags VIRTIO_CONSOLE_HDR_END_DATA) {
+is_complete = true;
+break;
+}
+if (buf == QTAILQ_FIRST(port-unflushed_buffers)
+ !(buf-flags VIRTIO_CONSOLE_HDR_START_DATA)) {
+
+/* There's some data that arrived without a START flag. Flush it.
*/
+is_complete = true;
+break;
+}
+
+if (buf-flags VIRTIO_CONSOLE_HDR_START_DATA) {
+if (start_seen) {
+/*
+ * There's some data that arrived without an END
+ * flag. Flush it.
+ */
+size -= buf-len + buf-offset;
+is_complete = true;
+break;
+}
+start_seen = true;
+}
+}
+return is_complete ? size : 0;
+}
+
+/*
+ * The guest could have sent the data corresponding to one write
+ * request split up in multiple buffers. The first buffer has the
+ * VIRTIO_CONSOLE_HDR_START_DATA flag set and the last buffer has the
+ * VIRTIO_CONSOLE_HDR_END_DATA flag set. Using this information, merge
+ * the parts into one buf here to process it for output.
+ */
+static VirtIOSerialPortBuffer *get_complete_buf(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf, *buf2;
+uint8_t *outbuf;
+size_t out_offset, out_size;
+
+out_size =
Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests
On 12/23/2009 01:52 PM, Amit Shah wrote:
Guests send us one buffer at a time. Current guests send buffers sized
4K bytes. If guest userspace applications sent out 4K bytes in one
write() syscall, the write request actually sends out multiple buffers,
each of 4K in size.
This usually isn't a problem but for some apps, like VNC, the entire
data has to be sent in one go to make copy/paste work fine. So if an app
on the guest sends out guest clipboard contents, it has to be sent to
the vnc server in one go as the guest app sent it.
For this to be done, we need the guest to send us START and END markers
for each write request so that we can find out complete buffers and send
them off to ports.
This needs us to buffer all the data that comes in from the guests, hold
it off till we see all the data corresponding to one write request,
merge it all in one buffer and then send it to the port the data was
destined for.
Also, we add support for caching of these buffers till a port indicates
it's ready to receive data.
We keep caching data the guest sends us till a port accepts it. However,
this could lead to an OOM condition where a rogue process on the guest
could continue pumping in data while the host continues to cache it.
We introduce a per-port byte-limit property to alleviate this condition.
When this limit is reached, we send a control message to the guest
indicating it to not send us any more data till further indication. When
the number of bytes cached go lesser than the limit specified, we open
tell the guest to restart sending data.
Signed-off-by: Amit Shahamit.s...@redhat.com
---
hw/virtio-serial-bus.c | 315 +++-
hw/virtio-serial.c |7 +
hw/virtio-serial.h | 44 +++
3 files changed, 364 insertions(+), 2 deletions(-)
+static void flush_all_ports(VirtIOSerial *vser)
+{
+struct VirtIOSerialPort *port;
+
+QTAILQ_FOREACH(port,vser-ports, next) {
+pthread_mutex_lock(port-unflushed_buffers_lock);
+if (port-has_activity) {
+port-has_activity = false;
+flush_queue(port);
+}
+pthread_mutex_unlock(port-unflushed_buffers_lock);
+}
+}
+
+static void remove_port_buffers(VirtIOSerialPort *port)
+{
+struct VirtIOSerialPortBuffer *buf, *buf2;
+
+pthread_mutex_lock(port-unflushed_buffers_lock);
+QTAILQ_FOREACH_SAFE(buf,port-unflushed_buffers, next, buf2) {
+QTAILQ_REMOVE(port-unflushed_buffers, buf, next);
+free_buf(buf);
+}
+pthread_mutex_unlock(port-unflushed_buffers_lock);
+}
The locking here is unnecessary.
Regards,
Anthony Liguori
