On Thu, 2015-09-10 at 12:16 -0700, Andy Grover wrote: > On 09/10/2015 11:18 AM, Andy Grover wrote: > > On 09/10/2015 03:53 AM, joeue Deng wrote: > >> Using 'targetcli /backstores/block/ create name dev [readonly=true]' to > >> set block with "ro write-thru" stat. > >> > >> however, the initiator can still write to the iscsi device. > >> > >> and I also find something similar, > >> https://bugzilla.redhat.com/show_bug.cgi?id=1036052 > >> > >> Is this not supported current? > >> > >> the version i am using is : > >> > >> targetcli-2.1.fb37-3.el7.src.rpm > >> python-configshell-1.1.fb14-1.el7.src.rpm > >> python-rtslib-2.1.fb50-1.el7.src.rpm > > > >> targetcli-fb-devel mailing list > > > > [CCing target-devel for their thoughts] > > > > This is because iblock's readonly parameter just results in the block > > device being opened without FMODE_WRITE flag set. If the underlying > > block device is actually read-only then iblock will pass them down and > > they will fail in blkdev_write_iter(), like they should. > > > > But if the iblock device is set read-only but the underlying device is > > not then we get this situation, because opening without FMODE_WRITE does > > nothing to prevent write bios from being performed. > > > > I see two solutions: > > > > 1) Change iblock to fail write/write_same/discard requests to devices it > > has as read-only, before they get to the backing block device > > > > 2) Change targetcli/rtslib so read-only is not settable by the user, but > > instead is based on the state of the backing device (by checking > > /sys/block/<x>/ro). > > > > I lean towards #2. I tried #1 and it works, but the original goal (see > > 44bfd0185) wasn't to add a read-only feature to lio exports, it was to > > get read-only block devices working. > > And of course we also have write protect on the target side.. Maybe what > we do is #2, and then userspace can also set write protect on tpg luns > if backed by a read-only block device. After thinking about this a bit more, I'd prefer the following to propagate up a read-only bit using se_device->dev_flags, and subsequently override the passed lun_access in core_tpg_add_lun(). WDYT..? diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c index 5a9982f..0f19e11 100644 --- a/drivers/target/target_core_iblock.c +++ b/drivers/target/target_core_iblock.c @@ -105,6 +105,8 @@ static int iblock_configure_device(struct se_device *dev) mode = FMODE_READ|FMODE_EXCL; if (!ib_dev->ibd_readonly) mode |= FMODE_WRITE; + else + dev->dev_flags |= DF_READ_ONLY; bd = blkdev_get_by_path(ib_dev->ibd_udev_path, mode, ib_dev); if (IS_ERR(bd)) { diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index 2d0381d..5fb9dd7 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -668,7 +668,10 @@ int core_tpg_add_lun( list_add_tail(&lun->lun_dev_link, &dev->dev_sep_list); spin_unlock(&dev->se_port_lock); - lun->lun_access = lun_access; + if (dev->dev_flags & DF_READ_ONLY) + lun->lun_access = TRANSPORT_LUNFLAGS_READ_ONLY; + else + lun->lun_access = lun_access; if (!(dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE)) hlist_add_head_rcu(&lun->link, &tpg->tpg_lun_hlist); mutex_unlock(&tpg->tpg_lun_mutex); diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index ac9bf1c..5f48754 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -730,6 +730,7 @@ struct se_device { #define DF_EMULATED_VPD_UNIT_SERIAL 0x00000004 #define DF_USING_UDEV_PATH 0x00000008 #define DF_USING_ALIAS 0x00000010 +#define DF_READ_ONLY 0x00000020 /* Physical device queue depth */ u32 queue_depth; /* Used for SPC-2 reservations enforce of ISIDs */ -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html