Re: [PATCH] vhost-user: Fix protocol feature bit conflict

[Stefan Hajnoczi]

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

On Mon, 16 Oct 2023 at 04:33, Hanna Czenczek <hreitz@redhat.com> wrote:
>
> The VHOST_USER_PROTOCOL_F_XEN_MMAP feature bit was defined in
> f21e95ee97d, which has been part of qemu's 8.1.0 release.  However, it
> seems it was never added to qemu's code, but it is well possible that it
> is already used by different front-ends outside of qemu (i.e., Xen).
>
> VHOST_USER_PROTOCOL_F_SHARED_OBJECT in contrast was added to qemu's code
> in 16094766627, but never defined in the vhost-user specification.  As a
> consequence, both bits were defined to be 17, which cannot work.
>
> Regardless of whether actual code or the specification should take
> precedence, F_XEN_MMAP is already part of a qemu release, while
> F_SHARED_OBJECT is not.  Therefore, bump the latter to take number 18
> instead of 17, and add this to the specification.
>
> Take the opportunity to add at least a little note on the
> VhostUserShared structure to the specification.  This structure is
> referenced by the new commands introduced in 16094766627, but was not
> defined.
>
> Fixes: 160947666276c5b7f6bca4d746bcac2966635d79
>        ("vhost-user: add shared_object msg")
> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
> ---
>  docs/interop/vhost-user.rst               | 11 +++++++++++
>  include/hw/virtio/vhost-user.h            |  3 ++-
>  subprojects/libvhost-user/libvhost-user.h |  3 ++-
>  3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 415bb47a19..768fb5c28c 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -275,6 +275,16 @@ Inflight description
>
>  :queue size: a 16-bit size of virtqueues
>
> +VhostUserShared
> +^^^^^^^^^^^^^^^
> +
> ++------+
> +| UUID |
> ++------+
> +
> +:UUID: 16 bytes UUID, whose first three components (a 32-bit value, then
> +  two 16-bit values) are stored in big endian.
> +
>  C structure
>  -----------
>
> @@ -885,6 +895,7 @@ Protocol features
>    #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS  15
>    #define VHOST_USER_PROTOCOL_F_STATUS               16
>    #define VHOST_USER_PROTOCOL_F_XEN_MMAP             17
> +  #define VHOST_USER_PROTOCOL_F_SHARED_OBJECT        18
>
>  Front-end message types
>  -----------------------
> diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
> index 9f9ddf878d..1d4121431b 100644
> --- a/include/hw/virtio/vhost-user.h
> +++ b/include/hw/virtio/vhost-user.h
> @@ -29,7 +29,8 @@ enum VhostUserProtocolFeature {
>      VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
>      VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
>      VHOST_USER_PROTOCOL_F_STATUS = 16,
> -    VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 17,
> +    /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
> +    VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
>      VHOST_USER_PROTOCOL_F_MAX
>  };
>
> diff --git a/subprojects/libvhost-user/libvhost-user.h 
> b/subprojects/libvhost-user/libvhost-user.h
> index b36a42a7ca..c2352904f0 100644
> --- a/subprojects/libvhost-user/libvhost-user.h
> +++ b/subprojects/libvhost-user/libvhost-user.h
> @@ -65,7 +65,8 @@ enum VhostUserProtocolFeature {
>      VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS = 14,
>      VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
>      /* Feature 16 is reserved for VHOST_USER_PROTOCOL_F_STATUS. */
> -    VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 17,
> +    /* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
> +    VHOST_USER_PROTOCOL_F_SHARED_OBJECT = 18,
>      VHOST_USER_PROTOCOL_F_MAX
>  };
>
> --
> 2.41.0
>
>