Re: [PATCH v10 09/10] hw/nvme: add reservation protocal command

[Klaus Jensen]

On Aug 29 16:42, Changqi Lu wrote:
> Add reservation acquire, reservation register,
> reservation release and reservation report commands
> in the nvme device layer.
> 
> By introducing these commands, this enables the nvme
> device to perform reservation-related tasks, including
> querying keys, querying reservation status, registering
> reservation keys, initiating and releasing reservations,
> as well as clearing and preempting reservations held by
> other keys.
> 
> These commands are crucial for management and control of
> shared storage resources in a persistent manner.
> Signed-off-by: Changqi Lu <luchangqi.123@bytedance.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> Acked-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/nvme/ctrl.c       | 330 +++++++++++++++++++++++++++++++++++++++++++
>  hw/nvme/nvme.h       |   4 +
>  include/block/nvme.h |  37 +++++
>  3 files changed, 371 insertions(+)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index ad212de723..1f8a5659c9 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -1747,6 +1755,13 @@ static void nvme_aio_err(NvmeRequest *req, int ret)
>      case NVME_CMD_READ:
>          status = NVME_UNRECOVERED_READ;
>          break;
> +    case NVME_CMD_RESV_REPORT:
> +        if (ret == -ENOTSUP) {
> +            status = NVME_INVALID_OPCODE;
> +        } else {
> +            status = NVME_UNRECOVERED_READ;
> +        }
> +        break;
>      case NVME_CMD_FLUSH:
>      case NVME_CMD_WRITE:
>      case NVME_CMD_WRITE_ZEROES:
> @@ -1754,6 +1769,15 @@ static void nvme_aio_err(NvmeRequest *req, int ret)
>      case NVME_CMD_COPY:
>          status = NVME_WRITE_FAULT;
>          break;
> +    case NVME_CMD_RESV_REGISTER:
> +    case NVME_CMD_RESV_ACQUIRE:
> +    case NVME_CMD_RESV_RELEASE:
> +        if (ret == -ENOTSUP) {
> +            status = NVME_INVALID_OPCODE;
> +        } else {
> +            status = NVME_WRITE_FAULT;
> +        }
> +        break;
>      default:
>          status = NVME_INTERNAL_DEV_ERROR;
>          break;

This wasn't exactly what I had in mind ;) See below.

> @@ -2692,6 +2716,304 @@ static uint16_t nvme_verify(NvmeCtrl *n, NvmeRequest 
> *req)
>      return NVME_NO_COMPLETE;
>  }
>  
> +typedef struct NvmeKeyInfo {
> +    uint64_t cr_key;
> +    uint64_t nr_key;
> +} NvmeKeyInfo;
> +
> +static uint16_t nvme_resv_register(NvmeCtrl *n, NvmeRequest *req)
> +{
> +    int ret;
> +    NvmeKeyInfo key_info;
> +    NvmeNamespace *ns = req->ns;
> +    uint32_t cdw10 = le32_to_cpu(req->cmd.cdw10);
> +    bool ignore_key = cdw10 >> 3 & 0x1;
> +    uint8_t action = cdw10 & 0x7;
> +    uint8_t ptpl = cdw10 >> 30 & 0x3;
> +    bool aptpl;
> +

Here, check ns->id_ns.rescap for reservations support and return
NVME_INVALID_OPCODE. There is no need to call the block layer otherwise.

Same for the below commands.