[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] comments for the virtio spec - Introduction section
On Thu, Jan 30, 2014 at 12:38:38PM +0200, Michael S. Tsirkin wrote: > On Wed, Jan 29, 2014 at 03:04:50PM +1030, Rusty Russell wrote: > > Hi Laura, > > > > Thanks for the feedback! > > > > Laura Novich <lnovich@redhat.com> writes: > > > If you can't open the document here are my comments > > > > Hmm, attachment got stripped? > > > > > INTRODUCTION > > > =============== > > > 1. Remove The entire first paragraph with the exveption of the first sentence. > > > 2. Replace with - A virtual device in its truest sense is not any different from a physical device. The virtual machine sees a virtual device just as a physical machine would see a physical one. As such, this document will treat virtual devices as physical devices. > > > > This isn't quite correct. Consider: > > > > This document describes the specifications of the “virtio” > > family of devices. These devices are found in virtual > > environments > > > > vs: > > This document describes the specifications of the “virtio” > > family of devices. A virtual device in its truest sense is not > > any different from a physical device. > > > > We need to say that virtio devices are found in virtual environments. > > > > Secondly, you are missing historical context: Xen and HyperV treat > > virtual devices as very different beasts than physical devices. It is > > not a fundamental feature of these devices that they are no different > > from a physical device, but rather a conscious decision by the spec > > authors. > > > > > 3. The purpose of ... > > > 4. replace rather than using boutique to rather than using vendor-specific, per-environment, or per-OS mechanisms. > > > > Is "boutique" too obscure? I like it because it casts an image of the > > alternatives being niche. That may be wishful thinking, but it was a > > deliberate choice :) > > > > > 5. ADD -The categories described above are further defined as follows: > > > Straightforward: Revise last sentence to read - There aren’t any exotic page-flipping or COW mechanisms and it functions as a normal device.[1] > > > > The end of this sentence is not meaningful. Compare: > > > > There is no exotic page-flipping or COW mechanism: it's just > > a normal device. > > > > The ": it's just..." emphasizes the former. With your variant: > > > > > > There aren’t any exotic page-flipping or COW mechanisms and it > > functions as a normal device. > > > > It's not clear what "functions as a normal device" means. > > > > > 6. Efficient: ADD to input and output (I/O), > > > > Again, I disagree. I/O conflates input and output, whereas I wish to > > call them both out separately. Perhaps we should add the word "both", > > ie: > > > > descriptors for both input and output... > > > > > 7. effects from both guest and (not sure but i think you should ADD host) device writing to the same cache > > > lines. > > > > Throughout the document, we explicitly refer to "device" and "driver" > > rather than host and guest. This reinforces the design decision that > > they should appear "normal", as well as being more specific. > > > > > 8. Standard: CHANGE TO READ: Virtio makes no assumptions about the environment in which > > > it operates, beyond what is required in order to allow the bus to attach the host device to the driver on the guest. Virtio > > > devices are implemented over PCI and other buses, and earlier drafts that > > > have been implemented on other buses not included in this specification.[2] > > > > You suggest replacing "beyond supporting the bus attaching the device." > > with "beyond what is required in order to allow the bus to attach the > > host device to the driver on the guest." > > > > What do these extra words add? > > > > There is a missing word "have", but it doesn't need "that". > > > > ...and earlier drafts been implemented on other buses not included > > in this spec. > > > > And I'll accept changing "spec" to "specification". > > > > So this is the difference I have so far from your feedback: > > > > diff --git a/introduction.tex b/introduction.tex > > index 5d57f78..98e1e0d 100644 > > --- a/introduction.tex > > +++ b/introduction.tex > > @@ -13,14 +13,14 @@ inter-guest communication) requires copying. > > } > > > > Efficient: Virtio devices consist of rings of descriptors > > - for input and output, which are neatly separated to avoid cache > > + for both input and output, which are neatly separated to avoid cache > > effects from both driver and device writing to the same cache > > lines. > > missed this one. I agree. Actually, rereading this, it's still IMHO not very clear what is separated from what. descriptors? I think output here means avail and input used? If yes then let's say so: + for both input and output. Input rings are neatly separated from output rings + to avoid cache Alternatively if we want to be more precise: Really descriptors are not in a ring at all: they are in the buffer. OTOH used ring is separated from avail ring, but they are not for input and output: input and output are in descriptor buffer. How about + Efficient: Virtio devices consist of a memory buffer with descriptors + for both input and output as well as memory rings of indices + for driver-to-device and device-to-driver communication. + device-to-driver rings are neatly separated from driver-to-device + rings to avoid cache Though I think the shorter version is fine for introduction. > > Standard: Virtio makes no assumptions about the environment in which > > it operates, beyond supporting the bus attaching the device. Virtio > > devices are implemented over PCI and other buses, and earlier drafts > > - been implemented on other buses not included in this spec. > > + have been implemented on other buses not included in this specification. > > \footnote{The Linux implementation further separates the PCI virtio code > > from the specific virtio drivers: these drivers are shared with > > the non-PCI implementations (currently lguest and S/390). > > I tweaked this one a bit more. > Can you pls tell me whether you agree with the patchset I posted? > I think it clarifies misunderstandings (Laura might not be the > only one misreading this text) without losing context. > > > Cheers, > > Rusty. > > > > > > This publicly archived list offers a means to provide input to the > > OASIS Virtual I/O Device (VIRTIO) TC. > > > > In order to verify user consent to the Feedback License terms and > > to minimize spam in the list archive, subscription is required > > before posting. > > > > Subscribe: virtio-comment-subscribe@lists.oasis-open.org > > Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org > > List help: virtio-comment-help@lists.oasis-open.org > > List archive: https://lists.oasis-open.org/archives/virtio-comment/ > > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf > > List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists > > Committee: https://www.oasis-open.org/committees/virtio/ > > Join OASIS: https://www.oasis-open.org/join/ > > This publicly archived list offers a means to provide input to the > OASIS Virtual I/O Device (VIRTIO) TC. > > In order to verify user consent to the Feedback License terms and > to minimize spam in the list archive, subscription is required > before posting. > > Subscribe: virtio-comment-subscribe@lists.oasis-open.org > Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org > List help: virtio-comment-help@lists.oasis-open.org > List archive: https://lists.oasis-open.org/archives/virtio-comment/ > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf > List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists > Committee: https://www.oasis-open.org/committees/virtio/ > Join OASIS: https://www.oasis-open.org/join/
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]