From 672588d736c4bc2e9b499b4aac5ad4eac0d28203 Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Fri, 26 Jan 2018 14:09:39 +0100 Subject: [PATCH] vhost: unify vdev removal Instead of: * spdk_vhost_scsi_dev_remove(vdev) * spdk_vhost_blk_dev_remove(vdev) we now have * spdk_vhost_dev_remove(vdev) All the logic is already handled internally. This patch only changes the API. Also, previous vhost_dev_construct()/remove() functions have been renamed to vhost_dev_register()/unregister() because that's what they really do. Change-Id: I7dd0d77bc5b633bec075e0a71345ddbed62697b4 Signed-off-by: Dariusz Stojaczyk Reviewed-on: https://review.gerrithub.io/396574 Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris Reviewed-by: --- include/spdk/vhost.h | 16 +++------------- lib/vhost/vhost.c | 10 +++++----- lib/vhost/vhost_blk.c | 14 ++++++++------ lib/vhost/vhost_internal.h | 9 ++++----- lib/vhost/vhost_rpc.c | 2 +- lib/vhost/vhost_scsi.c | 11 ++++++----- test/unit/lib/vhost/test_vhost.c | 18 +++++++++--------- test/unit/lib/vhost/vhost.c/vhost_ut.c | 16 ++++++++-------- test/unit/lib/vhost/vhost_blk.c/vhost_blk_ut.c | 10 +++++----- .../lib/vhost/vhost_scsi.c/vhost_scsi_ut.c | 8 ++++---- 10 files changed, 53 insertions(+), 61 deletions(-) diff --git a/include/spdk/vhost.h b/include/spdk/vhost.h index 60c3170fc..c7285b737 100644 --- a/include/spdk/vhost.h +++ b/include/spdk/vhost.h @@ -155,16 +155,6 @@ int spdk_vhost_set_coalescing(struct spdk_vhost_dev *vdev, uint32_t delay_base_u */ int spdk_vhost_scsi_dev_construct(const char *name, const char *cpumask); -/** - * Remove an empty vhost SCSI device. The vdev must not - * have any SCSI devices attached nor have any open connection on - * it's socket. - * - * \param vdev vhost SCSI device - * \return 0 on success, negative errno on error. - */ -int spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev); - /** * Construct and attach new SCSI target to the vhost SCSI device * on given (unoccupied) slot. The device will be created with a single @@ -246,13 +236,13 @@ int spdk_vhost_blk_construct(const char *name, const char *cpumask, const char * bool readonly); /** - * Remove a vhost blk device. The device must not have any - * open connections on it's socket. + * Remove a vhost device. The device must not have any open + * connections on it's socket. * * \param vdev vhost blk device * \return 0 on success, negative errno on error. */ -int spdk_vhost_blk_destroy(struct spdk_vhost_dev *dev); +int spdk_vhost_dev_remove(struct spdk_vhost_dev *vdev); /** * Get underlying SPDK bdev from vhost blk device. The diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 167450f9d..b4efd5f00 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -609,8 +609,8 @@ spdk_vhost_parse_core_mask(const char *mask, struct spdk_cpuset *cpumask) } int -spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, const char *mask_str, - enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend) +spdk_vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *mask_str, + enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend) { unsigned ctrlr_num; char path[PATH_MAX]; @@ -734,7 +734,7 @@ out: } int -spdk_vhost_dev_remove(struct spdk_vhost_dev *vdev) +spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev) { unsigned ctrlr_num; @@ -1197,9 +1197,9 @@ spdk_vhost_dump_config_json(struct spdk_vhost_dev *vdev, } int -spdk_remove_vhost_controller(struct spdk_vhost_dev *vdev) +spdk_vhost_dev_remove(struct spdk_vhost_dev *vdev) { - return vdev->backend->vhost_remove_controller(vdev); + return vdev->backend->remove_device(vdev); } static int diff --git a/lib/vhost/vhost_blk.c b/lib/vhost/vhost_blk.c index 8a72db158..97cbf3d8a 100644 --- a/lib/vhost/vhost_blk.c +++ b/lib/vhost/vhost_blk.c @@ -611,6 +611,8 @@ spdk_vhost_blk_dump_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_wr spdk_json_write_object_end(w); } +static int spdk_vhost_blk_destroy(struct spdk_vhost_dev *dev); + static const struct spdk_vhost_dev_backend vhost_blk_device_backend = { .virtio_features = SPDK_VHOST_FEATURES | (1ULL << VIRTIO_BLK_F_SIZE_MAX) | (1ULL << VIRTIO_BLK_F_SEG_MAX) | @@ -625,7 +627,7 @@ static const struct spdk_vhost_dev_backend vhost_blk_device_backend = { .start_device = spdk_vhost_blk_start, .stop_device = spdk_vhost_blk_stop, .dump_config_json = spdk_vhost_blk_dump_config_json, - .vhost_remove_controller = spdk_vhost_blk_destroy, + .remove_device = spdk_vhost_blk_destroy, }; int @@ -703,8 +705,8 @@ spdk_vhost_blk_construct(const char *name, const char *cpumask, const char *dev_ bvdev->bdev = bdev; bvdev->readonly = readonly; - ret = spdk_vhost_dev_construct(&bvdev->vdev, name, cpumask, SPDK_VHOST_DEV_T_BLK, - &vhost_blk_device_backend); + ret = spdk_vhost_dev_register(&bvdev->vdev, name, cpumask, SPDK_VHOST_DEV_T_BLK, + &vhost_blk_device_backend); if (ret != 0) { spdk_bdev_close(bvdev->bdev_desc); ret = -1; @@ -715,7 +717,7 @@ spdk_vhost_blk_construct(const char *name, const char *cpumask, const char *dev_ SPDK_ERRLOG("Controller %s: failed to set as a readonly\n", name); spdk_bdev_close(bvdev->bdev_desc); - if (spdk_vhost_dev_remove(&bvdev->vdev) != 0) { + if (spdk_vhost_dev_unregister(&bvdev->vdev) != 0) { SPDK_ERRLOG("Controller %s: failed to remove controller\n", name); } @@ -732,7 +734,7 @@ out: return ret; } -int +static int spdk_vhost_blk_destroy(struct spdk_vhost_dev *vdev) { struct spdk_vhost_blk_dev *bvdev = to_blk_dev(vdev); @@ -742,7 +744,7 @@ spdk_vhost_blk_destroy(struct spdk_vhost_dev *vdev) return -EINVAL; } - rc = spdk_vhost_dev_remove(&bvdev->vdev); + rc = spdk_vhost_dev_unregister(&bvdev->vdev); if (rc != 0) { return rc; } diff --git a/lib/vhost/vhost_internal.h b/lib/vhost/vhost_internal.h index 453dd385e..b36a2e62b 100644 --- a/lib/vhost/vhost_internal.h +++ b/lib/vhost/vhost_internal.h @@ -134,7 +134,7 @@ struct spdk_vhost_dev_backend { uint32_t offset, uint32_t size, uint32_t flags); void (*dump_config_json)(struct spdk_vhost_dev *vdev, struct spdk_json_write_ctx *w); - int (*vhost_remove_controller)(struct spdk_vhost_dev *vdev); + int (*remove_device)(struct spdk_vhost_dev *vdev); }; struct spdk_vhost_dev { @@ -239,9 +239,9 @@ int spdk_vhost_vring_desc_to_iov(struct spdk_vhost_dev *vdev, struct iovec *iov, uint16_t *iov_index, const struct vring_desc *desc); bool spdk_vhost_dev_has_feature(struct spdk_vhost_dev *vdev, unsigned feature_id); -int spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, const char *mask_str, - enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend); -int spdk_vhost_dev_remove(struct spdk_vhost_dev *vdev); +int spdk_vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *mask_str, + enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend); +int spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev); int spdk_vhost_scsi_controller_construct(void); int spdk_vhost_blk_controller_construct(void); @@ -249,6 +249,5 @@ void spdk_vhost_dump_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_w void spdk_vhost_dev_backend_event_done(void *event_ctx, int response); void spdk_vhost_lock(void); void spdk_vhost_unlock(void); -int spdk_remove_vhost_controller(struct spdk_vhost_dev *vdev); #endif /* SPDK_VHOST_INTERNAL_H */ diff --git a/lib/vhost/vhost_rpc.c b/lib/vhost/vhost_rpc.c index baed56cfe..01b2260b9 100644 --- a/lib/vhost/vhost_rpc.c +++ b/lib/vhost/vhost_rpc.c @@ -391,7 +391,7 @@ spdk_rpc_remove_vhost_controller_cb(struct spdk_vhost_dev *vdev, void *arg) goto invalid; } - rc = spdk_remove_vhost_controller(vdev); + rc = spdk_vhost_dev_remove(vdev); if (rc < 0) { goto invalid; } diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index 47e188a75..1f3846fa9 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -108,6 +108,7 @@ struct spdk_vhost_scsi_task { static int spdk_vhost_scsi_start(struct spdk_vhost_dev *, void *); static int spdk_vhost_scsi_stop(struct spdk_vhost_dev *, void *); static void spdk_vhost_scsi_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_write_ctx *w); +static int spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev); const struct spdk_vhost_dev_backend spdk_vhost_scsi_device_backend = { .virtio_features = SPDK_VHOST_SCSI_FEATURES, @@ -115,7 +116,7 @@ const struct spdk_vhost_dev_backend spdk_vhost_scsi_device_backend = { .start_device = spdk_vhost_scsi_start, .stop_device = spdk_vhost_scsi_stop, .dump_config_json = spdk_vhost_scsi_config_json, - .vhost_remove_controller = spdk_vhost_scsi_dev_remove, + .remove_device = spdk_vhost_scsi_dev_remove, }; static void @@ -697,8 +698,8 @@ spdk_vhost_scsi_dev_construct(const char *name, const char *cpumask) } spdk_vhost_lock(); - rc = spdk_vhost_dev_construct(&svdev->vdev, name, cpumask, SPDK_VHOST_DEV_T_SCSI, - &spdk_vhost_scsi_device_backend); + rc = spdk_vhost_dev_register(&svdev->vdev, name, cpumask, SPDK_VHOST_DEV_T_SCSI, + &spdk_vhost_scsi_device_backend); if (rc) { spdk_dma_free(svdev); @@ -708,7 +709,7 @@ spdk_vhost_scsi_dev_construct(const char *name, const char *cpumask) return rc; } -int +static int spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev) { struct spdk_vhost_scsi_dev *svdev = to_scsi_dev(vdev); @@ -725,7 +726,7 @@ spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev) } } - rc = spdk_vhost_dev_remove(vdev); + rc = spdk_vhost_dev_unregister(vdev); if (rc != 0) { return rc; } diff --git a/test/unit/lib/vhost/test_vhost.c b/test/unit/lib/vhost/test_vhost.c index 4e4387921..e1ad5ce43 100644 --- a/test/unit/lib/vhost/test_vhost.c +++ b/test/unit/lib/vhost/test_vhost.c @@ -100,17 +100,17 @@ DEFINE_STUB(spdk_json_write_array_begin, int, (struct spdk_json_write_ctx *w), 0 DEFINE_STUB(spdk_json_write_object_end, int, (struct spdk_json_write_ctx *w), 0); DEFINE_STUB(spdk_json_write_array_end, int, (struct spdk_json_write_ctx *w), 0); -/* This sets spdk_vhost_dev_remove to either to fail or success */ -DEFINE_STUB(spdk_vhost_dev_remove_fail, bool, (void), false); -/* This sets spdk_vhost_dev_construct to either to fail or success */ -DEFINE_STUB(spdk_vhost_dev_construct_fail, bool, (void), false); +/* This sets spdk_vhost_dev_unregister to either to fail or success */ +DEFINE_STUB(spdk_vhost_dev_unregister_fail, bool, (void), false); +/* This sets spdk_vhost_dev_register to either to fail or success */ +DEFINE_STUB(spdk_vhost_dev_register_fail, bool, (void), false); static struct spdk_vhost_dev *g_spdk_vhost_device; int -spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, const char *mask_str, - enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend) +spdk_vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *mask_str, + enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend) { - if (spdk_vhost_dev_construct_fail()) { + if (spdk_vhost_dev_register_fail()) { return -1; } @@ -119,9 +119,9 @@ spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, const ch } int -spdk_vhost_dev_remove(struct spdk_vhost_dev *vdev) +spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev) { - if (spdk_vhost_dev_remove_fail()) { + if (spdk_vhost_dev_unregister_fail()) { return -1; } diff --git a/test/unit/lib/vhost/vhost.c/vhost_ut.c b/test/unit/lib/vhost/vhost.c/vhost_ut.c index fa025d1bc..3bae466b8 100644 --- a/test/unit/lib/vhost/vhost.c/vhost_ut.c +++ b/test/unit/lib/vhost/vhost.c/vhost_ut.c @@ -222,25 +222,25 @@ create_controller_test(void) /* Create device with no name */ vdev = alloc_vdev(); - ret = spdk_vhost_dev_construct(vdev, NULL, "0x1", SPDK_VHOST_DEV_T_BLK, &backend); + ret = spdk_vhost_dev_register(vdev, NULL, "0x1", SPDK_VHOST_DEV_T_BLK, &backend); CU_ASSERT(ret != 0); /* Create device with incorrect cpumask */ - ret = spdk_vhost_dev_construct(vdev, "vdev_name_0", "0x2", SPDK_VHOST_DEV_T_BLK, &backend); + ret = spdk_vhost_dev_register(vdev, "vdev_name_0", "0x2", SPDK_VHOST_DEV_T_BLK, &backend); CU_ASSERT(ret != 0); /* Create device with too long name and path */ memset(long_name, 'x', sizeof(long_name)); long_name[PATH_MAX - 1] = 0; snprintf(dev_dirname, sizeof(dev_dirname), "some_path/"); - ret = spdk_vhost_dev_construct(vdev, long_name, "0x1", SPDK_VHOST_DEV_T_BLK, &backend); + ret = spdk_vhost_dev_register(vdev, long_name, "0x1", SPDK_VHOST_DEV_T_BLK, &backend); CU_ASSERT(ret != 0); dev_dirname[0] = 0; /* Create device when device name is already taken */ vdev->name = strdup("vdev_name_0"); g_spdk_vhost_devices[0] = vdev; - ret = spdk_vhost_dev_construct(vdev, "vdev_name_0", "0x1", SPDK_VHOST_DEV_T_BLK, &backend); + ret = spdk_vhost_dev_register(vdev, "vdev_name_0", "0x1", SPDK_VHOST_DEV_T_BLK, &backend); CU_ASSERT(ret != 0); /* Create device when max number of devices is reached */ @@ -248,7 +248,7 @@ create_controller_test(void) g_spdk_vhost_devices[ctrlr_num] = vdev; } - ret = spdk_vhost_dev_construct(vdev, "vdev_name_1", "0x1", SPDK_VHOST_DEV_T_BLK, &backend); + ret = spdk_vhost_dev_register(vdev, "vdev_name_1", "0x1", SPDK_VHOST_DEV_T_BLK, &backend); CU_ASSERT(ret != 0); free_vdev(vdev); @@ -286,7 +286,7 @@ remove_controller_test(void) vdev->name = strdup("vdev_name_0"); /* Remove device when controller is in use */ - ret = spdk_vhost_dev_remove(vdev); + ret = spdk_vhost_dev_unregister(vdev); CU_ASSERT(ret != 0); if (ret == 0) { vdev->name = strdup("vdev_name_0"); @@ -294,7 +294,7 @@ remove_controller_test(void) /* Remove nonexistent device */ vdev->lcore = -1; - ret = spdk_vhost_dev_remove(vdev); + ret = spdk_vhost_dev_unregister(vdev); CU_ASSERT(ret != 0); if (ret != 0) { free(vdev->name); @@ -306,7 +306,7 @@ remove_controller_test(void) snprintf(dev_dirname, sizeof(dev_dirname), "some_path/"); vdev->name = strdup(long_name); g_spdk_vhost_devices[0] = vdev; - ret = spdk_vhost_dev_remove(vdev); + ret = spdk_vhost_dev_unregister(vdev); CU_ASSERT(ret != 0); if (ret == 0) { vdev->name = NULL; diff --git a/test/unit/lib/vhost/vhost_blk.c/vhost_blk_ut.c b/test/unit/lib/vhost/vhost_blk.c/vhost_blk_ut.c index 186dfe005..b9810c86d 100644 --- a/test/unit/lib/vhost/vhost_blk.c/vhost_blk_ut.c +++ b/test/unit/lib/vhost/vhost_blk.c/vhost_blk_ut.c @@ -98,8 +98,8 @@ vhost_blk_construct_test(void) int rc; struct spdk_bdev *ut_p_spdk_bdev = MOCK_PASS_THRU_P; - MOCK_SET(spdk_vhost_dev_remove_fail, bool, false); - MOCK_SET(spdk_vhost_dev_construct_fail, bool, false); + MOCK_SET(spdk_vhost_dev_unregister_fail, bool, false); + MOCK_SET(spdk_vhost_dev_register_fail, bool, false); /* Create device with invalid name */ MOCK_SET_P(spdk_bdev_get_by_name, struct spdk_bdev *, NULL); @@ -114,7 +114,7 @@ vhost_blk_construct_test(void) /* Failed to construct controller */ MOCK_SET(spdk_bdev_open, int, 0); - MOCK_SET(spdk_vhost_dev_construct_fail, bool, true); + MOCK_SET(spdk_vhost_dev_register_fail, bool, true); rc = spdk_vhost_blk_construct("vhost.0", "0x1", "Malloc0", true); CU_ASSERT(rc != 0); @@ -124,7 +124,7 @@ vhost_blk_construct_test(void) CU_ASSERT(rc != 0); /* Failed to set readonly as a feature and failde to remove controller */ - MOCK_SET(spdk_vhost_dev_remove_fail, bool, true); + MOCK_SET(spdk_vhost_dev_unregister_fail, bool, true); rc = spdk_vhost_blk_construct("vhost.0", "0x1", "Malloc0", true); CU_ASSERT(rc != 0); } @@ -144,7 +144,7 @@ vhost_blk_destroy_test(void) /* Failed to remove device */ bvdev->vdev.type = SPDK_VHOST_DEV_T_BLK; - MOCK_SET(spdk_vhost_dev_remove_fail, bool, true); + MOCK_SET(spdk_vhost_dev_unregister_fail, bool, true); rc = spdk_vhost_blk_destroy(&bvdev->vdev); CU_ASSERT(rc == -1); diff --git a/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c b/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c index e642cb7e3..e7ca3ac2a 100644 --- a/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c +++ b/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c @@ -141,7 +141,7 @@ vhost_scsi_controller_construct_test(void) CU_ASSERT(g_spdk_vhost_device != NULL); /* Remove created device */ - MOCK_SET(spdk_vhost_dev_remove_fail, bool, false); + MOCK_SET(spdk_vhost_dev_unregister_fail, bool, false); rc = spdk_vhost_scsi_dev_remove(g_spdk_vhost_device); CU_ASSERT(rc == 0); } @@ -153,7 +153,7 @@ vhost_scsi_dev_remove_test(void) struct spdk_vhost_scsi_dev *svdev = NULL; struct spdk_scsi_dev *scsi_dev; - MOCK_SET(spdk_vhost_dev_remove_fail, bool, false); + MOCK_SET(spdk_vhost_dev_unregister_fail, bool, false); /* Try to remove controller which is occupied */ svdev = alloc_svdev(); @@ -165,7 +165,7 @@ vhost_scsi_dev_remove_test(void) svdev->scsi_dev[0] = NULL; /* Failed to remove device */ - MOCK_SET(spdk_vhost_dev_remove_fail, bool, true); + MOCK_SET(spdk_vhost_dev_unregister_fail, bool, true); rc = spdk_vhost_scsi_dev_remove(&svdev->vdev); CU_ASSERT(rc == -1); @@ -178,7 +178,7 @@ vhost_scsi_dev_construct_test(void) int rc; /* Failed to construct vhost device */ - MOCK_SET(spdk_vhost_dev_construct_fail, bool, true); + MOCK_SET(spdk_vhost_dev_register_fail, bool, true); rc = spdk_vhost_scsi_dev_construct("vhost.0", "0x1"); CU_ASSERT(rc != 0); }