diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index bf1538fac..ca3aee980 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -46,8 +46,6 @@ #include "spdk/vhost.h" #include "vhost_internal.h" -#include "spdk_internal/assert.h" - /* Features supported by SPDK VHOST lib. */ #define SPDK_VHOST_SCSI_FEATURES (SPDK_VHOST_FEATURES | \ (1ULL << VIRTIO_SCSI_F_INOUT) | \ @@ -71,13 +69,11 @@ struct spdk_vhost_scsi_dev { struct spdk_vhost_dev vdev; struct spdk_scsi_dev *scsi_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS]; - bool detached_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS]; + bool removed_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS]; struct spdk_ring *task_pool; struct spdk_poller *requestq_poller; struct spdk_poller *mgmt_poller; - - struct spdk_ring *vhost_events; } __rte_cache_aligned; struct spdk_vhost_scsi_task { @@ -97,18 +93,6 @@ struct spdk_vhost_scsi_task { struct rte_vhost_vring *vq; }; -enum spdk_vhost_scsi_event_type { - SPDK_VHOST_SCSI_EVENT_HOTATTACH, - SPDK_VHOST_SCSI_EVENT_HOTDETACH, -}; - -struct spdk_vhost_scsi_event { - enum spdk_vhost_scsi_event_type type; - unsigned dev_index; - struct spdk_scsi_dev *dev; - struct spdk_scsi_lun *lun; -}; - static int new_device(struct spdk_vhost_dev *, void *); static int destroy_device(struct spdk_vhost_dev *, void *); static void spdk_vhost_scsi_config_json(struct spdk_vhost_dev *vdev, struct spdk_json_write_ctx *w); @@ -154,107 +138,6 @@ spdk_vhost_get_tasks(struct spdk_vhost_scsi_dev *svdev, struct spdk_vhost_scsi_t svdev->vdev.task_cnt += res_count; } -static void -spdk_vhost_scsi_event_process(struct spdk_vhost_scsi_dev *svdev, struct spdk_vhost_scsi_event *ev, - struct virtio_scsi_event *desc_ev) -{ - int event_id, reason_id; - int dev_id, lun_id; - - assert(ev->dev); - assert(ev->dev_index < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS); - dev_id = ev->dev_index; - - switch (ev->type) { - case SPDK_VHOST_SCSI_EVENT_HOTATTACH: - event_id = VIRTIO_SCSI_T_TRANSPORT_RESET; - reason_id = VIRTIO_SCSI_EVT_RESET_RESCAN; - - spdk_scsi_dev_allocate_io_channels(ev->dev); - svdev->scsi_dev[dev_id] = ev->dev; - svdev->detached_dev[dev_id] = false; - SPDK_NOTICELOG("%s: hot-attached device %d\n", svdev->vdev.name, dev_id); - break; - case SPDK_VHOST_SCSI_EVENT_HOTDETACH: - event_id = VIRTIO_SCSI_T_TRANSPORT_RESET; - reason_id = VIRTIO_SCSI_EVT_RESET_REMOVED; - - if (ev->lun == NULL) { - svdev->detached_dev[dev_id] = true; - SPDK_NOTICELOG("%s: marked 'Dev %d' for hot-detach\n", svdev->vdev.name, dev_id); - } else { - SPDK_NOTICELOG("%s: hotremoved LUN '%s'\n", svdev->vdev.name, spdk_scsi_lun_get_name(ev->lun)); - } - break; - default: - SPDK_UNREACHABLE(); - } - - /* some events may apply to the entire device via lun id set to 0 */ - lun_id = ev->lun == NULL ? 0 : spdk_scsi_lun_get_id(ev->lun); - - if (desc_ev) { - desc_ev->event = event_id; - desc_ev->lun[0] = 1; - desc_ev->lun[1] = dev_id; - desc_ev->lun[2] = lun_id >> 8; /* relies on linux kernel implementation */ - desc_ev->lun[3] = lun_id & 0xFF; - memset(&desc_ev->lun[4], 0, 4); - desc_ev->reason = reason_id; - } -} - -/** - * Process vhost event, send virtio event - */ -static void -process_event(struct spdk_vhost_scsi_dev *svdev, struct spdk_vhost_scsi_event *ev) -{ - struct vring_desc *desc; - struct virtio_scsi_event *desc_ev; - uint32_t req_size; - uint16_t req; - struct rte_vhost_vring *vq = &svdev->vdev.virtqueue[VIRTIO_SCSI_EVENTQ]; - - if (spdk_vhost_vq_avail_ring_get(vq, &req, 1) != 1) { - SPDK_ERRLOG("%s: no avail virtio eventq ring entries. virtio event won't be sent.\n", - svdev->vdev.name); - desc = NULL; - desc_ev = NULL; - req_size = 0; - /* even though we can't send virtio event, - * the spdk vhost event should still be processed - */ - } else { - desc = spdk_vhost_vq_get_desc(vq, req); - desc_ev = spdk_vhost_gpa_to_vva(&svdev->vdev, desc->addr); - req_size = sizeof(*desc_ev); - - if (desc->len < sizeof(*desc_ev) || desc_ev == NULL) { - SPDK_ERRLOG("%s: invalid eventq descriptor.\n", svdev->vdev.name); - desc_ev = NULL; - req_size = 0; - } - } - - spdk_vhost_scsi_event_process(svdev, ev, desc_ev); - - if (desc) { - spdk_vhost_vq_used_ring_enqueue(&svdev->vdev, vq, req, req_size); - } -} - -static void -process_eventq(struct spdk_vhost_scsi_dev *svdev) -{ - struct spdk_vhost_scsi_event *ev; - - while (spdk_ring_dequeue(svdev->vhost_events, (void **)&ev, 1) == 1) { - process_event(svdev, ev); - spdk_dma_free(ev); - } -} - static void process_removed_devs(struct spdk_vhost_scsi_dev *svdev) { @@ -264,41 +147,76 @@ process_removed_devs(struct spdk_vhost_scsi_dev *svdev) for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; ++i) { dev = svdev->scsi_dev[i]; - if (dev && svdev->detached_dev[i] && !spdk_scsi_dev_has_pending_tasks(dev)) { + if (dev && svdev->removed_dev[i] && !spdk_scsi_dev_has_pending_tasks(dev)) { spdk_scsi_dev_free_io_channels(dev); spdk_scsi_dev_destruct(dev); svdev->scsi_dev[i] = NULL; - SPDK_NOTICELOG("%s: hot-detached 'Dev %d'.\n", svdev->vdev.name, i); + SPDK_NOTICELOG("%s: hot-detached device 'Dev %u'.\n", svdev->vdev.name, i); } } } static void -enqueue_vhost_event(struct spdk_vhost_scsi_dev *svdev, enum spdk_vhost_scsi_event_type type, - int dev_index, struct spdk_scsi_dev *dev, struct spdk_scsi_lun *lun) +eventq_enqueue(struct spdk_vhost_scsi_dev *svdev, const struct spdk_scsi_dev *dev, + const struct spdk_scsi_lun *lun, uint32_t event, + uint32_t reason) { - struct spdk_vhost_scsi_event *ev; + int dev_id, lun_id; + struct rte_vhost_vring *vq; + struct vring_desc *desc; + struct virtio_scsi_event *desc_ev; + uint32_t req_size; + uint16_t req; if (dev == NULL) { - SPDK_ERRLOG("%s: vhost event device cannot be NULL.\n", svdev->vdev.name); + SPDK_ERRLOG("%s: eventq device cannot be NULL.\n", svdev->vdev.name); return; } - ev = spdk_dma_zmalloc(sizeof(*ev), SPDK_CACHE_LINE_SIZE, NULL); - if (ev == NULL) { - SPDK_ERRLOG("%s: failed to alloc vhost event.\n", svdev->vdev.name); + for (dev_id = 0; dev_id < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; dev_id++) { + if (svdev->scsi_dev[dev_id] == dev) { + break; + } + } + + if (dev_id == SPDK_VHOST_SCSI_CTRLR_MAX_DEVS) { + SPDK_ERRLOG("Dev %s is not a part of vhost scsi controller '%s'.\n", spdk_scsi_dev_get_name(dev), + svdev->vdev.name); return; } - ev->type = type; - ev->dev_index = dev_index; - ev->dev = dev; - ev->lun = lun; + /* some events may apply to the entire device via lun id set to 0 */ + lun_id = lun == NULL ? 0 : spdk_scsi_lun_get_id(lun); - if (spdk_ring_enqueue(svdev->vhost_events, (void **)&ev, 1) != 1) { - SPDK_ERRLOG("%s: failed to enqueue vhost event (no room in ring?).\n", svdev->vdev.name); - spdk_dma_free(ev); + vq = &svdev->vdev.virtqueue[VIRTIO_SCSI_EVENTQ]; + + if (spdk_vhost_vq_avail_ring_get(vq, &req, 1) != 1) { + SPDK_ERRLOG("Controller %s: Failed to send virtio event (no avail ring entries?).\n", + svdev->vdev.name); + return; } + + desc = spdk_vhost_vq_get_desc(vq, req); + desc_ev = spdk_vhost_gpa_to_vva(&svdev->vdev, desc->addr); + + if (desc->len < sizeof(*desc_ev) || desc_ev == NULL) { + SPDK_ERRLOG("Controller %s: Invalid eventq descriptor.\n", svdev->vdev.name); + req_size = 0; + } else { + desc_ev->event = event; + desc_ev->lun[0] = 1; + desc_ev->lun[1] = dev_id; + desc_ev->lun[2] = lun_id >> 8; /* relies on linux kernel implementation */ + desc_ev->lun[3] = lun_id & 0xFF; + /* virtio doesn't specify any strict format for LUN id (bytes 2 and 3) + * current implementation relies on linux kernel sources + */ + memset(&desc_ev->lun[4], 0, 4); + desc_ev->reason = reason; + req_size = sizeof(*desc_ev); + } + + spdk_vhost_vq_used_ring_enqueue(&svdev->vdev, vq, req, req_size); } static void @@ -355,11 +273,6 @@ mgmt_task_submit(struct spdk_vhost_scsi_task *task, enum spdk_scsi_task_func fun static void invalid_request(struct spdk_vhost_scsi_task *task) { - /* Flush eventq so that guest is instantly notified about any hotremoved luns. - * This might prevent him from sending more invalid requests and trying to reset - * the device. - */ - process_eventq(task->svdev); spdk_vhost_vq_used_ring_enqueue(&task->svdev->vdev, task->vq, task->req_idx, 0); spdk_vhost_scsi_task_put(task); @@ -382,10 +295,10 @@ spdk_vhost_scsi_task_init_target(struct spdk_vhost_scsi_task *task, const __u8 * dev = task->svdev->scsi_dev[lun[1]]; task->scsi_dev = dev; if (dev == NULL) { - /* If dev has been hot-detached, return 0 to allow sending additional - * scsi hotremove event via sense codes. + /* If dev has been hotdetached, return 0 to allow sending + * additional hotremove event via sense codes. */ - return task->svdev->detached_dev[lun[1]] ? 0 : -1; + return task->svdev->removed_dev[lun[1]] ? 0 : -1; } task->scsi.target_port = spdk_scsi_dev_find_port_by_id(task->scsi_dev, 0); @@ -673,7 +586,6 @@ vdev_mgmt_worker(void *arg) struct spdk_vhost_scsi_dev *svdev = arg; process_removed_devs(svdev); - process_eventq(svdev); process_controlq(svdev, &svdev->vdev.virtqueue[VIRTIO_SCSI_CONTROLQ]); } @@ -720,7 +632,6 @@ spdk_vhost_scsi_dev_construct(const char *name, const char *cpumask) &spdk_vhost_scsi_device_backend); if (rc) { - spdk_ring_free(svdev->vhost_events); spdk_dma_free(svdev); } @@ -750,7 +661,6 @@ spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev) return rc; } - spdk_ring_free(svdev->vhost_events); spdk_dma_free(svdev); return 0; } @@ -770,8 +680,6 @@ static void spdk_vhost_scsi_lun_hotremove(const struct spdk_scsi_lun *lun, void *arg) { struct spdk_vhost_scsi_dev *svdev = arg; - const struct spdk_scsi_dev *scsi_dev; - unsigned scsi_dev_num; assert(lun != NULL); assert(svdev != NULL); @@ -780,25 +688,8 @@ spdk_vhost_scsi_lun_hotremove(const struct spdk_scsi_lun *lun, void *arg) return; } - scsi_dev = spdk_scsi_lun_get_dev(lun); - - for (scsi_dev_num = 0; scsi_dev_num < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; ++scsi_dev_num) { - if (svdev->scsi_dev[scsi_dev_num] == scsi_dev) { - break; - } - } - - if (scsi_dev_num == SPDK_VHOST_SCSI_CTRLR_MAX_DEVS) { - SPDK_ERRLOG("%s: '%s' is not a part of this controller.\n", svdev->vdev.name, - spdk_scsi_dev_get_name(scsi_dev)); - return; - } - - enqueue_vhost_event(svdev, SPDK_VHOST_SCSI_EVENT_HOTDETACH, scsi_dev_num, - (struct spdk_scsi_dev *) scsi_dev, (struct spdk_scsi_lun *) lun); - - SPDK_NOTICELOG("%s: queued LUN '%s' for hotremove\n", svdev->vdev.name, - spdk_scsi_dev_get_name(scsi_dev)); + eventq_enqueue(svdev, spdk_scsi_lun_get_dev(lun), lun, VIRTIO_SCSI_T_TRANSPORT_RESET, + VIRTIO_SCSI_EVT_RESET_REMOVED); } int @@ -806,7 +697,6 @@ spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, const char *lun_name) { struct spdk_vhost_scsi_dev *svdev; - struct spdk_scsi_dev *scsi_dev; char dev_name[SPDK_SCSI_DEV_MAX_NAME]; int lun_id_list[1]; char *lun_names_list[1]; @@ -835,9 +725,8 @@ spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, return -EINVAL; } - if (vdev->lcore != -1 && !spdk_vhost_dev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { - SPDK_ERRLOG("%s: 'Dev %u' is in use and hot-attach is not enabled for this controller\n", - vdev->name, scsi_dev_num); + if (vdev->lcore != -1) { + SPDK_ERRLOG("Controller %s is in use and hotplug is not supported\n", vdev->name); return -ENOTSUP; } @@ -853,23 +742,16 @@ spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, lun_id_list[0] = 0; lun_names_list[0] = (char *)lun_name; - scsi_dev = spdk_scsi_dev_construct(dev_name, lun_names_list, lun_id_list, 1, - SPDK_SPC_PROTOCOL_IDENTIFIER_SAS, spdk_vhost_scsi_lun_hotremove, svdev); - if (scsi_dev == NULL) { + svdev->removed_dev[scsi_dev_num] = false; + svdev->scsi_dev[scsi_dev_num] = spdk_scsi_dev_construct(dev_name, lun_names_list, lun_id_list, 1, + SPDK_SPC_PROTOCOL_IDENTIFIER_SAS, spdk_vhost_scsi_lun_hotremove, svdev); + + if (svdev->scsi_dev[scsi_dev_num] == NULL) { SPDK_ERRLOG("Couldn't create spdk SCSI device '%s' using lun device '%s' in controller: %s\n", dev_name, lun_name, vdev->name); return -EINVAL; } - - spdk_scsi_dev_add_port(scsi_dev, 0, "vhost"); - - if (vdev->lcore == -1) { - svdev->detached_dev[scsi_dev_num] = false; - svdev->scsi_dev[scsi_dev_num] = scsi_dev; - } else { - enqueue_vhost_event(svdev, SPDK_VHOST_SCSI_EVENT_HOTATTACH, scsi_dev_num, scsi_dev, NULL); - } - + spdk_scsi_dev_add_port(svdev->scsi_dev[scsi_dev_num], 0, "vhost"); SPDK_NOTICELOG("Controller %s: defined device '%s' using lun '%s'\n", vdev->name, dev_name, lun_name); return 0; @@ -911,7 +793,8 @@ spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_nu return -ENOTSUP; } - enqueue_vhost_event(svdev, SPDK_VHOST_SCSI_EVENT_HOTDETACH, scsi_dev_num, scsi_dev, NULL); + svdev->removed_dev[scsi_dev_num] = true; + eventq_enqueue(svdev, scsi_dev, NULL, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_REMOVED); SPDK_NOTICELOG("%s: queued 'Dev %u' for hot-detach.\n", vdev->name, scsi_dev_num); return 0; @@ -1055,7 +938,7 @@ new_device(struct spdk_vhost_dev *vdev, void *event_ctx) { struct spdk_vhost_scsi_dev *svdev; uint32_t i; - int rc = 0; + int rc; svdev = to_scsi_dev(vdev); if (svdev == NULL) { @@ -1070,14 +953,6 @@ new_device(struct spdk_vhost_dev *vdev, void *event_ctx) goto out; } - 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.\n", vdev->name); - rc = -1; - goto out; - } - for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; i++) { if (svdev->scsi_dev[i] == NULL) { continue; @@ -1107,7 +982,6 @@ destroy_device_poller_cb(void *arg) { struct spdk_vhost_dev_destroy_ctx *ctx = arg; struct spdk_vhost_scsi_dev *svdev = ctx->svdev; - void *ev; uint32_t i; if (svdev->vdev.task_cnt > 0) { @@ -1124,15 +998,6 @@ destroy_device_poller_cb(void *arg) SPDK_NOTICELOG("Stopping poller for vhost controller %s\n", svdev->vdev.name); spdk_vhost_dev_mem_unregister(&svdev->vdev); - /* Flush not sent events */ - while (spdk_ring_dequeue(svdev->vhost_events, &ev, 1) == 1) { - /* process vhost event, but don't send virtio event */ - spdk_vhost_scsi_event_process(svdev, ev, NULL); - spdk_dma_free(ev); - } - - spdk_ring_free(svdev->vhost_events); - free_task_pool(svdev); spdk_poller_unregister(&ctx->poller, NULL); diff --git a/test/vhost/fiotest/autotest.sh b/test/vhost/fiotest/autotest.sh index 25416522e..6e92aa2db 100755 --- a/test/vhost/fiotest/autotest.sh +++ b/test/vhost/fiotest/autotest.sh @@ -250,26 +250,6 @@ done $COMMON_DIR/vm_run.sh $x --work-dir=$TEST_DIR $used_vms vm_wait_for_boot 600 $used_vms -if [[ $test_type == "spdk_vhost_scsi" ]]; then - for vm_conf in ${vms[@]}; do - IFS=',' read -ra conf <<< "$vm_conf" - while IFS=':' read -ra disks; do - for disk in "${disks[@]}"; do - echo "INFO: Hotdetach test. Trying to remove existing device from a controller naa.$disk.${conf[0]}" - $rpc_py remove_vhost_scsi_dev naa.$disk.${conf[0]} 0 - - sleep 0.1 - - echo "INFO: Hotattach test. Re-adding device 0 to naa.$disk.${conf[0]}" - $rpc_py add_vhost_scsi_lun naa.$disk.${conf[0]} 0 $disk - done - done <<< "${conf[2]}" - unset IFS; - done -fi - -sleep 0.1 - echo "===============" echo "" echo "INFO: Testing..."