All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Daniel Wagner <dwagner@suse.de>
Cc: Christoph Hellwig <hch@lst.de>, Keith Busch <kbusch@kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Subject: Re: libnvme API design
Date: Fri, 10 Oct 2025 09:26:58 +0200	[thread overview]
Message-ID: <20251010072658.GA17812@lst.de> (raw)
In-Reply-To: <9812534f-6a15-4126-a4c0-7b4ae39e327b@flourine.local>

On Thu, Oct 09, 2025 at 01:07:47PM +0200, Daniel Wagner wrote:
> Hi Christoph,
> 
> As you have suggested the command API should be inverted, that is the
> only stable API will be nvme_submit_admin_passthru and struct
> nvme_passthru_cmd and a bunch of helpers to initialize the cmd.
> 
> I've done updated a bunch of the identify commands and below is the
> result. I think we could reduce the number of helpers a bit,
> 
>   nvme_nvm_init_identify_ns vs nvme_nvm_init_identify_ns_csi.
> 
> Is this what you had in mind

Yes.

> (obviously I haven't gone the whole way to
> macros only, I like some type safty)?

What actual type safety do we actually gain here?

> static inline
> void nvme_nvm_init_identify_ns(struct nvme_passthru_cmd *cmd,
> 			       __u32 nsid, struct nvme_id_ns *ns)
> {
> 	nvme_nvm_init_identify(cmd,
> 			       NVME_IDENTIFY_CNS_NS,
> 			       ns, sizeof(*ns));
> 	cmd->nsid = nsid;

Given that identify includes nsid and just specified how it is
cleared to zero for the non-ns commands, maybe pass a nsid to
nvme_nvm_init_identify to simplify things?

> 	cmd->cdw11 |=  NVME_SET(nvmsetid, IDENTIFY_CDW11_CNSSPECID);

And NVME_SET still feels very oddly name..



  reply	other threads:[~2025-10-10  7:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-09 11:07 libnvme API design Daniel Wagner
2025-10-10  7:26 ` Christoph Hellwig [this message]
2025-10-10  8:44   ` Daniel Wagner
2025-10-13  6:47     ` Christoph Hellwig
2025-10-13 13:02       ` Daniel Wagner
2025-10-13 14:22         ` Daniel Wagner
2025-10-14  4:20           ` Christoph Hellwig
2025-10-14 15:29             ` Daniel Wagner
2025-10-15  4:11               ` Christoph Hellwig
2025-10-14  4:20         ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251010072658.GA17812@lst.de \
    --to=hch@lst.de \
    --cc=dwagner@suse.de \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.