qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC v3] virtio/vsock: add two more queues for datagram types


From: Stefano Garzarella
Subject: Re: [RFC v3] virtio/vsock: add two more queues for datagram types
Date: Mon, 9 Aug 2021 12:49:34 +0200

On Thu, Aug 05, 2021 at 12:00:05PM -0700, Jiang Wang . wrote:
On Tue, Aug 3, 2021 at 11:49 PM Stefano Garzarella <sgarzare@redhat.com> wrote:

On Wed, Aug 4, 2021 at 8:41 AM Stefano Garzarella <sgarzare@redhat.com>
wrote:
>
> On Tue, Aug 03, 2021 at 11:58:27AM -0700, Jiang Wang . wrote:
> >On Wed, Jul 7, 2021 at 10:27 AM Stefano Garzarella <sgarzare@redhat.com> 
wrote:
> >> On Wed, Jul 07, 2021 at 09:52:46AM -0700, Jiang Wang . wrote:
> >> >On Wed, Jul 7, 2021 at 1:33 AM Stefano Garzarella <sgarzare@redhat.com> 
wrote:
> >> >> On Tue, Jul 06, 2021 at 10:26:07PM +0000, Jiang Wang wrote:
>
> [...]
>
> >> >> >+
> >> >> >+    if (nvqs < 0)
> >> >> >+        nvqs = MAX_VQS_WITHOUT_DGRAM;
> >> >> >+
> >> >> >+    if (nvqs == MAX_VQS_WITH_DGRAM) {
> >> >> >+        vvc->dgram_recv_vq = virtio_add_queue(vdev, 
VHOST_VSOCK_QUEUE_SIZE,
> >> >> >+                                              
vhost_vsock_common_handle_output);
> >> >> >+        vvc->dgram_trans_vq = virtio_add_queue(vdev, 
VHOST_VSOCK_QUEUE_SIZE,
> >> >> >+                                               
vhost_vsock_common_handle_output);
> >> >> >+    }
> >> >> >+
> >> >> >     /* The event queue belongs to QEMU */
> >> >> >     vvc->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> >> >> >                                        
vhost_vsock_common_handle_output);
> >> >>
> >> >> Did you do a test with a guest that doesn't support datagram with QEMU
> >> >> and hosts that do?
> >> >>
> >> >Yes, and it works.
> >> >
> >> >> I repost my thoughts that I had on v2:
> >> >>
> >> >>      What happen if the guest doesn't support dgram?
> >> >>
> >> >>      I think we should dynamically use the 3rd queue or the 5th queue 
for
> >> >>      the events at runtime after the guest acked the features.
> >> >>
> >> >>      Maybe better to switch to an array of VirtQueue.
> >> >>
> >> >I think in current V3, it  already dynamically use 3rd or 5th queue
> >> >depending
> >> >on the feature bit.
> >>
> >> I'm not sure. IIUC when vhost_vsock_common_realize() is called, we don't
> >> know the features acked by the guest, so how can it be dynamic?
> >>
> >> Here we should know only if the host kernel supports it.
> >>
> >> Maybe it works, because in QEMU we use the event queue only after a
> >> migration to send a reset event, so you can try to migrate a guest to
> >> check this path.
> >>
> >I tried VM migration and didn't see any problems. The migration looks fine
> >and vsock dgram still works after migration. Is there any more specific test
> >you want to run to check for this code path?
> >
>
> I meant a migration of a guest from QEMU without this patch to a QEMU
> with this patch. Of course in that case testing a socket stream.
>

Sorry, I meant the opposite.

You should try to migrate a guest that does not support dgrams starting
from a QEMU with this patch (and kernel that supports dgram, so qemu
uses the 5th queue for event), to a QEMU without this patch.

Got it. I tried what you said and saw errors on the destination qemu. Then
I moved event queue up to be number 3 (before adding dgram vqs),
as the following:

+    /* The event queue belongs to QEMU */
+    vvc->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
+                                       vhost_vsock_common_handle_output);
+

    nvqs = vhost_vsock_get_max_qps(enable_dgram);

@@ -253,10 +257,6 @@ void vhost_vsock_common_realize(VirtIODevice
*vdev, const char *name, bool enabl

vhost_vsock_common_handle_output);
    }

-    /* The event queue belongs to QEMU */
-    vvc->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
-                                       vhost_vsock_common_handle_output);
-

But I still see following errors on the destination qemu:
qemu-system-x86_64: Error starting vhost vsock: 14

Any idea if my above code change is wrong or missing something?

No, sorry.
Do you have some kernel messages in the host?


Take one step back, what should be the host kernel version? With or
without dgram support? I tried both.  The new dest kernel shows the above error.
The old dest kernel shows a msr error probably not related to vsock.

I think the best is to try the host kernel with DGRAM support, so QEMU will allocate all the queues.


To explain the above qemu 14 error, I think the issue is that the
source host kernel
supports dgram by always setting the DGRAM feature bit(in my
implementation). Then the source
qemu query the source host kernel, and use 5 for event vq. Even if the source
guest kernel does not support dgram, it currently has no way to tell the source
host or the source qemu.

Initially I think is better to try the migration on the same host, so we can exclude other issues. When it works properly, you can try to migrate to a different host kernel.


On the source machine, when we start qemu process,  and the guest VM
is still in BIOS or early boot process ( before vsock is initialized), I think
at this time, the qemu vhost_vsock_common_realize() is already called.
So qemu can only check if the host kernel supports dgram or not, but has
no knowledge about the guest kernel. After the guest kernel is fully boot up,
it can tell qemu or the host if it supports dgram or not ( or the host or qemu
detect for that). But I don't think we will change the vq numbers at that time.

Maybe we should fix that too and change vq numbers ( delete vq and add
back?) at a later
time after the guest kernel is fully boot-up?

IIRC vhost-net allocates the maximum number of queues, and then it uses only the queues supported/requested, so I think we can do something similar.

Allocates 5 queues and, at runtime, we can decide which queue to use.

Thanks,
Stefano




reply via email to

[Prev in Thread] Current Thread [Next in Thread]