[PATCH] NVMe: Do not save result for range type feature

[Keith Busch]

On Wed, 3 Apr 2013, Matthew Wilcox wrote:

> On Tue, Apr 02, 2013 at 02:31:35PM -0600, Keith Busch wrote:
>> The LBA range type feature being optional, we do not want to save
>> the result or propagate it up the call stack when a device does not
>> support it.
>
> Looking at the whole function, the description of what it returns
> is rather odd.  "If nvme_setup_io_queues fails, returns that error.
> If nvme_identify for the controller fails, returns -EIO.  Otherwise,
> return an error if nvme_identify for the last namespace fails or if
> nvme_get_features for the last namespace fails."
>
> So we always discard errors for not-the-last namespace, but return an
> error if identify or get_features for the last namespace gets an error.
> Worse, on returning an error, the caller will free the queues and all the
> controller structures, even though the namespaces have been registered as
> devices!  I really didn't think through the error path here, did I?  :-)
>
> So, I would propose that we _always_ discard any errors we hit going through
> the namespace addition loop.  As long as we've been able to set up queues
> and identify the controller, this function should return success.
>
> And I think *that* patch is as simple as adding:
>
>                        list_add_tail(&ns->list, &dev->namespaces);
>        }
> +	res = 0;
>        list_for_each_entry(ns, &dev->namespaces, list)
>                add_disk(ns->disk);
>
> What do you think?

Great idea! Better than waiting for each possible failure to occur and
have a bunch of patches to fix essentially the same problem. :)