Re: [PATCH v4] virtio/vsock: add two more queues for datagram types

[Stefano Garzarella <sgarzare@redhat.com>]

On Tue, Sep 07, 2021 at 12:15:30PM +0200, Stefano Garzarella wrote:
>On Sun, Sep 05, 2021 at 11:08:34AM -0700, Jiang Wang . wrote:
>>On Mon, Aug 9, 2021 at 3:58 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>>On Thu, Aug 05, 2021 at 12:07:02PM -0700, Jiang Wang . wrote:
>>>>On Wed, Aug 4, 2021 at 1:13 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>>>>> On Tue, Aug 03, 2021 at 11:41:32PM +0000, Jiang Wang wrote:
>
>[...]
>
>>>>> >+
>>>>> >+    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);
>>>>> >+    }
>>>>> >+
>>>>>
>>>>> I'm still concerned about compatibility with guests that don't
>>>>> support
>>>>> dgram, as I mentioned in the previous email.
>>>>>
>>>>> I need to do some testing, but my guess is it won't work if the host
>>>>> (QEMU and vhost-vsock) supports it and the guest doesn't.
>>>>>
>>>>> I still think that we should allocate an array of queues and then decide
>>>>> at runtime which one to use for events (third or fifth) after the guest
>>>>> has acked the features.
>>>>>
>>>>Agree. I will check where the guest ack the feature. If you have any
>>>
>>>I'm not sure we should delete them, I think we can allocate 5 queues and
>>>then use queue 3 or 5 for events in vhost_vsock_common_start(), when the
>>>guest already acked the features.
>>>
>>
>>I think I just solved most compatibility issues during migration. The
>>previous error I saw was due to a bug in vhost-vsock kernel module.
>
>Please post the fix upstream.
>
>>After fixing that, I did not change anything for qemu ( i.e, still the same
>>version 4, btw I will fix fd issue in v5) and did a few migration tests.
>>Most of them are good.
>>
>>There are two test cases that migration failed with "Features 0x130000002
>>unsupported"error, which is due to
>>SEQPACKET qemu patch (my dgram patch
>>is based on seqpacket patch). Not sure if we need to
>>fix it or not.  I think the compatibility is good as of now. Let me
>
>I'm a bit lost. Do you mean because one of the QEMU doesn't support 
>SEQPACKET?
>
>
>>know if you have other concerns or more test cases to test.
>>Otherwise, I will submit V5 soon.
>>
>>Test results:
>>Left three columns are the source set-up,  right are destination set up.
>>Host and Guest refers to the host and guest kernel respectively. These
>>tests are not complete, and I make the src and dest kernel mostly the
>>same version. But I did test one case where source kernel has dgram
>>support while dest kernel does not and it is good. Btw, if the src kernel
>>and dest kernel have a big difference( like 5.14 vs 4.19), then QEMU
>>will show some msr errors which I guess is kind of expected.
>>
>>Host        QEMU        Guest            --> Host        QEMU            result
>>dgram       no-dgram    no-dgram        dgram       no-dgram        Good
>>dgram       no-dgram    dgram           dgram       no-dgram        Good
>>dgram       dgram       no-dgram        dgram       dgram           Good
>>dgram       dgram       no-dgram        dgram       no-dgram        Good
>>dgram       dgram       dgram           dgram       no-dgram
>>load feature error *1
>>
>>no-dgram    no-dgram    dgram           no-dgram    no-dgram        Good
>>no-dgram    dgram       dgram           no-dgram    dgram             Good
>>no-dgram    dgram       no-dgram        no-dgram    dgram           Good
>>no-dgram    dgram       no-dgram        no-dgram    no-dgram        Good
>>no-dgram    dgram       dgram           no-dgram    no-dgram
>>load feature error *1
>>
>>dgram       dgram       no-dgram        no-dgram    no-dgram        Good
>>
>>*1 Qemu shows following error messages:
>>qemu-system-x86_64: Features 0x130000002 unsupported. Allowed
>>features: 0x179000000
>>qemu-system-x86_64: Failed to load virtio-vhost_vsock:virtio
>>qemu-system-x86_64: error while loading state for instance 0x0 of
>>device '0000:00:05.0/virtio-vhost_vsock'
>>qemu-system-x86_64: load of migration failed: Operation not permitted
>>
>>This is due to SEQPACKET feature bit.

I think I found the issue, I'm sending the patch to the mailing list.

If you want to try it is here: 
https://gitlab.com/sgarzarella/qemu/-/commits/vsock-fix-seqpacket-migration

Maybe we can reuse the `features` field also for dgram.

Stefano