Skip to contentSkip to conversation

libosd: Support for device lookup by osd_device_info

PATCH In review Boaz Harrosh 2 participants 4 messages Jun 30, 2009 SCSI

lore
Share & tools
lore
#0 lore raw

cover libosd: Support for device lookup by osd_device_info

These two patches are for Linux=2.6.32

They are for supporting what is needed by the pNFS-objects layout driver and exofs when exporting osd(s) on the network.

Benny you will need to carry these in the pnfs tree until 2.6.32-rc1 as prerequisite of the osd get_device_info support (will send next)

list of patches:
[PATCH 1/2] libosd: osd_dev_info: Unique Identification of an OSD device [PATCH 2/2] libosd: osd_dev_is_ver1 - Minor API cleanup

Please review
Thanks
Boaz

#1 lore raw

PATCH 1/2 libosd: osd_dev_info: Unique Identification of an OSD device

Define an osd_dev_info structure that Uniquely identifies an OSD device lun on the network. The identification is built from unique target attributes and is the same for all network/SAN machines.

osduld_info_lookup() - NEW
New API that will lookup an osd_dev by its osd_dev_info. This is used by pNFS-objects for cross network global device identification.

osduld_device_info() - NEW
Given an osd_dev handle returns its associated osd_dev_info. This is used by exofs to encode the device information for network clients. (Get-device-info). The ULD fetches this information at startup and hangs it on each OSD device. (This is a fast operation that can be called at any condition)

osd_auto_detect_ver() - REVISED
Now returns an osd_dev_info structure. Is only called once by ULD as before. See added comments for how to use.

---
 drivers/scsi/osd/osd_initiator.c |   19 ++++++++++----
 drivers/scsi/osd/osd_uld.c       |   50 +++++++++++++++++++++++++++++++++++++-
 include/scsi/osd_initiator.h     |   37 +++++++++++++++++++++++++---
 3 files changed, 95 insertions(+), 11 deletions(-)
drivers/scsi/osd/osd_initiator.c +13 -6
index 7a117c1..c36697e 100644
@@ -73,7 +73,8 @@ static const char *_osd_ver_desc(struct osd_request *or)
 
 #define ATTR_DEF_RI(id, len) ATTR_DEF(OSD_APAGE_ROOT_INFORMATION, id, len)
 
-static int _osd_print_system_info(struct osd_dev *od, void *caps)
+static int _osd_print_system_info(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)
 {
 	struct osd_request *or;
 	struct osd_attr get_attrs[] = {
@@ -137,8 +138,10 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 	OSD_INFO("PRODUCT_SERIAL_NUMBER  [%s]\n",
 		(char *)pFirst);
 
-	pFirst = get_attrs[a].val_ptr;
-	OSD_INFO("OSD_NAME               [%s]\n", (char *)pFirst);
+	odi->osdname_len = get_attrs[a].len;
+	odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL);
+	memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len);
+	OSD_INFO("OSD_NAME               [%s]\n", odi->osdname);
 	a++;
 
 	pFirst = get_attrs[a++].val_ptr;
@@ -171,6 +174,9 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 				   sid_dump, sizeof(sid_dump), true);
 		OSD_INFO("OSD_SYSTEM_ID(%d)\n"
 			 "        [%s]\n", len, sid_dump);
+
+		odi->systemid_len = len;
+		memcpy(odi->systemid, get_attrs[a].val_ptr, len);
 		a++;
 	}
 out:
@@ -178,16 +184,17 @@ out:
 	return ret;
 }
 
