From ah89892@sac.sfbay.sun.com Sun Mar 8 21:30:27 2009 Received: from sunmail4.singapore.sun.com (sunmail4.Singapore.Sun.COM [129.158.71.19]) by sac.sfbay.sun.com (8.13.8+Sun/8.13.8) with ESMTP id n294UQjh014756 for ; Sun, 8 Mar 2009 21:30:27 -0700 (PDT) Received: from nwk-avmta-1.SFBay.Sun.COM (nwk-avmta-1.SFBay.Sun.COM [129.146.11.74]) by sunmail4.singapore.sun.com (8.13.4+Sun/8.13.3/ENSMAIL,v2.2) with ESMTP id n294UNBu010570; Mon, 9 Mar 2009 12:30:25 +0800 (SGT) Received: from pmxchannel-daemon.nwk-avmta-1.sfbay.Sun.COM by nwk-avmta-1.sfbay.Sun.COM (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) id <0KG8007031UPC000@nwk-avmta-1.sfbay.Sun.COM>; Sun, 08 Mar 2009 21:30:25 -0700 (PDT) Received: from dm-sfbay-01.sfbay.sun.com ([129.145.155.118]) by nwk-avmta-1.sfbay.Sun.COM (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) with ESMTP id <0KG800HVM1UOYPB0@nwk-avmta-1.sfbay.Sun.COM>; Sun, 08 Mar 2009 21:30:24 -0700 (PDT) Received: from sac.sfbay.sun.com (sac.SFBay.Sun.COM [129.146.226.132]) by dm-sfbay-01.sfbay.sun.com (8.13.8+Sun/8.13.8/ENSMAIL,v2.2) with ESMTP id n294UOkw033730; Sun, 08 Mar 2009 21:30:24 -0700 (PDT) Received: from sac.sfbay.sun.com (localhost [127.0.0.1]) by sac.sfbay.sun.com (8.13.8+Sun/8.13.8) with ESMTP id n294UN0r014751; Sun, 08 Mar 2009 21:30:23 -0700 (PDT) Received: (from ah89892@localhost) by sac.sfbay.sun.com (8.13.8+Sun/8.13.8/Submit) id n294UNhF014747; Sun, 08 Mar 2009 21:30:23 -0700 (PDT) Date: Sun, 08 Mar 2009 21:30:23 -0700 (PDT) From: Alan Hargreaves Subject: Specification update for Pluggable fwflash [PSARC/2009/163 FastTrack timeout 03/16/2009] To: PSARC-ext@sun.com Cc: fwflash-ie@sun.com Message-id: <200903090430.n294UNhF014747@sac.sfbay.sun.com> Content-transfer-encoding: 7BIT X-PMX-Version: 5.4.1.325704 Status: RO Content-Length: 7061 I am sponsoring this Update Fast Track for behalf of James McPherson. Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI This information is Copyright 2009 Sun Microsystems 1. Introduction 1.1. Project/Component Working Name: Specification update for Pluggable fwflash 1.2. Name of Document Author/Supplier: Author: James McPherson 1.3 Date of This Document: 08 March, 2009 4. Technical Description Title ----- Specification update for Pluggable fwflash(1M) PSARC/2008/151 Problem ------- PSARC/2008/151 introduced Pluggable fwflash(1M), and defined a plugin architecture to make supporting new hardware a straight-forward task. It has become apparent as new plugins are defined that there are some limitations in the existing specification, which requires updating. Limitation #1 is that the identification plugin does not have a means of managing its own memory. This opens the door to plugin memory leaks, and core dumps if memory needs to be allocated inside the plugin rather than depending on an external library which provides its own memory management. Limitation #2 is that when we have many different possible VENDOR types reported for a driver (eg, SEAGATE, HITACHI, SAMSUNG, IBM....), then we could create a link farm in /usr/lib/fwflash/verify for every possible name, but this is tiresome and requires continual maintenance as new devices or vendors are supported. While writing a plugin to support flashing disks attached via sd(7D), we discovered that there are some SATA disks which report their vendor property inconsistently with others. For example, note the inquiry-vendor-id for this Seagate 320Gb SATA disk: name='inquiry-product-id' type=string items=1 value='ST3320620AS' name='inquiry-device-type' type=int items=1 value=00000000 name='inquiry-revision-id' type=string items=1 value='3.AAK' name='inquiry-vendor-id' type=string items=1 value='ST3320620AS' name='pm-capable' type=int items=1 value=00000001 name='sas-mpt' type=boolean name='compatible' type=string items=5 value='scsiclass,00.vATA.pST3320620AS.rK' + 'scsiclass,00.vATA.pST3320620AS' + 'scsa,00.bmpt' + 'scsiclass,00' + 'scsiclass' Compared with a Samsung 320Gb SATA disk: name='inquiry-revision-id' type=string items=1 value='CP100-12' name='inquiry-product-id' type=string items=1 value='HD321KJ' name='inquiry-vendor-id' type=string items=1 value='SAMSUNG' name='pm-capable' type=int items=1 value=00000001 name='guid' type=string items=1 value='50000f001bb01248' name='sas-mpt' type=boolean name='port-wwn' type=byte items=8 value=48.12.b0.1b.00.0f.00.50 name='target-port' type=string items=1 value='4812b01b000f0050' name='compatible' type=string items=5 value='scsiclass,00.vATA.pSAMSUNG_HD321KJ.r0-12' + 'scsiclass,00.vATA.pSAMSUNG_HD321KJ' + 'scsa,00.bmpt' + 'scsiclass,00' + 'scsiclass' Also note the inquiry-revision-id property, which in both cases differs from what is specified in the first 'compatible' property. Just using libdevinfo properties to identify these devices results in reporting information which is not as correct as possible, and which may confuse the user: Device[0] /dev/dsk/c4t5000CCA00510A7CCd0s2 Class [sd] Vendor : HITACHI Product : HUS1514SBSUN146G Firmware revision : SA02 Inquiry Serial Number : 000742B94YPC J4V94YPC GUID : 5000cca00510a7cc ... Device[5] /dev/dsk/c3t5d0s2 Class [sd] Vendor : SAMSUNG Product : HD321KJ Firmware revision : CP100-12 Inquiry Serial Number : S0MQJ1KPB01248 GUID : 50000f001bb01248 ... Device[7] /dev/dsk/c3t7d0s2 Class [sd] Vendor : ST3320620AS Product : ST3320620AS Firmware revision : 3.AAK Inquiry Serial Number : 6QF1N2MN GUID : (not supported) Allowing the identification plugin to manage its own memory allows us to report more correct information for such devices, based on the 'compatible' property: Device[5] /dev/dsk/c3t5d0s2 Class [sd] Vendor : SAMSUNG Product : HD321KJ Firmware revision : 0-12 Inquiry Serial Number : S0MQJ1KPB01248 GUID : 50000f001bb01248 ... Device[7] /dev/dsk/c3t7d0s2 Class [sd] Vendor : SEAGATE Product : ST3320620AS Firmware revision : K Inquiry Serial Number : 6QF1N2MN GUID : (not supported) Proposed solution ----------------- The proposal has two elements: #1. Allow verification plugins to be named in the form drivername-GENERIC.so which will be used as a fallback filename option in the verification loader. (For example, "sd-GENERIC.so"). This removes the need for a link farm in /usr/lib/fwflash/verify and decreases the maintenance burden. #2. Add two definitions to fwflash.h. Firstly #define FWPLUGIN_VERSION_2 2 and if an identification plugin defines unsigned int plugin_version = FWPLUGIN_VERSION_2; in its struct fw_plugin definition, then the fwflash cleanup function will invoke the plugin cleanup function at the appropriate point: int (*fw_cleanup)(struct devicelist *thisdev); It is an error condition for an identification plugin to define plugin_version >= FWPLUGIN_VERSION_2, and to not also define the fw_cleanup() function. Diffs for are found below. Stability classifications ------------------------- The precursor to this case, PSARC/2008/151 Pluggable fwflash(1m), indicated that struct fw_plugin was Committed. This classification remains current. Manpage changes --------------- No manpage changes are required for this change. diffs ------------------------- --- a/usr/src/cmd/fwflash/common/fwflash.h Tue Oct 14 20:37:29 2008 -0700 +++ b/usr/src/cmd/fwflash/common/fwflash.h Wed Mar 04 17:44:22 2009 +1000 @@ -19,7 +19,7 @@ * CDDL HEADER END */ /* - * Copyright 2008 Sun Microsystems, Inc. All rights reserved. + * Copyright 2009 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ @@ -57,7 +57,8 @@ */ #define FWPLUGIN_VERSION_1 1 - +#define FWPLUGIN_VERSION_2 2 + struct devicelist; struct fw_plugin { @@ -120,6 +121,16 @@ * All identification plugins must support this operation. */ int (*fw_devinfo)(struct devicelist *thisdev); + + /* + * Function entry point to allow the plugin to clean up its + * data structure use IF plugin_version == FWPLUGIN_VERSION_2. + * + * If this function is not defined in the plugin, that is not + * an error condition unless the plugin_version variable is + * defined. + */ + void (*fw_cleanup)(struct devicelist *thisdev); }; 6. Resources and Schedule 6.4. Steering Committee requested information 6.4.1. Consolidation C-team Name: ON 6.5. ARC review type: FastTrack 6.6. ARC Exposure: open From carlsonj@phorcys.east.sun.com Mon Mar 9 06:38:11 2009 Received: from newsunmail1brm.central.sun.com (newsunmail1brm.Central.Sun.COM [129.147.62.245]) by sac.sfbay.sun.com (8.13.8+Sun/8.13.8) with ESMTP id n29DcBZq007463 for ; Mon, 9 Mar 2009 06:38:11 -0700 (PDT) Received: from nwk-avmta-2.sfbay.sun.com (nwk-avmta-2.SFBay.Sun.COM [129.145.155.6]) by newsunmail1brm.central.sun.com (8.13.7+Sun/8.13.7/ENSMAIL,v2.2) with ESMTP id n29DcAqv020999; Mon, 9 Mar 2009 07:38:10 -0600 (MDT) Received: from pmxchannel-daemon.nwk-avmta-2.sfbay.sun.com by nwk-avmta-2.sfbay.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) id <0KG800I0NR7MNV00@nwk-avmta-2.sfbay.sun.com>; Mon, 09 Mar 2009 06:38:10 -0700 (PDT) Received: from phorcys.east.sun.com ([129.148.174.143]) by nwk-avmta-2.sfbay.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) with ESMTP id <0KG8006AIR7KWLD0@nwk-avmta-2.sfbay.sun.com>; Mon, 09 Mar 2009 06:38:09 -0700 (PDT) Received: from phorcys.east.sun.com (localhost [127.0.0.1]) by phorcys.east.sun.com (8.14.3+Sun/8.14.3) with ESMTP id n29DbwqL017274; Mon, 09 Mar 2009 09:37:58 -0400 (EDT) Received: (from carlsonj@localhost) by phorcys.east.sun.com (8.14.3+Sun/8.14.3/Submit) id n29DbwJm017271; Mon, 09 Mar 2009 09:37:58 -0400 (EDT) Date: Mon, 09 Mar 2009 09:37:58 -0400 From: James Carlson Subject: Re: Specification update for Pluggable fwflash [PSARC/2009/163 FastTrack timeout 03/16/2009] In-reply-to: <200903090430.n294UNhF014747@sac.sfbay.sun.com> To: Alan Hargreaves Cc: PSARC-ext@sun.com, fwflash-ie@sun.com Message-id: <18869.7094.644618.757253@gargle.gargle.HOWL> MIME-version: 1.0 X-Mailer: VM 7.01 under Emacs 21.3.1 Content-type: text/plain; charset=us-ascii Content-transfer-encoding: 7BIT X-PMX-Version: 5.4.1.325704 References: <200903090430.n294UNhF014747@sac.sfbay.sun.com> Status: RO Content-Length: 2002 Alan Hargreaves writes: > and if an identification plugin defines > > unsigned int plugin_version = FWPLUGIN_VERSION_2; > > in its struct fw_plugin definition, then the fwflash cleanup function I don't think I understand that. First of all (as a nit), 2008/151 didn't include versioning, so it'd be good to describe how it works here. Reading the existing header file, it seems that plugins must define a uint32_t variable (not a struct) that has the name "fwplugin_version", and that's what's used for the versioning, not "plugin_version" as above. Is that correct, or are there other changes here? Does the integer change to a structure? Is there a new variable? I would have expected just a new #define. > int (*fw_cleanup)(struct devicelist *thisdev); > > It is an error condition for an identification plugin to define > plugin_version >= FWPLUGIN_VERSION_2, and to not also define the > fw_cleanup() function. One tiny nit here: I think you'd probably be better off making it so that plugins simply use FWPLUGIN_VERSION_CURRENT, and the framework calls fw_cleanup if the version is >=2 and the function pointer is non-NULL. That way, old plugins will correctly inform the framework that they've been compiled with an old version of this structure (and thus the new fw_cleanup member is located one word off the end) by saying version 1, and new plugins that don't need to clean anything up won't be forced into providing a dummy function. (The real issue here is that structures like this should be allocated by the framework so that you don't have to bump a version number when new members are allocated. The versioning needs to be tied with the allocation of space for that structure. But I guess it's too late to fix that ...) -- James Carlson, Solaris Networking Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677 From James.McPherson@sun.com Tue Mar 10 17:06:04 2009 Received: from sunmail4.singapore.sun.com (sunmail4.Singapore.Sun.COM [129.158.71.19]) by sac.sfbay.sun.com (8.13.8+Sun/8.13.8) with ESMTP id n2B063sk021691 for ; Tue, 10 Mar 2009 17:06:03 -0700 (PDT) Received: from brm-avmta-1.central.sun.com (brm-avmta-1.Central.Sun.COM [129.147.4.11]) by sunmail4.singapore.sun.com (8.13.4+Sun/8.13.3/ENSMAIL,v2.2) with ESMTP id n2B05n7X005930; Wed, 11 Mar 2009 08:06:02 +0800 (SGT) Received: from pmxchannel-daemon.brm-avmta-1.central.sun.com by brm-avmta-1.central.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) id <0KGB00203EY25V00@brm-avmta-1.central.sun.com>; Tue, 10 Mar 2009 18:06:02 -0600 (MDT) Received: from sineb-mail-2.sun.com ([192.18.19.7]) by brm-avmta-1.central.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) with ESMTP id <0KGB00CAVEXZHD70@brm-avmta-1.central.sun.com>; Tue, 10 Mar 2009 18:06:00 -0600 (MDT) Received: from fe-apac-05.sun.com (fe-apac-05.sun.com [192.18.19.176] (may be forged)) by sineb-mail-2.sun.com (8.13.6+Sun/8.12.9) with ESMTP id n2B05waX008461; Wed, 11 Mar 2009 00:05:58 +0000 (GMT) Received: from conversion-daemon.mail-apac.sun.com by mail-apac.sun.com (Sun Java(tm) System Messaging Server 7.0-3.01 64bit (built Dec 23 2008)) id <0KGB00L00EQ7YK00@mail-apac.sun.com>; Wed, 11 Mar 2009 08:05:57 +0800 (SGT) Received: from blinder ([unknown] [220.157.71.44]) by mail-apac.sun.com (Sun Java(tm) System Messaging Server 7.0-3.01 64bit (built Dec 23 2008)) with ESMTPSA id <0KGB00LCXEXN4X90@mail-apac.sun.com>; Wed, 11 Mar 2009 08:05:49 +0800 (SGT) Date: Wed, 11 Mar 2009 10:05:43 +1000 From: "James C. McPherson" Subject: Re: Specification update for Pluggable fwflash [PSARC/2009/163 FastTrack timeout 03/16/2009] In-reply-to: <18869.7094.644618.757253@gargle.gargle.HOWL> Sender: James.McPherson@sun.com To: James Carlson Cc: Alan Hargreaves , PSARC-ext@sun.com, fwflash-ie@sun.com Message-id: <20090311100543.00000965@blinder> Organization: Sun Microsystems MIME-version: 1.0 X-Mailer: Claws Mail 3.7.0 (GTK+ 2.14.5; i386-pc-solaris2.11) Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7BIT X-PMX-Version: 5.4.1.325704 References: <200903090430.n294UNhF014747@sac.sfbay.sun.com> <18869.7094.644618.757253@gargle.gargle.HOWL> Status: RO Content-Length: 3356 Hi James, On Mon, 09 Mar 2009 09:37:58 -0400 James Carlson wrote: > Alan Hargreaves writes: > > and if an identification plugin defines > > > > unsigned int plugin_version = FWPLUGIN_VERSION_2; > > > > in its struct fw_plugin definition, then the fwflash cleanup function > > I don't think I understand that. First of all (as a nit), 2008/151 > didn't include versioning, so it'd be good to describe how it works > here. I didn't worry about versioning with 2008/151 - and I should have thought further when designing it. Benefit of hindsight :| The idea is that when a new structure element (for any of the existing structures) or a new structure is defined, we would bump the version number, so $SRC/cmd/fwflash/common/fwflash.c would be able to deal with the change in an appropriate fashion. Existing plugins would not need to be redelivered because their behaviour would not change. > Reading the existing header file, it seems that plugins must define a > uint32_t variable (not a struct) that has the name "fwplugin_version", > and that's what's used for the versioning, not "plugin_version" as > above. > Is that correct, or are there other changes here? Does the integer > change to a structure? Is there a new variable? Reviewing the existing header file, I see that there are extraneous comments about versioning left over from a very early version of the original spec. The existing comments are wrong, not only are plugins not required to define any version-related element in any structure, but even if they did (now), we have no way of using that information. We are proposing to add the integer plugin_version (or fwplugin_version, I'm not wedded to the exact name). There is no new structure. > I would have expected just a new #define. > > > int (*fw_cleanup)(struct devicelist *thisdev); > > > > It is an error condition for an identification plugin to define > > plugin_version >= FWPLUGIN_VERSION_2, and to not also define the > > fw_cleanup() function. > > One tiny nit here: I think you'd probably be better off making it so > that plugins simply use FWPLUGIN_VERSION_CURRENT, and the framework > calls fw_cleanup if the version is >=2 and the function pointer is > non-NULL. > That way, old plugins will correctly inform the framework that they've > been compiled with an old version of this structure (and thus the new > fw_cleanup member is located one word off the end) by saying version > 1, and new plugins that don't need to clean anything up won't be > forced into providing a dummy function. For the teardown part, that's pretty much how I've got it implemented (except for the #define name) in my prototype. > (The real issue here is that structures like this should be allocated > by the framework so that you don't have to bump a version number when > new members are allocated. The versioning needs to be tied with the > allocation of space for that structure. But I guess it's too late to > fix that ...) Yes, I agree. Again, benefit of hindsight. For that matter, if I was designing Pluggable fwflash all over again I'd skip use of the header and just use libnvpair instead. James C. McPherson -- Senior Kernel Software Engineer, Solaris Sun Microsystems http://blogs.sun.com/jmcp http://www.jmcp.homeunix.com/blog From carlsonj@phorcys.east.sun.com Wed Mar 11 06:24:46 2009 Received: from sunmail3mpk.sfbay.sun.com (sunmail3mpk.SFBay.Sun.COM [129.146.11.52]) by sac.sfbay.sun.com (8.13.8+Sun/8.13.8) with ESMTP id n2BDOkon028923 for ; Wed, 11 Mar 2009 06:24:46 -0700 (PDT) Received: from brm-avmta-1.central.sun.com (brm-avmta-1.Central.Sun.COM [129.147.4.11]) by sunmail3mpk.sfbay.sun.com (8.13.7+Sun/8.13.7/ENSMAIL,v2.2) with ESMTP id n2BDOF2O016637; Wed, 11 Mar 2009 06:24:43 -0700 (PDT) Received: from pmxchannel-daemon.brm-avmta-1.central.sun.com by brm-avmta-1.central.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) id <0KGC00EV3FX74E00@brm-avmta-1.central.sun.com>; Wed, 11 Mar 2009 07:24:43 -0600 (MDT) Received: from phorcys.east.sun.com ([129.148.174.143]) by brm-avmta-1.central.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) with ESMTP id <0KGC003B2FX0PN80@brm-avmta-1.central.sun.com>; Wed, 11 Mar 2009 07:24:37 -0600 (MDT) Received: from phorcys.east.sun.com (localhost [127.0.0.1]) by phorcys.east.sun.com (8.14.3+Sun/8.14.3) with ESMTP id n2BDOZoE023113; Wed, 11 Mar 2009 09:24:35 -0400 (EDT) Received: (from carlsonj@localhost) by phorcys.east.sun.com (8.14.3+Sun/8.14.3/Submit) id n2BDOYuG023110; Wed, 11 Mar 2009 09:24:34 -0400 (EDT) Date: Wed, 11 Mar 2009 09:24:34 -0400 From: James Carlson Subject: Re: Specification update for Pluggable fwflash [PSARC/2009/163 FastTrack timeout 03/16/2009] In-reply-to: <20090311100543.00000965@blinder> To: "James C. McPherson" Cc: Alan Hargreaves , PSARC-ext@sun.com, fwflash-ie@sun.com Message-id: <18871.48018.936083.925765@gargle.gargle.HOWL> MIME-version: 1.0 X-Mailer: VM 7.01 under Emacs 21.3.1 Content-type: text/plain; charset=us-ascii Content-transfer-encoding: 7BIT X-PMX-Version: 5.4.1.325704 References: <200903090430.n294UNhF014747@sac.sfbay.sun.com> <18869.7094.644618.757253@gargle.gargle.HOWL> <20090311100543.00000965@blinder> Status: RO Content-Length: 2268 James C. McPherson writes: > > Reading the existing header file, it seems that plugins must define a > > uint32_t variable (not a struct) that has the name "fwplugin_version", > > and that's what's used for the versioning, not "plugin_version" as > > above. > > Is that correct, or are there other changes here? Does the integer > > change to a structure? Is there a new variable? > > Reviewing the existing header file, I see that there are > extraneous comments about versioning left over from a very > early version of the original spec. Ah, ok. I was trying to match up this proposal with what I thought was implemented in that file ... that's obviously a mistake. > The existing comments are wrong, not only are plugins not required > to define any version-related element in any structure, but even > if they did (now), we have no way of using that information. > > We are proposing to add the integer plugin_version (or fwplugin_version, > I'm not wedded to the exact name). There is no new structure. OK. In that case, if there's no existing code, I don't care what you use. The "plugin_version" you proposed is fine. > For the teardown part, that's pretty much how I've got it implemented > (except for the #define name) in my prototype. The differences are: - I'm suggesting it's not an error to leave out [leave as NULL] fw_cleanup in a newly compiled plug-in. (The spec here seems to say it is an error, but perhaps it's actually referring to truncating the structure in some way -- which seems like it should be a logical impossibility, and impossible to detect as well.) - Having a neutrally-named #define means you can get source level compatibility. (That is, you can recompile on a new system, get whatever structure is in the header file on that system, and get a properly matching version number for the new structure without having to change your source.) They're attempts to smooth out future changes, but they're minor nits, and regardless of the direction you go, +1. -- James Carlson, Solaris Networking Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677 From Alan.Hargreaves@sun.com Wed Mar 11 16:19:18 2009 Received: from sunmail2sca.sfbay.sun.com (sunmail2sca.SFBay.Sun.COM [129.145.155.234]) by sac.sfbay.sun.com (8.13.8+Sun/8.13.8) with ESMTP id n2BNJIth017290 for ; Wed, 11 Mar 2009 16:19:18 -0700 (PDT) Received: from brm-avmta-1.central.sun.com (brm-avmta-1.Central.Sun.COM [129.147.4.11]) by sunmail2sca.sfbay.sun.com (8.13.7+Sun/8.13.7/ENSMAIL,v2.2) with ESMTP id n2BNIqjO015330; Wed, 11 Mar 2009 16:19:15 -0700 (PDT) Received: from pmxchannel-daemon.brm-avmta-1.central.sun.com by brm-avmta-1.central.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) id <0KGD0000H7FRCO00@brm-avmta-1.central.sun.com>; Wed, 11 Mar 2009 17:19:03 -0600 (MDT) Received: from sineb-mail-2.sun.com ([192.18.19.7]) by brm-avmta-1.central.sun.com (Sun Java System Messaging Server 6.2-3.04 (built Jul 15 2005)) with ESMTP id <0KGD005SL7FPTME0@brm-avmta-1.central.sun.com>; Wed, 11 Mar 2009 17:19:02 -0600 (MDT) Received: from fe-apac-06.sun.com (fe-apac-06.sun.com [192.18.19.177] (may be forged)) by sineb-mail-2.sun.com (8.13.6+Sun/8.12.9) with ESMTP id n2BNJ1EE008414; Wed, 11 Mar 2009 23:19:01 +0000 (GMT) Received: from conversion-daemon.mail-apac.sun.com by mail-apac.sun.com (Sun Java(tm) System Messaging Server 7.0-3.01 64bit (built Dec 23 2008)) id <0KGD000007FHI000@mail-apac.sun.com>; Thu, 12 Mar 2009 07:19:01 +0800 (SGT) Received: from [129.158.12.60] ([unknown] [129.158.12.60]) by mail-apac.sun.com (Sun Java(tm) System Messaging Server 7.0-3.01 64bit (built Dec 23 2008)) with ESMTPSA id <0KGD00CE77FF54D0@mail-apac.sun.com>; Thu, 12 Mar 2009 07:19:00 +0800 (SGT) Date: Thu, 12 Mar 2009 10:18:51 +1100 From: Alan Hargreaves Subject: Re: Specification update for Pluggable fwflash [PSARC/2009/163 FastTrack timeout 03/16/2009] In-reply-to: <200903090430.n294UNhF014747@sac.sfbay.sun.com> Sender: Alan.Hargreaves@sun.com To: Alan Hargreaves Cc: PSARC-ext@sun.com, fwflash-ie@sun.com Message-id: <49B846DB.9050108@Sun.COM> MIME-version: 1.0 Content-type: text/plain; format=flowed; charset=ISO-8859-1 Content-transfer-encoding: 7BIT X-PMX-Version: 5.4.1.325704 References: <200903090430.n294UNhF014747@sac.sfbay.sun.com> User-Agent: Thunderbird 2.0.0.18 (X11/20090209) Status: RO Content-Length: 7772 This case was approved at the meeting on March 11. Update the IAM file to reflect this. alan. Alan Hargreaves wrote: > I am sponsoring this Update Fast Track for behalf of James McPherson. > > Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI > This information is Copyright 2009 Sun Microsystems > 1. Introduction > 1.1. Project/Component Working Name: > Specification update for Pluggable fwflash > 1.2. Name of Document Author/Supplier: > Author: James McPherson > 1.3 Date of This Document: > 08 March, 2009 > 4. Technical Description > > Title > ----- > > Specification update for Pluggable fwflash(1M) PSARC/2008/151 > > Problem > ------- > > PSARC/2008/151 introduced Pluggable fwflash(1M), and defined a plugin > architecture to make supporting new hardware a straight-forward task. > > It has become apparent as new plugins are defined that there are some > limitations in the existing specification, which requires updating. > > > Limitation #1 is that the identification plugin does not have a means > of managing its own memory. This opens the door to plugin memory leaks, > and core dumps if memory needs to be allocated inside the plugin rather > than depending on an external library which provides its own memory > management. > > Limitation #2 is that when we have many different possible VENDOR types > reported for a driver (eg, SEAGATE, HITACHI, SAMSUNG, IBM....), then > we could create a link farm in /usr/lib/fwflash/verify for every possible > name, but this is tiresome and requires continual maintenance as new devices > or vendors are supported. > > While writing a plugin to support flashing disks attached via sd(7D), we > discovered that there are some SATA disks which report their vendor > property inconsistently with others. > > > For example, note the inquiry-vendor-id for this Seagate 320Gb SATA disk: > > > name='inquiry-product-id' type=string items=1 > value='ST3320620AS' > name='inquiry-device-type' type=int items=1 > value=00000000 > name='inquiry-revision-id' type=string items=1 > value='3.AAK' > name='inquiry-vendor-id' type=string items=1 > value='ST3320620AS' > name='pm-capable' type=int items=1 > value=00000001 > name='sas-mpt' type=boolean > name='compatible' type=string items=5 > value='scsiclass,00.vATA.pST3320620AS.rK' + > 'scsiclass,00.vATA.pST3320620AS' + > 'scsa,00.bmpt' + 'scsiclass,00' + 'scsiclass' > > > Compared with a Samsung 320Gb SATA disk: > > > name='inquiry-revision-id' type=string items=1 > value='CP100-12' > name='inquiry-product-id' type=string items=1 > value='HD321KJ' > name='inquiry-vendor-id' type=string items=1 > value='SAMSUNG' > name='pm-capable' type=int items=1 > value=00000001 > name='guid' type=string items=1 > value='50000f001bb01248' > name='sas-mpt' type=boolean > name='port-wwn' type=byte items=8 > value=48.12.b0.1b.00.0f.00.50 > name='target-port' type=string items=1 > value='4812b01b000f0050' > name='compatible' type=string items=5 > value='scsiclass,00.vATA.pSAMSUNG_HD321KJ.r0-12' + > 'scsiclass,00.vATA.pSAMSUNG_HD321KJ' + > 'scsa,00.bmpt' + 'scsiclass,00' + 'scsiclass' > > > Also note the inquiry-revision-id property, which in both cases differs from > what is specified in the first 'compatible' property. > > Just using libdevinfo properties to identify these devices results in > reporting information which is not as correct as possible, and which may > confuse the user: > > Device[0] /dev/dsk/c4t5000CCA00510A7CCd0s2 > Class [sd] > Vendor : HITACHI > Product : HUS1514SBSUN146G > Firmware revision : SA02 > Inquiry Serial Number : 000742B94YPC J4V94YPC > GUID : 5000cca00510a7cc > > ... > Device[5] /dev/dsk/c3t5d0s2 > Class [sd] > Vendor : SAMSUNG > Product : HD321KJ > Firmware revision : CP100-12 > Inquiry Serial Number : S0MQJ1KPB01248 > GUID : 50000f001bb01248 > > ... > Device[7] /dev/dsk/c3t7d0s2 > Class [sd] > Vendor : ST3320620AS > Product : ST3320620AS > Firmware revision : 3.AAK > Inquiry Serial Number : 6QF1N2MN > GUID : (not supported) > > > Allowing the identification plugin to manage its own memory allows us > to report more correct information for such devices, based on the > 'compatible' property: > > > Device[5] /dev/dsk/c3t5d0s2 > Class [sd] > Vendor : SAMSUNG > Product : HD321KJ > Firmware revision : 0-12 > Inquiry Serial Number : S0MQJ1KPB01248 > GUID : 50000f001bb01248 > > ... > Device[7] /dev/dsk/c3t7d0s2 > Class [sd] > Vendor : SEAGATE > Product : ST3320620AS > Firmware revision : K > Inquiry Serial Number : 6QF1N2MN > GUID : (not supported) > > > > > > Proposed solution > ----------------- > > The proposal has two elements: > > > #1. Allow verification plugins to be named in the form > > drivername-GENERIC.so > > which will be used as a fallback filename option in the > verification loader. (For example, "sd-GENERIC.so"). This removes > the need for a link farm in /usr/lib/fwflash/verify and decreases > the maintenance burden. > > > > #2. Add two definitions to fwflash.h. Firstly > > #define FWPLUGIN_VERSION_2 2 > > and if an identification plugin defines > > unsigned int plugin_version = FWPLUGIN_VERSION_2; > > in its struct fw_plugin definition, then the fwflash cleanup function > will invoke the plugin cleanup function at the appropriate point: > > int (*fw_cleanup)(struct devicelist *thisdev); > > It is an error condition for an identification plugin to define > plugin_version >= FWPLUGIN_VERSION_2, and to not also define the > fw_cleanup() function. > > > Diffs for are found below. > > > > Stability classifications > ------------------------- > > The precursor to this case, PSARC/2008/151 Pluggable fwflash(1m), indicated > that struct fw_plugin was Committed. This classification remains current. > > > > > > Manpage changes > --------------- > > No manpage changes are required for this change. > > > > diffs > ------------------------- > > > --- a/usr/src/cmd/fwflash/common/fwflash.h Tue Oct 14 20:37:29 2008 -0700 > +++ b/usr/src/cmd/fwflash/common/fwflash.h Wed Mar 04 17:44:22 2009 +1000 > @@ -19,7 +19,7 @@ > * CDDL HEADER END > */ > /* > - * Copyright 2008 Sun Microsystems, Inc. All rights reserved. > + * Copyright 2009 Sun Microsystems, Inc. All rights reserved. > * Use is subject to license terms. > */ > > @@ -57,7 +57,8 @@ > */ > > #define FWPLUGIN_VERSION_1 1 > - > +#define FWPLUGIN_VERSION_2 2 > + > struct devicelist; > > struct fw_plugin { > @@ -120,6 +121,16 @@ > * All identification plugins must support this operation. > */ > int (*fw_devinfo)(struct devicelist *thisdev); > + > + /* > + * Function entry point to allow the plugin to clean up its > + * data structure use IF plugin_version == FWPLUGIN_VERSION_2. > + * > + * If this function is not defined in the plugin, that is not > + * an error condition unless the plugin_version variable is > + * defined. > + */ > + void (*fw_cleanup)(struct devicelist *thisdev); > }; > > > > > 6. Resources and Schedule > 6.4. Steering Committee requested information > 6.4.1. Consolidation C-team Name: > ON > 6.5. ARC review type: FastTrack > 6.6. ARC Exposure: open > > -- Alan Hargreaves - http://blogs.sun.com/tpenta Staff Engineer (Kernel/VOSJEC/Performance) Asia Pacific/Emerging Markets Sun Microsystems