[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: [OASIS Issue Tracker] (VIRTIO-90) VIRTIO Spec feedback #6 From: Thomas Huth
[ https://tools.oasis-open.org/issues/browse/VIRTIO-90?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Michael Tsirkin updated VIRTIO-90: ---------------------------------- Affects Version/s: virtio 1.0 csprd01 Fix Version/s: virtio 1.0 csprd02 > VIRTIO Spec feedback #6 From: Thomas Huth > ------------------------------------------ > > Key: VIRTIO-90 > URL: https://tools.oasis-open.org/issues/browse/VIRTIO-90 > Project: OASIS Virtual I/O Device (VIRTIO) TC > Issue Type: Bug > Affects Versions: virtio 1.0 csprd01 > Reporter: Michael Tsirkin > Fix For: virtio 1.0 csprd02 > > > Date: Wed, 29 Jan 2014 21:36:30 +0100 > Link to Mail: https://lists.oasis-open.org/archives/virtio-comment/201401/msg00055.html > Commenter name: Thomas Huth <thuth@linux.vnet.ibm.com> > 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. -- This message was sent by Atlassian JIRA (v6.1.1#6155)
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]