-int osd_auto_detect_ver(struct osd_dev *od, void *caps)
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)
 {
 	int ret;
 
 	/* Auto-detect the osd version */
-	ret = _osd_print_system_info(od, caps);
+	ret = _osd_print_system_info(od, caps, odi);
 	if (ret) {
 		osd_dev_set_ver(od, OSD_VER1);
 		OSD_DEBUG("converting to OSD1\n");
-		ret = _osd_print_system_info(od, caps);
+		ret = _osd_print_system_info(od, caps, odi);
 	}
 
 	return ret;
drivers/scsi/osd/osd_uld.c +49 -1

80 lines not on this page · raw message

include/scsi/osd_initiator.h +33 -4
index 02bd9f7..0d29cc3 100644
@@ -56,10 +56,23 @@ struct osd_dev {
 #endif
 };
 
-/* Retrieve/return osd_dev(s) for use by Kernel clients */
-struct osd_dev *osduld_path_lookup(const char *dev_name); /*Use IS_ERR/ERR_PTR*/
+/* Unique Identification of an OSD device */
+struct osd_dev_info {
+	unsigned systemid_len;
+	u8 systemid[OSD_SYSTEMID_LEN];
+	unsigned osdname_len;
+	u8 *osdname;
+};
+
+/* Retrieve/return osd_dev(s) for use by Kernel clients
+ * Use IS_ERR/ERR_PTR on returned "osd_dev *".
+ */
+struct osd_dev *osduld_path_lookup(const char *dev_name);
+struct osd_dev *osduld_info_lookup(const struct osd_dev_info *odi);
 void osduld_put_device(struct osd_dev *od);
 
+const struct osd_dev_info *osduld_device_info(struct osd_dev *od);
+
 /* Add/remove test ioctls from external modules */
 typedef int (do_test_fn)(struct osd_dev *od, unsigned cmd, unsigned long arg);
 int osduld_register_test(unsigned ioctl, do_test_fn *do_test);
@@ -69,8 +82,24 @@ void osduld_unregister_test(unsigned ioctl);
 void osd_dev_init(struct osd_dev *od, struct scsi_device *scsi_device);
 void osd_dev_fini(struct osd_dev *od);
 
-/* some hi level device operations */
-int osd_auto_detect_ver(struct osd_dev *od, void *caps);    /* GFP_KERNEL */
+/**
+ * osd_auto_detect_ver - Detect the OSD version, return Unique Identification
+ *
+ * @od:     OSD target lun handle
+ * @caps:   Capabilities authorizing OSD root read attributes access
+ * @odi:    Retrieved information uniquely identifying the osd target lun
+ *          Note: odi->osdname must be kfreed by caller.
+ *
+ * Auto detects the OSD version of the OSD target and sets the @od
+ * accordingly. Meanwhile also returns the "system id" and "osd name" root
+ * attributes which uniquely identify the OSD target. This member is usually
+ * called by the ULD. ULD users should call osduld_device_info().
+ * This rutine allocates osd requests and memory at GFP_KERNEL level and might
+ * sleep.
+ */
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi);
+
 static inline struct request_queue *osd_request_queue(struct osd_dev *od)
 {
 	return od->scsi_device->request_queue;
Signature · 1 line
1.6.2.1
#2 lore raw

Re: [PATCH 1/2] libosd: osd_dev_info: Unique Identification of an OSD device

Quoted 32 lines · Boaz Harrosh

Define an osd_dev_info structure that Uniquely identifies an OSD device lun on the network. The identification is built from unique target attributes and is the same for all network/SAN machines.

osduld_info_lookup() - NEW
New API that will lookup an osd_dev by its osd_dev_info. This is used by pNFS-objects for cross network global device identification.

osduld_device_info() - NEW
Given an osd_dev handle returns its associated osd_dev_info. This is used by exofs to encode the device information for network clients. (Get-device-info). The ULD fetches this information at startup and hangs it on each OSD device. (This is a fast operation that can be called at any condition)

osd_auto_detect_ver() - REVISED
Now returns an osd_dev_info structure. Is only called once by ULD as before. See added comments for how to use.

---
 drivers/scsi/osd/osd_initiator.c |   19 ++++++++++----
 drivers/scsi/osd/osd_uld.c       |   50 +++++++++++++++++++++++++++++++++++++-
 include/scsi/osd_initiator.h     |   37 +++++++++++++++++++++++++---
 3 files changed, 95 insertions(+), 11 deletions(-)
drivers/scsi/osd/osd_initiator.c +0 -0
index 7a117c1..c36697e 100644

drivers/scsi/osd/osd_initiator.c static const char *_osd_ver_desc(struct osd_request *or) · line 77

Quoted 6 lines
drivers/scsi/osd/osd_initiator.c +2 -1
@@ -73,7 +73,8 @@ static const char *_osd_ver_desc(struct osd_request *or)
 
 #define ATTR_DEF_RI(id, len) ATTR_DEF(OSD_APAGE_ROOT_INFORMATION, id, len)
 
-static int _osd_print_system_info(struct osd_dev *od, void *caps)
+static int _osd_print_system_info(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)

Now that it's doing much more than printing how about renaming it to _osd_prep_system_info or _osd_get_system_info?

drivers/scsi/osd/osd_initiator.c static int _osd_print_system_info(struct osd_dev *od, void *caps) · line 144

Quoted 13 lines
drivers/scsi/osd/osd_initiator.c +4 -2
 {
 	struct osd_request *or;
 	struct osd_attr get_attrs[] = {
@@ -137,8 +138,10 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 	OSD_INFO("PRODUCT_SERIAL_NUMBER  [%s]\n",
 		(char *)pFirst);
 
-	pFirst = get_attrs[a].val_ptr;
-	OSD_INFO("OSD_NAME               [%s]\n", (char *)pFirst);
+	odi->osdname_len = get_attrs[a].len;
+	odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL);
+	memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len);
+	OSD_INFO("OSD_NAME               [%s]\n", odi->osdname);

The osd name isn't necessarily a (printable) string...
I'd treat it the same as the systemid and not even bother allocating space for it if osdname_len==0 (and handle the null pointer correctly in this case where it's accessed)

