qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [PATCH 1/1] vhost-user: modify SET_LOG_BASE only if VHOST_USER_PROTOCOL_F_LOG_SHMFD is set
Date: Mon, 16 Nov 2015 19:09:54 +0200

On Mon, Nov 16, 2015 at 05:53:10PM +0100, Thibaut Collet wrote:
> On Mon, Nov 16, 2015 at 5:21 PM, Michael S. Tsirkin <address@hidden> wrote:
> > 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?
> >
> 
> Agree. With the migration blocker on VHOST_USER_PROTOCOL_F_LOG_SHMFD
> message with the new payload can not be sent to old vhost backend.
> The documentation is a little bit confusing. The message payload for
> SET_LOG_BASE still indicates a u64 whereas it is no more the case.
> 
> Modification on the vhost-user.c file is not really needed but maybe
> we can keep the patch on the doc ?

Absolutely but let's say something like
        Historically migration would still happen when
        VHOST_USER_PROTOCOL_F_LOG_SHMFD was not negotiated.
        In that case, the value sent was u64 with no file
        descriptors. This message should be ignored.

> >> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]