On 05/14/2014 05:57 PM, Nicholas A. Bellinger wrote:
On Wed, 2014-05-14 at 17:22 -0700, Andy Grover wrote:
On 05/14/2014 05:07 PM, Nicholas A. Bellinger wrote:
On Wed, 2014-05-14 at 15:48 -0700, Andy Grover wrote:
Just like for pSCSI, if the transport sets get_write_cache, then it is
not valid to enable write cache emulation for it. Return an error.
see https://bugzilla.redhat.com/show_bug.cgi?id=1082675
Reviewed-by: Chris Leech <cleech@xxxxxxxxxx>
Signed-off-by: Andy Grover <agrover@xxxxxxxxxx>
---
drivers/target/target_core_device.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 65001e1..d461ecb 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -798,10 +798,10 @@ int se_dev_set_emulate_write_cache(struct se_device *dev, int flag)
pr_err("emulate_write_cache not supported for pSCSI\n");
return -EINVAL;
}
- if (dev->transport->get_write_cache) {
- pr_warn("emulate_write_cache cannot be changed when underlying"
- " HW reports WriteCacheEnabled, ignoring request\n");
- return 0;
+ if (flag &&
+ dev->transport->get_write_cache) {
+ pr_err("emulate_write_cache not supported for this device\n");
+ return -EINVAL;
}
dev->dev_attrib.emulate_write_cache = flag;
Allowing the target WCE bit to be disabled when the underlying device
has WCE enabled is a recipe for disaster.
How is the initiator supposed to know when to flush writes if the target
is telling it that WCE is disabled.
I'll take change to return -EINVAL here, but the other part is dangerous
and wrong.
This is for backstores like iblock, where we're actually getting WCE
yes/no from the backing block device. This configfs entry is not for WCE
yes/no, it is for WCE *emulation* yes/no.
Huh..? The only thing that emulate_write_cache controls is the
reporting of WCE via the caching mode page + EVPD=0x86 for FILEIO + RD
backends.
So we should allow emulate_write_cache to be set to 0, because we're not
emulating the write cache value, we're using the actual value... but we
shouldn't allow it to be enabled if we're getting real WCE yes/no values.
There is no point in touching the emulate_write_cache value for IBLOCK.
It's already being ignored in spc_check_dev_wce() anyways.
Right, but userspace expects that any value it has read from configfs is
a valid value to write to that entry when restoring target
configuration. So for iblock emulate_write_cache should always read 0,
and writing back 0 should succeed. Writing 1 should fail. For this we
need to check the flag.
Regards -- Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
