On (Tue) Nov 10 2009 [17:27:03], Rusty Russell wrote: > On Tue, 10 Nov 2009 04:57:30 pm Rusty Russell wrote: > > > > Rather than assume a single port, add a 'struct ports' with an array > > of ports. Currently, there's always only one, but that will change. > > Now, from here we need several more patches. At least: Hey Rusty, Thanks. I'll look through these. I've split up my code as well, have put it up at http://git.kernel.org/?p=linux/kernel/git/amit/virtio-console.git > 1) Generalize the output path so we can sleep, rather than spinning. > This means a non-NULL callback, a 'done' flag and a struct completion. Instead of the buffer pool? I think we can go with the buffer pool of fixed-sized buffers, but a lot lesser than the 1024 I had. (Also meshes with the vnc comment below) On the other hand, performance isn't critical, so we could do away with the pool. > 2) Do we really need more than input buffer at a time? If not, it's easy to > generalize the input callback. This will be slow, but shouldn't be a > problem. In my testing of a vnc clipboard copy/paste, the vnc client only sent the clipboard data once and didn't bother about retransmitting if write() returned < len. It's a problem with the vnc client I used (tigervnc, which is based off tightvnc) though but adding that support would hurt? > 3) Add a header. I suggest we change your proposed format a little, rather > than an explicit length use a "continues" bit: > > struct virtio_console_header { > /* Port number */ > __u32 id; > /* VIRTIO_CONSOLE_HDR_CONTROLMSG, VIRTIO_CONSOLE_HDR_CONTINUES. */ > __u32 flags; > } __attribute__((packed)); Good idea. > This should be really easy to construct, and for input in the !multiport > path we can fake one up. We ignore CONTINUES on input since we don't have > a userspace API which understands framing (we'd need recvmsg). The header should only be sent (and expected) in the multiport case, so this won't matter when the .._F_MULTIPORT feature is not found. > 4) Hook it all together with the new feature bit. I've actually split it into 4 feature bits: MULTIPORT means multiple ports and a header THROTTLE to save host and guest from OOM CACHING to allow ports to buffer data even after the char device is closed UNPLUG to allow port unplug (I added these as part of the splitting effort because they're now in individual patches) > 5) Add the debugfs and sysfs stuff in stages. Agreed (and have done in my scratch series). > 6) See if we really need throttling now we're only allowing 1 buffer at a > time. That also depends if we cache data per port -- the throttling I had was for the per-port unconsumed buffers. Thanks again, Amit _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/virtualization
