Re: [PATCH 1/2] target: Don't allow setting WC emulation if device doesn't support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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.

So I'm still not seeing how this is not the right thing to do, especially since it's identical to the pscsi case immediately above.

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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux