From d1d69a169cc07e1d45b8fcd45429151217be7724 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Sat, 20 Jul 2019 17:03:04 +0200 Subject: [PATCH] vhost: remove session type checks When rte_vhost tells us to start a session with given vid, we lookup the corresponsing session object from an spdk-internal session list and tell it to start polling without even specifying any backend. The vsession->vdev->type checks could only fail as a result of some spdk data corruption, so replace those with just asserts now. This code path could have never been hit in our tests anyway. Change-Id: I97c6cbe7088f338b684d291c93cbc59c44cfdc4e Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/466042 Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris Reviewed-by: Vitaliy Mysak Tested-by: SPDK CI Jenkins --- lib/vhost/vhost_blk.c | 53 +++++------------------------------- lib/vhost/vhost_scsi.c | 61 ++++++++---------------------------------- 2 files changed, 17 insertions(+), 97 deletions(-) diff --git a/lib/vhost/vhost_blk.c b/lib/vhost/vhost_blk.c index fd0795841..d3e996467 100644 --- a/lib/vhost/vhost_blk.c +++ b/lib/vhost/vhost_blk.c @@ -528,15 +528,7 @@ no_bdev_vdev_worker(void *arg) static struct spdk_vhost_blk_session * to_blk_session(struct spdk_vhost_session *vsession) { - if (vsession == NULL) { - return NULL; - } - - if (vsession->vdev->backend != &vhost_blk_device_backend) { - SPDK_ERRLOG("%s: not a vhost-blk device\n", vsession->vdev->name); - return NULL; - } - + assert(vsession->vdev->backend == &vhost_blk_device_backend); return (struct spdk_vhost_blk_session *)vsession; } @@ -580,7 +572,6 @@ _spdk_vhost_session_bdev_remove_cb(struct spdk_vhost_dev *vdev, struct spdk_vhos struct spdk_vhost_blk_dev *bvdev = to_blk_dev(vdev); assert(bvdev != NULL); - spdk_bdev_close(bvdev->bdev_desc); bvdev->bdev_desc = NULL; bvdev->bdev = NULL; @@ -676,18 +667,10 @@ static int spdk_vhost_blk_start_cb(struct spdk_vhost_dev *vdev, struct spdk_vhost_session *vsession, void *unused) { + struct spdk_vhost_blk_session *bvsession = to_blk_session(vsession); struct spdk_vhost_blk_dev *bvdev; - struct spdk_vhost_blk_session *bvsession; int i, rc = 0; - bvsession = to_blk_session(vsession); - if (bvsession == NULL) { - SPDK_ERRLOG("%s: trying to start non-blk session as a blk one.\n", - vdev->name); - rc = -1; - goto out; - } - bvdev = to_blk_dev(vdev); assert(bvdev != NULL); bvsession->bvdev = bvdev; @@ -783,22 +766,12 @@ static int spdk_vhost_blk_stop_cb(struct spdk_vhost_dev *vdev, struct spdk_vhost_session *vsession, void *unused) { - struct spdk_vhost_blk_session *bvsession; - - bvsession = to_blk_session(vsession); - if (bvsession == NULL) { - SPDK_ERRLOG("Trying to stop non-blk controller as a blk one.\n"); - goto err; - } + struct spdk_vhost_blk_session *bvsession = to_blk_session(vsession); spdk_poller_unregister(&bvsession->requestq_poller); bvsession->stop_poller = spdk_poller_register(destroy_session_poller_cb, bvsession, 1000); return 0; - -err: - spdk_vhost_session_stop_done(vsession, -1); - return -1; } static int @@ -815,10 +788,6 @@ spdk_vhost_blk_dump_info_json(struct spdk_vhost_dev *vdev, struct spdk_json_writ struct spdk_vhost_blk_dev *bvdev; bvdev = to_blk_dev(vdev); - if (bvdev == NULL) { - return; - } - assert(bvdev != NULL); spdk_json_write_named_object_begin(w, "block"); @@ -840,10 +809,7 @@ spdk_vhost_blk_write_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_w struct spdk_vhost_blk_dev *bvdev; bvdev = to_blk_dev(vdev); - if (bvdev == NULL) { - return; - } - + assert(bvdev != NULL); if (!bvdev->bdev) { return; } @@ -874,11 +840,7 @@ spdk_vhost_blk_get_config(struct spdk_vhost_dev *vdev, uint8_t *config, uint64_t blkcnt; bvdev = to_blk_dev(vdev); - if (bvdev == NULL) { - SPDK_ERRLOG("Trying to get virito_blk configuration failed\n"); - return -1; - } - + assert(bvdev != NULL); bdev = bvdev->bdev; if (bdev == NULL) { /* We can't just return -1 here as this GET_CONFIG message might @@ -1075,10 +1037,7 @@ spdk_vhost_blk_destroy(struct spdk_vhost_dev *vdev) struct spdk_vhost_blk_dev *bvdev = to_blk_dev(vdev); int rc; - if (!bvdev) { - return -EINVAL; - } - + assert(bvdev != NULL); rc = spdk_vhost_dev_unregister(&bvdev->vdev); if (rc != 0) { return rc; diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index 4c989a1f8..fca1c1cec 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -815,15 +815,7 @@ to_scsi_dev(struct spdk_vhost_dev *ctrlr) static struct spdk_vhost_scsi_session * to_scsi_session(struct spdk_vhost_session *vsession) { - if (vsession == NULL) { - return NULL; - } - - if (vsession->vdev->backend != &spdk_vhost_scsi_device_backend) { - SPDK_ERRLOG("%s: not a vhost-scsi device.\n", vsession->vdev->name); - return NULL; - } - + assert(vsession->vdev->backend == &spdk_vhost_scsi_device_backend); return (struct spdk_vhost_scsi_session *)vsession; } @@ -855,10 +847,7 @@ spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev) struct spdk_vhost_scsi_dev *svdev = to_scsi_dev(vdev); int rc, i; - if (svdev == NULL) { - return -EINVAL; - } - + assert(svdev != NULL); for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; ++i) { if (svdev->scsi_dev_state[i].dev) { if (vdev->registered) { @@ -890,7 +879,8 @@ spdk_vhost_scsi_dev_get_tgt(struct spdk_vhost_dev *vdev, uint8_t num) assert(num < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS); svdev = to_scsi_dev(vdev); - if (svdev == NULL || svdev->scsi_dev_state[num].status != VHOST_SCSI_DEV_PRESENT) { + assert(svdev != NULL); + if (svdev->scsi_dev_state[num].status != VHOST_SCSI_DEV_PRESENT) { return NULL; } @@ -998,10 +988,7 @@ spdk_vhost_scsi_dev_add_tgt(struct spdk_vhost_dev *vdev, int scsi_tgt_num, const char *bdev_names_list[1]; svdev = to_scsi_dev(vdev); - if (svdev == NULL) { - return -EINVAL; - } - + assert(svdev != NULL); if (scsi_tgt_num < 0) { for (scsi_tgt_num = 0; scsi_tgt_num < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; scsi_tgt_num++) { if (svdev->scsi_dev_state[scsi_tgt_num].dev == NULL) { @@ -1127,10 +1114,7 @@ spdk_vhost_scsi_dev_remove_tgt(struct spdk_vhost_dev *vdev, unsigned scsi_tgt_nu } svdev = to_scsi_dev(vdev); - if (svdev == NULL) { - return -ENODEV; - } - + assert(svdev != NULL); scsi_dev_state = &svdev->scsi_dev_state[scsi_tgt_num]; if (scsi_dev_state->dev == NULL || scsi_dev_state->status == VHOST_SCSI_DEV_ADDING) { SPDK_ERRLOG("%s: SCSI target %u is not occupied\n", vdev->name, scsi_tgt_num); @@ -1296,16 +1280,12 @@ static int spdk_vhost_scsi_start_cb(struct spdk_vhost_dev *vdev, struct spdk_vhost_session *vsession, void *unused) { - struct spdk_vhost_scsi_dev *svdev; - struct spdk_vhost_scsi_session *svsession; + struct spdk_vhost_scsi_session *svsession = to_scsi_session(vsession); + struct spdk_vhost_scsi_dev *svdev = svsession->svdev; struct spdk_scsi_dev_vhost_state *state; uint32_t i; int rc; - svsession = to_scsi_session(vsession); - assert(svsession != NULL); - svdev = svsession->svdev; - /* validate all I/O queues are in a contiguous index range */ for (i = VIRTIO_SCSI_REQUESTQ; i < vsession->max_queues; i++) { if (vsession->virtqueue[i].vring.desc == NULL) { @@ -1360,17 +1340,10 @@ out: static int spdk_vhost_scsi_start(struct spdk_vhost_session *vsession) { - struct spdk_vhost_scsi_session *svsession; + struct spdk_vhost_scsi_session *svsession = to_scsi_session(vsession); struct spdk_vhost_scsi_dev *svdev; int rc; - svsession = to_scsi_session(vsession); - if (svsession == NULL) { - SPDK_ERRLOG("%s: trying to start non-scsi session as a scsi one.\n", - vsession->name); - return -1; - } - svdev = to_scsi_dev(vsession->vdev); assert(svdev != NULL); svsession->svdev = svdev; @@ -1450,10 +1423,7 @@ static int spdk_vhost_scsi_stop_cb(struct spdk_vhost_dev *vdev, struct spdk_vhost_session *vsession, void *unused) { - struct spdk_vhost_scsi_session *svsession; - - svsession = to_scsi_session(vsession); - assert(svsession != NULL); + struct spdk_vhost_scsi_session *svsession = to_scsi_session(vsession); /* Stop receiving new I/O requests */ spdk_poller_unregister(&svsession->requestq_poller); @@ -1476,14 +1446,9 @@ spdk_vhost_scsi_stop_cb(struct spdk_vhost_dev *vdev, static int spdk_vhost_scsi_stop(struct spdk_vhost_session *vsession) { - struct spdk_vhost_scsi_session *svsession; + struct spdk_vhost_scsi_session *svsession = to_scsi_session(vsession); int rc; - svsession = to_scsi_session(vsession); - if (svsession == NULL) { - SPDK_ERRLOG("Trying to stop non-scsi session as a scsi one.\n"); - return -1; - } rc = spdk_vhost_session_send_event(vsession->poll_group, vsession, spdk_vhost_scsi_stop_cb, 3, "stop session"); if (rc != 0) { @@ -1552,10 +1517,6 @@ spdk_vhost_scsi_write_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_ struct spdk_scsi_lun *lun; uint32_t i; - if (to_scsi_dev(vdev) == NULL) { - return; - } - spdk_json_write_object_begin(w); spdk_json_write_named_string(w, "method", "construct_vhost_scsi_controller");