Re: [PATCH] SMC: do not allow load/unload of media that has been pinned down with PreventAllowMediumRemoval

[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

List, Tomo, Mark

Thanks for the review.
I have changed the patch to only check the src slot on unload as Mark suggested.

regards
ronnie sahlberg


On Sun, Feb 5, 2012 at 3:18 PM, Mark Harvey <markh794@xxxxxxxxx> wrote:
> On Sun, Feb 5, 2012 at 10:43 AM, ronnie sahlberg
> <ronniesahlberg@xxxxxxxxx> wrote:
>> List, Tomo
>>
>> Please find attached a small patch for SMC.
>> It now prevents the media changer commands from loading/unloading
>> media to a data transfer device IF that data transfer device has the
>> media removal prevention set via PreventAllowMediumRemoval.
>>
>> Initiators set this when they mount a device that is removable.
>>
>> For example if you have a removable SBC device like the sbc jukebox
>> example, Linux for example will set the PreventAllowMediumRemoval when
>> you:
>> mount -o ro,noload <device> <mountpoint>
>> just so that no one will eject the media while the filesystem is mounted.
>>
>> Similarly this also happens for MMC devices,   and I belive also on
>> SSC devices, but I cant check SSC devices.
>> (Mark, If I assume that backup applocations do use
>> PreventAllowMediumRemoval when they have loaded a tape into the dt
>> device?)
>>
>>
>> regards
>> ronnie sahlberg
>
> The IBM SCSI Reference (11th edition) states..
>
> ------------
> If the Prevent field is set, then eject requests from the front panel
> are ignored and
> Unload commands give Check Condition status. The Sense Key is set to Illegal
> Request and the ASC/ASCQ to Medium Removal Prevented (5302).
> All initiators that have prevented medium removal must enable it before the
> medium can be removed from the drive.
> -----------
>
> This patch also checks status on destination slot, which I think is a
> bit too aggressive.
> Checking on unload on source slot should be fine.
>
> Cheers

Attachment: 0001-SMC-Do-not-allow-load-unload-on-the-data-transfer-de.patch.gz
Description: GNU Zip compressed data

From 1f7d686e85d56b88d7c515ff439783dff22f4702 Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
Date: Mon, 6 Feb 2012 21:35:43 +1100
Subject: [PATCH] SMC: Do not allow load/unload on the data transfer device is prevent removal; is set

Update the smc move media command.
Do not allow loading or unloading media to the data transfer device if an initiator has locked the media with prevent-allow-medium-removal

Initiators such as linux open-iscsi sets the media prevention when mounting a SBC/MMC (and probably also SSC) device that has the RMB bit set in the standard

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
---
 usr/smc.c    |   14 ++++++++++++++
 usr/target.c |   31 ++++++++++++++++++++++++++++++-
 usr/tgtd.h   |    1 +
 3 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/usr/smc.c b/usr/smc.c
index 3e77f2a..197afd9 100644
--- a/usr/smc.c
+++ b/usr/smc.c
@@ -5,6 +5,7 @@
  * (C) 2004-2007 FUJITA Tomonori <tomof@xxxxxxx>
  * (C) 2005-2007 Mike Christie <michaelc@xxxxxxxxxxx>
  * (C) 2007      Mark Harvey <markh794@xxxxxxxxx>
+ * (C) 2012      Ronnie Sahlberg <ronniesahlberg@xxxxxxxxx>
  *
  * SCSI target emulation code is based on Ardis's iSCSI implementation.
  *   http://www.ardistech.com/iscsi/
@@ -47,6 +48,11 @@
 #include "smc.h"
 #include "media.h"
 
+static int check_slot_removable(struct slot *s)
+{
+	return dtd_check_removable(s->drive_tid, s->drive_lun);
+}
+
 static int set_slot_full(struct slot *s, uint16_t src, char *path)
 {
 	int err = 0;
@@ -443,6 +449,14 @@ static int smc_move_medium(int host_no, struct scsi_cmd *cmd)
 	if (invert && (s->sides == 1))	/* Use default INVALID FIELD IN CDB */
 		goto sense;
 
+	if (src_slot->element_type == ELEMENT_DATA_TRANSFER) {
+		if (check_slot_removable(src_slot)) {
+			key = ILLEGAL_REQUEST;
+			asc = ASC_MEDIUM_REMOVAL_PREVENTED;
+			goto sense;
+		}
+	}
+
 	memcpy(&dest_slot->barcode, &src_slot->barcode, sizeof(s->barcode));
 	if (dest_slot->element_type == ELEMENT_DATA_TRANSFER) {
 		char path[128];
diff --git a/usr/target.c b/usr/target.c
index daa115b..42a3a09 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -442,7 +442,9 @@ __device_lookup(int tid, uint64_t lun, struct target **t)
 	if (!lu)
 		return NULL;
 
-	*t = target;
+	if (t)
+		*t = target;
+
 	return lu;
 }
 
@@ -748,6 +750,30 @@ struct lu_phy_attr *lu_attr_lookup(int tid, uint64_t lun)
 }
 
 /**
+ * dtd_check_removable
+ * @tid:	Target ID
+ * @lun:	LUN
+ *
+ * check if a DT can have its media reoved or not
+ */
+int dtd_check_removable(int tid, uint64_t lun)
+{
+	struct scsi_lu *lu;
+
+	lu = __device_lookup(tid, lun, NULL);
+	if (!lu)
+		return TGTADM_NO_LUN;
+
+	if (lu_prevent_removal(lu))
+		return TGTADM_PREVENT_REMOVAL;
+
+	if (!lu->attrs.removable)
+		return TGTADM_INVALID_REQUEST;
+
+	return TGTADM_SUCCESS;
+}
+
+/**
  * dtd_load_unload  --  Load / unload media
  * @tid:	Target ID
  * @lun:	LUN
@@ -766,6 +792,9 @@ int dtd_load_unload(int tid, uint64_t lun, int load, char *file)
 	if (!lu)
 		return TGTADM_NO_LUN;
 
+	if (lu_prevent_removal(lu))
+		return TGTADM_PREVENT_REMOVAL;
+
 	if (!lu->attrs.removable)
 		return TGTADM_INVALID_REQUEST;
 
diff --git a/usr/tgtd.h b/usr/tgtd.h
index b50a887..49d0699 100644
--- a/usr/tgtd.h
+++ b/usr/tgtd.h
@@ -318,6 +318,7 @@ extern int device_type_register(struct device_type_template *);
 
 extern struct lu_phy_attr *lu_attr_lookup(int tid, uint64_t lun);
 extern int dtd_load_unload(int tid, uint64_t lun, int load, char *file);
+extern int dtd_check_removable(int tid, uint64_t lun);
 
 extern int register_backingstore_template(struct backingstore_template *bst);
 extern struct backingstore_template *get_backingstore_template(const char *name);
-- 
1.7.3.1