[OASIS Issue Tracker] (VIRTIO-90) VIRTIO Spec feedback #6 From: Thomas Huth

[OASIS Issues Tracker <workgroup_mailer@lists.oasis-open.org>]

     [ 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)