From 48aced6ebc481d02cf14014d6c4039f813e3b13a Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Mon, 20 Nov 2017 19:24:24 +0100 Subject: [PATCH] virtio: move virtio_dev allocation to upper layers This allows the to use a virtio_dev allocated as a part of bigger struct - possibly a SCSI/BLK specific wrapper. As a part of this refactor, multiple vdev-related functions have been renamed to keep the API clean. Change-Id: If84bf03e4a1642869c2f1ba93939fb32c8fb5d57 Signed-off-by: Dariusz Stojaczyk Reviewed-on: https://review.gerrithub.io/388298 Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris Reviewed-by: Daniel Verkamp --- lib/bdev/virtio/bdev_virtio.c | 43 +++++++++++----- lib/bdev/virtio/rte_virtio/virtio.c | 25 +++------- lib/bdev/virtio/rte_virtio/virtio.h | 62 +++++++++++++++--------- lib/bdev/virtio/rte_virtio/virtio_pci.c | 16 ++++-- lib/bdev/virtio/rte_virtio/virtio_user.c | 31 ++++++------ 5 files changed, 103 insertions(+), 74 deletions(-) diff --git a/lib/bdev/virtio/bdev_virtio.c b/lib/bdev/virtio/bdev_virtio.c index b592225d4d..f1fe1fdfa1 100644 --- a/lib/bdev/virtio/bdev_virtio.c +++ b/lib/bdev/virtio/bdev_virtio.c @@ -574,8 +574,9 @@ scan_target_abort(struct virtio_scsi_scan_base *base, int error) } TAILQ_REMOVE(&g_virtio_driver.init_ctrlrs, base->vdev, tailq); - virtio_dev_reset(base->vdev); - virtio_dev_free(base->vdev); + virtio_dev_stop(base->vdev); + virtio_dev_destruct(base->vdev); + free(base->vdev); if (base->cb_fn) { @@ -1147,19 +1148,26 @@ bdev_virtio_process_config(void) num_queues = 1; } + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) { + SPDK_ERRLOG("virtio device calloc failed\n"); + goto out; + } + name = spdk_conf_section_get_val(sp, "Name"); if (name == NULL) { default_name = spdk_sprintf_alloc("VirtioScsi%u", vdev_num); name = default_name; } - vdev = virtio_user_dev_init(name, path, num_queues, 512, SPDK_VIRTIO_SCSI_QUEUE_NUM_FIXED); + rc = virtio_user_dev_init(vdev, name, path, num_queues, 512, + SPDK_VIRTIO_SCSI_QUEUE_NUM_FIXED); free(default_name); default_name = NULL; - if (vdev == NULL) { - rc = -1; + if (rc != 0) { + free(vdev); goto out; } } @@ -1191,8 +1199,9 @@ bdev_virtio_scsi_free(struct virtio_dev *vdev) virtio_dev_release_queue(vdev, VIRTIO_SCSI_REQUESTQ); } - virtio_dev_reset(vdev); - virtio_dev_free(vdev); + virtio_dev_stop(vdev); + virtio_dev_destruct(vdev); + free(vdev); } static int @@ -1211,7 +1220,7 @@ bdev_virtio_scsi_scan(struct virtio_dev *vdev, virtio_create_device_cb cb_fn, vo base->cb_fn = cb_fn; base->cb_arg = cb_arg; - rc = virtio_dev_init(vdev, VIRTIO_SCSI_DEV_SUPPORTED_FEATURES); + rc = virtio_dev_restart(vdev, VIRTIO_SCSI_DEV_SUPPORTED_FEATURES); if (rc != 0) { spdk_dma_free(base); return rc; @@ -1312,8 +1321,9 @@ virtio_scsi_dev_unregister_cb(void *io_device) virtio_dev_release_queue(vdev, VIRTIO_SCSI_CONTROLQ); } - virtio_dev_reset(vdev); - virtio_dev_free(vdev); + virtio_dev_stop(vdev); + virtio_dev_destruct(vdev); + free(vdev); TAILQ_REMOVE(&g_virtio_driver.attached_ctrlrs, vdev, tailq); finish_module = TAILQ_EMPTY(&g_virtio_driver.attached_ctrlrs); @@ -1345,10 +1355,17 @@ create_virtio_user_scsi_device(const char *base_name, const char *path, unsigned struct virtio_dev *vdev; int rc; - vdev = virtio_user_dev_init(base_name, path, num_queues, queue_size, - SPDK_VIRTIO_SCSI_QUEUE_NUM_FIXED); - if (!vdev) { + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) { + SPDK_ERRLOG("calloc failed for virtio device %s: %s\n", base_name, path); + return -ENOMEM; + } + + rc = virtio_user_dev_init(vdev, base_name, path, num_queues, queue_size, + SPDK_VIRTIO_SCSI_QUEUE_NUM_FIXED); + if (rc != 0) { SPDK_ERRLOG("Failed to create virito device %s: %s\n", base_name, path); + free(vdev); return -EINVAL; } diff --git a/lib/bdev/virtio/rte_virtio/virtio.c b/lib/bdev/virtio/rte_virtio/virtio.c index 013f596706..a4227baccf 100644 --- a/lib/bdev/virtio/rte_virtio/virtio.c +++ b/lib/bdev/virtio/rte_virtio/virtio.c @@ -315,31 +315,23 @@ virtio_negotiate_features(struct virtio_dev *dev, uint64_t req_features) return 0; } -struct virtio_dev * - virtio_dev_construct(const struct virtio_dev_ops *ops, void *ctx) +int +virtio_dev_construct(struct virtio_dev *vdev, const struct virtio_dev_ops *ops, void *ctx) { - struct virtio_dev *vdev; - - vdev = calloc(1, sizeof(*vdev)); - if (vdev == NULL) { - SPDK_ERRLOG("virtio device calloc failed\n"); - return NULL; - } - pthread_mutex_init(&vdev->mutex, NULL); vdev->backend_ops = ops; vdev->ctx = ctx; - return vdev; + return 0; } int -virtio_dev_init(struct virtio_dev *dev, uint64_t req_features) +virtio_dev_restart(struct virtio_dev *dev, uint64_t req_features) { int ret; /* Reset the device although not necessary at startup */ - virtio_dev_reset(dev); + virtio_dev_stop(dev); /* Tell the host we've noticed this device. */ virtio_dev_set_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE); @@ -358,12 +350,11 @@ virtio_dev_init(struct virtio_dev *dev, uint64_t req_features) } void -virtio_dev_free(struct virtio_dev *dev) +virtio_dev_destruct(struct virtio_dev *dev) { virtio_free_queues(dev); - virtio_dev_backend_ops(dev)->free_vdev(dev); + virtio_dev_backend_ops(dev)->destruct_dev(dev); pthread_mutex_destroy(&dev->mutex); - free(dev); } static void @@ -688,7 +679,7 @@ virtio_dev_write_dev_config(struct virtio_dev *dev, size_t offset, } void -virtio_dev_reset(struct virtio_dev *dev) +virtio_dev_stop(struct virtio_dev *dev) { virtio_dev_backend_ops(dev)->set_status(dev, VIRTIO_CONFIG_S_RESET); /* flush status write */ diff --git a/lib/bdev/virtio/rte_virtio/virtio.h b/lib/bdev/virtio/rte_virtio/virtio.h index 6cd77150bc..32f3836ec5 100644 --- a/lib/bdev/virtio/rte_virtio/virtio.h +++ b/lib/bdev/virtio/rte_virtio/virtio.h @@ -112,8 +112,8 @@ struct virtio_dev_ops { */ int (*set_features)(struct virtio_dev *vdev, uint64_t features); - /** Deinit and free virtio device */ - void (*free_vdev)(struct virtio_dev *vdev); + /** Destruct virtio device */ + void (*destruct_dev)(struct virtio_dev *vdev); uint16_t (*get_queue_num)(struct virtio_dev *hw, uint16_t queue_id); int (*setup_queue)(struct virtio_dev *hw, struct virtqueue *vq); @@ -210,22 +210,42 @@ uint16_t virtio_recv_pkts(struct virtqueue *vq, struct virtio_req **reqs, int virtio_xmit_pkt(struct virtqueue *vq, struct virtio_req *req); /** - * Construct virtio device. This will set vdev->id field. - * The device has to be freed with \c virtio_dev_free. + * Construct a virtio device. The device will be in stopped state by default. + * Before doing any I/O, it has to be manually started via \c virtio_dev_restart. * + * \param vdev memory for virtio device, must be zeroed * \param ops backend callbacks * \param ctx argument for the backend callbacks */ -struct virtio_dev *virtio_dev_construct(const struct virtio_dev_ops *ops, void *ctx); +int virtio_dev_construct(struct virtio_dev *vdev, const struct virtio_dev_ops *ops, + void *ctx); /** - * Reset and reinit a virtio device. This will also renegotiate feature flags. + * Notify the host to start processing this virtio device. This is + * a blocking call that won't return until the host has started. + * This call will also allocate virtqueues and renegotiate feature flags. * - * \param vdev vhost device + * \param vdev virtio device * \param req_features features this driver supports */ -int virtio_dev_init(struct virtio_dev *vdev, uint64_t req_features); -void virtio_dev_free(struct virtio_dev *dev); +int virtio_dev_restart(struct virtio_dev *vdev, uint64_t req_features); + +/** + * Stop the host from processing the device. This is a blocking call + * that won't return until all outstanding I/O has been processed on + * the host (virtio device) side. + * + * \param vdev virtio device + */ +void virtio_dev_stop(struct virtio_dev *vdev); + +/** + * Destruct a virtio device. Note that it must be in the stopped state. + * The virtio_dev should be manually freed afterwards. + * + * \param vdev virtio device + */ +void virtio_dev_destruct(struct virtio_dev *vdev); /** * Bind a virtqueue with given index to the current thread; @@ -286,14 +306,6 @@ bool virtio_dev_queue_is_acquired(struct virtio_dev *vdev, uint16_t index); */ void virtio_dev_release_queue(struct virtio_dev *vdev, uint16_t index); -/** - * Reset given virtio device. This will leave the device in unusable state. - * To reuse the device, call \c virtio_dev_init. - * - * \param vdev virtio device - */ -void virtio_dev_reset(struct virtio_dev *vdev); - /** * Get Virtio status flags. * @@ -303,9 +315,9 @@ uint8_t virtio_dev_get_status(struct virtio_dev *vdev); /** * Set Virtio status flag. The flags have to be set in very specific order - * defined the VirtIO 1.0 spec section 3.1.1. To unset the flags, call - * \c virtio_dev_reset or set \c VIRTIO_CONFIG_S_RESET flag. There is - * no way to unset particular flags. + * defined the VIRTIO 1.0 spec section 3.1.1. To unset the flags, stop the + * device or set \c VIRTIO_CONFIG_S_RESET status flag. There is no way to + * unset only particular flags. * * \param vdev virtio device * \param flag flag to set @@ -367,8 +379,10 @@ void virtio_dev_dump_json_config(struct virtio_dev *vdev, struct spdk_json_write int virtio_enumerate_pci(void); /** - * Connect to a vhost-user device and create corresponding virtio_dev. + * Connect to a vhost-user device and init corresponding virtio_dev struct. + * The virtio_dev will have to be freed with \c virtio_dev_free. * + * \param vdev preallocated vhost device struct to operate on * \param name name of this virtio device * \param path path to the Unix domain socket of the vhost-user device * \param requested_queues maximum number of request queues that this @@ -379,8 +393,8 @@ int virtio_enumerate_pci(void); * additional event and control queues. * \return virtio device */ -struct virtio_dev *virtio_user_dev_init(const char *name, const char *path, - uint16_t requested_queues, - uint32_t queue_size, uint16_t fixed_queue_num); +int virtio_user_dev_init(struct virtio_dev *vdev, const char *name, const char *path, + uint16_t requested_queues, uint32_t queue_size, + uint16_t fixed_queue_num); #endif /* SPDK_VIRTIO_H */ diff --git a/lib/bdev/virtio/rte_virtio/virtio_pci.c b/lib/bdev/virtio/rte_virtio/virtio_pci.c index a6682695fe..10e60dee81 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_pci.c +++ b/lib/bdev/virtio/rte_virtio/virtio_pci.c @@ -198,7 +198,7 @@ modern_set_features(struct virtio_dev *dev, uint64_t features) } static void -modern_free_vdev(struct virtio_dev *vdev) +modern_destruct_dev(struct virtio_dev *vdev) { struct virtio_hw *hw = vdev->ctx; @@ -301,7 +301,7 @@ static const struct virtio_dev_ops modern_ops = { .set_status = modern_set_status, .get_features = modern_get_features, .set_features = modern_set_features, - .free_vdev = modern_free_vdev, + .destruct_dev = modern_destruct_dev, .get_queue_num = modern_get_queue_num, .setup_queue = modern_setup_queue, .del_queue = modern_del_queue, @@ -470,8 +470,16 @@ pci_enum_virtio_probe_cb(void *ctx, struct spdk_pci_device *pci_dev) return -1; } - vdev = virtio_dev_construct(&modern_ops, hw); + vdev = calloc(1, sizeof(*vdev)); if (vdev == NULL) { + SPDK_ERRLOG("calloc failed\n"); + free_virtio_hw(hw); + return -1; + } + + rc = virtio_dev_construct(vdev, &modern_ops, hw); + if (rc != 0) { + free(vdev); free_virtio_hw(hw); return -1; } @@ -486,7 +494,7 @@ pci_enum_virtio_probe_cb(void *ctx, struct spdk_pci_device *pci_dev) return 0; err: - virtio_dev_free(vdev); + virtio_dev_destruct(vdev); return -1; } diff --git a/lib/bdev/virtio/rte_virtio/virtio_user.c b/lib/bdev/virtio/rte_virtio/virtio_user.c index 19dee4b166..5ec3af6465 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_user.c +++ b/lib/bdev/virtio/rte_virtio/virtio_user.c @@ -345,7 +345,7 @@ virtio_user_notify_queue(struct virtio_dev *vdev, struct virtqueue *vq) } static void -virtio_user_free(struct virtio_dev *vdev) +virtio_user_destroy(struct virtio_dev *vdev) { struct virtio_user_dev *dev = vdev->ctx; @@ -373,7 +373,7 @@ static const struct virtio_dev_ops virtio_user_ops = { .set_status = virtio_user_set_status, .get_features = virtio_user_get_features, .set_features = virtio_user_set_features, - .free_vdev = virtio_user_free, + .destruct_dev = virtio_user_destroy, .get_queue_num = virtio_user_get_queue_num, .setup_queue = virtio_user_setup_queue, .del_queue = virtio_user_del_queue, @@ -381,34 +381,33 @@ static const struct virtio_dev_ops virtio_user_ops = { .dump_json_config = virtio_user_dump_json_config, }; -struct virtio_dev * -virtio_user_dev_init(const char *name, const char *path, uint16_t requested_queues, - uint32_t queue_size, - uint16_t fixed_queue_num) +int +virtio_user_dev_init(struct virtio_dev *vdev, const char *name, const char *path, + uint16_t requested_queues, uint32_t queue_size, uint16_t fixed_queue_num) { - struct virtio_dev *vdev; struct virtio_user_dev *dev; uint64_t max_queues; char err_str[64]; + int rc; if (name == NULL) { SPDK_ERRLOG("No name gived for controller: %s\n", path); - return NULL; + return -1; } else if (requested_queues == 0) { SPDK_ERRLOG("Can't create controller with no queues: %s\n", path); - return NULL; + return -1; } dev = calloc(1, sizeof(*dev)); if (dev == NULL) { - return NULL; + return -1; } - vdev = virtio_dev_construct(&virtio_user_ops, dev); - if (vdev == NULL) { + rc = virtio_dev_construct(vdev, &virtio_user_ops, dev); + if (rc != 0) { SPDK_ERRLOG("Failed to init device: %s\n", path); free(dev); - return NULL; + return -1; } vdev->is_hw = 0; @@ -447,9 +446,9 @@ virtio_user_dev_init(const char *name, const char *path, uint16_t requested_queu } TAILQ_INSERT_TAIL(&g_virtio_driver.init_ctrlrs, vdev, tailq); - return vdev; + return 0; err: - virtio_dev_free(vdev); - return NULL; + virtio_dev_destruct(vdev); + return -1; }