From 6159c8c69be16f01a9cde1e3de970fe0612da8db Mon Sep 17 00:00:00 2001 From: Evgeniy Kochetov Date: Wed, 30 Jun 2021 18:51:58 +0300 Subject: [PATCH] bdev: Fail nvme passthru command if not supported by bdev The originally detected problem is that SPDK NVMf target fails command with invalid opcode with status code INTERNAL_DEVICE_ERROR instead of INVALID_OPCODE. All unknown commands on IO queue are passed to underlying block device layer as NVME_IO type. It is not checked if this type of commands is supported and, when command fails, INTERNAL_DEVICE_ERROR is set as status code. If command fails on submission, status code is set to INVALID_OPCODE which is more relevant. This patch adds check if command type is supported to bdev_nvme_*_passthru functions. If not supported, it is failed with ENOTSUP. Signed-off-by: Evgeniy Kochetov Change-Id: I4d7f7639da17dd3b1dc3eee7eb1b4a4f876117a2 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8567 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Reviewed-by: Paul Luse --- lib/bdev/bdev.c | 12 ++++++++++++ test/unit/lib/bdev/bdev.c/bdev_ut.c | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 4f9db0025..651543515 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -4927,6 +4927,10 @@ spdk_bdev_nvme_admin_passthru(struct spdk_bdev_desc *desc, struct spdk_io_channe return -EBADF; } + if (spdk_unlikely(!bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_NVME_ADMIN))) { + return -ENOTSUP; + } + bdev_io = bdev_channel_get_io(channel); if (!bdev_io) { return -ENOMEM; @@ -4965,6 +4969,10 @@ spdk_bdev_nvme_io_passthru(struct spdk_bdev_desc *desc, struct spdk_io_channel * return -EBADF; } + if (spdk_unlikely(!bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_NVME_IO))) { + return -ENOTSUP; + } + bdev_io = bdev_channel_get_io(channel); if (!bdev_io) { return -ENOMEM; @@ -5003,6 +5011,10 @@ spdk_bdev_nvme_io_passthru_md(struct spdk_bdev_desc *desc, struct spdk_io_channe return -EBADF; } + if (spdk_unlikely(!bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_NVME_IO_MD))) { + return -ENOTSUP; + } + bdev_io = bdev_channel_get_io(channel); if (!bdev_io) { return -ENOMEM; diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index bd6602f9b..7a5f7b886 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -967,6 +967,20 @@ bdev_io_types_test(void) ut_enable_io_type(SPDK_BDEV_IO_TYPE_WRITE_ZEROES, true); ut_enable_io_type(SPDK_BDEV_IO_TYPE_WRITE, true); + /* NVME_IO, NVME_IO_MD and NVME_ADMIN are not supported */ + ut_enable_io_type(SPDK_BDEV_IO_TYPE_NVME_IO, false); + ut_enable_io_type(SPDK_BDEV_IO_TYPE_NVME_IO_MD, false); + ut_enable_io_type(SPDK_BDEV_IO_TYPE_NVME_ADMIN, false); + rc = spdk_bdev_nvme_io_passthru(desc, io_ch, NULL, NULL, 0, NULL, NULL); + CU_ASSERT(rc == -ENOTSUP); + rc = spdk_bdev_nvme_io_passthru_md(desc, io_ch, NULL, NULL, 0, NULL, 0, NULL, NULL); + CU_ASSERT(rc == -ENOTSUP); + rc = spdk_bdev_nvme_admin_passthru(desc, io_ch, NULL, NULL, 0, NULL, NULL); + CU_ASSERT(rc == -ENOTSUP); + ut_enable_io_type(SPDK_BDEV_IO_TYPE_NVME_IO, true); + ut_enable_io_type(SPDK_BDEV_IO_TYPE_NVME_IO_MD, true); + ut_enable_io_type(SPDK_BDEV_IO_TYPE_NVME_ADMIN, true); + spdk_put_io_channel(io_ch); spdk_bdev_close(desc); free_bdev(bdev);