Commit e051a536 authored by Darek Stojaczyk's avatar Darek Stojaczyk Committed by Jim Harris
Browse files

vhost: ignore sessions that weren't ever started 



The previous patches described as optimizations also
fixed some issues. They seem sufficient to cover all
the error cases, but the real source of the problem
lies in foreach_session() initiated by the device backend,
which can use sessions that were never seen by the
backend.

The backends are only notified when a session is
*started*, but foreach_session() iterates through
all the sessions - even those that were never started.
Vhost SCSI, for example, in the foreach_session() callbacks
used to expect svsession->svdev to be always set, but
that field is only set when the session gets started.

A perfect solution would to introduce a new backend
callback to be called on new connection. Vhost SCSI
could set e.g. svsession->svdev inside. For now we go
with much easier solution that prevents sessions from
being used in foreach-session() unless they were
started at least once. (...and e.g. got their ->svdev set)

Change-Id: Ida30a1f27f99977360d08a71a64fc92931b25b75
Signed-off-by: default avatarDarek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/449394


Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent bf77d4b7

lib/vhost/vhost.c

+9 −4
Original line number Diff line number Diff line
@@ -943,7 +943,7 @@ spdk_vhost_event_async_foreach_fn(void *arg1, void *arg2) 
	}



	vsession = spdk_vhost_session_find_by_id(vdev, ctx->vsession_id);

	if (vsession == NULL) {

	if (vsession == NULL || !vsession->initialized) {

		/* The session must have been removed in the meantime, so we

		 * just skip it in our foreach chain

		 */
@@ -1183,6 +1183,7 @@ start_device(int vid) 


	spdk_vhost_session_set_coalescing(vdev, vsession, NULL);

	spdk_vhost_session_mem_register(vsession);

	vsession->initialized = true;

	rc = vdev->backend->start_session(vsession);

	if (rc != 0) {

		spdk_vhost_session_mem_unregister(vsession);
@@ -1346,6 +1347,7 @@ new_connection(int vid) 
	vsession->id = vdev->vsessions_num++;

	vsession->vid = vid;

	vsession->lcore = -1;

	vsession->initialized = false;

	vsession->next_stats_check_time = 0;

	vsession->stats_check_interval = SPDK_VHOST_STATS_CHECK_INTERVAL_MS *

					 spdk_get_ticks_hz() / 1000UL;
@@ -1390,10 +1392,13 @@ spdk_vhost_external_event_foreach_continue(struct spdk_vhost_dev *vdev, 
	}



	while (vsession->lcore == -1) {

		if (vsession->initialized) {

			rc = fn(vdev, vsession, arg);

			if (rc < 0) {

				return;

			}

		}



		vsession = spdk_vhost_session_next(vdev, vsession->id);

		if (vsession == NULL) {

			goto out_finish_foreach;

lib/vhost/vhost_internal.h

+1 −0
Original line number Diff line number Diff line
@@ -126,6 +126,7 @@ struct spdk_vhost_session { 


	int32_t lcore;



	bool initialized;

	bool needs_restart;

	bool forced_polling;