vhost: added new/destroy_device callback layer

new/destroy_device implementations in particular backends now take
spdk_vhost_dev pointer.

This patch is a first step towards fixing synchronization issues between
RPC and vhost reactor. See the next patch which introduces generic
vhost mutex in vhost.c.

Change-Id: I448492330734726c21189f71ec7a9a8ed81c8195
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/372073
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>

lib/vhost/vhost.c

@@ -50,6 +50,14 @@ static char dev_dirname[PATH_MAX] = "";
 
 #define MAX_VHOST_DEVICES	64
 
+static int new_device(int vid);
+static void destroy_device(int vid);
+
+const struct vhost_device_ops g_spdk_vhost_ops = {
+	.new_device =  new_device,
+	.destroy_device = destroy_device,
+};
+
 static struct spdk_vhost_dev *g_spdk_vhost_devices[MAX_VHOST_DEVICES];
 
 void *spdk_vhost_gpa_to_vva(struct spdk_vhost_dev *vdev, uint64_t addr)
@@ -184,7 +192,7 @@ spdk_vhost_dev_has_feature(struct spdk_vhost_dev *vdev, unsigned feature_id)
 	return vdev->negotiated_features & (1ULL << feature_id);
 }
 
-struct spdk_vhost_dev *
+static struct spdk_vhost_dev *
 spdk_vhost_dev_find_by_vid(int vid)
 {
 	unsigned i;
@@ -356,7 +364,7 @@ spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, uint64_t
 		return -EIO;
 	}
 
