From ad9a842c71d0edd165967de90d9218a7cd4f43e3 Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Tue, 25 Jul 2017 15:42:27 +0200 Subject: [PATCH] rpc/vhost_scsi: defer remove_scsi_dev RPC response Defer writing RPC response until after the device has been hotremoved. Change-Id: I052280b205415e4a2ffa1421653cc49b8e3b1445 Signed-off-by: Dariusz Stojaczyk Reviewed-on: https://review.gerrithub.io/371119 Reviewed-by: Pawel Wodkowski Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp --- include/spdk/vhost.h | 3 +- lib/vhost/vhost_rpc.c | 36 ++++++++++------ lib/vhost/vhost_scsi.c | 41 +++++++++++++++---- .../lib/vhost/vhost_scsi.c/vhost_scsi_ut.c | 6 +-- 4 files changed, 62 insertions(+), 24 deletions(-) diff --git a/include/spdk/vhost.h b/include/spdk/vhost.h index 74ca34b64a..69074386ea 100644 --- a/include/spdk/vhost.h +++ b/include/spdk/vhost.h @@ -73,7 +73,8 @@ int spdk_vhost_scsi_dev_remove(struct spdk_vhost_dev *vdev); struct spdk_scsi_dev *spdk_vhost_scsi_dev_get_dev(struct spdk_vhost_dev *ctrl, uint8_t num); int spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, const char *lun_name); -int spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num); +int spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, + spdk_vhost_event_fn cb_fn, void *cb_arg); int spdk_vhost_blk_construct(const char *name, const char *cpumask, const char *dev_name, bool readonly); diff --git a/lib/vhost/vhost_rpc.c b/lib/vhost/vhost_rpc.c index 5d6743fe55..69937c291e 100644 --- a/lib/vhost/vhost_rpc.c +++ b/lib/vhost/vhost_rpc.c @@ -307,23 +307,11 @@ static const struct spdk_json_object_decoder rpc_vhost_remove_dev[] = { }; static int -spdk_rpc_remove_vhost_scsi_dev_cb(struct spdk_vhost_dev *vdev, void *arg) +spdk_rpc_remove_vhost_scsi_dev_finish_cb(struct spdk_vhost_dev *vdev, void *arg) { struct rpc_remove_vhost_scsi_ctrlr_dev *rpc = arg; struct spdk_jsonrpc_request *request = rpc->request; struct spdk_json_write_ctx *w; - int rc; - char buf[64]; - - if (vdev == NULL) { - rc = -ENODEV; - goto invalid; - } - - rc = spdk_vhost_scsi_dev_remove_dev(vdev, rpc->scsi_dev_num); - if (rc < 0) { - goto invalid; - } free_rpc_remove_vhost_scsi_ctrlr_dev(rpc); @@ -335,6 +323,28 @@ spdk_rpc_remove_vhost_scsi_dev_cb(struct spdk_vhost_dev *vdev, void *arg) spdk_json_write_bool(w, true); spdk_jsonrpc_end_result(request, w); return 0; +} + +static int +spdk_rpc_remove_vhost_scsi_dev_cb(struct spdk_vhost_dev *vdev, void *arg) +{ + struct rpc_remove_vhost_scsi_ctrlr_dev *rpc = arg; + struct spdk_jsonrpc_request *request = rpc->request; + char buf[64]; + int rc; + + if (vdev == NULL) { + rc = -ENODEV; + goto invalid; + } + + rc = spdk_vhost_scsi_dev_remove_dev(vdev, rpc->scsi_dev_num, + spdk_rpc_remove_vhost_scsi_dev_finish_cb, rpc); + if (rc < 0) { + goto invalid; + } + + return 0; invalid: free_rpc_remove_vhost_scsi_ctrlr_dev(rpc); diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index 5c5406bf1c..52b3bebf71 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -66,10 +66,16 @@ #define VIRTIO_SCSI_EVENTQ 1 #define VIRTIO_SCSI_REQUESTQ 2 +struct spdk_scsi_dev_vhost_state { + bool removed; + spdk_vhost_event_fn remove_cb; + void *remove_ctx; +}; + struct spdk_vhost_scsi_dev { struct spdk_vhost_dev vdev; struct spdk_scsi_dev *scsi_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS]; - bool removed_dev[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS]; + struct spdk_scsi_dev_vhost_state scsi_dev_state[SPDK_VHOST_SCSI_CTRLR_MAX_DEVS]; struct spdk_ring *task_pool; struct spdk_poller *requestq_poller; @@ -142,15 +148,21 @@ static void process_removed_devs(struct spdk_vhost_scsi_dev *svdev) { struct spdk_scsi_dev *dev; + struct spdk_scsi_dev_vhost_state *state; int i; for (i = 0; i < SPDK_VHOST_SCSI_CTRLR_MAX_DEVS; ++i) { dev = svdev->scsi_dev[i]; + state = &svdev->scsi_dev_state[i]; - if (dev && svdev->removed_dev[i] && !spdk_scsi_dev_has_pending_tasks(dev)) { + if (dev && state->removed && !spdk_scsi_dev_has_pending_tasks(dev)) { spdk_scsi_dev_free_io_channels(dev); spdk_scsi_dev_destruct(dev); svdev->scsi_dev[i] = NULL; + if (state->remove_cb) { + state->remove_cb(&svdev->vdev, state->remove_ctx); + state->remove_cb = NULL; + } SPDK_NOTICELOG("%s: hot-detached device 'Dev %u'.\n", svdev->vdev.name, i); } } @@ -281,7 +293,7 @@ spdk_vhost_scsi_task_init_target(struct spdk_vhost_scsi_task *task, const __u8 * /* If dev has been hotdetached, return 0 to allow sending * additional hotremove event via sense codes. */ - return task->svdev->removed_dev[lun[1]] ? 0 : -1; + return task->svdev->scsi_dev_state[lun[1]].removed ? 0 : -1; } task->scsi.target_port = spdk_scsi_dev_find_port_by_id(task->scsi_dev, 0); @@ -741,7 +753,7 @@ 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; - svdev->removed_dev[scsi_dev_num] = false; + svdev->scsi_dev_state[scsi_dev_num].removed = 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); @@ -763,10 +775,13 @@ spdk_vhost_scsi_dev_add_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, } int -spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num) +spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_num, + spdk_vhost_event_fn cb_fn, void *cb_arg) { struct spdk_vhost_scsi_dev *svdev; struct spdk_scsi_dev *scsi_dev; + struct spdk_scsi_dev_vhost_state *scsi_dev_state; + int rc = 0; if (scsi_dev_num >= SPDK_VHOST_SCSI_CTRLR_MAX_DEVS) { SPDK_ERRLOG("%s: invalid device number %d\n", vdev->name, scsi_dev_num); @@ -788,8 +803,11 @@ spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_nu /* controller is not in use, remove dev and exit */ spdk_scsi_dev_destruct(scsi_dev); svdev->scsi_dev[scsi_dev_num] = NULL; + if (cb_fn) { + rc = cb_fn(vdev, cb_arg); + } SPDK_NOTICELOG("%s: removed device 'Dev %u'\n", vdev->name, scsi_dev_num); - return 0; + return rc; } if (!spdk_vhost_dev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { @@ -798,7 +816,16 @@ spdk_vhost_scsi_dev_remove_dev(struct spdk_vhost_dev *vdev, unsigned scsi_dev_nu return -ENOTSUP; } - svdev->removed_dev[scsi_dev_num] = true; + scsi_dev_state = &svdev->scsi_dev_state[scsi_dev_num]; + if (scsi_dev_state->removed) { + SPDK_WARNLOG("%s: 'Dev %u' has been already marked to hotremove.\n", svdev->vdev.name, + scsi_dev_num); + return -EBUSY; + } + + scsi_dev_state->remove_cb = cb_fn; + scsi_dev_state->remove_ctx = cb_arg; + scsi_dev_state->removed = true; eventq_enqueue(svdev, scsi_dev_num, 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); diff --git a/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c b/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c index 25f067ceeb..beebd1427c 100644 --- a/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c +++ b/test/unit/lib/vhost/vhost_scsi.c/vhost_scsi_ut.c @@ -282,11 +282,11 @@ vhost_scsi_dev_remove_dev_test(void) svdev->vdev.name = strdup("vhost.0"); /* Invalid device number */ - rc = spdk_vhost_scsi_dev_remove_dev(&svdev->vdev, SPDK_VHOST_SCSI_CTRLR_MAX_DEVS + 1); + rc = spdk_vhost_scsi_dev_remove_dev(&svdev->vdev, SPDK_VHOST_SCSI_CTRLR_MAX_DEVS + 1, NULL, NULL); CU_ASSERT(rc == -EINVAL); /* Try to remove nonexistent device */ - rc = spdk_vhost_scsi_dev_remove_dev(&svdev->vdev, 0); + rc = spdk_vhost_scsi_dev_remove_dev(&svdev->vdev, 0, NULL, NULL); CU_ASSERT(rc == -ENODEV); /* Try to remove device when controller is in use */ @@ -294,7 +294,7 @@ vhost_scsi_dev_remove_dev_test(void) scsi_dev = alloc_scsi_dev(); svdev->scsi_dev[0] = scsi_dev; MOCK_SET(spdk_vhost_dev_has_feature, bool, false); - rc = spdk_vhost_scsi_dev_remove_dev(&svdev->vdev, 0); + rc = spdk_vhost_scsi_dev_remove_dev(&svdev->vdev, 0, NULL, NULL); CU_ASSERT(rc == -ENOTSUP); free(scsi_dev); free(svdev->vdev.name);