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

Reply via email to