Hi Tim,
Thanks for the webrev.

I think the proposed change is fine, but it lacks one small part.
Because IPS is back-published to the updates for older OpenSolaris 
builds and libbe is not, we will need to check weather the new function 
is available and if it is not then we will need to disable the renaming 
functionallity.

Can we do something similar to PyGTK, so during import require the 
specific version of the libbe and if it is not met then we will disable 
the functions?

like:

 >>> import libbe
 >>> libbe.require("2")
Traceback (most recent call last):
   File "<stdin>", line 1, in ?
AttributeError: 'module' object has no attribute 'require'

At this stage we can cach the AttributeError as we are sure we are using 
old version of the libbe and we can disable the user naming functionallity.
Other options are also welcome, but we do need to check if the function 
is available before actual using it. We could do:
 >>> 'beVerifyBEName' in dir(libbe)
True

but it would be hack rather then nice solution.

Brock, as you know more about API requirements, what do you think?

best
Michal Pryc

Tim Knitter wrote:
>
> Ethan,
>
>> Tim,
>>
>> Just a couple more comments...
>>
>> BootEnvironment.py - 42 - Where is this variable used?
>>
>
> No where. I was initially going to store the max length there but used a 
> class variable instead. I removed it.
>
>> beadm.py - 193,649 - why does the '%' symbol have to be
>> passed in as a variable into the error message string?
>>
>
> It wasn't possible to escape it in the message string so I had to pass it in. 
> Python kept interpreting it to be a format character.
>
>> libbe.c - 799 - should this somehow be a boolean
>> function instead?
>>
>
> There isn't a way to pass back a true boolean. Or put another way, there 
> isn't a boolean format specifier when using Py_BuildValue(). There is the "b" 
> specifier but when used it doesn't produce a Python "True" or "False". It 
> returns an int so I left this as is.
>
> Webrev updated.
>
> Thanks
> Tim
>
>> thanks,
>> -ethan
>>
>>
>> Tim Knitter wrote:
>>>
>>> Ethan Quach wrote:
>>>>
>>>> Tim Knitter wrote:
>>>>> Hi Evan,
>>>>>
>>>>>> Hi Tim,
>>>>>>
>>>>>> I understand the reasoning behind the length limitation for the BE
>>>>>> name but I'm not clear why the description is also limited to 64
>>>>>> characters. What was the reasoning behind the description length
>>>>>> limit?
>>>>>>
>>>>> None other than having a sane limit so the text doesn't wrap in grub
>>>>> and in the boot menu (menu.lst for x86).
>>>> If there's no technical reason why we should be limiting this, I
>>>> think we should leave it alone.  What's so magical about 64 being
>>>> within limits of a wrapped-around grub title anyway?
>>>>
>>> Nothing magical maybe a little mystical though. :-) I removed
>>> description checking throughout as well as checking the length of the
>>> root dataset before zfs works on the dataset later in the code as was
>>> discussed in the snap meeting.
>>>
>>> The webrev has been updated.
>>>
>>> Thanks
>>> Tim
>>>
>>>> -ethan
>>>>
>>>>
>>>>> Thanks
>>>>> Tim
>>>>>
>>>>>> Thanks!
>>>>>> -evan
>>>>>>
>>>>>> Tim Knitter wrote:
>>>>>>> Please review the following bug fix:
>>>>>>>
>>>>>>> 5749 libbe to provide public interface to validate BE name
>>>>>>>
>>>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5749
>>>>>>> http://cr.opensolaris.org/~tsk/5749/
>>>>>>>
>>>>>>> Thanks
>>>>>>> Tim
>>>>>>> _______________________________________________
>>>>>>> caiman-discuss mailing list
>>>>>>> caiman-discuss at opensolaris.org
>>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>> _______________________________________________
>>>>> caiman-discuss mailing list
>>>>> caiman-discuss at opensolaris.org
>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss


Reply via email to