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 <m...@redhat.com> 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 <thibaut.col...@6wind.com> > >> --- > >> 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