Thread (13 messages) 13 messages, 5 authors, 2021-10-12

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

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help