-	if (rte_vhost_driver_callback_register(path, &backend->ops) != 0) {
+	if (rte_vhost_driver_callback_register(path, &g_spdk_vhost_ops) != 0) {
 		rte_vhost_driver_unregister(path);
 		SPDK_ERRLOG("Couldn't register callbacks for controller %s\n", name);
 		return -EIO;
@@ -368,7 +376,7 @@ spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, uint64_t
 	vdev->lcore = -1;
 	vdev->cpumask = cpumask;
 	vdev->type = type;
-	vdev->vhost_backend = backend;
+	vdev->backend = backend;
 
 	g_spdk_vhost_devices[ctrlr_num] = vdev;
 
@@ -519,64 +527,77 @@ spdk_vhost_allocate_reactor(uint64_t cpumask)
 	return selected_core;
 }
 
-void
+static void
-spdk_vhost_dev_unload(struct spdk_vhost_dev *vdev)
+destroy_device(int vid)
 {
+	struct spdk_vhost_dev *vdev;
 	struct rte_vhost_vring *q;
 	uint16_t i;
 
+	vdev = spdk_vhost_dev_find_by_vid(vid);
+	if (vdev == NULL) {
+		SPDK_ERRLOG("Couldn't find device with vid %d to stop.\n", vid);
+		return;
+	}
+
+	if (vdev->backend->destroy_device(vdev) != 0) {
+		SPDK_ERRLOG("Couldn't stop device with vid %d.\n", vid);
+		return;
+	}
+
 	for (i = 0; i < vdev->num_queues; i++) {
 		q = &vdev->virtqueue[i];
 		rte_vhost_set_vhost_vring_last_idx(vdev->vid, i, q->last_avail_idx, q->last_used_idx);
 	}
 
 	free(vdev->mem);
-
 	spdk_vhost_free_reactor(vdev->lcore);
 	vdev->lcore = -1;
+	vdev->vid = -1;
 }
 
-struct spdk_vhost_dev *
+static int
-spdk_vhost_dev_load(int vid)
+new_device(int vid)
 {
 	struct spdk_vhost_dev *vdev;
 	char ifname[PATH_MAX];
+	int rc;
 
 	uint16_t num_queues = rte_vhost_get_vring_num(vid);
 	uint16_t i;
 
 	if (rte_vhost_get_ifname(vid, ifname, PATH_MAX) < 0) {
 		SPDK_ERRLOG("Couldn't get a valid ifname for device %d\n", vid);
-		return NULL;
+		return -1;
 	}
 
 	vdev = spdk_vhost_dev_find(ifname);
 	if (vdev == NULL) {
 		SPDK_ERRLOG("Controller %s not found.\n", ifname);
-		return NULL;
+		return -1;
 	}
 
 	if (vdev->lcore != -1) {
 		SPDK_ERRLOG("Controller %s already connected.\n", ifname);
-		return NULL;
+		return -1;
 	}
 
 	if (num_queues > SPDK_VHOST_MAX_VQUEUES) {
 		SPDK_ERRLOG("vhost device %d: Too many queues (%"PRIu16"). Max %"PRIu16"\n", vid, num_queues,
 			    SPDK_VHOST_MAX_VQUEUES);
-		return NULL;
+		return -1;
 	}
 
 	for (i = 0; i < num_queues; i++) {
 		if (rte_vhost_get_vhost_vring(vid, i, &vdev->virtqueue[i])) {
 			SPDK_ERRLOG("vhost device %d: Failed to get information of queue %"PRIu16"\n", vid, i);
-			return NULL;
+			return -1;
 		}
 
 		/* Disable notifications. */
 		if (rte_vhost_enable_guest_notification(vid, i, 0) != 0) {
 			SPDK_ERRLOG("vhost device %d: Failed to disable guest notification on queue %"PRIu16"\n", vid, i);
-			return NULL;
+			return -1;
 		}
 
 	}
@@ -586,17 +607,25 @@ spdk_vhost_dev_load(int vid)
 
 	if (rte_vhost_get_negotiated_features(vid, &vdev->negotiated_features) != 0) {
 		SPDK_ERRLOG("vhost device %d: Failed to get negotiated driver features\n", vid);
-		return NULL;
+		return -1;
 	}
 
 	if (rte_vhost_get_mem_table(vid, &vdev->mem) != 0) {
 		SPDK_ERRLOG("vhost device %d: Failed to get guest memory table\n", vid);
-		return NULL;
+		return -1;
 	}
 
 	vdev->lcore = spdk_vhost_allocate_reactor(vdev->cpumask);
+	rc = vdev->backend->new_device(vdev);
+	if (rc != 0) {
+		free(vdev->mem);
+		spdk_vhost_free_reactor(vdev->lcore);
+		vdev->lcore = -1;
+		vdev->vid = -1;
+		return -1;
+	}
 
-	return vdev;
+	return 0;
 }
 
 void
@@ -733,8 +762,8 @@ spdk_vhost_timed_event_wait(struct spdk_vhost_timed_event *ev, const char *errms
 void
 spdk_vhost_dump_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_write_ctx *w)
 {
-	assert(vdev->vhost_backend->dump_config_json != NULL);
+	assert(vdev->backend->dump_config_json != NULL);
-	vdev->vhost_backend->dump_config_json(vdev, w);
+	vdev->backend->dump_config_json(vdev, w);
 }
 
 SPDK_LOG_REGISTER_TRACE_FLAG("vhost_ring", SPDK_TRACE_VHOST_RING)

lib/vhost/vhost_blk.c

@@ -518,48 +518,42 @@ alloc_task_pool(struct spdk_vhost_blk_dev *bvdev)
  *
  */
 static int
-new_device(int vid)
+new_device(struct spdk_vhost_dev *vdev)
 {
-	struct spdk_vhost_dev *vdev = spdk_vhost_dev_load(vid);
+	struct spdk_vhost_blk_dev *bvdev;
-	struct spdk_vhost_blk_dev *bvdev = to_blk_dev(vdev);
 	int rc = -1;
 
-	if (vdev == NULL) {
+	bvdev = to_blk_dev(vdev);
+	if (bvdev == NULL) {
+		SPDK_ERRLOG("Trying to start non-blk controller as a blk one.\n");
 		return -1;
 	} else if (bvdev == NULL) {
 		SPDK_ERRLOG("Trying to start non-blk controller as blk one.\n");
-		goto out;
+		return -1;
 	}
 
 	rc = alloc_task_pool(bvdev);
 	if (rc != 0) {
 		SPDK_ERRLOG("%s: failed to alloc task pool.", bvdev->vdev.name);
-		goto out;
+		return -1;
 	}
 
 	spdk_vhost_timed_event_send(bvdev->vdev.lcore, add_vdev_cb, bvdev, 1, "add blk vdev");
 
-out:
+	return 0;
-	if (rc != 0) {
-		spdk_vhost_dev_unload(&bvdev->vdev);
-	}
-
-	return rc;
 }
 
-static void
+static int
-destroy_device(int vid)
+destroy_device(struct spdk_vhost_dev *vdev)
 {
 	struct spdk_vhost_blk_dev *bvdev;
-	struct spdk_vhost_dev *vdev;
 	struct spdk_vhost_timed_event event = {0};
 	uint32_t i;
 
-	vdev = spdk_vhost_dev_find_by_vid(vid);
 	bvdev = to_blk_dev(vdev);
 	if (bvdev == NULL) {
-		SPDK_ERRLOG("Couldn't find device with vid %d to stop.\n", vid);
+		SPDK_ERRLOG("Trying to stop non-blk controller as a blk one.\n");
-		abort();
+		return -1;
 	}
 
 	spdk_vhost_timed_event_init(&event, vdev->lcore, NULL, NULL, 1);
@@ -579,7 +573,7 @@ destroy_device(int vid)
 	spdk_vhost_timed_event_send(vdev->lcore, remove_vdev_cb, bvdev, 1, "remove vdev");
 
 	free_task_pool(bvdev);
-	spdk_vhost_dev_unload(vdev);
+	return 0;
 }
 
 static void
@@ -616,11 +610,9 @@ static const struct spdk_vhost_dev_backend vhost_blk_device_backend = {
 	.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),
+	.new_device =  new_device,
+	.destroy_device = destroy_device,
 	.dump_config_json = spdk_vhost_blk_dump_config_json,
-	.ops = {
-		.new_device =  new_device,
-		.destroy_device = destroy_device,
-	}
 };
 
 int

lib/vhost/vhost_internal.h

@@ -85,8 +85,9 @@ enum spdk_vhost_dev_type {
 struct spdk_vhost_dev_backend {
 	uint64_t virtio_features;
 	uint64_t disabled_features;
+	int (*new_device)(struct spdk_vhost_dev *);
+	int (*destroy_device)(struct spdk_vhost_dev *);
 	void (*dump_config_json)(struct spdk_vhost_dev *vdev, struct spdk_json_write_ctx *w);
-	const struct vhost_device_ops ops;
 };
 
 struct spdk_vhost_dev {
@@ -100,12 +101,12 @@ struct spdk_vhost_dev {
 	uint64_t cpumask;
 
 	enum spdk_vhost_dev_type type;
+	const struct spdk_vhost_dev_backend *backend;
 
 	uint16_t num_queues;
 	uint64_t negotiated_features;
 	struct rte_vhost_vring virtqueue[SPDK_VHOST_MAX_VQUEUES] __attribute((aligned(
 				SPDK_CACHE_LINE_SIZE)));
-	const struct spdk_vhost_dev_backend *vhost_backend;
 };
 
 void spdk_vhost_dev_mem_register(struct spdk_vhost_dev *vdev);
@@ -129,15 +130,11 @@ bool spdk_vhost_vring_desc_is_wr(struct vring_desc *cur_desc);
 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);
-struct spdk_vhost_dev *spdk_vhost_dev_find_by_vid(int vid);
 
 int spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, uint64_t cpumask,
 			     enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend);
 int spdk_vhost_dev_remove(struct spdk_vhost_dev *vdev);
 
-struct spdk_vhost_dev *spdk_vhost_dev_load(int vid);
-void spdk_vhost_dev_unload(struct spdk_vhost_dev *dev);
-
 typedef void (*spdk_vhost_timed_event_fn)(void *);
 
 struct spdk_vhost_timed_event {

lib/vhost/vhost_scsi.c

@@ -109,18 +109,16 @@ struct spdk_vhost_scsi_event {
 	struct spdk_scsi_lun *lun;
 };
 
-static int new_device(int vid);
+static int new_device(struct spdk_vhost_dev *);
-static void destroy_device(int vid);
+static int destroy_device(struct spdk_vhost_dev *);
 static void spdk_vhost_scsi_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_write_ctx *w);
 
 const struct spdk_vhost_dev_backend spdk_vhost_scsi_device_backend = {
 	.virtio_features = SPDK_VHOST_SCSI_FEATURES,
 	.disabled_features = SPDK_VHOST_SCSI_DISABLED_FEATURES,
+	.new_device =  new_device,
+	.destroy_device = destroy_device,
 	.dump_config_json = spdk_vhost_scsi_config_json,
-	.ops = {
-		.new_device =  new_device,
-		.destroy_device = destroy_device,
-	}
 };
 
 static void
@@ -1099,64 +1097,48 @@ alloc_task_pool(struct spdk_vhost_scsi_dev *svdev)
  * and then allocated to a specific data core.
  */
 static int
-new_device(int vid)
+new_device(struct spdk_vhost_dev *vdev)
 {
-	struct spdk_vhost_dev *vdev;
+	struct spdk_vhost_scsi_dev *svdev;
-	struct spdk_vhost_scsi_dev *svdev = NULL;
+	int rc;
-	int rc = -1;
-
-	vdev = spdk_vhost_dev_load(vid);
-	if (vdev == NULL) {
-		SPDK_ERRLOG("Trying start a controller with unknown vid: %d.\n", vid);
-		return -1;
-	}
 
 	svdev = to_scsi_dev(vdev);
 	if (svdev == NULL) {
-		SPDK_ERRLOG("Trying to start non-scsi controller as scsi one.\n");
+		SPDK_ERRLOG("Trying to start non-scsi controller as a scsi one.\n");
-		goto out;
+		return -1;
 	}
 
 	rc = alloc_task_pool(svdev);
 	if (rc != 0) {
 		SPDK_ERRLOG("%s: failed to alloc task pool.", vdev->name);
-		goto out;
+		return -1;
 	}
 
 	svdev->vhost_events = spdk_ring_create(SPDK_RING_TYPE_MP_SC, 16,
 					       spdk_env_get_socket_id(vdev->lcore));
 	if (svdev->vhost_events == NULL) {
 		SPDK_ERRLOG("%s: failed to alloc event pool.", vdev->name);
-		goto out;
+		return -1;
 	}
 
 	spdk_vhost_timed_event_send(vdev->lcore, add_vdev_cb, svdev, 1, "add scsi vdev");
 
-	rc = 0;
+	return 0;
-
-out:
-	if (rc != 0) {
-		spdk_vhost_dev_unload(&svdev->vdev);
-	}
-
-	return rc;
 }
 
-static void
+static int
-destroy_device(int vid)
+destroy_device(struct spdk_vhost_dev *vdev)
 {
 	struct spdk_vhost_scsi_dev *svdev;
-	struct spdk_vhost_dev *vdev;
 	void *ev;
 	struct spdk_vhost_timed_event event = {0};
 	uint32_t i;
 
-	vdev = spdk_vhost_dev_find_by_vid(vid);
-	if (vdev == NULL) {
-		rte_panic("Couldn't find device with vid %d to stop.\n", vid);
-	}
 	svdev = to_scsi_dev(vdev);
-	assert(svdev);
+	if (svdev == NULL) {
+		SPDK_ERRLOG("Trying to stop non-scsi controller as a scsi one.\n");
+		return -1;
+	}
 
 	spdk_vhost_timed_event_init(&event, vdev->lcore, NULL, NULL, 1);
 	spdk_poller_unregister(&svdev->requestq_poller, event.spdk_event);
@@ -1187,7 +1169,7 @@ destroy_device(int vid)
 	spdk_ring_free(svdev->vhost_events);
 
 	free_task_pool(svdev);
-	spdk_vhost_dev_unload(vdev);
+	return 0;
 }
 
 int