[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