Hi James, Garrett

><Garret D'Amore>
>><James Carlson>
>>><Chris Horne>
>>>     For maximum flexibility, it is best if a driver does not have
>>>     compiled-in knowledge of its own name.  Existing  
>>What does "maximum flexibility" mean here?  Under what normal
>>conditions would a driver need to be installed under different names
>>... ?
>>I guess I have nothing against the interface -- it seems obvious
>>enough in design -- but the usage seems pretty obscure.
> 
> The only thing I can think of is in some weird situation, where, for 
> example, a driver or module is having something done on behalf of it in 
> some framework somewhere.
> 
> For example, something like xxx_framework_initops(struct modlinkage *).  
> It does seem of pretty limited utility, though.
> 
>>>     information.  The ddi_modname() interface was added in s10_59
>>>     as part of CR5033382 "sd driver should not depend on hard-coded
>>>     driver name". 
>>
>>That shows a debug example, where (I'd guess) someone could just as
>>easily recompile with a new name.  Are there other cases where
>>renaming on the fly makes sense?

For a developer, I think that having a driver cp(1)-clonable is easier
than requiring cloning of the driver source and changing makefiles.

Also, if a developer chooses to clone the driver source, they will
still need to avoid kstat/kmem_cache-like namespace collision. To do
this, the driver developer must find the origin of all global namespace
collisions.  Once identified, the developer needs to modify fixed
'driver_name' strings. When development is complete, and the cloned
source branch gets folded back into the original driver code base, the
developer needs to remember to change these strings back. This is
awkward.

With ddi_modname() the developer can identify the conflicts once,
switch to ddi_modname() use, and never have to deal with the issue
again.  For initialization off _init(9E), there is currently no way to
determine the driver/module name at run time: that is the problem
that ddi_modname() solves - for both source-clone development and
to enable future cp-clonable development.  After a driver is
cp-clonable, development can occur in-place (without setting up a
cloned source branch).

With a Solaris goal of 'supporting the developer', I believe that
enabling a driver to be cp-clonable makes the development of subsequent
bug fixes and enhancements easier - especially for drivers involved in
mounting the root device (where you don't want early use of development
bits to cause the machine to fail to boot).

There is also a weaker use-case for drivers, like sgen, where different
bindings can be established at different times for different purposes -
where a single 'rem_drv', when uninstalling software, will remove all
bindings. For example if one package establishes a self-identifying sgen
binding for a scanner, and another establishes a self-identifying
binding for a media changer - when a customer removes the scanner
software he also ends up deleting the alias for the media changer. The
proper way of doing this would be to add and remove aliases via
"update_drv -a/-d -i", but there is existing practice and third party
products that say "run 'rem_drv sgen'". Making a driver cp-clonable
could provide a workaround that accommodates existing practice

>>
>>>+      char *mod_modname(struct modlinkage *modlinkage);
>>>    
>>
>>Code review nit: I suggest 'const char *' as a return value here (and
>>perhaps a const argument as well).  The string this returns shouldn't
>>normally be modified by a user.  (And if it is ...)
>>  
> 
> YES PLEASE!  Its really quite annoying when folks don't use const char * 
> when they should.  Its more important, IMO, that the functions take 
> const pointers than return them.  (The fact that large portions of our 
> DDI are not const-ipated, makes it really hard to use const char * 
> elsewhere where we should.  Fixing this across the board may actually 
> gain some performance improvements due to better optimization.)

OK - I will ensure that the man page changes and implementation use
"const char *".

-Chris


Reply via email to