William,

I expect to get to this Thursday morning mountain time USA.

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


Reply via email to