Re: [PATCH v2] nvmet: fix Reservation Register Replace for unregistered host with IEKEY

[Guixin Liu <kanie@linux.alibaba.com>]

Hi Zhengrong:

在 2026/7/28 16:26, Zhengrong Li 写道:
> When a host sends a Reservation Register command with RREGA=Replace
> and IEKEY=1 without being previously registered, nvmet returns
> Reservation Conflict.  SPDK accepts this combination and creates a
> new registrant with the provided NRKEY, with an explicit unit test
> covering this scenario (test/unit/lib/nvmf/subsystem.c).
>
> Fix nvmet_pr_replace() to add a new registrant when the host is not
> found in the registrant list and IEKEY is set with a non-zero NRKEY,
> consistent with SPDK's behavior.
Just quote the NVMe spec rather than mentioning SPDK.
>
> Tested with nvme-cli against nvmet-tcp:
>
>    # no prior registration
>    nvme resv-register /dev/nvmeXn1 -n 1 --rrega=2 --iekey --nrkey=0x9999
>
>    Before: RESERVATION_CONFLICT (0x4083)
>    After:  success, registrant created with rkey 0x9999
Here need add: Fixes: 5a47c2080a73 ("nvmet: support reservation feature").
> Signed-off-by: Zhengrong Li <zhengrong_li@linux.alibaba.com>
> ---
> Changes since v1:
>   - Use kzalloc_obj() instead of kmalloc_obj() + memset() (Maurizio).
> ---
>   drivers/nvme/target/pr.c | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/target/pr.c b/drivers/nvme/target/pr.c
> index c71ae46244ff..944e3d3947d0 100644
> --- a/drivers/nvme/target/pr.c
> +++ b/drivers/nvme/target/pr.c
> @@ -355,12 +355,14 @@ static u16 nvmet_pr_replace(struct nvmet_req *req,
>   	u16 status = NVME_SC_RESERVATION_CONFLICT | NVME_STATUS_DNR;
>   	struct nvmet_ctrl *ctrl = req->sq->ctrl;
>   	struct nvmet_pr *pr = &req->ns->pr;
> -	struct nvmet_pr_registrant *reg;
> +	struct nvmet_pr_registrant *reg, *new;
>   	u64 nrkey = le64_to_cpu(d->nrkey);
> +	bool found = false;
>   
>   	down(&pr->pr_sem);
>   	list_for_each_entry_rcu(reg, &pr->registrant_list, entry) {
>   		if (uuid_equal(&reg->hostid, &ctrl->hostid)) {
> +			found = true;
>   			if (ignore_key || reg->rkey == le64_to_cpu(d->crkey))
>   				status = nvmet_pr_update_reg_attr(pr, reg,
>   						nvmet_pr_update_reg_rkey,
> @@ -368,6 +370,21 @@ static u16 nvmet_pr_replace(struct nvmet_req *req,
>   			break;
>   		}
>   	}
> +
> +	if (!found && ignore_key && nrkey) {
> +		new = kzalloc_obj(*new);
Nit: you can alloc new before down(&pr->pr_sem).
> +		if (!new) {
> +			status = NVME_SC_INTERNAL;
> +			goto out;
> +		}
> +		INIT_LIST_HEAD(&new->entry);
> +		new->rkey = nrkey;
> +		uuid_copy(&new->hostid, &ctrl->hostid);
> +		list_add_tail_rcu(&new->entry, &pr->registrant_list);
> +		status = NVME_SC_SUCCESS;
> +	}
> +
If the nrkey==0, should return NVME_SC_INVALID_FIELD | NVME_STATUS_DNR 
instead of

NVME_SC_RESERVATION_CONFLICT | NVME_STATUS_DNR;


> +out:
>   	up(&pr->pr_sem);
>   	return status;
>   }
Others looks good, thanks.

Best Regards,
Guixin Liu