Re: [PATCH] docs: fix vhost-user protocol doc

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

On Sun, Sep 08, 2024 at 10:49:54PM +0800, luzhixing12345 wrote:
> >On Fri, Sep 06, 2024 at 10:10:45AM +0800, luzhixing12345 wrote:
> >> Hi, can someone help review this patch?
> >> 
> >> Signed-off-by: luzhixing12345 <luzhixing12345@gmail.com>
> >
> >You got comments Aug 5, pls address them.
> 
> ok, the comments are addressed.

Pls post v2, list the comments addressed in the changelog,
coming after ---

Thanks!


> >On Sun, Aug 04, 2024 at 01:04:20PM GMT, luzhixing12345 wrote:
> >>add a ref link to Memory region description
> >>
> >>add extra type(64 bits) to Log description structure fields
> >>
> >>fix 's to 's
> >>
> >>---
> >> docs/interop/vhost-user.rst | 22 +++++++++++++---------
> >> 1 file changed, 13 insertions(+), 9 deletions(-)
> >
> >Please run `scripts/checkpatch.pl` before sending.
> >
> >S-o-b missing here.
> >
> >>
> >>diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> >>index d8419fd2f1..e34b305bd9 100644
> >>--- a/docs/interop/vhost-user.rst
> >>+++ b/docs/interop/vhost-user.rst
> >>@@ -167,6 +167,8 @@ A vring address description
> >> Note that a ring address is an IOVA if ``VIRTIO_F_IOMMU_PLATFORM`` has
> >> been negotiated. Otherwise it is a user address.
> >>
> >>+.. _memory_region_description:
> >>+
> >> Memory region description
> >> ^^^^^^^^^^^^^^^^^^^^^^^^^
> >>
> >>@@ -180,7 +182,7 @@ Memory region description
> >>
> >> :user address: a 64-bit user address
> >>
> >>-:mmap offset: 64-bit offset where region starts in the mapped memory
> >>+:mmap offset: a 64-bit offset where region starts in the mapped memory
> >>
> >> When the ``VHOST_USER_PROTOCOL_F_XEN_MMAP`` protocol feature has been
> >> successfully negotiated, the memory region description contains two extra
> >>@@ -190,7 +192,7 @@ fields at the end.
> >> | guest address | size | user address | mmap offset | xen mmap flags | domid |
> >> +---------------+------+--------------+-------------+----------------+-------+
> >>
> >>-:xen mmap flags: 32-bit bit field
> >>+:xen mmap flags: a 32-bit bit field
> >>
> >> - Bit 0 is set for Xen foreign memory mapping.
> >> - Bit 1 is set for Xen grant memory mapping.
> >>@@ -211,6 +213,8 @@ Single memory region description
> >>
> >> :padding: 64-bit
> >>
> >>+:region: :ref:`Memory region description <memory_region_description>`
> >>+
> >> A region is represented by Memory region description.
> >
> >Should we merge this line with the one added?
> 
> Desciptions about memory regions are merged into one line.
> 
> >>
> >> Multiple Memory regions description
> >
> >Should we extend also the Multiple Memory region description?
> 
> Yes, this patch adds a ref link to it.
> 
> >>@@ -233,9 +237,9 @@ Log description
> >> | log size | log offset |
> >> +----------+------------+
> >>
> >>-:log size: size of area used for logging
> >>+:log size: a 64-bit size of area used for logging
> >>
> >>-:log offset: offset from start of supplied file descriptor where
> >>+:log offset: a 64-bit offset from start of supplied file descriptor where
> >>              logging starts (i.e. where guest address 0 would be
> >>              logged)
> >>
> >>@@ -382,7 +386,7 @@ the kernel implementation.
> >>
> >> The communication consists of the *front-end* sending message requests and
> >> the *back-end* sending message replies. Most of the requests don't require
> >>-replies. Here is a list of the ones that do:
> >>+replies, except for the following requests:
> >>
> >> * ``VHOST_USER_GET_FEATURES``
> >> * ``VHOST_USER_GET_PROTOCOL_FEATURES``
> >>@@ -1239,11 +1243,11 @@ Front-end message types
> >>   (*a vring descriptor index for split virtqueues* vs. *vring descriptor
> >>   indices for packed virtqueues*).
> >>
> >>-  When and as long as all of a device's vrings are stopped, it is
> >>+  When and as long as all of a device's vrings are stopped, it is
> >>   *suspended*, see :ref:`Suspended device state
> >>   <suspended_device_state>`.
> >>
> >>-  The request payload's *num* field is currently reserved and must be
> >>+  The request payload's *num* field is currently reserved and must be
> >>   set to 0.
> >>
> >> ``VHOST_USER_SET_VRING_KICK``
> >>@@ -1662,7 +1666,7 @@ Front-end message types
> >>   :reply payload: ``u64``
> >>
> >>   Front-end and back-end negotiate a channel over which to transfer the
> >>-  back-end's internal state during migration.  Either side (front-end or
> >>+  back-end's internal state during migration.  Either side (front-end or
> >>   back-end) may create the channel.  The nature of this channel is not
> >>   restricted or defined in this document, but whichever side creates it
> >>   must create a file descriptor that is provided to the respectively
> >>@@ -1714,7 +1718,7 @@ Front-end message types
> >>   :request payload: N/A
> >>   :reply payload: ``u64``
> >>
> >>-  After transferring the back-end's internal state during migration (see
> >>+  After transferring the back-end's internal state during migration (see
> >>   the :ref:`Migrating back-end state <migrating_backend_state>`
> >>   section), check whether the back-end was able to successfully fully
> >>   process the state.
> >>-- 
> >>2.34.1
> >>
> 
> Signed-off-by: luzhixing12345 <luzhixing12345@gmail.com>
> ---
>  docs/interop/vhost-user.rst | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index d8419fd2f1..2e50f2ddfa 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -167,6 +167,8 @@ A vring address description
>  Note that a ring address is an IOVA if ``VIRTIO_F_IOMMU_PLATFORM`` has
>  been negotiated. Otherwise it is a user address.
>  
> +.. _memory_region_description:
> +
>  Memory region description
>  ^^^^^^^^^^^^^^^^^^^^^^^^^
>  
> @@ -180,7 +182,7 @@ Memory region description
>  
>  :user address: a 64-bit user address
>  
> -:mmap offset: 64-bit offset where region starts in the mapped memory
> +:mmap offset: a 64-bit offset where region starts in the mapped memory
>  
>  When the ``VHOST_USER_PROTOCOL_F_XEN_MMAP`` protocol feature has been
>  successfully negotiated, the memory region description contains two extra
> @@ -190,7 +192,7 @@ fields at the end.
>  | guest address | size | user address | mmap offset | xen mmap flags | domid |
>  +---------------+------+--------------+-------------+----------------+-------+
>  
> -:xen mmap flags: 32-bit bit field
> +:xen mmap flags: a 32-bit bit field
>  
>  - Bit 0 is set for Xen foreign memory mapping.
>  - Bit 1 is set for Xen grant memory mapping.
> @@ -211,7 +213,7 @@ Single memory region description
>  
>  :padding: 64-bit
>  
> -A region is represented by Memory region description.
> +:region: region is represented by :ref:`Memory region description <memory_region_description>`.
>  
>  Multiple Memory regions description
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> @@ -224,7 +226,7 @@ Multiple Memory regions description
>  
>  :padding: 32-bit
>  
> -A region is represented by Memory region description.
> +:regions: regions field contains 8 regions of type :ref:`Memory region description <memory_region_description>`.
>  
>  Log description
>  ^^^^^^^^^^^^^^^
> @@ -233,9 +235,9 @@ Log description
>  | log size | log offset |
>  +----------+------------+
>  
> -:log size: size of area used for logging
> +:log size: a 64-bit size of area used for logging
>  
> -:log offset: offset from start of supplied file descriptor where
> +:log offset: a 64-bit offset from start of supplied file descriptor where
>               logging starts (i.e. where guest address 0 would be
>               logged)
>  
> @@ -382,7 +384,7 @@ the kernel implementation.
>  
>  The communication consists of the *front-end* sending message requests and
>  the *back-end* sending message replies. Most of the requests don't require
> -replies. Here is a list of the ones that do:
> +replies, except for the following requests:
>  
>  * ``VHOST_USER_GET_FEATURES``
>  * ``VHOST_USER_GET_PROTOCOL_FEATURES``
> @@ -1239,11 +1241,11 @@ Front-end message types
>    (*a vring descriptor index for split virtqueues* vs. *vring descriptor
>    indices for packed virtqueues*).
>  
> -  When and as long as all of a device’s vrings are stopped, it is
> +  When and as long as all of a device's vrings are stopped, it is
>    *suspended*, see :ref:`Suspended device state
>    <suspended_device_state>`.
>  
> -  The request payload’s *num* field is currently reserved and must be
> +  The request payload's *num* field is currently reserved and must be
>    set to 0.
>  
>  ``VHOST_USER_SET_VRING_KICK``
> @@ -1662,7 +1664,7 @@ Front-end message types
>    :reply payload: ``u64``
>  
>    Front-end and back-end negotiate a channel over which to transfer the
> -  back-end’s internal state during migration.  Either side (front-end or
> +  back-end's internal state during migration.  Either side (front-end or
>    back-end) may create the channel.  The nature of this channel is not
>    restricted or defined in this document, but whichever side creates it
>    must create a file descriptor that is provided to the respectively
> @@ -1714,7 +1716,7 @@ Front-end message types
>    :request payload: N/A
>    :reply payload: ``u64``
>  
> -  After transferring the back-end’s internal state during migration (see
> +  After transferring the back-end's internal state during migration (see
>    the :ref:`Migrating back-end state <migrating_backend_state>`
>    section), check whether the back-end was able to successfully fully
>    process the state.
> -- 
> 2.34.1