[PATCH 6/6] nvme: Add consistency check for zone count

[Javier González]

On 26.06.2020 00:04, Damien Le Moal wrote:
>On 2020/06/26 6:49, Keith Busch wrote:
>> On Thu, Jun 25, 2020 at 02:21:52PM +0200, Javier González wrote:
>>>  drivers/nvme/host/zns.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
>>> index 7d8381fe7665..de806788a184 100644
>>> --- a/drivers/nvme/host/zns.c
>>> +++ b/drivers/nvme/host/zns.c
>>> @@ -234,6 +234,13 @@ static int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
>>>  		sector += ns->zsze * nz;
>>>  	}
>>>
>>> +	if (nr_zones < 0 && zone_idx != ns->nr_zones) {
>>> +		dev_err(ns->ctrl->device, "inconsistent zone count %u/%u\n",
>>> +				zone_idx, ns->nr_zones);
>>> +		ret = -EINVAL;
>>> +		goto out_free;
>>> +	}
>>> +
>>>  	ret = zone_idx;
>>
>> nr_zones is unsigned, so it's never < 0.
>>
>> The API we're providing doesn't require zone_idx equal the namespace's
>> nr_zones at the end, though. A subset of the total number of zones can
>> be requested here.
>>

I did see nr_zones coming with -1; guess it is my compiler.

>
>Yes, absolutely. zone_idx is not an absolute zone number. It is the index of the
>reported zone descriptor in the current report range requested by the user,
>which is not necessarily for the entire drive (i.e., provided nr zones is less
>than the total number of zones of the disk and/or start sector is > 0). So
>zone_idx indicates the actual number of zones reported, it is not the total

I see. As I can see, when nr_zones comes undefined I believed we could
assume that zone_idx is absolute, but I can be wrong.

Does it make sense to support this check with an additional counter and
a explicit nr_zones initialization when undefined or you
prefer to just remove it as Matias suggested?

Javier