[PATCH v5 01/13] hw/block/nvme: fix zone management receive reporting too many zones

Klaus Jensen posted 13 patches 4 years, 11 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Klaus Jensen <its@irrelevant.dk>, Keith Busch <kbusch@kernel.org>, Fam Zheng <fam@euphon.net>, Max Reitz <mreitz@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>
[PATCH v5 01/13] hw/block/nvme: fix zone management receive reporting too many zones
Posted by Klaus Jensen 4 years, 11 months ago
From: Klaus Jensen <k.jensen@samsung.com>

nvme_zone_mgmt_recv uses nvme_ns_nlbas() to get the number of LBAs in
the namespace and then calculates the number of zones to report by
incrementing slba with ZSZE until exceeding the number of LBAs as
returned by nvme_ns_nlbas().

This is bad because the namespace might be of such as size that some
LBAs are valid, but are not part of any zone, causing zone management
receive to report one additional (but non-existing) zone.

Fix this with a conventional loop on i < ns->num_zones instead.

Fixes: a479335bfaf3 ("hw/block/nvme: Support Zoned Namespace Command Set")
Cc: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d439e44db839..c7b9a1663dd7 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2619,12 +2619,13 @@ static uint16_t nvme_zone_mgmt_recv(NvmeCtrl *n, NvmeRequest *req)
     uint32_t zone_idx, zra, zrasf, partial;
     uint64_t max_zones, nr_zones = 0;
     uint16_t status;
-    uint64_t slba, capacity = nvme_ns_nlbas(ns);
+    uint64_t slba;
     NvmeZoneDescr *z;
     NvmeZone *zone;
     NvmeZoneReportHeader *header;
     void *buf, *buf_p;
     size_t zone_entry_sz;
+    int i;
 
     req->status = NVME_SUCCESS;
 
@@ -2666,7 +2667,7 @@ static uint16_t nvme_zone_mgmt_recv(NvmeCtrl *n, NvmeRequest *req)
     buf = g_malloc0(data_size);
 
     zone = &ns->zone_array[zone_idx];
-    for (; slba < capacity; slba += ns->zone_size) {
+    for (i = zone_idx; i < ns->num_zones; i++) {
         if (partial && nr_zones >= max_zones) {
             break;
         }
-- 
2.30.1


RE: [PATCH v5 01/13] hw/block/nvme: fix zone management receive reporting too many zones
Posted by Dmitry Fomichev 4 years, 11 months ago
LGTM,
Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

> -----Original Message-----
> From: Klaus Jensen <its@irrelevant.dk>
> Sent: Wednesday, March 10, 2021 4:54 AM
> To: qemu-devel@nongnu.org
> Cc: Stefan Hajnoczi <stefanha@redhat.com>; Klaus Jensen
> <its@irrelevant.dk>; Fam Zheng <fam@euphon.net>; Max Reitz
> <mreitz@redhat.com>; Kevin Wolf <kwolf@redhat.com>; qemu-
> block@nongnu.org; Gollu Appalanaidu <anaidu.gollu@samsung.com>; Keith
> Busch <kbusch@kernel.org>; Klaus Jensen <k.jensen@samsung.com>;
> Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
> Subject: [PATCH v5 01/13] hw/block/nvme: fix zone management receive
> reporting too many zones
> 
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> nvme_zone_mgmt_recv uses nvme_ns_nlbas() to get the number of LBAs in
> the namespace and then calculates the number of zones to report by
> incrementing slba with ZSZE until exceeding the number of LBAs as
> returned by nvme_ns_nlbas().
> 
> This is bad because the namespace might be of such as size that some
> LBAs are valid, but are not part of any zone, causing zone management
> receive to report one additional (but non-existing) zone.
> 
> Fix this with a conventional loop on i < ns->num_zones instead.
> 
> Fixes: a479335bfaf3 ("hw/block/nvme: Support Zoned Namespace Command
> Set")
> Cc: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/block/nvme.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index d439e44db839..c7b9a1663dd7 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -2619,12 +2619,13 @@ static uint16_t
> nvme_zone_mgmt_recv(NvmeCtrl *n, NvmeRequest *req)
>      uint32_t zone_idx, zra, zrasf, partial;
>      uint64_t max_zones, nr_zones = 0;
>      uint16_t status;
> -    uint64_t slba, capacity = nvme_ns_nlbas(ns);
> +    uint64_t slba;
>      NvmeZoneDescr *z;
>      NvmeZone *zone;
>      NvmeZoneReportHeader *header;
>      void *buf, *buf_p;
>      size_t zone_entry_sz;
> +    int i;
> 
>      req->status = NVME_SUCCESS;
> 
> @@ -2666,7 +2667,7 @@ static uint16_t nvme_zone_mgmt_recv(NvmeCtrl
> *n, NvmeRequest *req)
>      buf = g_malloc0(data_size);
> 
>      zone = &ns->zone_array[zone_idx];
> -    for (; slba < capacity; slba += ns->zone_size) {
> +    for (i = zone_idx; i < ns->num_zones; i++) {
>          if (partial && nr_zones >= max_zones) {
>              break;
>          }
> --
> 2.30.1


Re: [PATCH v5 01/13] hw/block/nvme: fix zone management receive reporting too many zones
Posted by Klaus Jensen 4 years, 11 months ago
On Mar 14 19:31, Dmitry Fomichev wrote:
> LGTM,
> Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> 

Thanks Dmitry!

If you could give [10/13], [11/13] and [12/13] a look-see as well that
would be awesome :)


-
k