[PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features

[PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features

[Daniil Tatianin]

This patch set attempts to align vhost-user-blk with virtio-blk in
terms of backward compatibility and flexibility. It also improves
the virtio core by introducing new common code that can be used by
a virtio device to calculate its config space size.

In particular it adds the following things:
- Common virtio code for deducing the required device config size based
  on provided host features.
- Ability to disable modern virtio-blk features like
  discard/write-zeroes for vhost-user-blk.
- Dynamic configuration space resizing based on enabled features,
  by reusing the common code introduced in the earlier commits.
- Cleans up the VHostUserBlk structure by reusing parent fields.

Changes since v1 (mostly addresses Stefan's feedback):
- Introduce VirtIOConfigSizeParams & virtio_get_config_size
- Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
  only hold the virtio-blk config size parameters.
- Reuse parent fields in vhost-user-blk instead of introducing new ones.

Changes since v2:
- Squash the first four commits into one
- Set .min_size for virtio-net as well
- Move maintainer/meson user-blk bits to the last commit

Daniil Tatianin (5):
  virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
  virtio-blk: move config size params to virtio-blk-common
  vhost-user-blk: make it possible to disable write-zeroes/discard
  vhost-user-blk: make 'config_wce' part of 'host_features'
  vhost-user-blk: dynamically resize config space based on features

 MAINTAINERS                           |  4 +++
 hw/block/meson.build                  |  4 +--
 hw/block/vhost-user-blk.c             | 29 +++++++++++---------
 hw/block/virtio-blk-common.c          | 39 +++++++++++++++++++++++++++
 hw/block/virtio-blk.c                 | 28 +++----------------
 hw/net/virtio-net.c                   |  9 +++++--
 hw/virtio/virtio.c                    | 10 ++++---
 include/hw/virtio/vhost-user-blk.h    |  1 -
 include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
 include/hw/virtio/virtio.h            | 10 +++++--
 10 files changed, 105 insertions(+), 49 deletions(-)
 create mode 100644 hw/block/virtio-blk-common.c
 create mode 100644 include/hw/virtio/virtio-blk-common.h

-- 
2.25.1

[PATCH v3 1/5] virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size

[Daniil Tatianin]

This is the first step towards moving all device config size calculation
logic into the virtio core code. In particular, this adds a struct that
contains all the necessary information for common virtio code to be able
to calculate the final config size for a device. This is expected to be
used with the new virtio_get_config_size helper, which calculates the
final length based on the provided host features.

This builds on top of already existing code like VirtIOFeature and
virtio_feature_get_config_size(), but adds additional fields, as well as
sanity checking so that device-specifc code doesn't have to duplicate it.

An example usage would be:

    static const VirtIOFeature dev_features[] = {
        {.flags = 1ULL << FEATURE_1_BIT,
         .end = endof(struct virtio_dev_config, feature_1)},
        {.flags = 1ULL << FEATURE_2_BIT,
         .end = endof(struct virtio_dev_config, feature_2)},
        {}
    };

    static const VirtIOConfigSizeParams dev_cfg_size_params = {
        .min_size = DEV_BASE_CONFIG_SIZE,
        .max_size = sizeof(struct virtio_dev_config),
        .feature_sizes = dev_features
    };

    // code inside my_dev_device_realize()
    size_t config_size = virtio_get_config_size(&dev_cfg_size_params,
                                                host_features);
    virtio_init(vdev, VIRTIO_ID_MYDEV, config_size);

Currently every device is expected to write its own boilerplate from the
example above in device_realize(), however, the next step of this
transition is moving VirtIOConfigSizeParams into VirtioDeviceClass,
so that it can be done automatically by the virtio initialization code.

All of the users of virtio_feature_get_config_size have been converted
to use virtio_get_config_size so it's no longer needed and is removed
with this commit.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
 hw/block/virtio-blk.c      | 16 +++++++---------
 hw/net/virtio-net.c        |  9 +++++++--
 hw/virtio/virtio.c         | 10 ++++++----
 include/hw/virtio/virtio.h | 10 ++++++++--
 4 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e9ba752f6b..10c47c2934 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -49,13 +49,11 @@ static const VirtIOFeature feature_sizes[] = {
     {}
 };
 
-static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features)
-{
-    s->config_size = MAX(VIRTIO_BLK_CFG_SIZE,
-        virtio_feature_get_config_size(feature_sizes, host_features));
-
-    assert(s->config_size <= sizeof(struct virtio_blk_config));
-}
+static const VirtIOConfigSizeParams cfg_size_params = {
+    .min_size = VIRTIO_BLK_CFG_SIZE,
+    .max_size = sizeof(struct virtio_blk_config),
+    .feature_sizes = feature_sizes
+};
 
 static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
                                     VirtIOBlockReq *req)
@@ -1204,8 +1202,8 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    virtio_blk_set_config_size(s, s->host_features);
-
+    s->config_size = virtio_get_config_size(&cfg_size_params,
+                                            s->host_features);
     virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
 
     s->blk = conf->conf.blk;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd0d056fde..78b003a158 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -106,6 +106,12 @@ static const VirtIOFeature feature_sizes[] = {
     {}
 };
 
+static const VirtIOConfigSizeParams cfg_size_params = {
+    .min_size = endof(struct virtio_net_config, mac),
+    .max_size = sizeof(struct virtio_net_config),
+    .feature_sizes = feature_sizes
+};
+
 static VirtIONetQueue *virtio_net_get_subqueue(NetClientState *nc)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
@@ -3246,8 +3252,7 @@ static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
 {
     virtio_add_feature(&host_features, VIRTIO_NET_F_MAC);
 
-    n->config_size = virtio_feature_get_config_size(feature_sizes,
-                                                    host_features);
+    n->config_size = virtio_get_config_size(&cfg_size_params, host_features);
 }
 
 void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5d607aeaa0..c0bf8dd6fd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2999,11 +2999,12 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
     return ret;
 }
 
