Re: [Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests

[Jamie Lokier <jamie@shareable.org>]

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