Hi Tomo, I think for safety it is better to require this as an explicit option, but I dont have any strong opinion either for or against. What changes do you you want me to implement for this patch ? Do you want me to re-spin the patch and remove the option ? regards ronnie sahlberg On Mon, Apr 2, 2012 at 10:26 AM, FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: > On Mon, 02 Apr 2012 09:06:11 +0900 (JST) > FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: > >> On Sun, 1 Apr 2012 08:26:27 +1000 >> ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: >> >>> From 2830abc3935294eba621b781f72aecda604c65a7 Mon Sep 17 00:00:00 2001 >>> From: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx> >>> Date: Sun, 1 Apr 2012 08:04:06 +1000 >>> Subject: [PATCH 2/2] SBC UNMAP: Add support for thin-provisioning and the UNMAP command. >>> >>> The UNMAP command is implemented using FALLOC_FL_PUNCH_HOLE and will >>> release UNMAPPED blocks back to the underlying filesystem. >>> >>> FALLOC_FL_PUNCH_HOLE is fairly new addition to Linux but works on >>> ext4 and XFS filesystems currently. >>> >>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx> >>> --- >>> doc/tgtadm.8.xml | 25 ++++++++++++++ >>> usr/bs_rdwr.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> usr/sbc.c | 76 +++++++++++++++++++++++++++++++++++++++++- >>> usr/scsi.h | 1 + >>> usr/spc.c | 43 +++++++++++++++++++++-- >>> usr/target.c | 2 + >>> usr/tgtd.h | 2 + >>> 7 files changed, 241 insertions(+), 5 deletions(-) >>> >>> diff --git a/doc/tgtadm.8.xml b/doc/tgtadm.8.xml >>> index 668e184..a40f659 100644 >>> --- a/doc/tgtadm.8.xml >>> +++ b/doc/tgtadm.8.xml >>> @@ -352,6 +352,31 @@ tgtadm --lld iscsi --mode logicalunit --op update --tid 1 --lun 1 \ >>> --params readonly=1 >>> </screen> >>> >>> + <varlistentry><term><option>thin_provisioning=<0|1></option></term> >>> + <listitem> >>> + <para> >>> + This controls the provisioning for the LUN. A thin-provisioned >>> + LUN is represented as a sparse file. >>> + TGTD supports provisioning type 2 for sparse files. >>> + When initiators use the SCSI UNMAP command TGTD will release >>> + the affected areas back to the filesystem using >>> + FALLOC_FL_PUNCH_HOLE. >>> + </para> >>> + <para> >>> + This parameter only applies to DISK devices. >>> + </para> >>> + <para> >>> + Thin-provisioning only works for LUNs stored on filesystems >>> + that support FALLOC_FL_PUNCH_HOLE. >>> + </para> >>> + </listitem> >>> + </varlistentry> >>> + >>> + <screen format="linespecific"> >>> +tgtadm --lld iscsi --mode logicalunit --op update --tid 1 --lun 1 \ >>> + --params thin_provisioning=1 >>> + </screen> >> >> Users need to enable this explicitly? >> >> I mean that when a lu is added, tgtd can check if the backing storage >> supports FALLOC_FL_PUNCH_HOLE then enables it automatically? > > Note that I'm not against providing the explict option to users but > normally users has the option for this (e.g. btrfs mount option) on > the initiator side. So even if tgtd automatically enables this, users > can avoid using this feature. So I wonder that it's worth providing > the explict option. -- To unsubscribe from this list: send the line "unsubscribe stgt" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html