-size_t virtio_feature_get_config_size(const VirtIOFeature *feature_sizes,
-                                      uint64_t host_features)
+size_t virtio_get_config_size(const VirtIOConfigSizeParams *params,
+                              uint64_t host_features)
 {
-    size_t config_size = 0;
-    int i;
+    size_t config_size = params->min_size;
+    const VirtIOFeature *feature_sizes = params->feature_sizes;
+    size_t i;
 
     for (i = 0; feature_sizes[i].flags != 0; i++) {
         if (host_features & feature_sizes[i].flags) {
@@ -3011,6 +3012,7 @@ size_t virtio_feature_get_config_size(const VirtIOFeature *feature_sizes,
         }
     }
 
+    assert(config_size <= params->max_size);
     return config_size;
 }
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index db1c0ddf6b..f3ff6710d5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -44,8 +44,14 @@ typedef struct VirtIOFeature {
     size_t end;
 } VirtIOFeature;
 
-size_t virtio_feature_get_config_size(const VirtIOFeature *features,
-                                      uint64_t host_features);
+typedef struct VirtIOConfigSizeParams {
+    size_t min_size;
+    size_t max_size;
+    const VirtIOFeature *feature_sizes;
+} VirtIOConfigSizeParams;
+
+size_t virtio_get_config_size(const VirtIOConfigSizeParams *params,
+                              uint64_t host_features);
 
 typedef struct VirtQueue VirtQueue;
 
-- 
2.25.1

[PATCH v3 2/5] virtio-blk: move config size params to virtio-blk-common

[Daniil Tatianin]

This way we can reuse it for other virtio-blk devices, e.g
vhost-user-blk, which currently does not control its config space size
dynamically.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
 MAINTAINERS                           |  2 ++
 hw/block/meson.build                  |  2 +-
 hw/block/virtio-blk-common.c          | 39 +++++++++++++++++++++++++++
 hw/block/virtio-blk.c                 | 24 ++---------------
 include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
 5 files changed, 64 insertions(+), 23 deletions(-)
 create mode 100644 hw/block/virtio-blk-common.c
 create mode 100644 include/hw/virtio/virtio-blk-common.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 5ce4227ff6..2858577c5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2030,8 +2030,10 @@ virtio-blk
 M: Stefan Hajnoczi <stefanha@redhat.com>
 L: qemu-block@nongnu.org
 S: Supported
+F: hw/block/virtio-blk-common.c
 F: hw/block/virtio-blk.c
 F: hw/block/dataplane/*
+F: include/hw/virtio/virtio-blk-common.h
 F: tests/qtest/virtio-blk-test.c
 T: git https://github.com/stefanha/qemu.git block
 
diff --git a/hw/block/meson.build b/hw/block/meson.build
index 2389326112..8ee1f1f850 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -16,7 +16,7 @@ softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
 softmmu_ss.add(when: 'CONFIG_TC58128', if_true: files('tc58128.c'))
 
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
+specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 'virtio-blk-common.c'))
 specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c'))
 
 subdir('dataplane')
diff --git a/hw/block/virtio-blk-common.c b/hw/block/virtio-blk-common.c
new file mode 100644
index 0000000000..ac52d7c176
--- /dev/null
+++ b/hw/block/virtio-blk-common.c
@@ -0,0 +1,39 @@
+/*
+ * Virtio Block Device common helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "standard-headers/linux/virtio_blk.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-blk-common.h"
+
+/* Config size before the discard support (hide associated config fields) */
+#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
+                                     max_discard_sectors)
+
+/*
+ * Starting from the discard feature, we can use this array to properly
+ * set the config size depending on the features enabled.
+ */
+static const VirtIOFeature feature_sizes[] = {
+    {.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
+     .end = endof(struct virtio_blk_config, discard_sector_alignment)},
+    {.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES,
+     .end = endof(struct virtio_blk_config, write_zeroes_may_unmap)},
+    {}
+};
+
+const VirtIOConfigSizeParams virtio_blk_cfg_size_params = {
+    .min_size = VIRTIO_BLK_CFG_SIZE,
+    .max_size = sizeof(struct virtio_blk_config),
+    .feature_sizes = feature_sizes
+};
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 10c47c2934..8131ec2dbc 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -32,29 +32,9 @@
 #include "hw/virtio/virtio-bus.h"
 #include "migration/qemu-file-types.h"
 #include "hw/virtio/virtio-access.h"
