[PATCH v2 0/2] vhost-vsock: fixes for seqpacket support

[PATCH v2 0/2] vhost-vsock: fixes for seqpacket support

[Stefano Garzarella]

This series includes 2 patches: the first is similar of v1, but using
OnOffAuto property.
The second patch moves the `seqpacket` handling in the vhost-vsock-common
class to reuse the same code/properties also in vhost-user-vsock devices.

Perhaps the patches can be merged into a single one, but to simplify the
backport I left them separate.

v1: https://lore.kernel.org/qemu-devel/20210907124935.147164-1-sgarzare@redhat.com/

v2:
- used OnOffAuto `seqpacket` property instead of boolean `features` [stefanha]
- moved `seqpacket` property to vhost-vsock-common class

Stefano Garzarella (2):
  vhost-vsock: fix migration issue when seqpacket is supported
  vhost-vsock: handle common features in vhost-vsock-common

 include/hw/virtio/vhost-vsock-common.h |  5 +++++
 hw/core/machine.c                      |  5 ++++-
 hw/virtio/vhost-user-vsock.c           |  4 +++-
 hw/virtio/vhost-vsock-common.c         | 31 ++++++++++++++++++++++++++
 hw/virtio/vhost-vsock.c                | 11 +--------
 5 files changed, 44 insertions(+), 12 deletions(-)

-- 
2.31.1

[PATCH v2 1/2] vhost-vsock: fix migration issue when seqpacket is supported

[Stefano Garzarella]

Commit 1e08fd0a46 ("vhost-vsock: SOCK_SEQPACKET feature bit support")
enabled the SEQPACKET feature bit.
This commit is released with QEMU 6.1, so if we try to migrate a VM where
the host kernel supports SEQPACKET but machine type version is less than
6.1, we get the following errors:

    Features 0x130000002 unsupported. Allowed features: 0x179000000
    Failed to load virtio-vhost_vsock:virtio
    error while loading state for instance 0x0 of device '0000:00:05.0/virtio-vhost_vsock'
    load of migration failed: Operation not permitted

Let's disable the feature bit for machine types < 6.1.
We add a new OnOffAuto property for this, called `seqpacket`.
When it is `auto` (default), QEMU behaves as before, trying to enable the
feature, when it is `on` QEMU will fail if the backend (vhost-vsock
kernel module) doesn't support it.

Fixes: 1e08fd0a46 ("vhost-vsock: SOCK_SEQPACKET feature bit support")
Cc: qemu-stable@nongnu.org
Reported-by: Jiang Wang <jiang.wang@bytedance.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- used OnOffAuto `seqpacket` property instead of boolean `features` [stefanha]
---
 include/hw/virtio/vhost-vsock.h |  3 +++
 hw/core/machine.c               |  1 +
 hw/virtio/vhost-vsock.c         | 19 ++++++++++++++++---
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
index 84f4e727c7..3f121a624f 100644
--- a/include/hw/virtio/vhost-vsock.h
+++ b/include/hw/virtio/vhost-vsock.h
@@ -30,6 +30,9 @@ struct VHostVSock {
     VHostVSockCommon parent;
     VHostVSockConf conf;
 
+    /* features */
+    OnOffAuto seqpacket;
+
     /*< public >*/
 };
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 067f42b528..7e2851feb9 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -46,6 +46,7 @@ GlobalProperty hw_compat_6_0[] = {
     { "nvme-ns", "eui64-default", "off"},
     { "e1000", "init-vet", "off" },
     { "e1000e", "init-vet", "off" },
+    { "vhost-vsock-device", "seqpacket", "off" },
 };
 const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
 
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 1b1a5c70ed..dade0da031 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -114,10 +114,21 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
                                          Error **errp)
 {
     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
+    VHostVSock *vsock = VHOST_VSOCK(vdev);
+
+    if (vsock->seqpacket != ON_OFF_AUTO_OFF) {
+        virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_SEQPACKET);
+    }
+
+    requested_features = vhost_get_features(&vvc->vhost_dev, feature_bits,
+                                            requested_features);
+
+    if (vsock->seqpacket == ON_OFF_AUTO_ON &&
+        !virtio_has_feature(requested_features, VIRTIO_VSOCK_F_SEQPACKET)) {
+        error_setg(errp, "vhost-vsock backend doesn't support seqpacket");
+    }
 
