On Thu, Apr 23, 2020 at 03:39:38PM +0300, Max Gurtovoy wrote:
>>> + if (ctrl->subsys->pi_support && ctrl->port->pi_enable) {
>>> + if (ctrl->port->pi_capable) {
>>> + ctrl->pi_support = true;
>>> + pr_info("controller %d T10-PI enabled\n", ctrl->cntlid);
>>> + } else {
>>> + ctrl->pi_support = false;
>>> + pr_warn("T10-PI is not supported on controller %d\n",
>>> + ctrl->cntlid);
>>> + }
>> I think the printks are a little verbose. Also why can we set
>> ctrl->port->pi_enable if ctrl->port->pi_capable is false? Shoudn't
>> we reject that earlier? In that case this could simply become:
>>
>> ctrl->pi_support = ctrl->subsys->pi_support && ctrl->port->pi_enable;
>
> for that we'll need to check pi_capable during add_port process and disable
> pi_enable bit if user set it.
Which seems pretty sensible. In fact I think the configfs write for
pi enable should fail if we don't have the capability.
> User should set it before enable the port (this will always succeed).
>
> I'll make this change as well.
>
> re the verbosity, sometimes I get many requests from users to get
> indication for some features.
>
> We can remove this as well if needed.
I'd rather keep debug prints silent. You could add a verbose mode
in nvmetcli that prints all the settings applied if that helps these
users.
