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

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

Alan,

On 1/23/24 09:11, 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.
>
> 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 ...

-ck