From 1dae563373d948609de5b3f3ec646e6ffddb5409 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Fri, 9 Aug 2019 03:32:06 +0200 Subject: [PATCH] vhost: move rte_vhost socket creation to rte_vhost_compat.c rte_vhost_compat.c will now not only handle vhost-user messages over the unix domain socket, but also setup that unix domain socket with rte_vhost's APIs. What was previously called vhost_dev_install_rte_compat_hooks() is now called vhost_register_unix_socket() and is responsible for creating the entire unix domain socket. This enables us to write more advanced unit tests for vhost. Instead of mocking low-level rte_vhost APIs, we could now potentially mock vhost_register_unix_socket() and create vhost devices and sessions without any actual unix domain sockets involved. Change-Id: Ifb18b92b37915c3f683b6d4fcdcc9259a3770561 Signed-off-by: Darek Stojaczyk Signed-off-by: Vitaliy Mysak Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470455 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/vhost/rte_vhost_compat.c | 78 ++++++++++++++++++++------ lib/vhost/vhost.c | 48 +--------------- lib/vhost/vhost_internal.h | 4 +- test/unit/lib/vhost/vhost.c/vhost_ut.c | 4 +- 4 files changed, 68 insertions(+), 66 deletions(-) diff --git a/lib/vhost/rte_vhost_compat.c b/lib/vhost/rte_vhost_compat.c index f40f5b25b..a1b35b99f 100644 --- a/lib/vhost/rte_vhost_compat.c +++ b/lib/vhost/rte_vhost_compat.c @@ -48,9 +48,10 @@ #include "spdk_internal/vhost_user.h" -#ifndef SPDK_CONFIG_VHOST_INTERNAL_LIB extern const struct vhost_device_ops g_spdk_vhost_ops; +#ifndef SPDK_CONFIG_VHOST_INTERNAL_LIB + static enum rte_vhost_msg_result spdk_extern_vhost_pre_msg_handler(int vid, void *_msg) { @@ -215,26 +216,71 @@ vhost_session_install_rte_compat_hooks(struct spdk_vhost_session *vsession) } } -void -vhost_dev_install_rte_compat_hooks(struct spdk_vhost_dev *vdev) -{ - uint64_t protocol_features = 0; - - rte_vhost_driver_get_protocol_features(vdev->path, &protocol_features); - protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG); - rte_vhost_driver_set_protocol_features(vdev->path, protocol_features); -} - #else /* SPDK_CONFIG_VHOST_INTERNAL_LIB */ + void vhost_session_install_rte_compat_hooks(struct spdk_vhost_session *vsession) { /* nothing to do. all the changes are already incorporated into rte_vhost */ } -void -vhost_dev_install_rte_compat_hooks(struct spdk_vhost_dev *vdev) -{ - /* nothing to do */ -} #endif + +int +vhost_register_unix_socket(const char *path, const char *ctrl_name, + uint64_t virtio_features, uint64_t disabled_features) +{ + struct stat file_stat; +#ifndef SPDK_CONFIG_VHOST_INTERNAL_LIB + uint64_t protocol_features = 0; +#endif + + /* Register vhost driver to handle vhost messages. */ + if (stat(path, &file_stat) != -1) { + if (!S_ISSOCK(file_stat.st_mode)) { + SPDK_ERRLOG("Cannot create a domain socket at path \"%s\": " + "The file already exists and is not a socket.\n", + path); + return -EIO; + } else if (unlink(path) != 0) { + SPDK_ERRLOG("Cannot create a domain socket at path \"%s\": " + "The socket already exists and failed to unlink.\n", + path); + return -EIO; + } + } + + if (rte_vhost_driver_register(path, 0) != 0) { + SPDK_ERRLOG("Could not register controller %s with vhost library\n", ctrl_name); + SPDK_ERRLOG("Check if domain socket %s already exists\n", path); + return -EIO; + } + if (rte_vhost_driver_set_features(path, virtio_features) || + rte_vhost_driver_disable_features(path, disabled_features)) { + SPDK_ERRLOG("Couldn't set vhost features for controller %s\n", ctrl_name); + + rte_vhost_driver_unregister(path); + return -EIO; + } + + 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", ctrl_name); + return -EIO; + } + +#ifndef SPDK_CONFIG_VHOST_INTERNAL_LIB + rte_vhost_driver_get_protocol_features(path, &protocol_features); + protocol_features |= (1ULL << VHOST_USER_PROTOCOL_F_CONFIG); + rte_vhost_driver_set_protocol_features(path, protocol_features); +#endif + + if (rte_vhost_driver_start(path) != 0) { + SPDK_ERRLOG("Failed to start vhost driver for controller %s (%d): %s\n", + ctrl_name, errno, spdk_strerror(errno)); + rte_vhost_driver_unregister(path); + return -EIO; + } + + return 0; +} diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 8d46240e9..c002be370 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -675,7 +675,6 @@ vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *ma const struct spdk_vhost_dev_backend *backend) { char path[PATH_MAX]; - struct stat file_stat; struct spdk_cpuset *cpumask; int rc; @@ -711,51 +710,11 @@ vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *ma goto out; } - /* Register vhost driver to handle vhost messages. */ - if (stat(path, &file_stat) != -1) { - if (!S_ISSOCK(file_stat.st_mode)) { - SPDK_ERRLOG("Cannot create a domain socket at path \"%s\": " - "The file already exists and is not a socket.\n", - path); - rc = -EIO; - goto out; - } else if (unlink(path) != 0) { - SPDK_ERRLOG("Cannot create a domain socket at path \"%s\": " - "The socket already exists and failed to unlink.\n", - path); - rc = -EIO; - goto out; - } - } - - if (rte_vhost_driver_register(path, 0) != 0) { - SPDK_ERRLOG("Could not register controller %s with vhost library\n", name); - SPDK_ERRLOG("Check if domain socket %s already exists\n", path); - rc = -EIO; - goto out; - } - 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", name); - - rte_vhost_driver_unregister(path); - rc = -EIO; - goto out; - } - - 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); - rc = -EIO; - goto out; - } - vdev->name = strdup(name); vdev->path = strdup(path); if (vdev->name == NULL || vdev->path == NULL) { free(vdev->name); free(vdev->path); - rte_vhost_driver_unregister(path); rc = -EIO; goto out; } @@ -769,12 +728,7 @@ vhost_dev_register(struct spdk_vhost_dev *vdev, const char *name, const char *ma vhost_dev_set_coalescing(vdev, SPDK_VHOST_COALESCING_DELAY_BASE_US, SPDK_VHOST_VQ_IOPS_COALESCING_THRESHOLD); - vhost_dev_install_rte_compat_hooks(vdev); - - if (rte_vhost_driver_start(path) != 0) { - SPDK_ERRLOG("Failed to start vhost driver for controller %s (%d): %s\n", - name, errno, spdk_strerror(errno)); - rte_vhost_driver_unregister(path); + if (vhost_register_unix_socket(path, name, backend->virtio_features, backend->disabled_features)) { TAILQ_REMOVE(&g_vhost_devices, vdev, tailq); free(vdev->name); free(vdev->path); diff --git a/lib/vhost/vhost_internal.h b/lib/vhost/vhost_internal.h index e1681a95f..d914a1fd3 100644 --- a/lib/vhost/vhost_internal.h +++ b/lib/vhost/vhost_internal.h @@ -385,7 +385,9 @@ void vhost_session_stop_done(struct spdk_vhost_session *vsession, int response); struct spdk_vhost_session *vhost_session_find_by_vid(int vid); void vhost_session_install_rte_compat_hooks(struct spdk_vhost_session *vsession); -void vhost_dev_install_rte_compat_hooks(struct spdk_vhost_dev *vdev); + +int vhost_register_unix_socket(const char *path, const char *ctrl_name, + uint64_t virtio_features, uint64_t disabled_features); struct vhost_poll_group *vhost_get_poll_group(struct spdk_cpuset *cpumask); diff --git a/test/unit/lib/vhost/vhost.c/vhost_ut.c b/test/unit/lib/vhost/vhost.c/vhost_ut.c index 8bfb2b685..985ec259b 100644 --- a/test/unit/lib/vhost/vhost.c/vhost_ut.c +++ b/test/unit/lib/vhost/vhost.c/vhost_ut.c @@ -48,8 +48,8 @@ DEFINE_STUB(rte_vhost_get_vring_base, int, (int vid, uint16_t queue_id, uint16_t *last_avail_idx, uint16_t *last_used_idx), 0); DEFINE_STUB_V(vhost_session_install_rte_compat_hooks, (struct spdk_vhost_session *vsession)); -DEFINE_STUB_V(vhost_dev_install_rte_compat_hooks, - (struct spdk_vhost_dev *vdev)); +DEFINE_STUB(vhost_register_unix_socket, int, (const char *path, const char *name, + uint64_t virtio_features, uint64_t disabled_features), 0); DEFINE_STUB(rte_vhost_driver_unregister, int, (const char *path), 0); DEFINE_STUB(spdk_mem_register, int, (void *vaddr, size_t len), 0); DEFINE_STUB(spdk_mem_unregister, int, (void *vaddr, size_t len), 0);