-    virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_SEQPACKET);
-    return vhost_get_features(&vvc->vhost_dev, feature_bits,
-                                requested_features);
+    return requested_features;
 }
 
 static const VMStateDescription vmstate_virtio_vhost_vsock = {
@@ -218,6 +229,8 @@ static void vhost_vsock_device_unrealize(DeviceState *dev)
 static Property vhost_vsock_properties[] = {
     DEFINE_PROP_UINT64("guest-cid", VHostVSock, conf.guest_cid, 0),
     DEFINE_PROP_STRING("vhostfd", VHostVSock, conf.vhostfd),
+    DEFINE_PROP_ON_OFF_AUTO("seqpacket", VHostVSock, seqpacket,
+                            ON_OFF_AUTO_AUTO),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.31.1

[PATCH v2 2/2] vhost-vsock: handle common features in vhost-vsock-common

[Stefano Garzarella]

virtio-vsock features, like VIRTIO_VSOCK_F_SEQPACKET, can be handled
by vhost-vsock-common parent class. In this way, we can reuse the
same code for all virtio-vsock backends (i.e. vhost-vsock,
vhost-user-vsock).

Let's move `seqpacket` property to vhost-vsock-common class, add
vhost_vsock_common_get_features() used by children, and disable
`seqpacket` for vhost-user-vsock device for machine types < 6.2.

The behavior of vhost-vsock device doesn't change; vhost-user-vsock
device now supports `seqpacket` property.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 include/hw/virtio/vhost-vsock-common.h |  5 +++++
 include/hw/virtio/vhost-vsock.h        |  3 ---
 hw/core/machine.c                      |  4 +++-
 hw/virtio/vhost-user-vsock.c           |  4 +++-
 hw/virtio/vhost-vsock-common.c         | 31 ++++++++++++++++++++++++++
 hw/virtio/vhost-vsock.c                | 24 +-------------------
 6 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/include/hw/virtio/vhost-vsock-common.h b/include/hw/virtio/vhost-vsock-common.h
index e412b5ee98..d8b565b4da 100644
--- a/include/hw/virtio/vhost-vsock-common.h
+++ b/include/hw/virtio/vhost-vsock-common.h
@@ -35,6 +35,9 @@ struct VHostVSockCommon {
     VirtQueue *trans_vq;
 
     QEMUTimer *post_load_timer;
+
+    /* features */
+    OnOffAuto seqpacket;
 };
 
 int vhost_vsock_common_start(VirtIODevice *vdev);
@@ -43,5 +46,7 @@ int vhost_vsock_common_pre_save(void *opaque);
 int vhost_vsock_common_post_load(void *opaque, int version_id);
 void vhost_vsock_common_realize(VirtIODevice *vdev, const char *name);
 void vhost_vsock_common_unrealize(VirtIODevice *vdev);
+uint64_t vhost_vsock_common_get_features(VirtIODevice *vdev, uint64_t features,
+                                         Error **errp);
 
 #endif /* _QEMU_VHOST_VSOCK_COMMON_H */
diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
index 3f121a624f..84f4e727c7 100644
--- a/include/hw/virtio/vhost-vsock.h
+++ b/include/hw/virtio/vhost-vsock.h
@@ -30,9 +30,6 @@ struct VHostVSock {
     VHostVSockCommon parent;
     VHostVSockConf conf;
 
-    /* features */
-    OnOffAuto seqpacket;
-
     /*< public >*/
 };
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 7e2851feb9..28318b3436 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -37,7 +37,9 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-pci.h"
 
-GlobalProperty hw_compat_6_1[] = {};
+GlobalProperty hw_compat_6_1[] = {
+    { "vhost-user-vsock-device", "seqpacket", "off" },
+};
 const size_t hw_compat_6_1_len = G_N_ELEMENTS(hw_compat_6_1);
 
 GlobalProperty hw_compat_6_0[] = {
diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c
index 6095ed7349..52bd682c34 100644
--- a/hw/virtio/vhost-user-vsock.c
+++ b/hw/virtio/vhost-user-vsock.c
@@ -81,7 +81,9 @@ static uint64_t vuv_get_features(VirtIODevice *vdev,
 {
     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
 
-    return vhost_get_features(&vvc->vhost_dev, user_feature_bits, features);
+    features = vhost_get_features(&vvc->vhost_dev, user_feature_bits, features);
+
+    return vhost_vsock_common_get_features(vdev, features, errp);
 }
 
 static const VMStateDescription vuv_vmstate = {
diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
index 4ad6e234ad..3f3771274e 100644
--- a/hw/virtio/vhost-vsock-common.c
+++ b/hw/virtio/vhost-vsock-common.c
@@ -18,6 +18,30 @@
 #include "qemu/iov.h"
 #include "monitor/monitor.h"
 
+const int feature_bits[] = {
+    VIRTIO_VSOCK_F_SEQPACKET,
+    VHOST_INVALID_FEATURE_BIT
+};
+
+uint64_t vhost_vsock_common_get_features(VirtIODevice *vdev, uint64_t features,
+                                         Error **errp)
+{
+    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
+
+    if (vvc->seqpacket != ON_OFF_AUTO_OFF) {
+        virtio_add_feature(&features, VIRTIO_VSOCK_F_SEQPACKET);
+    }
+
+    features = vhost_get_features(&vvc->vhost_dev, feature_bits, features);
+
+    if (vvc->seqpacket == ON_OFF_AUTO_ON &&
+        !virtio_has_feature(features, VIRTIO_VSOCK_F_SEQPACKET)) {
+        error_setg(errp, "vhost-vsock backend doesn't support seqpacket");
+    }
+
+    return features;
+}
+
 int vhost_vsock_common_start(VirtIODevice *vdev)
 {
     VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
@@ -231,11 +255,18 @@ void vhost_vsock_common_unrealize(VirtIODevice *vdev)
     virtio_cleanup(vdev);
 }
 
+static Property vhost_vsock_common_properties[] = {
+    DEFINE_PROP_ON_OFF_AUTO("seqpacket", VHostVSockCommon, seqpacket,
+                            ON_OFF_AUTO_AUTO),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void vhost_vsock_common_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
 
+    device_class_set_props(dc, vhost_vsock_common_properties);
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     vdc->guest_notifier_mask = vhost_vsock_common_guest_notifier_mask;
     vdc->guest_notifier_pending = vhost_vsock_common_guest_notifier_pending;
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index dade0da031..478c0c9a87 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -21,11 +21,6 @@
 #include "hw/virtio/vhost-vsock.h"
 #include "monitor/monitor.h"
 
-const int feature_bits[] = {
-    VIRTIO_VSOCK_F_SEQPACKET,
-    VHOST_INVALID_FEATURE_BIT
-};
-
 static void vhost_vsock_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VHostVSock *vsock = VHOST_VSOCK(vdev);
@@ -113,22 +108,7 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
                                          uint64_t requested_features,
                                          Error **errp)
 {
-    VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
-    VHostVSock *vsock = VHOST_VSOCK(vdev);
-
-    if (vsock->seqpacket != ON_OFF_AUTO_OFF) {
-        virtio_add_feature(&requested_features, VIRTIO_VSOCK_F_SEQPACKET);
-    }
-
-    requested_features = vhost_get_features(&vvc->vhost_dev, feature_bits,
-                                            requested_features);
-
-    if (vsock->seqpacket == ON_OFF_AUTO_ON &&
-        !virtio_has_feature(requested_features, VIRTIO_VSOCK_F_SEQPACKET)) {
-        error_setg(errp, "vhost-vsock backend doesn't support seqpacket");
-    }
-
-    return requested_features;
+    return vhost_vsock_common_get_features(vdev, requested_features, errp);
 }
 
 static const VMStateDescription vmstate_virtio_vhost_vsock = {
@@ -229,8 +209,6 @@ static void vhost_vsock_device_unrealize(DeviceState *dev)
 static Property vhost_vsock_properties[] = {
     DEFINE_PROP_UINT64("guest-cid", VHostVSock, conf.guest_cid, 0),
     DEFINE_PROP_STRING("vhostfd", VHostVSock, conf.vhostfd),
-    DEFINE_PROP_ON_OFF_AUTO("seqpacket", VHostVSock, seqpacket,
-                            ON_OFF_AUTO_AUTO),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.31.1