Quoted 68 lines
drivers/scsi/osd/osd_initiator.c +7 -3
 	a++;
 
 	pFirst = get_attrs[a++].val_ptr;
@@ -171,6 +174,9 @@ static int _osd_print_system_info(struct osd_dev *od, void *caps)
 				   sid_dump, sizeof(sid_dump), true);
 		OSD_INFO("OSD_SYSTEM_ID(%d)\n"
 			 "        [%s]\n", len, sid_dump);
+
+		odi->systemid_len = len;
+		memcpy(odi->systemid, get_attrs[a].val_ptr, len);
 		a++;
 	}
 out:
@@ -178,16 +184,17 @@ out:
 	return ret;
 }
 
-int osd_auto_detect_ver(struct osd_dev *od, void *caps)
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi)
 {
 	int ret;
 
 	/* Auto-detect the osd version */
-	ret = _osd_print_system_info(od, caps);
+	ret = _osd_print_system_info(od, caps, odi);
 	if (ret) {
 		osd_dev_set_ver(od, OSD_VER1);
 		OSD_DEBUG("converting to OSD1\n");
-		ret = _osd_print_system_info(od, caps);
+		ret = _osd_print_system_info(od, caps, odi);
 	}
 
 	return ret;
drivers/scsi/osd/osd_uld.c +19 -0
index 0bdef33..77e63f1 100644
@@ -87,6 +87,7 @@ struct osd_uld_device {
 	struct osd_dev od;
 	struct gendisk *disk;
 	struct device *class_member;
+	struct osd_dev_info odi;
 };
 
 static void __uld_get(struct osd_uld_device *oud);
@@ -216,6 +217,45 @@ free_od:
 }
 EXPORT_SYMBOL(osduld_path_lookup);
 
