From bf77d4b774ccfae15282dc0d0bec52eeb5539a82 Mon Sep 17 00:00:00 2001 From: Darek Stojaczyk Date: Wed, 3 Apr 2019 07:44:58 +0200 Subject: [PATCH] vhost/scsi: fix newly attached targets being hotremoved by mistake Before SCSI target is removed, all vhost sessions need to drain their pending I/O and put their I/O channels. After a session puts it channel, it sends an async notification to the entire vhost device. The device will check if there are any other sessions still referencing the SCSI target and if not - it will continue removing the spdk_scsi_dev object. There may be multiple sessions sending those async events at the same time, and while we do protect from removing the same spdk_scsi_dev twice, we can still remove a different spdk_scsi_dev that was hot-attached in the meantime with the same target ID. 1. SCSI target hotremove (e.g. via RPC or bdev hotremove) / \ / \ session A session B drain I/O drain I/O | | v | done v send event done \ send event* \ All sessions have detached the SCSI target, remove it from the entire vhost device. From this point a new target can be hot-attached (e.g. via RPC). 2. Attach a SCSI target with with same target ID. 3. Hotremove event* from the previous SCSI target gets finally executed. SCSI target with that ID is occupied (again) and may be hotremoved by mistake. The role of that hotremove event is just to kick the vhost device and make it remove any scsi targets that can be removed, so add a check preventing it from removing devices in states other than REMOVING. Change-Id: Ia1cc7cae797fd8859d485e63f0ef37aeac2945d0 Signed-off-by: Darek Stojaczyk Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/449990 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Changpeng Liu --- lib/vhost/vhost_scsi.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index f8e24e45c..fe28d8137 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -184,11 +184,6 @@ remove_scsi_tgt(struct spdk_vhost_scsi_dev *svdev, struct spdk_scsi_dev *dev; state = &svdev->scsi_dev_state[scsi_tgt_num]; - if (state->dev == NULL) { - /* we've been already removed in the meantime */ - return 0; - } - dev = state->dev; state->dev = NULL; assert(state->status == VHOST_SCSI_DEV_REMOVING); @@ -216,6 +211,11 @@ spdk_vhost_scsi_session_process_removed(struct spdk_vhost_dev *vdev, struct spdk_vhost_scsi_dev *svdev = SPDK_CONTAINEROF(vdev, struct spdk_vhost_scsi_dev, vdev); + if (svdev->scsi_dev_state[scsi_tgt_num].status != VHOST_SCSI_DEV_REMOVING) { + /* device was already removed in the meantime */ + return 0; + } + return remove_scsi_tgt(svdev, scsi_tgt_num); }