Feedback for chapter 5

[Thomas Huth <thuth@linux.vnet.ibm.com>]

Here's my review for Virtio draft 01, chapter 05:

Generic remark: Quite a lot of structures use a mixture of "u8"
and "char" types for bytes, e.g. struct virtio_blk_req. Is that
intended? If not, I'd suggest to always use "u8" here instead.


Page 38 / Device Types:

Is the information about other virtio devices like 9P documented
somewhere else? If yes, you might want to put a pointer to these
documents here.


Page 43 and page 44:

Some of the #defines are indented with an additional space.
That looks a little bit ugly, would be great if you could get rid
of the indentation here.


Page 43 / Setting MAC Address Filtering

ETH_ALEN is a Linux-internal #define only, which is not defined
in this spec, so I'd suggest to simply replace ETH_ALEN with 6 here.


Page 48 / Virtqueues

The initial numbered list names the second pair of console virtqueues
"port1" but the next sentence talks about "Port 2" ... that's confusing.
==> Maybe replace "Ports 2 onwards only exist ..." with something like
"All ports except the first one only exist ..." ?


Page 50:

The paragraph "5." says:
"If the device specified a port `name', a sysfs attribute is created with
the name filled in, so that udev rules can be written that can create a
symlink from the port's name to the char device for port discovery by
applications in the driver."
==> That's completely specific to Linux as far as I can tell, so I think
this should not go into a generic specification document, or at least it
should be marked as an example for Linux.
Also, this paragraph talks about the "name" which is not introduced before
paragraph "6.", so I think if you really want to keep this paragraph "5.",
it should be moved behind "6." instead.

Concerning paragraph "6.":
The console port change events are hardly documented ... I think this
paragraph needs some love. How does the "value" look like for the
various message types? For example, how is the size of the console
passed when the VIRTIO_CONSOLE_RESIZE event occured? Or the name? What
is the difference between VIRTIO_CONSOLE_PORT_ADD and
VIRTIO_CONSOLE_PORT_OPEN?

As a further remark, I also I wonder whether should be a way to signal
the terminal type (like "vt100") to the guest?


Page 55 / Device Operation: Request Queues

While reading this chapter, I first got a little bit confused about the
terms "read-only" and "write-only" since I read chapter 4.1.3.1.1
("Virtio Device Configuration Layout Detection") shortly before, where
the terms are used in the opposite way - since "read-only" and
"write-only" are dependend on the view, whether you talk about the device
or the driver.
So even it's clear when you read the various chapters twice and think
about everything logically, it might be more consisten and easier to
read if you always say something more explicit like "read-only for the
device" or "read-only for the driver" throughout the specification.


 Thomas