OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: RE: [PATCH v2] virtio-net: Define configuration field layout before its description


> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Friday, February 17, 2023 4:29 AM
> 
> I think it's ok that this combines text movement and rewording. Splitting up
> would have made very small patches.

Yes. the description modified is linked to the structure change. Hence, it was in one patch.
But will relook again if there is logical boundary that I can use to split.

> In fact something I think applies to other contributions too.
> But please do reflect it in the subject.
> Something like "config field layout reorg" would better describe what is going
> on.
> 
> Also pls try to keep subject line length below 50 characters.
> Abbreviation is ok there.
> 
Ok. will do.

> > -Device configuration fields are listed below, they are read-only for
> > a driver. The \field{mac} address field -always exists (though is only
> > valid if VIRTIO_NET_F_MAC is set), and -\field{status} only exists if
> > VIRTIO_NET_F_STATUS is set. Two -read-only bits (for the driver) are
> currently defined for the status field:
> > -VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> > +The network device has the following device configuration layout.
> > +These device configuration fields are read-only for the driver.
> 
> First time we are mentioning fields, which "These"?
> Really this sentence is part of field description so I feel it should come after the
> structure.
Make sense.

> And I feel read-only part needs more stress.
> Maybe "All device configuration fields are read-only for the driver"?

Ok.
> Maybe add a driver conformance statement too?
> 
Will add.

And once reviewed will raise for vote as you explained in other thread.


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]