* osd patches for the v2.6.32 merge window
@ 2009-09-07 11:25 Boaz Harrosh
2009-09-07 11:26 ` [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device Boaz Harrosh
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Boaz Harrosh @ 2009-09-07 11:25 UTC (permalink / raw)
To: James Bottomley, linux-scsi, open-osd mailing-list
James Hi
please submit the flowing patches into scsi-misc for
inclusion into the v2.6.32 Kernel.
These patches have been reviewed on the osd mailing-list and
sat in linux-next for a while. Some other trees will depend on
these changes.
Patches:
[PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device
[PATCH 2/3] libosd: osd_dev_is_ver1 - Minor API cleanup
[PATCH 3/3] libosd: osd_sense: OSD_CFO_PERMISSIONS
Thanks
Boaz
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device 2009-09-07 11:25 osd patches for the v2.6.32 merge window Boaz Harrosh @ 2009-09-07 11:26 ` Boaz Harrosh 2009-09-16 17:47 ` James Bottomley 2009-09-27 8:14 ` [PATCH 1/3 version 2] " Boaz Harrosh 2009-09-07 11:26 ` [PATCH 2/3] libosd: osd_dev_is_ver1 - Minor API cleanup Boaz Harrosh 2009-09-07 11:27 ` [PATCH 3/3] libosd: osd_sense: OSD_CFO_PERMISSIONS Boaz Harrosh 2 siblings, 2 replies; 8+ messages in thread From: Boaz Harrosh @ 2009-09-07 11:26 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd 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. Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- drivers/scsi/osd/osd_initiator.c | 22 +++++++++++---- drivers/scsi/osd/osd_uld.c | 53 +++++++++++++++++++++++++++++++++++++- include/scsi/osd_initiator.h | 37 +++++++++++++++++++++++--- 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index 7a117c1..26e1b41 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -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_get_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,13 @@ 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; + if (odi->osdname_len) { + odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); + memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); + } else + odi->osdname = NULL; + OSD_INFO("OSD_NAME [%s]\n", odi->osdname); a++; pFirst = get_attrs[a++].val_ptr; @@ -171,6 +177,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 +187,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_get_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_get_print_system_info(od, caps, odi); } return ret; diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c index 0bdef33..8c069f9 100644 --- a/drivers/scsi/osd/osd_uld.c +++ b/drivers/scsi/osd/osd_uld.c @@ -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,48 @@ 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 (!a2_len) /* User string is Empty means don't care */ + return true; + + if (a1_len != a2_len) + return false; + + return 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; + + 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)) { + OSD_DEBUG("found device sysid_len=%d osdname=%d\n", + odi->systemid_len, odi->osdname_len); + return od; + } + osduld_put_device(od); + } + + return ERR_PTR(-ENODEV); +} +EXPORT_SYMBOL(osduld_info_lookup); + void osduld_put_device(struct osd_dev *od) { @@ -230,6 +273,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 +300,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 +452,7 @@ static void __remove(struct kref *kref) put_disk(oud->disk); ida_remove(&osd_minor_ida, oud->minor); + kfree(oud->odi.osdname); kfree(oud); } diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h index 02bd9f7..0d29cc3 100644 --- a/include/scsi/osd_initiator.h +++ b/include/scsi/osd_initiator.h @@ -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; -- 1.6.2.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device 2009-09-07 11:26 ` [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device Boaz Harrosh @ 2009-09-16 17:47 ` James Bottomley 2009-09-17 11:28 ` Boaz Harrosh 2009-09-27 8:14 ` [PATCH 1/3 version 2] " Boaz Harrosh 1 sibling, 1 reply; 8+ messages in thread From: James Bottomley @ 2009-09-16 17:47 UTC (permalink / raw) To: Boaz Harrosh; +Cc: linux-scsi, open-osd On Mon, 2009-09-07 at 14:26 +0300, Boaz Harrosh wrote: > 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. > > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> > --- > drivers/scsi/osd/osd_initiator.c | 22 +++++++++++---- > drivers/scsi/osd/osd_uld.c | 53 +++++++++++++++++++++++++++++++++++++- > include/scsi/osd_initiator.h | 37 +++++++++++++++++++++++--- > 3 files changed, 101 insertions(+), 11 deletions(-) > > diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c > index 7a117c1..26e1b41 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -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_get_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,13 @@ 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; > + if (odi->osdname_len) { > + odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); > + memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); > + } else > + odi->osdname = NULL; > + OSD_INFO("OSD_NAME [%s]\n", odi->osdname); > a++; > > pFirst = get_attrs[a++].val_ptr; > @@ -171,6 +177,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 +187,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_get_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_get_print_system_info(od, caps, odi); > } > > return ret; > diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c > index 0bdef33..8c069f9 100644 > --- a/drivers/scsi/osd/osd_uld.c > +++ b/drivers/scsi/osd/osd_uld.c > @@ -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,48 @@ 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 (!a2_len) /* User string is Empty means don't care */ > + return true; > + > + if (a1_len != a2_len) > + return false; > + > + return 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; > + > + sprintf(dev_str, "/dev/osd%d", i); Embedding specific forms user device paths into the kernel really looks wrong here ... why are you doing this? What happens if the user uses udev to give them different names? > + 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)) Why not NULL terminate these strings as you pick them out of the device info? Then you can dispense with the length and use standard kernel string comparisons instead of rolling your own. James ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device 2009-09-16 17:47 ` James Bottomley @ 2009-09-17 11:28 ` Boaz Harrosh 2009-09-27 8:13 ` Boaz Harrosh 0 siblings, 1 reply; 8+ messages in thread From: Boaz Harrosh @ 2009-09-17 11:28 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, open-osd On 09/16/2009 08:47 PM, James Bottomley wrote: > On Mon, 2009-09-07 at 14:26 +0300, Boaz Harrosh wrote: >> 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. >> >> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> >> --- >> drivers/scsi/osd/osd_initiator.c | 22 +++++++++++---- >> drivers/scsi/osd/osd_uld.c | 53 +++++++++++++++++++++++++++++++++++++- >> include/scsi/osd_initiator.h | 37 +++++++++++++++++++++++--- >> 3 files changed, 101 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c >> index 7a117c1..26e1b41 100644 >> --- a/drivers/scsi/osd/osd_initiator.c >> +++ b/drivers/scsi/osd/osd_initiator.c >> @@ -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_get_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,13 @@ 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; >> + if (odi->osdname_len) { >> + odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); >> + memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); >> + } else >> + odi->osdname = NULL; >> + OSD_INFO("OSD_NAME [%s]\n", odi->osdname); >> a++; >> >> pFirst = get_attrs[a++].val_ptr; >> @@ -171,6 +177,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 +187,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_get_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_get_print_system_info(od, caps, odi); >> } >> >> return ret; >> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c >> index 0bdef33..8c069f9 100644 >> --- a/drivers/scsi/osd/osd_uld.c >> +++ b/drivers/scsi/osd/osd_uld.c >> @@ -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,48 @@ 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 (!a2_len) /* User string is Empty means don't care */ >> + return true; >> + >> + if (a1_len != a2_len) >> + return false; >> + >> + return 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; >> + >> + sprintf(dev_str, "/dev/osd%d", i); > > Embedding specific forms user device paths into the kernel really looks > wrong here ... why are you doing this? What happens if the user uses > udev to give them different names? > Two things [1] These names are created here below, exactly as shown. Now udev usually creates more links and soft-links, I've never seen a delete? (I agree that maybe a comment or a define could help readability as this format string is repeated twice, though slightly different). Lets say that "udev can do anything but not delete the primary file created by Kernel", as a prerequisite. [2] I could easily just add a global device list, like all the other parts of the Kernel are trigger happy to shoot this problem. But I hate it. All these devices are already held on a Kernel structure called the name_space. The life_time rules, locking, reference counting, and visibility, is bug-free and tried out to the bone. Why should I have to reinvent the wheel? Why introduce all these subtle corner cases, and sleepless debugging nights? The way I see it Kernel keeps my devices for me and gives me a "Key" to fetch for them. The key is the name. That said. I'll do what people want, it is 5 minutes to put all these on a global list_head. It's not lack of effort on my part. >> + 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)) > > Why not NULL terminate these strings as you pick them out of the device > info? Then you can dispense with the length and use standard kernel > string comparisons instead of rolling your own. > No! these are, by osd definition, not char-strings they are large-binary signatures. The system_id is defined by scsi, a 4-byte header + 16-bytes cryptographic key. I use hex-dump to print it. It is set by the target device at the factory. The osdname is user settable variable-length binary blob. To be used as a system policy anyway chosen by the application. We are pushing, and that is what the mkexofs--format is doing to conveniently store a uuid as generated with the uuidgen utility in it's string, including "-" form. (36 asci chars). When loaded we allocate an extra null-terminator (if needed) and print it as string every-where. So when worse comes to worse and it is not a string, only the print-out is bogus. (But safe) But when actually compering it for authentication, the pNFS standard mandates binary blob exact match out. > James > > Thanks Boaz ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device 2009-09-17 11:28 ` Boaz Harrosh @ 2009-09-27 8:13 ` Boaz Harrosh 0 siblings, 0 replies; 8+ messages in thread From: Boaz Harrosh @ 2009-09-27 8:13 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, open-osd On 09/17/2009 02:28 PM, Boaz Harrosh wrote: > On 09/16/2009 08:47 PM, James Bottomley wrote: >>> + sprintf(dev_str, "/dev/osd%d", i); >> >> Embedding specific forms user device paths into the kernel really looks >> wrong here ... why are you doing this? What happens if the user uses >> udev to give them different names? >> > Two things > > [1] > These names are created here below, exactly as shown. Now udev usually > creates more links and soft-links, I've never seen a delete? > (I agree that maybe a comment or a define could help readability as this > format string is repeated twice, though slightly different). > Lets say that "udev can do anything but not delete the primary file created > by Kernel", as a prerequisite. > > [2] > I could easily just add a global device list, like all the other parts of > the Kernel are trigger happy to shoot this problem. > But I hate it. > All these devices are already held on a Kernel structure called the name_space. > The life_time rules, locking, reference counting, and visibility, is bug-free and > tried out to the bone. Why should I have to reinvent the wheel? Why introduce all > these subtle corner cases, and sleepless debugging nights? > > The way I see it Kernel keeps my devices for me and gives me a "Key" to fetch for > them. The key is the name. > > That said. I'll do what people want, it is 5 minutes to put all these on a global > list_head. It's not lack of effort on my part. > I'm resending the same patch with minor changes: 1. Same defined format-string is now used both at creation of device and at look-up, so it is clear that these must and are the same. 2. I've added a comment to osduld_info_lookup that documents it's behavior So it should be clear from the get go and there are no surprises. I'm using this code for a long time now, under a pNFS setups, and what can I say, it does the job. Also I like the fact that I can use the same code as osduld_path_lookup() and open a file handle on the device, which solved some bugs in the past. Please consider for inclusion. The other two trivial patches are independent please include those as well. (I'll post as ver2 in reply to original patch) Thanks Boaz ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3 version 2] libosd: osd_dev_info: Unique Identification of an OSD device 2009-09-07 11:26 ` [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device Boaz Harrosh 2009-09-16 17:47 ` James Bottomley @ 2009-09-27 8:14 ` Boaz Harrosh 1 sibling, 0 replies; 8+ messages in thread From: Boaz Harrosh @ 2009-09-27 8:14 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd 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. Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- drivers/scsi/osd/osd_initiator.c | 22 +++++++++--- drivers/scsi/osd/osd_uld.c | 65 ++++++++++++++++++++++++++++++++++++- include/scsi/osd_initiator.h | 37 +++++++++++++++++++-- 3 files changed, 112 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index 7a117c1..26e1b41 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -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_get_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,13 @@ 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; + if (odi->osdname_len) { + odi->osdname = kzalloc(odi->osdname_len + 1, GFP_KERNEL); + memcpy(odi->osdname, get_attrs[a].val_ptr, odi->osdname_len); + } else + odi->osdname = NULL; + OSD_INFO("OSD_NAME [%s]\n", odi->osdname); a++; pFirst = get_attrs[a++].val_ptr; @@ -171,6 +177,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 +187,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_get_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_get_print_system_info(od, caps, odi); } return ret; diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c index 0bdef33..7134257 100644 --- a/drivers/scsi/osd/osd_uld.c +++ b/drivers/scsi/osd/osd_uld.c @@ -80,6 +80,8 @@ MODULE_LICENSE("GPL"); MODULE_ALIAS_CHARDEV_MAJOR(SCSI_OSD_MAJOR); MODULE_ALIAS_SCSI_DEVICE(TYPE_OSD); +#define OSD_FMT_d "osd%d" + struct osd_uld_device { int minor; struct kref kref; @@ -87,6 +89,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 +219,56 @@ 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 (!a2_len) /* User string is Empty means don't care */ + return true; + + if (a1_len != a2_len) + return false; + + return 0 == memcmp(a1, a2, a1_len); +} + +/* osduld_info_lookup - Loop through all devices, return the requested osd_dev. + * + * if @odi->systemid_len and/or @odi->osdname_len are zero, they act as a don't + * care. .e.g if they're both zero /dev/osd0 is returned. + * We are searching for /dev/osd%d in the name-space, so user-mode who would + * like to remove a device from the search can "mv /dev/osdX /my_devs/osdX" and + * the device will not be found. + */ +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; + + sprintf(dev_str, "/dev/" OSD_FMT_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)) { + OSD_DEBUG("found device sysid_len=%d osdname=%d\n", + odi->systemid_len, odi->osdname_len); + return od; + } + osduld_put_device(od); + } + + return ERR_PTR(-ENODEV); +} +EXPORT_SYMBOL(osduld_info_lookup); + void osduld_put_device(struct osd_dev *od) { @@ -230,6 +283,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 +310,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; @@ -302,7 +362,7 @@ static int osd_probe(struct device *dev) } disk->major = SCSI_OSD_MAJOR; disk->first_minor = oud->minor; - sprintf(disk->disk_name, "osd%d", oud->minor); + sprintf(disk->disk_name, OSD_FMT_d, oud->minor); oud->disk = disk; /* hold one more reference to the scsi_device that will get released @@ -402,6 +462,7 @@ static void __remove(struct kref *kref) put_disk(oud->disk); ida_remove(&osd_minor_ida, oud->minor); + kfree(oud->odi.osdname); kfree(oud); } diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h index 02bd9f7..0d29cc3 100644 --- a/include/scsi/osd_initiator.h +++ b/include/scsi/osd_initiator.h @@ -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; -- 1.6.2.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] libosd: osd_dev_is_ver1 - Minor API cleanup 2009-09-07 11:25 osd patches for the v2.6.32 merge window Boaz Harrosh 2009-09-07 11:26 ` [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device Boaz Harrosh @ 2009-09-07 11:26 ` Boaz Harrosh 2009-09-07 11:27 ` [PATCH 3/3] libosd: osd_sense: OSD_CFO_PERMISSIONS Boaz Harrosh 2 siblings, 0 replies; 8+ messages in thread From: Boaz Harrosh @ 2009-09-07 11:26 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd define a new osd_dev_is_ver1 that operates on devices and the old osd_req_is_ver1 uses that new API. Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- include/scsi/osd_initiator.h | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h index 0d29cc3..7bda109 100644 --- a/include/scsi/osd_initiator.h +++ b/include/scsi/osd_initiator.h @@ -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); } /* -- 1.6.2.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] libosd: osd_sense: OSD_CFO_PERMISSIONS 2009-09-07 11:25 osd patches for the v2.6.32 merge window Boaz Harrosh 2009-09-07 11:26 ` [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device Boaz Harrosh 2009-09-07 11:26 ` [PATCH 2/3] libosd: osd_dev_is_ver1 - Minor API cleanup Boaz Harrosh @ 2009-09-07 11:27 ` Boaz Harrosh 2 siblings, 0 replies; 8+ messages in thread From: Boaz Harrosh @ 2009-09-07 11:27 UTC (permalink / raw) To: James Bottomley, linux-scsi, open-osd Add one more important cdb_field_offset that can be returned with scsi_invalid_field_in_cdb. It is the offset of the permissions_bit_mask field in the capabilities structure. Interestingly, the offset is the same for V1/V2 Signed-off-by: Boaz Harrosh <bharrosh@panasas.com> --- include/scsi/osd_sense.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/include/scsi/osd_sense.h b/include/scsi/osd_sense.h index ff9b33c..91db543 100644 --- a/include/scsi/osd_sense.h +++ b/include/scsi/osd_sense.h @@ -255,6 +255,9 @@ enum osdv2_cdb_field_offset { OSD_CFO_STARTING_BYTE = OSD_CDB_OFFSET(v2.start_address), OSD_CFO_PARTITION_ID = OSD_CDB_OFFSET(partition), OSD_CFO_OBJECT_ID = OSD_CDB_OFFSET(object), + OSD_CFO_PERMISSIONS = sizeof(struct osd_cdb_head) + + offsetof(struct osd_capability_head, + permissions_bit_mask), }; #endif /* ndef __OSD_SENSE_H__ */ -- 1.6.2.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-09-27 8:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-09-07 11:25 osd patches for the v2.6.32 merge window Boaz Harrosh 2009-09-07 11:26 ` [PATCH 1/3] libosd: osd_dev_info: Unique Identification of an OSD device Boaz Harrosh 2009-09-16 17:47 ` James Bottomley 2009-09-17 11:28 ` Boaz Harrosh 2009-09-27 8:13 ` Boaz Harrosh 2009-09-27 8:14 ` [PATCH 1/3 version 2] " Boaz Harrosh 2009-09-07 11:26 ` [PATCH 2/3] libosd: osd_dev_is_ver1 - Minor API cleanup Boaz Harrosh 2009-09-07 11:27 ` [PATCH 3/3] libosd: osd_sense: OSD_CFO_PERMISSIONS Boaz Harrosh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.