Re: [Qemu-devel] [PATCH 1/1] vhost-user: modify SET_LOG_BASE only if VHOST_USER_PROTOCOL_F_LOG_SHMFD is set

[Michael S. Tsirkin]

On Mon, Nov 16, 2015 at 05:14:37PM +0100, Thibaut Collet wrote:
> Fixes: 2b8819c6eee5 ("vhost-user: modify SET_LOG_BASE to pass mmap size and
> offset")
> 
> For compatibility with old vhost backend content of the SET_LOG_BASE message
> can not be modified.

Hmm that's true. Interesting. But this only happens on migration,
right? And if VHOST_USER_PROTOCOL_F_LOG_SHMFD is not set
then we block migration. So how come the old message
is ever sent?

> The SET_LOG_BASE message payload is modified only if the
> VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature has been negociated.
> 
> The documentation has been updated accordingly with remarks from Marc André
> Lureau.
> 
> Signed-off-by: Thibaut Collet <address@hidden>
> ---
>  docs/specs/vhost-user.txt | 16 ++++++++++++++--
>  hw/virtio/vhost-user.c    | 12 +++++++++---
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 26dde2e..da4bf9c 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -87,6 +87,14 @@ Depending on the request type, payload can be:
>     User address: a 64-bit user address
>     mmap offset: 64-bit offset where region starts in the mapped memory
>  
> + * vhost user log description
> +   -----------------
> +   | size | offset |
> +   -----------------
> +
> +   size: a 64-bit size
> +   Offset: a 64-bit offset where log starts in the mapped memory
> +
>  In QEMU the vhost-user message is implemented with the following struct:
>  
>  typedef struct VhostUserMsg {
> @@ -280,14 +288,18 @@ Message types
>  
>        Id: 6
>        Equivalent ioctl: VHOST_SET_LOG_BASE
> -      Master payload: u64
> +      Master payload: - u64 if slave has not the 
> VHOST_USER_PROTOCOL_F_LOG_SHMFD
> +                        protocol feature
> +                      - vhost user log if slave has the
> +                        VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature
>        Slave payload: N/A
>  
>        Sets logging shared memory space.
>        When slave has VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol
>        feature, the log memory fd is provided in the ancillary data of
>        VHOST_USER_SET_LOG_BASE message, the size and offset of shared
> -      memory area provided in the message.
> +      memory area provided in the message and the message reply is an
> +      empty message (size of 0).
>  
>  
>   * VHOST_USER_SET_LOG_FD
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index c443602..dcdfd40 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -206,11 +206,17 @@ static int vhost_user_set_log_base(struct vhost_dev 
> *dev, uint64_t base,
>      VhostUserMsg msg = {
>          .request = VHOST_USER_SET_LOG_BASE,
>          .flags = VHOST_USER_VERSION,
> -        .payload.log.mmap_size = log->size,
> -        .payload.log.mmap_offset = 0,
> -        .size = sizeof(msg.payload.log),
>      };
>  
> +    if (shmfd) {
> +        msg.payload.log.mmap_size = log->size;
> +        msg.payload.log.mmap_offset = 0;
> +        msg.size = sizeof(msg.payload.log);
> +    } else {
> +        msg.payload.u64 = base;
> +        msg.size = sizeof(msg.payload.u64);
> +    }
> +
>      if (shmfd && log->fd != -1) {
>          fds[fd_num++] = log->fd;
>      }
> -- 
> 2.1.4