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.
>>>
>
>