+#include "hw/virtio/virtio-blk-common.h"
 #include "qemu/coroutine.h"
 
-/* Config size before the discard support (hide associated config fields) */
-#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
-                                     max_discard_sectors)
-/*
- * Starting from the discard feature, we can use this array to properly
- * set the config size depending on the features enabled.
- */
-static const VirtIOFeature feature_sizes[] = {
-    {.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
-     .end = endof(struct virtio_blk_config, discard_sector_alignment)},
-    {.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES,
-     .end = endof(struct virtio_blk_config, write_zeroes_may_unmap)},
-    {}
-};
-
-static const VirtIOConfigSizeParams cfg_size_params = {
-    .min_size = VIRTIO_BLK_CFG_SIZE,
-    .max_size = sizeof(struct virtio_blk_config),
-    .feature_sizes = feature_sizes
-};
-
 static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
                                     VirtIOBlockReq *req)
 {
@@ -1202,7 +1182,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    s->config_size = virtio_get_config_size(&cfg_size_params,
+    s->config_size = virtio_get_config_size(&virtio_blk_cfg_size_params,
                                             s->host_features);
     virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
 
diff --git a/include/hw/virtio/virtio-blk-common.h b/include/hw/virtio/virtio-blk-common.h
new file mode 100644
index 0000000000..31daada3e3
--- /dev/null
+++ b/include/hw/virtio/virtio-blk-common.h
@@ -0,0 +1,20 @@
+/*
+ * Virtio Block Device common helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef VIRTIO_BLK_COMMON_H
+#define VIRTIO_BLK_COMMON_H
+
+#include "hw/virtio/virtio.h"
+
+extern const VirtIOConfigSizeParams virtio_blk_cfg_size_params;
+
+#endif
-- 
2.25.1

[PATCH v3 3/5] vhost-user-blk: make it possible to disable write-zeroes/discard

[Daniil Tatianin]

It is useful to have the ability to disable these features for
compatibility with older VMs that don't have these implemented.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
 hw/block/vhost-user-blk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9117222456..4c9727e08c 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -259,8 +259,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
     virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
     virtio_add_feature(&features, VIRTIO_BLK_F_FLUSH);
     virtio_add_feature(&features, VIRTIO_BLK_F_RO);
-    virtio_add_feature(&features, VIRTIO_BLK_F_DISCARD);
-    virtio_add_feature(&features, VIRTIO_BLK_F_WRITE_ZEROES);
 
     if (s->config_wce) {
         virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
@@ -592,6 +590,10 @@ static Property vhost_user_blk_properties[] = {
                        VHOST_USER_BLK_AUTO_NUM_QUEUES),
     DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
     DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
+    DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
+                      VIRTIO_BLK_F_DISCARD, true),
+    DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, parent_obj.host_features,
+                      VIRTIO_BLK_F_WRITE_ZEROES, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.25.1

[PATCH v3 4/5] vhost-user-blk: make 'config_wce' part of 'host_features'

[Daniil Tatianin]

No reason to have this be a separate field. This also makes it more akin
to what the virtio-blk device does.

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
 hw/block/vhost-user-blk.c          | 6 ++----
 include/hw/virtio/vhost-user-blk.h | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 4c9727e08c..0d916edefa 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -260,9 +260,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
     virtio_add_feature(&features, VIRTIO_BLK_F_FLUSH);
     virtio_add_feature(&features, VIRTIO_BLK_F_RO);
 
-    if (s->config_wce) {
-        virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
-    }
     if (s->num_queues > 1) {
         virtio_add_feature(&features, VIRTIO_BLK_F_MQ);
     }
@@ -589,7 +586,8 @@ static Property vhost_user_blk_properties[] = {
     DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
                        VHOST_USER_BLK_AUTO_NUM_QUEUES),
     DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
-    DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
+    DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
+                      VIRTIO_BLK_F_CONFIG_WCE, true),
     DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
                       VIRTIO_BLK_F_DISCARD, true),
     DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, parent_obj.host_features,
diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
index 7c91f15040..ea085ee1ed 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -34,7 +34,6 @@ struct VHostUserBlk {
     struct virtio_blk_config blkcfg;
     uint16_t num_queues;
     uint32_t queue_size;
-    uint32_t config_wce;
     struct vhost_dev dev;
     struct vhost_inflight *inflight;
     VhostUserState vhost_user;
-- 
2.25.1

[PATCH v3 5/5] vhost-user-blk: dynamically resize config space based on features

[Daniil Tatianin]

Make vhost-user-blk backwards compatible when migrating from older VMs
running with modern features turned off, the same way it was done for
virtio-blk in 20764be0421c ("virtio-blk: set config size depending on the features enabled")

It's currently impossible to migrate from an older VM with
vhost-user-blk (with disable-legacy=off) because of errors like this:

qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: 41 device: 1 cmask: ff wmask: 80 w1cmask:0
qemu-system-x86_64: Failed to load PCIDevice:config
qemu-system-x86_64: Failed to load virtio-blk:virtio
qemu-system-x86_64: error while loading state for instance 0x0 of device '0000:00:05.0:00.0:02.0/virtio-blk'
qemu-system-x86_64: load of migration failed: Invalid argument

This is caused by the newer (destination) VM requiring a bigger BAR0
alignment because it has to cover a bigger configuration space, which
isn't actually needed since those additional config fields are not
active (write-zeroes/discard).

Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
 MAINTAINERS               |  2 ++
 hw/block/meson.build      |  2 +-
 hw/block/vhost-user-blk.c | 17 ++++++++++-------
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2858577c5b..a7d3914735 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2273,11 +2273,13 @@ S: Maintained
 F: contrib/vhost-user-blk/
 F: contrib/vhost-user-scsi/
 F: hw/block/vhost-user-blk.c
+F: hw/block/virtio-blk-common.c
 F: hw/scsi/vhost-user-scsi.c
 F: hw/virtio/vhost-user-blk-pci.c
 F: hw/virtio/vhost-user-scsi-pci.c
 F: include/hw/virtio/vhost-user-blk.h
 F: include/hw/virtio/vhost-user-scsi.h
+F: include/hw/virtio/virtio-blk-common.h
 
 vhost-user-gpu
 M: Marc-André Lureau <marcandre.lureau@redhat.com>
diff --git a/hw/block/meson.build b/hw/block/meson.build
index 8ee1f1f850..1908abd45c 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -17,6 +17,6 @@ softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
 softmmu_ss.add(when: 'CONFIG_TC58128', if_true: files('tc58128.c'))
 
 specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 'virtio-blk-common.c'))
-specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c'))
+specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c', 'virtio-blk-common.c'))
 
 subdir('dataplane')
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 0d916edefa..8dd063eb96 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -23,6 +23,7 @@
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
+#include "hw/virtio/virtio-blk-common.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-user-blk.h"
 #include "hw/virtio/virtio.h"
@@ -63,7 +64,7 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
     /* Our num_queues overrides the device backend */
     virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
 
-    memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
+    memcpy(config, &s->blkcfg, vdev->config_len);
 }
 
 static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
@@ -92,12 +93,12 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
 {
     int ret;
     struct virtio_blk_config blkcfg;
+    VirtIODevice *vdev = dev->vdev;
     VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
     Error *local_err = NULL;
 
     ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
-                               sizeof(struct virtio_blk_config),
-                               &local_err);
+                               vdev->config_len, &local_err);
     if (ret < 0) {
         error_report_err(local_err);
         return ret;
@@ -106,7 +107,7 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
     /* valid for resize only */
     if (blkcfg.capacity != s->blkcfg.capacity) {
         s->blkcfg.capacity = blkcfg.capacity;
-        memcpy(dev->vdev->config, &s->blkcfg, sizeof(struct virtio_blk_config));
+        memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len);
         virtio_notify_config(dev->vdev);
     }
 
