> -----Original Message----- > From: Klaus Jensen <[email protected]> > Sent: Wednesday, October 21, 2020 6:26 AM > To: Dmitry Fomichev <[email protected]> > Cc: Keith Busch <[email protected]>; Klaus Jensen > <[email protected]>; Kevin Wolf <[email protected]>; Philippe > Mathieu-Daudé <[email protected]>; Maxim Levitsky > <[email protected]>; Fam Zheng <[email protected]>; Niklas Cassel > <[email protected]>; Damien Le Moal <[email protected]>; > [email protected]; [email protected]; Alistair Francis > <[email protected]>; Matias Bjorling <[email protected]> > Subject: Re: [PATCH v7 05/11] hw/block/nvme: Support Zoned Namespace > Command Set > > On Oct 19 11:17, Dmitry Fomichev wrote: > > +/* > > + * Close or finish all the zones that are currently open. > > + */ > > +static void nvme_zoned_clear_ns(NvmeNamespace *ns) > > +{ > > + NvmeZone *zone; > > + uint32_t set_state; > > + int i; > > + > > + zone = ns->zone_array; > > + for (i = 0; i < ns->num_zones; i++, zone++) { > > + switch (nvme_get_zone_state(zone)) { > > + case NVME_ZONE_STATE_IMPLICITLY_OPEN: > > + QTAILQ_REMOVE(&ns->imp_open_zones, zone, entry); > > + break; > > + case NVME_ZONE_STATE_EXPLICITLY_OPEN: > > + QTAILQ_REMOVE(&ns->exp_open_zones, zone, entry); > > + break; > > + case NVME_ZONE_STATE_CLOSED: > > + /* fall through */ > > + default: > > + continue; > > + } > > + > > + if (zone->d.wp == zone->d.zslba) { > > + set_state = NVME_ZONE_STATE_EMPTY; > > + } else { > > + set_state = NVME_ZONE_STATE_CLOSED; > > + } > > + > > + switch (set_state) { > > + case NVME_ZONE_STATE_CLOSED: > > + trace_pci_nvme_clear_ns_close(nvme_get_zone_state(zone), > > + zone->d.zslba); > > + QTAILQ_INSERT_TAIL(&ns->closed_zones, zone, entry); > > + break; > > + case NVME_ZONE_STATE_EMPTY: > > + trace_pci_nvme_clear_ns_reset(nvme_get_zone_state(zone), > > + zone->d.zslba); > > + break; > > + case NVME_ZONE_STATE_FULL: > > + trace_pci_nvme_clear_ns_full(nvme_get_zone_state(zone), > > + zone->d.zslba); > > + zone->d.wp = nvme_zone_wr_boundary(zone); > > + QTAILQ_INSERT_TAIL(&ns->full_zones, zone, entry); > > + } > > No need for the switch here - just add to the closed list in the > conditional.
The switch becomes handy later in the series, particularly after adding descriptor extensions. For easier reviewing, it makes sense to add it from the beginning even though it is rudimentary at this point. > > The NVME_ZONE_STATE_FULL case is unreachable. Indeed. This should be introduced in the next patch. Now, I've looked at this code again and the active/open counting in this function ends up to be not quite right, I am fixing it. > > > + > > + zone->w_ptr = zone->d.wp; > > + nvme_set_zone_state(zone, set_state); > > + } > > +}
