Re: [virtio-comment] [PATCH v3] virtio-net: fix Driver Notification description related to VIRTIO_F_NOTIF_CONFIG_DATA

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Feb 03, 2021 at 11:38:27AM +0100, Cornelia Huck wrote:
> On Wed, 3 Feb 2021 01:30:08 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Thu, 7 Jan 2021 17:39:48 +0000
> > Vitaly Mireyno <vmireyno@marvell.com> wrote:
> > 
> > > Incorporated comments for the "[PATCH v9] virtio-net: Add support for the flexible driver notification structure".
> > > Made Driver Notifications description more consistent throughout the document wrt VIRTIO_F_NOTIF_CONFIG_DATA.
> > > 
> > > Changes from v2:
> > >  * Reworked 'vqn' name and definition
> > >   
> > 
> > First sorry for being late.
> 
> This had somehow fallen through the cracks on my side as well.

That's why we have the voting process :)

> > 
> > > 
> > > Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
> > > ---
> > >  content.tex        | 30 ++++++++++++++----------------
> > >  notifications-be.c |  2 +-
> > >  notifications-le.c |  2 +-
> > >  3 files changed, 16 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/content.tex b/content.tex
> > > index 00bc050..19f09d9 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -337,8 +337,12 @@ \section{Driver Notifications} \label{sec:Virtqueues / Driver notifications}
> > >  notification to the device.
> > >  
> > >  When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
> > > -this notification involves sending the
> > > -virtqueue number to the device (method depending on the transport).
> > > +this notification involves sending to the device the virtqueue token
> > > +(method depending on the transport).  
> > 
> > I don't think changing the word order is a good idea. I would prefer
> > 'this notification involves sending the virtqueue token to the device'
> 
> Agreed.
> 
> > 
> > > +if VIRTIO_F_NOTIF_CONFIG_DATA has not been negotiated, the virtqueue token is  
> > 
> > This is a new sentence. You should start it with a capital letter.
> 
> Nod.
> 
> > 
> > > +the virtqueue number to be notified  
> > 
> > To be notified is off here. I believe it can be omitted, or we need
> > something like 'the virtqueue number of the virtqueue to be notified'
> 
> "the number of the virtqueue to be notified" ?
> 
> > 
> > > +if VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated, the virtqueue token is  
> > 
> > This is probably supposed to be a new sentence as well, but you lack
> > both the period and the capital initial letter.
> 
> Nod.
> 
> > 
> > > +the queue notification data of the virtqueue to be notified.
> > >