Great! Thanks
William Schumann wrote:
> JV,
>
> jv35168 wrote:
>> William,
>>
>> I've looked over these changes and they all look fine to me. I only
>> have a couple of miner issues.
>>
>>
>> Comments:
>>
>> Regarding the orchestrator test driver. I realize it is not being
>> used at this time. But it could be again in the future.
>>
>> Please update bug 3112 to describe how the changes to the API, which
>> you are making, will impact the liborchestrator test driver.
>>
>> "3112 liborchestrator test driver should be enhanced to address QA
>> requirements"
> Done.
>>
>> Also as you suggest, removing it from the Makefile might make sense.
> I think that in the short term we can keep it around. The test drivers
> are not being built by nightly.
>>
>> Question:
>>
>> usr/src/lib/liborchestrator/disk_slices.c
>>
>> 888 om_debug_print(OM_DBGLVL_WARN, "failed to delete slice %d
>> from table\n",
>>
>> Is it possible on line 888 that the slice was already deleted, much
>> like on line 515?
> Indeed - appropriate warnings are made elsewhere. Removed.
> Thanks,
> William
>>
>>
>>
>> Joe
>>
>>
>> William Schumann wrote:
>>> Joe,
>>> Upon testing, I noticed a problem when the Solaris partition is
>>> deleted and re-created at another location. In that case, the slice
>>> information collected at TD is no longer valid and should be
>>> invalidated. Added code in disk_slices.c to invalidate the slice
>>> information.
>>> I also decided to include the fix for 6628 installation shouldn't
>>> fail when trying to delete nonexistent slice.
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=6628
>>> I extended this to include partitions, so that if you specify a
>>> partition or slice for deletion and it is not there, only a warning
>>> is issued to the log file and the installation proceeds.
>>>
>>> Retested on SPARC and x86, deleting and recreating partitions and
>>> slices.
>>>
>>> Responses to your comments are below.
>>>
>>> Joseph J VLcek wrote:
>>>> William Schumann wrote:
>>>>> The AI manifest can specify partitions existing on a disk to be
>>>>> deleted. Currently there is only support for deleting partitions
>>>>> if you know the starting offset sector and length, which must match
>>>>> those for the partition in fdisk(1m), (which you can find from what
>>>>> 'fdisk -W - /dev/rdsk/cxtxdxp0' dumps).
>>>>>
>>>>> This provides support for deleting partitions using the manifest if
>>>>> you know only the partition ID - (1-4)
>>>>>
>>>>> Affects x86-only
>>>>>
>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5654
>>>>> http://cr.opensolaris.org/~wmsch/bug-5654/
>>>>>
>>>>> Tested:
>>>>> - deleted multiple partitions and added new partitions
>>>>> - tried to delete partition that doesn't exist
>>>>> - regression tested deleting by starting sector + length
>>>>> _______________________________________________
>>>>> caiman-discuss mailing list
>>>>> caiman-discuss at opensolaris.org
>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>
>>>> William,
>>>>
>>>> Things look good. I only have some nits...
>>>>
>>>> Hope this helps! Joe
>>>>
>>>>
>>>> lib/liborchestrator/test_driver.c
>>>> ---------------------------------
>>>>
>>>> Invocation to om_delete_partition needs to be updated to include the
>>>> new parameter.
>>> We are no longer maintaining test_driver.c officially. I have not
>>> yet removed it from the Makefile, but it is neither built or included
>>> in any packages at the moment.
>>>>
>>>> usr/src/lib/liborchestrator/disk_parts.c
>>>> ----------------------------------------
>>>>
>>>> Issue 1: to be consistent FD_NUMPART should be be OM_NUMPART on lines:
>>>>
>>>> 190 for (ipart = 0; ipart < FD_NUMPART; ipart++, pinfo++) {
>>>> 1165 for (ipart = 0; ipart < FD_NUMPART; ipart++) {
>>>> 1191 "%d (0-%d) for deletion\n", ipart, FD_NUMPART - 1);
>>>> 1194 (FD_NUMPART - ipart - 1) * sizeof (*pinfo));
>>>> 1196 set_partition_unused(&pinfo[FD_NUMPART - 1]);
>>>>
>>> Changed.
>>>> Issue 2: Question/Idea
>>>>
>>>> Wouldn't the old post-delete om_debug_print loop be valuable prior
>>>> to returning from function om_delete_partition?
>>>>
>>>> 1144 for (ip = 0; ip < OM_NUMPART; ip++)
>>>> 1145 om_debug_print(OM_DBGLVL_INFO,
>>>> 1146 "post-delete dump: part_id=%d size=%d\n",
>>>> 1147 pinfo[ip].partition_id,
>>>> 1148 pinfo[ip].partition_size);
>>>>
>>>> Making this block of code a macro could keep the code clean... Just
>>>> an idea...
>>> When you look at the log, there seems to be enough debugging
>>> information, and the complexity of the code does not warrant lots of
>>> debugging info in the log, I think.
>>>
>>> Joe,
>>> Would you please review the additional changes?
>>> Thank you,
>>> William
>>
