From 5ffe337846c583dcdf0d15fd8edab13e55dc23b0 Mon Sep 17 00:00:00 2001 From: Vitaliy Mysak Date: Tue, 3 Dec 2019 21:58:42 +0100 Subject: [PATCH] vhost: change features initialization for vhost-blk Replace usage of rte_vhost_driver_enable_features() by setting disabled_features field dynamically. This patch doesn't change functionality, but simplifies initialization and removes usage of socket operation. Call to drive_enable_features() had to be done when features field was static, but now it is mutable so the call became redundant. Change-Id: I6efc63883773e4ba6d931efd057a38a705c53217 Signed-off-by: Vitaliy Mysak Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476616 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Community-CI: SPDK CI Jenkins --- lib/vhost/vhost_blk.c | 73 ++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/lib/vhost/vhost_blk.c b/lib/vhost/vhost_blk.c index 531e93f2b..00c475701 100644 --- a/lib/vhost/vhost_blk.c +++ b/lib/vhost/vhost_blk.c @@ -45,6 +45,22 @@ #include "vhost_internal.h" +/* Supported features for every SPDK VHOST-blk driver instance + * Some of these get disabled depending on backend bdev */ +#define SPDK_VHOST_BLK_SUPPORTED_FEATURES (SPDK_VHOST_FEATURES | \ + (1ULL << VIRTIO_BLK_F_SIZE_MAX) | (1ULL << VIRTIO_BLK_F_SEG_MAX) | \ + (1ULL << VIRTIO_BLK_F_GEOMETRY) | (1ULL << VIRTIO_BLK_F_RO) | \ + (1ULL << VIRTIO_BLK_F_BLK_SIZE) | (1ULL << VIRTIO_BLK_F_TOPOLOGY) | \ + (1ULL << VIRTIO_BLK_F_BARRIER) | (1ULL << VIRTIO_BLK_F_SCSI) | \ + (1ULL << VIRTIO_BLK_F_FLUSH) | (1ULL << VIRTIO_BLK_F_CONFIG_WCE) | \ + (1ULL << VIRTIO_BLK_F_MQ) | (1ULL << VIRTIO_BLK_F_DISCARD) | \ + (1ULL << VIRTIO_BLK_F_WRITE_ZEROES)) + +/* Not supported features */ +#define SPDK_VHOST_BLK_DISABLED_FEATURES (SPDK_VHOST_DISABLED_FEATURES | \ + (1ULL << VIRTIO_BLK_F_GEOMETRY) | (1ULL << VIRTIO_BLK_F_CONFIG_WCE) | \ + (1ULL << VIRTIO_BLK_F_BARRIER) | (1ULL << VIRTIO_BLK_F_SCSI)) + struct spdk_vhost_blk_task { struct spdk_bdev_io *bdev_io; struct spdk_vhost_blk_session *bvsession; @@ -943,8 +959,8 @@ int spdk_vhost_blk_construct(const char *name, const char *cpumask, const char *dev_name, bool readonly) { struct spdk_vhost_blk_dev *bvdev = NULL; + struct spdk_vhost_dev *vdev; struct spdk_bdev *bdev; - uint64_t features = 0; int ret = 0; spdk_vhost_lock(); @@ -962,18 +978,22 @@ spdk_vhost_blk_construct(const char *name, const char *cpumask, const char *dev_ goto out; } - bvdev->vdev.virtio_features = SPDK_VHOST_FEATURES | - (1ULL << VIRTIO_BLK_F_SIZE_MAX) | (1ULL << VIRTIO_BLK_F_SEG_MAX) | - (1ULL << VIRTIO_BLK_F_GEOMETRY) | (1ULL << VIRTIO_BLK_F_RO) | - (1ULL << VIRTIO_BLK_F_BLK_SIZE) | (1ULL << VIRTIO_BLK_F_TOPOLOGY) | - (1ULL << VIRTIO_BLK_F_BARRIER) | (1ULL << VIRTIO_BLK_F_SCSI) | - (1ULL << VIRTIO_BLK_F_FLUSH) | (1ULL << VIRTIO_BLK_F_CONFIG_WCE) | - (1ULL << VIRTIO_BLK_F_MQ) | (1ULL << VIRTIO_BLK_F_DISCARD) | - (1ULL << VIRTIO_BLK_F_WRITE_ZEROES); - bvdev->vdev.disabled_features = SPDK_VHOST_DISABLED_FEATURES | (1ULL << VIRTIO_BLK_F_GEOMETRY) | - (1ULL << VIRTIO_BLK_F_RO) | (1ULL << VIRTIO_BLK_F_FLUSH) | (1ULL << VIRTIO_BLK_F_CONFIG_WCE) | - (1ULL << VIRTIO_BLK_F_BARRIER) | (1ULL << VIRTIO_BLK_F_SCSI) | (1ULL << VIRTIO_BLK_F_DISCARD) | - (1ULL << VIRTIO_BLK_F_WRITE_ZEROES); + vdev = &bvdev->vdev; + vdev->virtio_features = SPDK_VHOST_BLK_SUPPORTED_FEATURES; + vdev->disabled_features = SPDK_VHOST_BLK_DISABLED_FEATURES; + + if (!spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_UNMAP)) { + vdev->disabled_features |= (1ULL << VIRTIO_BLK_F_DISCARD); + } + if (!spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_WRITE_ZEROES)) { + vdev->disabled_features |= (1ULL << VIRTIO_BLK_F_WRITE_ZEROES); + } + if (!readonly) { + vdev->disabled_features |= (1ULL << VIRTIO_BLK_F_RO); + } + if (!spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_FLUSH)) { + vdev->disabled_features |= (1ULL << VIRTIO_BLK_F_FLUSH); + } ret = spdk_bdev_open(bdev, true, bdev_remove_cb, bvdev, &bvdev->bdev_desc); if (ret != 0) { @@ -984,37 +1004,12 @@ spdk_vhost_blk_construct(const char *name, const char *cpumask, const char *dev_ bvdev->bdev = bdev; bvdev->readonly = readonly; - ret = vhost_dev_register(&bvdev->vdev, name, cpumask, &vhost_blk_device_backend); + ret = vhost_dev_register(vdev, name, cpumask, &vhost_blk_device_backend); if (ret != 0) { spdk_bdev_close(bvdev->bdev_desc); goto out; } - if (spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_UNMAP)) { - features |= (1ULL << VIRTIO_BLK_F_DISCARD); - } - if (spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_WRITE_ZEROES)) { - features |= (1ULL << VIRTIO_BLK_F_WRITE_ZEROES); - } - if (readonly) { - features |= (1ULL << VIRTIO_BLK_F_RO); - } - if (spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_FLUSH)) { - features |= (1ULL << VIRTIO_BLK_F_FLUSH); - } - - if (features && rte_vhost_driver_enable_features(bvdev->vdev.path, features)) { - SPDK_ERRLOG("%s: failed to enable features 0x%"PRIx64"\n", name, features); - - if (vhost_dev_unregister(&bvdev->vdev) != 0) { - SPDK_ERRLOG("%s: failed to remove device\n", name); - } - - spdk_bdev_close(bvdev->bdev_desc); - ret = -1; - goto out; - } - SPDK_INFOLOG(SPDK_LOG_VHOST, "%s: using bdev '%s'\n", name, dev_name); out: if (ret != 0 && bvdev) {