Commit 3f12b5fa authored Oct 16, 2018 by Darek Stojaczyk Committed by Jim Harris Oct 23, 2018

vhost/blk: check against hotremoved bdev in GET_CONFIG handler



We always need to ensure bvdev->bdev is not NULL before
accessing it. GET_CONFIG handler was standing out in this
matter, causing segfaults when connecting to a vhost block
device that went through a bdev hotremove.

If the bdev has been hotremoved we'll now report
blocksize == 0 and blockcount == 0. From what I checked,
QEMU will still expose a virtio-blk device to the VM, but
linux (because that's what I checked) won't create a block
device for it.

Note: The behavior above can be tricky to investigate as
neither QEMU nor VM doesn't log any messages about it.

We should eventually reject any new connections to a vhost-blk
socket with hotremoved bdev, but that's a task for later.

Fixes #460

Change-Id: I266257f4b35d07f35ba03adf46cb18425f3cf39a
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/417934


Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

parent decb5957

lib/vhost/vhost_blk.c

+18 −2

Original line number	Diff line number	Diff line
		@@ -718,8 +718,24 @@ spdk_vhost_blk_get_config(struct spdk_vhost_dev vdev, uint8_t config,
		}

		bdev = bvdev->bdev;
		if (bdev == NULL) {
		/* We can't just return -1 here as this GET_CONFIG message might
		* be caused by a QEMU VM reboot. Returning -1 will indicate an
		* error to QEMU, who might then decide to terminate itself.
		* We don't want that. A simple reboot shouldn't break the system.
		*
		* Presenting a block device with block size 0 and block count 0
		* doesn't cause any problems on QEMU side and the virtio-pci
		* device is even still available inside the VM, but there will
		* be no block device created for it - the kernel drivers will
		* silently reject it.
		*/
		blk_size = 0;
		blkcnt = 0;
		} else {
		blk_size = spdk_bdev_get_block_size(bdev);
		blkcnt = spdk_bdev_get_num_blocks(bdev);
		}

		memset(blkcfg, 0, sizeof(*blkcfg));
		blkcfg->blk_size = blk_size;