[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL
From: |
Felipe Franciosi |
Subject: |
Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL |
Date: |
Fri, 13 Jan 2017 15:09:46 +0000 |
Hi Marc-Andre,
> On 13 Jan 2017, at 07:03, Marc-André Lureau <address@hidden> wrote:
>
> Hi
>
> ----- Original Message -----
>> Currently, VQs are started as soon as a SET_VRING_KICK is received. That
>> is too early in the VQ setup process, as the backend might not yet have
>
> I think we may want to reconsider queue_set_started(), move it elsewhere,
> since kick/call fds aren't mandatory to process the rings.
Hmm. The fds aren't mandatory, but I imagine in that case we should still
receive SET_VRING_KICK/CALL messages without an fd (ie. with the
VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?
>
>> a callfd to notify in case it received a kick and fully processed the
>> request/command. This patch only starts a VQ when a SET_VRING_CALL is
>> received.
>
> I don't like that much, as soon as the kick fd is received, it should start
> polling it imho. callfd is optional, it may have one and not the other.
So the question is whether we should be receiving a SET_VRING_CALL anyway or
not, regardless of an fd being sent. (I think we do, but I haven't done
extensive testing with other device types.)
>
> Perhaps it's best for now to delay the callfd notification with a flag until
> it is received?
The other idea is to always kick when we receive the callfd. I remember
discussing that alternative with you before libvhost-user went in. The protocol
says both the driver and the backend must handle spurious kicks. This approach
also fixes the bug.
I'm happy with whatever alternative you want, as long it makes libvhost-user
usable for storage devices.
Thanks,
Felipe
>
>
>> Signed-off-by: Felipe Franciosi <address@hidden>
>> ---
>> contrib/libvhost-user/libvhost-user.c | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/contrib/libvhost-user/libvhost-user.c
>> b/contrib/libvhost-user/libvhost-user.c
>> index af4faad..a46ef90 100644
>> --- a/contrib/libvhost-user/libvhost-user.c
>> +++ b/contrib/libvhost-user/libvhost-user.c
>> @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg *vmsg)
>> DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index);
>> }
>>
>> - dev->vq[index].started = true;
>> - if (dev->iface->queue_set_started) {
>> - dev->iface->queue_set_started(dev, index, true);
>> - }
>> -
>> - if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
>> - dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
>> - vu_kick_cb, (void *)(long)index);
>> -
>> - DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
>> - dev->vq[index].kick_fd, index);
>> - }
>> -
>> return false;
>> }
>>
>> @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
>>
>> DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index);
>>
>> + dev->vq[index].started = true;
>> + if (dev->iface->queue_set_started) {
>> + dev->iface->queue_set_started(dev, index, true);
>> + }
>> +
>> + if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
>> + dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
>> + vu_kick_cb, (void *)(long)index);
>> +
>> + DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
>> + dev->vq[index].kick_fd, index);
>> + }
>> +
>> return false;
>> }
>>
>> --
>> 1.9.4
>>
>>
- [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/12
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Marc-André Lureau, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL,
Felipe Franciosi <=
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/13
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/17
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Felipe Franciosi, 2017/01/17
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/17
- Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Paolo Bonzini, 2017/01/19
Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL, Michael S. Tsirkin, 2017/01/16