Re: [Qemu-devel] [PATCH v8 7/7] virtio-console: Enable port throttling when chardev is slow to consume data

[Paul Brook]

> > > Sure.  My proposal is for qemu_chr_write() to succeed all the time.  If
> > > the backend can block, and the caller can handle it, it can get a
> > > -EAGAIN (or WSAEWOULDBLOCK) return.  When the backend becomes writable,
> > > the chardev can call the ->writes_unblocked() callback for that caller.
> > > Individual callers need not bother about re-submitting partial writes,
> > > since the buffering can be done in common code in one place
> > > (qemu-char.c).
> > 
> > That's only OK if you assume it's OK to buffer all but one byte of the
> > transmit request.
> 
> Is that a fair assumption to make?

No. See below.

> > > But that's entirely in guest memory, so it's limited to the amount of
> > > RAM that has been allocated to the guest.
> > 
> > Exactly. The guest can cause ram_size * nr_ports of additional host
> > memory to be allocated.  Not acceptable.
> 
> OK -- so this is how it adds up:
> 
> - guest vq
> - virtio-serial-bus converts iov to buf

This is an unbelievably lame piece of code. There's absolutely no reason to 
copy the data into a linear buffer. You should just be iterating over the 
elements of the sglist.

> - qemu-char stores the buf in case it wasn't able to send
>
> but then, since it's all async, we have:
> 
> - virtio-serial-bus frees the buf
> - guest deletes the buf and removes it from the vq
>
> So what's left is only the data in qemu-char's buf.  Now this can be
> (buf_size - 1) * nr_ports in the worst case.

Add at least another buf_size because you have to allocate the qemu-char 
buffer before you free the virtio-serial buffer. We can expect that
buf_size ~= guest ram size [1], so for practical purposes it may as well be 
unbounded.

Worst case the guest could submit a buffer consisting of many large 
overlapping regions, giving a buf_size many times greater then guest ram size.  
Technically such code isn't even doing anything wrong. We're only reading from 
guest RAM, so in principle the same memory can be submitted multiple times 
without causing a race condition.

> The alternative would be to keep using the guest buffer till all's done,

Yes.

> but then that depends on qemu getting async support - separating out the
> qemu_chr_write() into a separate thread and allowing vcpu and chr io
> operations to be run simultaneously.

You don't need any special async char API or threads.  Normal unix write 
semantics (i.e. short writes and EAGAIN) plus the unblock hook are sufficient.
As mentioned above, the virtio-serial code should be iterating over the 
sglist.  If the host won't accept all the data immediately then just remember 
how much has been sent, and resume iteration when the unblock hook is called.

Paul

[1] This kind of insanity does actually happen in the real world.  e.g. 
loading a 700Mb ramdisk image via the fw_cfg device, or a kernel panic handler 
that dumps the contents of RAM to a debug channel (i.e. serial port).