+static inline bool _the_same_or_null(const u8 *a1, unsigned a1_len,
+				     const u8 *a2, unsigned a2_len)
+{
+	if (a1_len && a2_len && (a1_len != a2_len))
+		return false;
+
+	return !a1_len || !a2_len || (0 == memcmp(a1, a2, a1_len));
+}
+
+struct osd_dev *osduld_info_lookup(const struct osd_dev_info *odi)
+{
+	unsigned i;
+
+	for (i = 0; i < SCSI_OSD_MAX_MINOR; i++) {
+		char dev_str[16];
+		struct osd_uld_device *oud;
+		struct osd_dev *od;
+

drivers/scsi/osd/osd_uld.c free_od: · line 247

Quoted 10 lines
drivers/scsi/osd/osd_uld.c +10 -0
+		sprintf(dev_str, "/dev/osd%d", i);
+		od = osduld_path_lookup(dev_str);
+		if (IS_ERR(od))
+			continue;
+
+		oud = od->file->private_data;
+		if (_the_same_or_null(oud->odi.systemid, oud->odi.systemid_len,
+				      odi->systemid, odi->systemid_len) &&
+		    _the_same_or_null(oud->odi.osdname, oud->odi.osdname_len,
+				      odi->osdname, odi->osdname_len)) {

This will match null oud->odi.systemid regardless of oud->odi.osdname. Shouldn't we check any non-NULL lookup key first?
for example:

static inline bool _same(const u8 *a1, unsigned a1_len,
const u8 *a2, unsigned a2_len)

{
	if (a1_len != a2_len)
		return false;

	return !a1_len || (0 == memcmp(a1, a2, a1_len));
}
		found = 1;
		if ((odi->systemid_len &&
		     !_same(oud->odi.systemid, oud->odi.systemid_len,
			    odi->systemid, odi->systemid_len)) ||
		    (odi->osdname_len &&
		     !_same(oud->odi.osdname, oud->odi.osdname_len,
				      odi->osdname, odi->osdname_len)))
			found = 0;
		if (found) {

drivers/scsi/osd/osd_uld.c free_od: · line 253

Quoted 6 lines
drivers/scsi/osd/osd_uld.c +6 -0
+			OSD_DEBUG("found device sysid_len=%d osdname=%d\n",
+				  odi->systemid_len, odi->osdname_len);
+			return od;
+		}
+		osduld_put_device(od);
+	}

Benny

Quoted 96 lines
drivers/scsi/osd/osd_uld.c +14 -1
+
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL(osduld_info_lookup);
+
 void osduld_put_device(struct osd_dev *od)
 {
 
@@ -230,6 +270,13 @@ void osduld_put_device(struct osd_dev *od)
 }
 EXPORT_SYMBOL(osduld_put_device);
 
+const struct osd_dev_info *osduld_device_info(struct osd_dev *od)
+{
+	struct osd_uld_device *oud = od->file->private_data;
+	return &oud->odi;
+}
+EXPORT_SYMBOL(osduld_device_info);
+
 /*
  * Scsi Device operations
  */
@@ -250,7 +297,7 @@ static int __detect_osd(struct osd_uld_device *oud)
 		OSD_ERR("warning: scsi_test_unit_ready failed\n");
 
 	osd_sec_init_nosec_doall_caps(caps, &osd_root_object, false, true);
-	if (osd_auto_detect_ver(&oud->od, caps))
+	if (osd_auto_detect_ver(&oud->od, caps, &oud->odi))
 		return -ENODEV;
 
 	return 0;
@@ -402,6 +449,7 @@ static void __remove(struct kref *kref)
 		put_disk(oud->disk);
 
 	ida_remove(&osd_minor_ida, oud->minor);
+	kfree(oud->odi.osdname);
 	kfree(oud);
 }
 
include/scsi/osd_initiator.h +33 -4
index 02bd9f7..0d29cc3 100644
@@ -56,10 +56,23 @@ struct osd_dev {
 #endif
 };
 
-/* Retrieve/return osd_dev(s) for use by Kernel clients */
-struct osd_dev *osduld_path_lookup(const char *dev_name); /*Use IS_ERR/ERR_PTR*/
+/* Unique Identification of an OSD device */
+struct osd_dev_info {
+	unsigned systemid_len;
+	u8 systemid[OSD_SYSTEMID_LEN];
+	unsigned osdname_len;
+	u8 *osdname;
+};
+
+/* Retrieve/return osd_dev(s) for use by Kernel clients
+ * Use IS_ERR/ERR_PTR on returned "osd_dev *".
+ */
+struct osd_dev *osduld_path_lookup(const char *dev_name);
+struct osd_dev *osduld_info_lookup(const struct osd_dev_info *odi);
 void osduld_put_device(struct osd_dev *od);
 
+const struct osd_dev_info *osduld_device_info(struct osd_dev *od);
+
 /* Add/remove test ioctls from external modules */
 typedef int (do_test_fn)(struct osd_dev *od, unsigned cmd, unsigned long arg);
 int osduld_register_test(unsigned ioctl, do_test_fn *do_test);
@@ -69,8 +82,24 @@ void osduld_unregister_test(unsigned ioctl);
 void osd_dev_init(struct osd_dev *od, struct scsi_device *scsi_device);
 void osd_dev_fini(struct osd_dev *od);
 
-/* some hi level device operations */
-int osd_auto_detect_ver(struct osd_dev *od, void *caps);    /* GFP_KERNEL */
+/**
+ * osd_auto_detect_ver - Detect the OSD version, return Unique Identification
+ *
+ * @od:     OSD target lun handle
+ * @caps:   Capabilities authorizing OSD root read attributes access
+ * @odi:    Retrieved information uniquely identifying the osd target lun
+ *          Note: odi->osdname must be kfreed by caller.
+ *
+ * Auto detects the OSD version of the OSD target and sets the @od
+ * accordingly. Meanwhile also returns the "system id" and "osd name" root
+ * attributes which uniquely identify the OSD target. This member is usually
+ * called by the ULD. ULD users should call osduld_device_info().
+ * This rutine allocates osd requests and memory at GFP_KERNEL level and might
+ * sleep.
+ */
+int osd_auto_detect_ver(struct osd_dev *od,
+	void *caps, struct osd_dev_info *odi);
+
 static inline struct request_queue *osd_request_queue(struct osd_dev *od)
 {
 	return od->scsi_device->request_queue;
#3 lore raw

PATCH 2/2 libosd: osd_dev_is_ver1 - Minor API cleanup

In reply to Boaz Harrosh

define a new osd_dev_is_ver1 that operates on devices and the old osd_req_is_ver1 uses that new API.

---
 include/scsi/osd_initiator.h |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)
include/scsi/osd_initiator.h +10 -6
index 0d29cc3..7bda109 100644
@@ -113,6 +113,15 @@ static inline void osd_dev_set_ver(struct osd_dev *od, enum osd_std_version v)
 #endif
 }
 
+static inline bool osd_dev_is_ver1(struct osd_dev *od)
+{
+#ifdef OSD_VER1_SUPPORT
+	return od->version == OSD_VER1;
+#else
+	return false;
+#endif
+}
+
 struct osd_request;
 typedef void (osd_req_done_fn)(struct osd_request *or, void *private);
 
@@ -149,14 +158,9 @@ struct osd_request {
 	int async_error;
 };
 
-/* OSD Version control */
 static inline bool osd_req_is_ver1(struct osd_request *or)
 {
-#ifdef OSD_VER1_SUPPORT
-	return or->osd_dev->version == OSD_VER1;
-#else
-	return false;
-#endif
+	return osd_dev_is_ver1(or->osd_dev);
 }
 
 /*
Signature · 1 line
1.6.2.1