On Tue, Jun 16, 2020 at 11:58:59AM -0400, Martin K. Petersen wrote:
> > @@ -1113,8 +1126,9 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
> > status = nvme_submit_sync_cmd(ctrl->admin_q, &c, data,
> > NVME_IDENTIFY_DATA_SIZE);
> > if (status) {
> > - dev_warn(ctrl->device,
> > - "Identify Descriptors failed (%d)\n", status);
> > + if (ctrl->vs >= NVME_VS(1, 3, 0))
> > + dev_warn(ctrl->device,
> > + "Identify Descriptors failed (%d)\n", status);
>
> Not a biggie but maybe this should be a separate patch?
Actually I think we can just get rid of this check before the warning.
We only call this function if the version is >= 1.3 or if multi-css was
selected. Both of those require this identification be supported.
> > @@ -1808,7 +1828,8 @@ static bool nvme_ns_ids_equal(struct
> > nvme_ns_ids *a, struct nvme_ns_ids *b) { return uuid_equal(&a->uuid,
> > &b->uuid) && memcmp(&a->nguid, &b->nguid, sizeof(a->nguid)) == 0 &&
> > - memcmp(&a->eui64, &b->eui64, sizeof(a->eui64)) == 0; +
> > memcmp(&a->eui64, &b->eui64, sizeof(a->eui64)) == 0 && +
> > a->csi == b->csi; }
>
> No objection to defensive programming. But wouldn't this be a broken
> device?
It could be a broken device, but I think it's checking against mistaken
identify, like if we're racing with namespace management commands
deleting and recreating namespaces that the driver is still bound to.
