vhost: expand spdk_vhost_dev_register functionality

Moved vdev initialization inside spdk_vhost_dev_register, so that
particular virtio device implementations don't have to e.g.
manage socket path.

spdk_vhost_dev_unregister was renamed to spdk_vhost_dev_remove

Change-Id: I240385ba25278c056737fed77f21695b2e787c6b
Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/363584
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

lib/vhost/vhost.c

@@ -257,20 +257,28 @@ spdk_vhost_dev_find(const char *ctrlr_name)
 }
 
 int
-spdk_vhost_dev_register(struct spdk_vhost_dev *vdev,
+spdk_vhost_dev_construct(struct spdk_vhost_dev *vdev, const char *name, uint64_t cpumask,
-			const struct spdk_vhost_dev_backend *backend)
+			 enum spdk_vhost_dev_type type, const struct spdk_vhost_dev_backend *backend)
 {
 	unsigned ctrlr_num;
 	char path[PATH_MAX];
 	struct stat file_stat;
 
-	if (vdev->name == NULL) {
+	assert(vdev);
+
+	if (name == NULL) {
 		SPDK_ERRLOG("Can't register controller with no name\n");
 		return -EINVAL;
 	}
 
-	if (spdk_vhost_dev_find(vdev->name)) {
+	if ((cpumask & spdk_app_get_core_mask()) != cpumask) {
-		SPDK_ERRLOG("vhost controller %s already exists.\n", vdev->name);
+		SPDK_ERRLOG("cpumask 0x%jx not a subset of app mask 0x%jx\n",
+			    cpumask, spdk_app_get_core_mask());
+		return -EINVAL;
+	}
+
+	if (spdk_vhost_dev_find(name)) {
+		SPDK_ERRLOG("vhost controller %s already exists.\n", name);
 		return -EEXIST;
 	}
 
@@ -285,9 +293,9 @@ spdk_vhost_dev_register(struct spdk_vhost_dev *vdev,
 		return -ENOSPC;
 	}
 
-	if (snprintf(path, sizeof(path), "%s%s", dev_dirname, vdev->name) >= (int)sizeof(path)) {
+	if (snprintf(path, sizeof(path), "%s%s", dev_dirname, name) >= (int)sizeof(path)) {
-		SPDK_ERRLOG("Resulting socket path for controller %s is too long: %s%s\n", vdev->name, dev_dirname,
+		SPDK_ERRLOG("Resulting socket path for controller %s is too long: %s%s\n", name, dev_dirname,
-			    vdev->name);
+			    name);
 		return -EINVAL;
 	}
 
@@ -295,42 +303,53 @@ spdk_vhost_dev_register(struct spdk_vhost_dev *vdev,
 	if (stat(path, &file_stat) != -1) {
 		if (!S_ISSOCK(file_stat.st_mode)) {
 			SPDK_ERRLOG("Cannot remove %s: not a socket.\n", path);
-			return -EINVAL;
+			return -EIO;
 		} else if (unlink(path) != 0) {
 			SPDK_ERRLOG("Cannot remove %s.\n", path);
-			abort();
+			return -EIO;
 		}
 	}
 
 	if (rte_vhost_driver_register(path, 0) != 0) {
-		SPDK_ERRLOG("Could not register controller %s with vhost library\n", vdev->name);
+		SPDK_ERRLOG("Could not register controller %s with vhost library\n", name);
 		SPDK_ERRLOG("Check if domain socket %s already exists\n", path);
 		return -EIO;
 	}
 	if (rte_vhost_driver_set_features(path, backend->virtio_features) ||
 	    rte_vhost_driver_disable_features(path, backend->disabled_features)) {
-		SPDK_ERRLOG("Couldn't set vhost features for controller %s\n", vdev->name);
+		SPDK_ERRLOG("Couldn't set vhost features for controller %s\n", name);
-		return -EINVAL;
-	}
 
-	if (rte_vhost_driver_callback_register(path, &backend->ops) != 0) {
+		rte_vhost_driver_unregister(path);
-		SPDK_ERRLOG("Couldn't register callbacks for controller %s\n", vdev->name);
-		return -ENOENT;
-	}
-
-	if (rte_vhost_driver_start(path) != 0) {
-		SPDK_ERRLOG("Failed to start vhost driver for controller %s (%d): %s", vdev->name, errno,
-			    strerror(errno));
 		return -EIO;
 	}
 
+	if (rte_vhost_driver_callback_register(path, &backend->ops) != 0) {
+		rte_vhost_driver_unregister(path);
+		SPDK_ERRLOG("Couldn't register callbacks for controller %s\n", name);
+		return -EIO;
+	}
+
+	vdev->name = strdup(name);
+	vdev->vid = -1;
+	vdev->lcore = -1;
+	vdev->cpumask = cpumask;
+	vdev->type = type;
+
 	g_spdk_vhost_devices[ctrlr_num] = vdev;
+
+	if (rte_vhost_driver_start(path) != 0) {
+		SPDK_ERRLOG("Failed to start vhost driver for controller %s (%d): %s", name, errno,
+			    strerror(errno));
+		rte_vhost_driver_unregister(path);
+		return -EIO;
+	}
+
 	SPDK_NOTICELOG("Controller %s: new controller added\n", vdev->name);
 	return 0;
 }
 
 int
-spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev)
+spdk_vhost_dev_remove(struct spdk_vhost_dev *vdev)
 {
 	unsigned ctrlr_num;
 	char path[PATH_MAX];
@@ -365,6 +384,7 @@ spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev)
 
 	SPDK_NOTICELOG("Controller %s: removed\n", vdev->name);
 
+	free(vdev->name);
 	g_spdk_vhost_devices[ctrlr_num] = NULL;
 	return 0;
 }

lib/vhost/vhost_internal.h

@@ -91,10 +91,12 @@ struct vring_desc *spdk_vhost_vring_desc_get_next(struct vring_desc *vq_desc,
 bool spdk_vhost_vring_desc_is_wr(struct vring_desc *cur_desc);
 
 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);
-int spdk_vhost_dev_register(struct spdk_vhost_dev *dev,
-			    const struct spdk_vhost_dev_backend *backend);
-int spdk_vhost_dev_unregister(struct spdk_vhost_dev *vdev);
 void spdk_vhost_dev_unload(struct spdk_vhost_dev *dev);
 
 #endif /* SPDK_VHOST_INTERNAL_H */

lib/vhost/vhost_scsi.c

@@ -609,41 +609,22 @@ remove_vdev_cb(void *arg1, void *arg2)
 int
 spdk_vhost_scsi_dev_construct(const char *name, uint64_t cpumask)
 {
-	struct spdk_vhost_scsi_dev *svdev;
+	struct spdk_vhost_scsi_dev *svdev = spdk_dma_zmalloc(sizeof(struct spdk_vhost_scsi_dev),
-	struct spdk_vhost_dev *vdev;
+					    SPDK_CACHE_LINE_SIZE, NULL);
-	int rc;
+	int ret;
 
-	if (name == NULL) {
-		SPDK_ERRLOG("Can't add controller with no name\n");
-		return -EINVAL;
-	}
-
-	if ((cpumask & spdk_app_get_core_mask()) != cpumask) {
-		SPDK_ERRLOG("cpumask 0x%jx not a subset of app mask 0x%jx\n",
-			    cpumask, spdk_app_get_core_mask());
-		return -EINVAL;
-	}
-
-	svdev = spdk_dma_zmalloc(sizeof(*svdev), SPDK_CACHE_LINE_SIZE, NULL);
 	if (svdev == NULL) {
-		SPDK_ERRLOG("Couldn't allocate memory for vhost dev\n");
 		return -ENOMEM;
 	}
 
-	vdev = &svdev->vdev;
+	ret = spdk_vhost_dev_construct(&svdev->vdev, name, cpumask, SPDK_VHOST_DEV_T_SCSI,
-	vdev->name =  strdup(name);
+				       &spdk_vhost_scsi_device_backend);
-	vdev->cpumask = cpumask;
-	vdev->lcore = -1;
 
-	vdev->type = SPDK_VHOST_DEV_T_SCSI;
+	if (ret) {
-
-	rc = spdk_vhost_dev_register(vdev, &spdk_vhost_scsi_device_backend);
-	if (rc < 0) {
-		free(vdev->name);
 		spdk_dma_free(svdev);
 	}
 
-	return rc;
+	return ret;
 }
 
 int
@@ -660,20 +641,11 @@ spdk_vhost_scsi_dev_remove(struct spdk_vhost_scsi_dev *svdev)
 		}
 	}
 
-	if (spdk_vhost_dev_unregister(vdev) != 0) {
+	if (spdk_vhost_dev_remove(vdev) != 0) {
-		SPDK_ERRLOG("Could not unregister scsi controller %s with vhost library\n", vdev->name);
 		return -EIO;
 	}
 
-	SPDK_NOTICELOG("Controller %s: removed\n", vdev->name);
-
-	/*
-	 * since spdk_vhost_scsi_vdev must not be in use,
-	 * it should be already *destructed* (spdk_vhost_dev_destruct)
-	 */
-	free(vdev->name);
 	spdk_dma_free(svdev);
-
 	return 0;
 }