Re: [PATCH 1/2] hw/nvme: fix namespace attachment

["Jesper Devantier" <foss@defmacro.it>]

On Tue Jun 3, 2025 at 2:59 PM CEST, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Commit 6ccca4b6bb9f ("hw/nvme: rework csi handling") introduced a bug in
> Namespace Attachment, causing it to
>
>   a) not allow a controller to attach namespaces to other controllers
>   b) assert if a valid non-attached namespace is detached
>
> This fixes both issues.
>
> Fixes: 6ccca4b6bb9f ("hw/nvme: rework csi handling")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2976
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/nvme/ctrl.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index fd935507bc0280c1f49822f9e3cb035df596ae47..8de900ef8aca9b510b072892f9f82c01acee4f7d 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -6816,7 +6816,7 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
>  
>          switch (sel) {
>          case NVME_NS_ATTACHMENT_ATTACH:
> -            if (nvme_ns(n, nsid)) {
> +            if (nvme_ns(ctrl, nsid)) {
>                  return NVME_NS_ALREADY_ATTACHED | NVME_DNR;
>              }
>  
> @@ -6824,7 +6824,7 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
>                  return NVME_NS_PRIVATE | NVME_DNR;
>              }
>  
> -            if (!nvme_csi_supported(n, ns->csi)) {
> +            if (!nvme_csi_supported(ctrl, ns->csi)) {
>                  return NVME_IOCS_NOT_SUPPORTED | NVME_DNR;
>              }
>  
> @@ -6834,6 +6834,10 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
>              break;
>  
>          case NVME_NS_ATTACHMENT_DETACH:
> +            if (!nvme_ns(ctrl, nsid)) {
> +                return NVME_NS_NOT_ATTACHED | NVME_DNR;
> +            }
> +
>              nvme_detach_ns(ctrl, ns);
>              nvme_update_dsm_limits(ctrl, NULL);
>  

Applied, built, read, and it seems in line with the specification :)

Reviewed-by: Jesper Wendel Devantier <foss@defmacro.it>