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

[Klaus Jensen]

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 <luchangqi.123@bytedance.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> Acked-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/nvme/ctrl.c       | 318 +++++++++++++++++++++++++++++++++++++++++++
>  hw/nvme/nvme.h       |   4 +
>  include/block/nvme.h |  37 +++++
>  3 files changed, 359 insertions(+)
> 

This looks good to me, but two comments.

  1. Do you have a small guide on how to get a simple test environment
     up for this?

  2. Can you touch on the justification for not supporting the remaining
     mandatory features required when indicating Reservation support?

     hw/nvme *does* compromise on mandatory features from time to time
     when it makes sense, so I'm not saying that not having the log
     pages, notifications and so on is a deal breaker, I'm just
     interested in the justification and/or motivation.

     For instance, I think the SPDK reserve test will fail on this due
     to lack of Host Identifier Feature support.

signature.asc
Description: PGP signature