@@ -442,7 +443,7 @@ static int vhost_user_blk_realize_connect(VHostUserBlk *s, Error **errp)
     assert(s->connected);
 
     ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
-                               sizeof(struct virtio_blk_config), errp);
+                               s->parent_obj.config_len, errp);
     if (ret < 0) {
         qemu_chr_fe_disconnect(&s->chardev);
         vhost_dev_cleanup(&s->dev);
@@ -457,6 +458,7 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
     ERRP_GUARD();
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostUserBlk *s = VHOST_USER_BLK(vdev);
+    size_t config_size;
     int retries;
     int i, ret;
 
@@ -487,8 +489,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    virtio_init(vdev, VIRTIO_ID_BLOCK,
-                sizeof(struct virtio_blk_config));
+    config_size = virtio_get_config_size(&virtio_blk_cfg_size_params,
+                                         vdev->host_features);
+    virtio_init(vdev, VIRTIO_ID_BLOCK, config_size);
 
     s->virtqs = g_new(VirtQueue *, s->num_queues);
     for (i = 0; i < s->num_queues; i++) {
-- 
2.25.1

Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features

[Raphael Norwitz]

Thanks for the changes. For the whole series:

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

On Tue, Sep 06, 2022 at 10:31:06AM +0300, Daniil Tatianin wrote:
> This patch set attempts to align vhost-user-blk with virtio-blk in
> terms of backward compatibility and flexibility. It also improves
> the virtio core by introducing new common code that can be used by
> a virtio device to calculate its config space size.
> 
> In particular it adds the following things:
> - Common virtio code for deducing the required device config size based
>   on provided host features.
> - Ability to disable modern virtio-blk features like
>   discard/write-zeroes for vhost-user-blk.
> - Dynamic configuration space resizing based on enabled features,
>   by reusing the common code introduced in the earlier commits.
> - Cleans up the VHostUserBlk structure by reusing parent fields.
> 
> Changes since v1 (mostly addresses Stefan's feedback):
> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
>   only hold the virtio-blk config size parameters.
> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
> 
> Changes since v2:
> - Squash the first four commits into one
> - Set .min_size for virtio-net as well
> - Move maintainer/meson user-blk bits to the last commit
> 
> Daniil Tatianin (5):
>   virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
>   virtio-blk: move config size params to virtio-blk-common
>   vhost-user-blk: make it possible to disable write-zeroes/discard
>   vhost-user-blk: make 'config_wce' part of 'host_features'
>   vhost-user-blk: dynamically resize config space based on features
> 
>  MAINTAINERS                           |  4 +++
>  hw/block/meson.build                  |  4 +--
>  hw/block/vhost-user-blk.c             | 29 +++++++++++---------
>  hw/block/virtio-blk-common.c          | 39 +++++++++++++++++++++++++++
>  hw/block/virtio-blk.c                 | 28 +++----------------
>  hw/net/virtio-net.c                   |  9 +++++--
>  hw/virtio/virtio.c                    | 10 ++++---
>  include/hw/virtio/vhost-user-blk.h    |  1 -
>  include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
>  include/hw/virtio/virtio.h            | 10 +++++--
>  10 files changed, 105 insertions(+), 49 deletions(-)
>  create mode 100644 hw/block/virtio-blk-common.c
>  create mode 100644 include/hw/virtio/virtio-blk-common.h
> 
> -- 
> 2.25.1
> 

Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features

[Daniil Tatianin]

Thanks for reviewing! Could you send a Pull request? Or do we need an 
ack from someone else?

On 9/7/22 7:02 AM, Raphael Norwitz wrote:
> Thanks for the changes. For the whole series:
> 
> Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> 
> On Tue, Sep 06, 2022 at 10:31:06AM +0300, Daniil Tatianin wrote:
>> This patch set attempts to align vhost-user-blk with virtio-blk in
>> terms of backward compatibility and flexibility. It also improves
>> the virtio core by introducing new common code that can be used by
>> a virtio device to calculate its config space size.
>>
>> In particular it adds the following things:
>> - Common virtio code for deducing the required device config size based
>>    on provided host features.
>> - Ability to disable modern virtio-blk features like
>>    discard/write-zeroes for vhost-user-blk.
>> - Dynamic configuration space resizing based on enabled features,
>>    by reusing the common code introduced in the earlier commits.
>> - Cleans up the VHostUserBlk structure by reusing parent fields.
>>
>> Changes since v1 (mostly addresses Stefan's feedback):
>> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
>> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
>>    only hold the virtio-blk config size parameters.
>> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
>>
>> Changes since v2:
>> - Squash the first four commits into one
>> - Set .min_size for virtio-net as well
>> - Move maintainer/meson user-blk bits to the last commit
>>
>> Daniil Tatianin (5):
>>    virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
>>    virtio-blk: move config size params to virtio-blk-common
>>    vhost-user-blk: make it possible to disable write-zeroes/discard
>>    vhost-user-blk: make 'config_wce' part of 'host_features'
>>    vhost-user-blk: dynamically resize config space based on features
>>
>>   MAINTAINERS                           |  4 +++
>>   hw/block/meson.build                  |  4 +--
>>   hw/block/vhost-user-blk.c             | 29 +++++++++++---------
>>   hw/block/virtio-blk-common.c          | 39 +++++++++++++++++++++++++++
>>   hw/block/virtio-blk.c                 | 28 +++----------------
>>   hw/net/virtio-net.c                   |  9 +++++--
>>   hw/virtio/virtio.c                    | 10 ++++---
>>   include/hw/virtio/vhost-user-blk.h    |  1 -
>>   include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
>>   include/hw/virtio/virtio.h            | 10 +++++--
>>   10 files changed, 105 insertions(+), 49 deletions(-)
>>   create mode 100644 hw/block/virtio-blk-common.c
>>   create mode 100644 include/hw/virtio/virtio-blk-common.h
>>
>> -- 
>> 2.25.1

Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features

[Raphael Norwitz]

> Thanks for reviewing! Could you send a Pull request? Or do we need an 
> ack from someone else?

mst typically includes the vhost-user-blk patches in his PRs. Usually a few
other people review but I'm not sure it's required.

A lot of folks have been busy prepping for KVM Forum the last few weeks
so I would expect delayed responses/reviews.

Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features

[Daniil Tatianin]

Ping :)

On 9/6/22 10:31 AM, Daniil Tatianin wrote:
> This patch set attempts to align vhost-user-blk with virtio-blk in
> terms of backward compatibility and flexibility. It also improves
> the virtio core by introducing new common code that can be used by
> a virtio device to calculate its config space size.
> 
> In particular it adds the following things:
> - Common virtio code for deducing the required device config size based
>    on provided host features.
> - Ability to disable modern virtio-blk features like
>    discard/write-zeroes for vhost-user-blk.
> - Dynamic configuration space resizing based on enabled features,
>    by reusing the common code introduced in the earlier commits.
> - Cleans up the VHostUserBlk structure by reusing parent fields.
> 
> Changes since v1 (mostly addresses Stefan's feedback):
> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
>    only hold the virtio-blk config size parameters.
> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
> 
> Changes since v2:
> - Squash the first four commits into one
> - Set .min_size for virtio-net as well
> - Move maintainer/meson user-blk bits to the last commit
> 
> Daniil Tatianin (5):
>    virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
>    virtio-blk: move config size params to virtio-blk-common
>    vhost-user-blk: make it possible to disable write-zeroes/discard
>    vhost-user-blk: make 'config_wce' part of 'host_features'
>    vhost-user-blk: dynamically resize config space based on features
> 
>   MAINTAINERS                           |  4 +++
>   hw/block/meson.build                  |  4 +--
>   hw/block/vhost-user-blk.c             | 29 +++++++++++---------
>   hw/block/virtio-blk-common.c          | 39 +++++++++++++++++++++++++++
>   hw/block/virtio-blk.c                 | 28 +++----------------
>   hw/net/virtio-net.c                   |  9 +++++--
>   hw/virtio/virtio.c                    | 10 ++++---
>   include/hw/virtio/vhost-user-blk.h    |  1 -
>   include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
>   include/hw/virtio/virtio.h            | 10 +++++--
>   10 files changed, 105 insertions(+), 49 deletions(-)
>   create mode 100644 hw/block/virtio-blk-common.c
>   create mode 100644 include/hw/virtio/virtio-blk-common.h
> 

Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features

[Daniil Tatianin]

On 10/11/22 10:20 AM, Daniil Tatianin wrote:
> Ping :)
Oops, didn't see the pull request. Disregard this.

> On 9/6/22 10:31 AM, Daniil Tatianin wrote:
>> This patch set attempts to align vhost-user-blk with virtio-blk in
>> terms of backward compatibility and flexibility. It also improves
>> the virtio core by introducing new common code that can be used by
>> a virtio device to calculate its config space size.
>>
>> In particular it adds the following things:
>> - Common virtio code for deducing the required device config size based
>>    on provided host features.
>> - Ability to disable modern virtio-blk features like
>>    discard/write-zeroes for vhost-user-blk.
>> - Dynamic configuration space resizing based on enabled features,
>>    by reusing the common code introduced in the earlier commits.
>> - Cleans up the VHostUserBlk structure by reusing parent fields.
>>
>> Changes since v1 (mostly addresses Stefan's feedback):
>> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
>> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
>>    only hold the virtio-blk config size parameters.
>> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
>>
>> Changes since v2:
>> - Squash the first four commits into one
>> - Set .min_size for virtio-net as well
>> - Move maintainer/meson user-blk bits to the last commit
>>
>> Daniil Tatianin (5):
>>    virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
>>    virtio-blk: move config size params to virtio-blk-common
>>    vhost-user-blk: make it possible to disable write-zeroes/discard
>>    vhost-user-blk: make 'config_wce' part of 'host_features'
>>    vhost-user-blk: dynamically resize config space based on features
>>
>>   MAINTAINERS                           |  4 +++
>>   hw/block/meson.build                  |  4 +--
>>   hw/block/vhost-user-blk.c             | 29 +++++++++++---------
>>   hw/block/virtio-blk-common.c          | 39 +++++++++++++++++++++++++++
>>   hw/block/virtio-blk.c                 | 28 +++----------------
>>   hw/net/virtio-net.c                   |  9 +++++--
>>   hw/virtio/virtio.c                    | 10 ++++---
>>   include/hw/virtio/vhost-user-blk.h    |  1 -
>>   include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
>>   include/hw/virtio/virtio.h            | 10 +++++--
>>   10 files changed, 105 insertions(+), 49 deletions(-)
>>   create mode 100644 hw/block/virtio-blk-common.c
>>   create mode 100644 include/hw/virtio/virtio-blk-common.h
>>