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

[alan.adamson at oracle.com]

>> sorry I didn't understand your question clearly ...
>>
>> In the original implementation we get two different struct device 
>> objects
>> for following commands in sysfs, that sets struct device:logging_enabled
>> flag,
>> which is also used to determine the logging in nvme_init_request() :-
>>
>> echo 1 > /sys/class/nvme/nvme0/passthru_err_log_enabled
>> echo 1 > /sys/class/nvme/nvme0/nvme0n1/passthru_err_log_enabled
>>
>> nvme_init_ctrl() already sets dev_set_drvdata(ctrl->device, ctrl). In 
>> this
>> proposal above change in nvme_alloc_ns() sets the
>> dev_set_drvdata(disk_to_dev(ns->disk), ns).
>>
>> 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?

If we create separate show/stores for the ctrl (nvme_dev_attrs) and ns 
(nvme_ns_attrs), using the exiting macro: DEVICE_ATTR(), the attribute 
name (passthru_err_log_enabled) can not be used for both nvme_dev_attrs 
and nvme_ns_attrs.

/sys/class/nvme/nvme0/adm_passthru_err_log_enabled

/sys/class/nvme/nvme0/nvme0n1/io_passthru_err_log_enabled


If I setup the attributes like:

static struct device_attribute dev_attr_adm_passthru_err_log_enabled = \
         __ATTR(passthru_err_log_enabled, S_IRUGO | S_IWUSR, \
         nvme_adm_passthru_err_log_enabled_show, 
nvme_adm_passthru_err_log_enabled_store);

static struct device_attribute dev_attr_io_passthru_err_log_enabled = \
         __ATTR(passthru_err_log_enabled, S_IRUGO | S_IWUSR, \
         nvme_io_passthru_err_log_enabled_show, 
nvme_io_passthru_err_log_enabled_store);


Then both attributes can be the same.

/sys/class/nvme/nvme0/passthru_err_log_enabled

/sys/class/nvme/nvme0/nvme0n1/passthru_err_log_enabled

What do we prefer?

Alan