Hi Jean, Thank you for looking this thing over! My comments are in-line.
New webrev: Full: http://cr.opensolaris.org/~clayb/10740/webrev3 Diff from webrev 2: http://cr.opensolaris.org/~clayb/10740/webrev_2_3_diff/ Thank you, Clay On Thu, 20 Aug 2009, jeanm wrote: > Nit: libaiscf.py line 64: service is misspelled. Fixed > libaiscf_pymod/Makefile: copyright date should be 2009 Grr, I thought I'd gotten that but I've ensured it's changed now... > General: Please add header block comments for all new methods. Some are > there, some aren't. I added header blocks to every function now I believe > libaiscf_instance.c line 411-412: Why the empty if? Good question, it's now set to if (ret != AI_SUCCESS) with no else. > libaiscf_service.c line201 and 209: Any reason why you did if (0 != > function()) and not if (function != 0) ? > Most people will be more used to seeing the later which makes it easier to > read. Yes, it's a rather long block that we're checking though. We're checking the return, so it's easier for one to see what we're checking it against than burrying the != 0 down below. > Jean > > Clay Baenziger wrote: >> Hi all, >> Per some initial feedback from Jean and Dave I've come to round two. >> Here are a few fixes for Python objects which had reference counts which >> were wrong (memory leaks) and I've added support for specifying the FMRI >> for AISCF() objects bringing them closer to being fully general. >> >> The diffs can be found at: >> http://cr.opensolaris.org/~clayb/10740/webrev_1_2_diff/ >> Or the full webrev at: >> http://cr.opensolaris.org/~clayb/10740/webrev2/ >> >> Thank you, >> Clay >> >> On Wed, 19 Aug 2009, Clay Baenziger wrote: >> >>> Hi all, >>> I believe the code is stable, clean, builds correctly and ready for a >>> full code review. I've implemented the last necessary for the library. >>> Now, one can view an instance's state (online, offline, maintenance, etc.) >>> and one can change the state too. For example: >>>>>> libaiscf.AISCF().state >>> 'online' >>>>>> libaiscf.AISCF().state="DISABLE" >>> [wait a bit (~5 sec), or it'll return online still] >>>>>> libaiscf.AISCF().state >>> 'offline' >>> >>> Webrev now at: >>> http://cr.opensolaris.org/~clayb/10740/webrev/ >>> >>> Thank you, >>> Clay >>> >>> On Tue, 18 Aug 2009, Clay Baenziger wrote: >>> >>>> Hi all, >>>> I've written a Python<->C bridge for libaiscf. It provides a few ways >>>> to interact with SMF. I'm looking for some folks to look the code over >>>> for memory management issues, for any potential SMF issues or any other >>>> issues others might see. This code has not be whacked to be c-style >>>> happy, so don't worry about that unless you really want. >>>> I am having issues with the delete section of >>>> AIservice_set_subscript(), if anyone has any suggestions what might be >>>> happening (dbx stack trace included). This is triggered when one tries to >>>> delete a property via: >>>> del(libaiscf.AIservice(libaiscf.AISCF(),"test")['status']) >>>> >>>> Thank you, >>>> Clay >>>> >>>> Webrev is at: >>>> http://cr.opensolaris.org/~clayb/10740/webrev_prelim1/ >>>> >>>> How the module works: >>>> --------------------- >>>> To load the module, one runs >>>> import libaiscf >>>> >>>> Then to create an SMF instance object, one can run >>>> (for svc:/system/install/server:default) >>>> instance=libaiscf.AISCF() >>>> (for svc:/system/install/server:someThingElse) >>>> instance=libaiscf.AISCF("someThingElse") >>> >>> This has changed to: >>> instance=libaiscf.AISCF(instance="someThingElse") >>> >>>> Further, to create an AI service object, one runs: >>> >>> For clarification, this is just an SMF property group representation >>> >>>> service=libaiscf.AIservice(instance,"serviceName") >>>> >>>> To see a property key under the service, one can do: >>>> service['boot-dir'] (which returns the string value or a KeyError as a >>>> dictionary would if the key doesn't exist) >>>> To set or change a property under the service, one can do: >>>> serivce['boot-dir']="/var/ai/service1_image" >>>> >>>> All actions query SMF and no data is cached in case something changes >>>> under the consumer. All actions are handed to SMF when executed. Other >>>> functions implemented can be read in the PyDoc output for the module, >>>> here attached. >>> > >