Hi,
I want to know if I understand it correctly.
```
static void nvme_aio_err(NvmeRequest *req, int ret)
{
uint16_t status = NVME_SUCCESS;
Error *local_err = NULL;
switch (req->cmd.opcode) {
case NVME_CMD_READ:
case NVME_CMD_RESV_REPORT:
status = NVME_UNRECOVERED_READ;
break;
case NVME_CMD_FLUSH:
case NVME_CMD_WRITE:
case NVME_CMD_WRITE_ZEROES:
case NVME_CMD_ZONE_APPEND:
case NVME_CMD_COPY:
case NVME_CMD_RESV_REGISTER:
case NVME_CMD_RESV_ACQUIRE:
case NVME_CMD_RESV_RELEASE:
status = NVME_WRITE_FAULT;
break;
default:
status = NVME_INTERNAL_DEV_ERROR;
break;
}
trace_pci_nvme_err_aio(nvme_cid(req), strerror(-ret), status);
error_setg_errno(&local_err, -ret, "aio failed");
error_report_err(local_err);
/*
* Set the command status code to the first encountered error but
allow a
* subsequent Internal Device Error to trump it.
*/
if (req->status && status != NVME_INTERNAL_DEV_ERROR) {
return;
}
req->status = status;
}
```
In the above use case, if it is a pr-related command and the error code
is not supported, the invalid error code should be returned instead of
the Fault error code.
On 2024/8/28 14:51, Klaus Jensen wrote:
> On Jul 12 10:36, 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
>> Signed-off-by: zhenwei pi
>> Acked-by: Klaus Jensen
>> ---
>> hw/nvme/ctrl.c | 318 +++++++++++++++++++++++++++++++++++++++++++
>> hw/nvme/nvme.h | 4 +
>> include/block/nvme.h | 37 +++++
>> 3 files changed, 359 insertions(+)
>>
>
>> +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;
>> +
>> + switch (ptpl) {
>> + case NVME_RESV_PTPL_NO_CHANGE:
>> + aptpl = (ns->id_ns.rescap & NVME_PR_CAP_PTPL) ? true : false;
>> + break;
>> + case NVME_RESV_PTPL_DISABLE:
>> + aptpl = false;
>> + break;
>> + case NVME_RESV_PTPL_ENABLE:
>> + aptpl = true;
>> + break;
>> + default:
>> + return NVME_INVALID_FIELD;
>> + }
>> +
>> + ret = nvme_h2c(n, (uint8_t *)&key_info, sizeof(NvmeKeyInfo), req);
>> + if (ret) {
>> + return ret;
>> + }
>> +
>> + switch (action) {
>> + case NVME_RESV_REGISTER_ACTION_REGISTER:
>> + req->aiocb = blk_aio_pr_register(ns->blkconf.blk, 0,
>> + key_info.nr_key, 0, aptpl,
>> + ignore_key, nvme_misc_cb,
>> + req);
>> + break;
>> + case NVME_RESV_REGISTER_ACTION_UNREGISTER:
>> + req->aiocb = blk_aio_pr_register(ns->blkconf.blk, key_info.cr_key, 0,
>> + 0, aptpl, ignore_key,
>> + nvme_misc_cb, req);
>> + break;
>> + case NVME_RESV_REGISTER_ACTION_REPLACE:
>> + req->aiocb = blk_aio_pr_register(ns->blkconf.blk, key_info.cr_key,
>> + key_info.nr_key, 0, aptpl, ignore_key,
>> + nvme_misc_cb, req);
>> + break;
>
> There should be some check on rescap I think. On a setup without
> reservation support from the block layer, these functions ends up
> returning ENOTSUP which causes hw/nvme to end up returning a Write Fault
> (which is a little wonky).
>
> Should they return invalid field, invalid opcode?
