On 8/24/22 9:13 PM, Stefan Hajnoczi wrote:
On Wed, Aug 24, 2022 at 12:18:34PM +0300, Daniil Tatianin wrote:
+size_t virtio_blk_common_get_config_size(uint64_t host_features)
+{
+ size_t config_size = MAX(VIRTIO_BLK_CFG_SIZE,
+ virtio_feature_get_config_size(feature_sizes, host_features));
+
+ assert(config_size <= sizeof(struct virtio_blk_config));
+ return config_size;
+}
This logic is common to all VIRTIO devices and I think it can be moved
to virtio_feature_get_config_size(). Then
virtio_blk_common_get_config_size() is no longer necessary and the
generic virtio_feature_get_config_size() can be called directly.
The only virtio-blk common part would be the
virtio_feature_get_config_size() parameter struct that describes the
minimum and maximum config space size, as well as how the feature bits
affect the size:
size = virtio_feature_get_config_size(virtio_blk_config_size_params,
host_features)
where virtio_blk_config_size_params is:
const VirtIOConfigSizeParams virtio_blk_config_size_params = {
.min_size = offsetof(struct virtio_blk_config, max_discard_sectors),
.max_size = sizeof(struct virtio_blk_config),
.features = {
{.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
.end = endof(struct virtio_blk_config, discard_sector_alignment)},
...,
},
};
Then virtio-blk-common.h just needs to define:
extern const VirtIOConfigSizeParams virtio_blk_config_size_params;
Taking it one step further, maybe VirtioDeviceClass should include a
const VirtIOConfigSizeParams *config_size_params field so
vdev->config_size can be computed by common VIRTIO code and the devices
only need to describe the parameters.
I think that's a great idea! Do you think it should be done
automatically in 'virtio_init' if this field is not NULL? One problem I
see with that is that you would have to make all virtio devices use
'parent_obj.host_features' for feature properties, which is currently
far from true, but then again this is very much opt-in. Another thing
you could do is add a separate helper for that, which maybe defeats the
purpose a little bit?
Stefan