Re: [PATCH] nvme: reject zoned namespaces whose zone info query failed

[Chris S]

On Fri, Aug 14, 2026 at 3:28 PM Keith Busch <kbusch@xxxxxxxxxx> wrote:
>
> On Fri, Aug 14, 2026 at 12:09:54PM -0400, Chao Shi wrote:
> > Before commit c85c9ab926a5 ("nvme: split nvme_update_zone_info") the
> > caller tested "if (ret)" and bailed out on any non-zero return.  Restore
> > that behaviour.
>
> I think the namespace is generally left up on purpose for controller
> reported errors so that we have a device handle for admin debugging
> purposes.
>
> Can you just skip the zone limits update when you have a bad response?
> Something like:
>
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2457,7 +2457,8 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
>                 capacity = 0;
>
>         if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&
> -           ns->head->ids.csi == NVME_CSI_ZNS)
> +           ns->head->ids.csi == NVME_CSI_ZNS &&
> +           zi.max_open_zones)
>                 nvme_update_zone_info(ns, &lim, &zi);
>
>         if ((ns->ctrl->vwc & NVME_CTRL_VWC_PRESENT) && !info->no_vwc)

Thanks -- keeping the namespace around is clearly the right, I
had not considered the admin handle.  v2 does that.

I did not use zi.max_open_zones as the condition though, because 0 is a
legal value there.  MOR and MAR are 0's based and FFFFFFFFh means "no
limit", so nvme_query_zone_info() does:

zi->max_open_zones = le32_to_cpu(id->mor) + 1;

which deliberately wraps FFFFFFFFh to 0, matching the block layer where
0 also means "no limit" -- blk_validate_zoned_limits() tests
"if (lim->max_active_zones && ...)" for exactly that reason.  So a
healthy device that reports no open resource limit has
zi.max_open_zones == 0, and gating on it would leave that device's
namespace unzoned.  QEMU hits this with its defaults: hw/nvme/ns.c has

id_ns_z->mor = cpu_to_le32(ns->params.max_open_zones - 1);

and zoned.max_open_zones defaults to 0.

zi.zone_size does not have that ambiguity.  Every path in
nvme_query_zone_info() that returns a positive status returns before
zone_size is assigned, and once it is assigned the only remaining
failure is -ENODEV, which the caller already handles.  A successful
query therefore always leaves it a non-zero power of two.  So v2 is:

if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&
   ns->head->ids.csi == NVME_CSI_ZNS && zi.zone_size)
nvme_update_zone_info(ns, &lim, &zi);

One open question in v2: on a first scan the namespace now comes up as
a non-zoned block device.  Setting capacity = 0 there as well would
match what this function already does for nvme_update_disk_info() and
nvme_init_integrity() failures, and would still keep the handle.  I
left it out to keep the change minimal -- happy to add it if you would
rather have it.

Sending v2 now.

Thanks,
Chao