Re: [PATCH V8 1/1] nvme: allow passthru cmd error logging

Christoph Hellwig
2024-01-24T09:06:23Z

On Tue, Jan 23, 2024 at 09:11:32AM -0800, alan.adamson@oracle.com wrote:
>>> This allows us to get the ctrl or ns object associated with the struct
>>> device
>>> we get in the sysfs, then based on the device class we update
>>> logging_enabled
>>> flags for either ctrl or ns respectively. In nvme_init_request() I use
>>> ctrl->logging_enabled and ns->logging_enabled based on admin or io cmd.
>>
>> I was asking why should we have a show/store that operate on both ns and
>> ctrl?
>>
>> Why not have a show/store in nvme_dev_attrs and a separate one in
>> nvme_ns_id_attrs ? Then you don't need the awkward is_nvme_class() ?
>> the ns attrs can access the ns in a normal way like the rest? Or am
>> I missing something?
>
> I did have a version that used nvme_ns_id_attrs but didn't think it was an
> appropriate place for it.  I can go back to that.

nvme_ns_id_attrs has been renamed now that we're using it for more than
just ids, so adding more attributes if they fit is fine.