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

[alan.adamson at oracle.com]

On 1/23/24 7:41 PM, Chaitanya Kulkarni wrote:
> Alan,
>
> On 1/23/24 09:11, alan.adamson at 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.
>>
>> Alan
>>
> can you please remove the struct device:logging_enabled and use Sagi's
> suggestion and post a new version ?
>
> There is a also comment from Christoph to use is_visible method instead
> of dynamically adding and removing groups, can you also add that to next
> version ? unless that cannot be done for some reason ...

Yes, I'll post a new version. I don't think the is_visible method is 
necessary since we will not need to create a new group, just use the 
existing groups (nvme_ns_id and nvme_dev).


Alan