RE: [PATCH v5 1/7] target: core: add common tpg/enable attribute
From: Dmitriy Bogdanov <hidden>
Date: 2021-09-16 08:30:43
Also in:
target-devel
From: Dmitriy Bogdanov <hidden>
Date: 2021-09-16 08:30:43
Also in:
target-devel
Hi Bodo,
quoted
+ if (se_tpg->enabled == op) + return count;
Sorry for jumping in lately.
Just one nit: In case someone tries to enable or disable the same tpg a second time, with the change we always do nothing and return count (--> OK). I just checked iscsi and qla2xxx. AFAICS iscsi before the patch rejected the second enable or disable with -EINVAL, while qla2xxx accepts the second disable and rejects the second enable with -EEXIST. Of course it sounds good to unify the behavior of existing enable attributes. OTOH: even if enabling/disabling the same tpg twice can be seen as suspicious behavior, are we sure to not confuse existing user space tools by changing the result?
targetcli tool does not check the result of disable/enable at all. Our proprietary application checks a result but does not check the particular return code, and the application does not expect the failure of the same second request. It's hardly to imagine why someone should expect the second enable and especially the second disable to fail - the requested operation(disable or enable) is successfully done for the caller. BR, Dmitry