On Thu, 30 Oct 2025 at 07:30, Klaus Jensen <[email protected]> wrote:
>
> From: Alan Adamson <[email protected]>
>
> Add support for the namespace atomic paramters: NAWUN and NAWUN. Namespace
> Atomic Compare and Write Unit (NACWU) is not currently supported.
>
> Writes that adhere to the NACWU and NAWUPF parameters are guaranteed to be
> atomic.
>
> New NVMe QEMU Paramters (See NVMe Specification for details):
> atomic.nawun=UINT16 (default: 0)
> atomic.nawupf=UINT16 (default: 0)
> atomic.nsfeat (default off) - Set Namespace Supported Atomic Boundary
> &
> Power (NSABP) bit in Namespace Features (NSFEAT) in the
> Identify
> Namespace Data Structure
>
> See the NVMe Specification for more information.
>
> Signed-off-by: Alan Adamson <[email protected]>
> Reviewed-by: Jesper Wendel Devantier <[email protected]>
> Signed-off-by: Klaus Jensen <[email protected]>
> + /* Set atomic write parameters */
> + if (ns->params.atomic_nsfeat) {
> + id_ns->nsfeat |= NVME_ID_NS_NSFEAT_NSABPNS;
> + id_ns->nawun = cpu_to_le16(ns->params.atomic_nawun);
> + if (!id->awupf || (id_ns->nawun && (id_ns->nawun < id->awun))) {
> + error_report("Invalid NAWUN: %x AWUN=%x", id_ns->nawun,
> id->awun);
> + }
This error check is about NAWUN, but the condition looks at id->awpuf.
Is that intentional ? (Coverity wonders if this is a copy-paste error:
CID 1642811.)
We should return early if we've detected an error case in the properties.
By default we'll fall on through. Similarly below.
This is a realize method, so error handling should be by
setting the 'error' argument, not by error_report().
> + id_ns->nawupf = cpu_to_le16(ns->params.atomic_nawupf);
> + if (!id->awupf || (id_ns->nawupf && (id_ns->nawupf < id->awupf))) {
> + error_report("Invalid NAWUPF: %x AWUPF=%x",
> + id_ns->nawupf, id->awupf);
> + }
> + if (id_ns->nawupf > id_ns->nawun) {
> + error_report("Invalid: NAWUN=%x NAWUPF=%x",
> + id_ns->nawun, id_ns->nawupf);
> + }
Personally I find this stack of checks a bit confusing -- we
are presumably catching various different invalid combinations
of the properties, but the error messages we produce are rather
unspecific. If it's the case that (for instance) the NAWUPF
cannot be larger than the NAWUN, we could tell the user that
specifically rather than just saying "Invalid" and making them
go look up what the requirements are in the spec or the code.
> + }
thanks
-- PMM
