Re: [Qemu-devel] [PATCH v11 1/7] vhost-user: use VHOST_USER_XXX macro for switch statement

[Marcel Apfelbaum <marcel@redhat.com>]

On 09/24/2015 01:18 PM, Michael S. Tsirkin wrote:
> On Thu, Sep 24, 2015 at 01:05:11PM +0300, Marcel Apfelbaum wrote:
>> On 09/23/2015 07:19 AM, Yuanhan Liu wrote:
>>> So that we could let vhost_user_call to handle extented requests,
>>> such as VHOST_USER_GET/SET_PROTOCOL_FEATURES, instead of invoking
>>> vhost_user_read/write and constructing the msg again by ourself.
>>>
>>> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>>> ---
>>>   hw/virtio/vhost-user.c | 38 ++++++++++++++++++++++----------------
>>>   1 file changed, 22 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>> index e7ab829..13677ac 100644
>>> --- a/hw/virtio/vhost-user.c
>>> +++ b/hw/virtio/vhost-user.c
>>> @@ -193,27 +193,33 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
>>>
>>>       assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>>>
>>> -    msg_request = vhost_user_request_translate(request);
>>> +    /* only translate vhost ioctl requests */
>>> +    if (request > VHOST_USER_MAX) {
>>> +        msg_request = vhost_user_request_translate(request);
>>> +    } else {
>>> +        msg_request = request;
>>> +    }
>>> +
>>>       msg.request = msg_request;
>>>       msg.flags = VHOST_USER_VERSION;
>>>       msg.size = 0;
>>>
>>> -    switch (request) {
>>> -    case VHOST_GET_FEATURES:
>>> +    switch (msg_request) {
>>> +    case VHOST_USER_GET_FEATURES:
>>>           need_reply = 1;
>>>           break;
>>>
>>> -    case VHOST_SET_FEATURES:
>>> -    case VHOST_SET_LOG_BASE:
>>> +    case VHOST_USER_SET_FEATURES:
>>> +    case VHOST_USER_SET_LOG_BASE:
>>>           msg.u64 = *((__u64 *) arg);
>>>           msg.size = sizeof(m.u64);
>>>           break;
>>>
>>> -    case VHOST_SET_OWNER:
>>> -    case VHOST_RESET_OWNER:
>>> +    case VHOST_USER_SET_OWNER:
>>> +    case VHOST_USER_RESET_OWNER:
>>>           break;
>>>
>>> -    case VHOST_SET_MEM_TABLE:
>>> +    case VHOST_USER_SET_MEM_TABLE:
>>>           for (i = 0; i < dev->mem->nregions; ++i) {
>>>               struct vhost_memory_region *reg = dev->mem->regions + i;
>>>               ram_addr_t ram_addr;
>>> @@ -246,30 +252,30 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request,
>>>
>>>           break;
>>>
>>> -    case VHOST_SET_LOG_FD:
>>> +    case VHOST_USER_SET_LOG_FD:
>>>           fds[fd_num++] = *((int *) arg);
>>>           break;
>>>
>>> -    case VHOST_SET_VRING_NUM:
>>> -    case VHOST_SET_VRING_BASE:
>>> +    case VHOST_USER_SET_VRING_NUM:
>>> +    case VHOST_USER_SET_VRING_BASE:
>>>           memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
>>>           msg.size = sizeof(m.state);
>>>           break;
>>>
>>> -    case VHOST_GET_VRING_BASE:
>>> +    case VHOST_USER_GET_VRING_BASE:
>>>           memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
>>>           msg.size = sizeof(m.state);
>>>           need_reply = 1;
>>>           break;
>>>
>>> -    case VHOST_SET_VRING_ADDR:
>>> +    case VHOST_USER_SET_VRING_ADDR:
>>>           memcpy(&msg.addr, arg, sizeof(struct vhost_vring_addr));
>>>           msg.size = sizeof(m.addr);
>>>           break;
>>>
>>> -    case VHOST_SET_VRING_KICK:
>>> -    case VHOST_SET_VRING_CALL:
>>> -    case VHOST_SET_VRING_ERR:
>>> +    case VHOST_USER_SET_VRING_KICK:
>>> +    case VHOST_USER_SET_VRING_CALL:
>>> +    case VHOST_USER_SET_VRING_ERR:
>>>           file = arg;
>>>           msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
>>>           msg.size = sizeof(m.u64);
>>>
>>
>>
>> --
>> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
>
> The -- is the signature separator.
> Please don't put text after that - I almost missed it.
>

Ooops, I'll be more careful next time (I did it already for this series...)

Thanks,
Marcel

>