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