Jan,
Upon retesting, I noticed a problem in recalculating the length of the 
1st partition if the user has requested it to start at sector 0.  The 
first cylinder of the first partition is used for control information, 
and the Orchestrator automatically moves the partition start to the 2nd 
cylinder and reduces the size of the partition by one cylinder.  
Roundoff error was occurring when the cylinder size in MB was being 
used.  Code change is in 
disk_parts.c:om_validate_and_resize_disk_partitions().  Patch provided 
below.

I also decided to take your advice and make the free space table to work 
if it hits the case of free space between all of the maximum number of 
partitions - as a future precautionary measure.

Retested a variety of partitions of different types and locations.
Retested GUI with and without existing DOS partitions varying Solaris 
partition size.

Patch for changes to disk_parts.c since code review 1st issued:
47c47
< static struct free_region free_space_table[OM_NUMPART];
---
 > static struct free_region free_space_table[OM_NUMPART + 1];
701a702,708
 >             /*
 >              * user changed partition size in GUI or size
 >              * was adjusted above.
 >              * Calculate new sector size information from megabytes
 >              */
 >
 >             om_set_part_sec_size_from_mb(p_new);
702a710
 >
711a720,721
 >                 p_new->partition_offset =
 >                     dt->dinfo.disk_cyl_size/BLOCKS_TO_MB;
712a723,724
 >                 p_new->partition_size_sec -=
 >                     dt->dinfo.disk_cyl_size;
714a727,731
 >                 om_debug_print(OM_DBGLVL_INFO,
 >                     "%d (%02X) is the first partition - will "
 >                     "start at the 2nd cylinder (sector %lld)\n",
 >                     i, p_new->partition_type,
 >                     p_new->partition_offset_sec);
719,726c736,737
<          * user changed partition size in GUI or size
<          * was adjusted above.
<          * Calculate new sector size information from megabytes
<          */
<
<         om_set_part_sec_size_from_mb(p_new);
<
<         /*
---
 >          * For the GUI:
 >          *
739c750
<         if (partition_allocation_scheme != AI_allocation) {
---
 >         if (partition_allocation_scheme == GUI_allocation) {
1850,1851c1861,1863
<     /* should not happen despite that fragments could outnumber parts */
<     if (n_fragments >= OM_NUMPART)
---
 >     /* should never exceed table size */
 >     if (n_fragments >=
 >         sizeof (free_space_table) / sizeof (free_space_table[0]))
Thanks again, Jan.
William

jan damborsky wrote:
> William,
>
> thank you for making those changes.
> They look good now.
>
> Jan
>
>
> On 02/13/09 17:18, William Schumann wrote:
>> Jan,
>>
>> jan damborsky wrote:
>>> Hi William,
>>>
>>>
>>> On 02/13/09 15:36, William Schumann wrote:
>>>> New Automated Installer code allows creating partitions of 
>>>> different types.
>>>>
>>>> Place partition type value in element "partition_type".
>>>>
>>>> Allows "FAT32", "DOS16", "SOLARIS", "OPENSOLARIS" or any numeric 
>>>> type specified in fdisk.h
>>>
>>> I am not sure, we should distinguish between SOLARIS and OPENSOLARIS
>>> partition, since those both create partition with 0xbf ID. Maybe we
>>> should use only SOLARIS which is more generic.
>> Removed.
>>>
>>> disk_part.c
>>> -----------
>>> pinfo->content_type is being set according to the partition
>>> ID to be created. I am not sure, if we could assume this.
>>> The reason is that 'content_type' should reflect what the
>>> partition actually contains and should be realized by looking
>>> into the partition.
>>> Since only ID is to be created and no other actions are taken,
>>> I think that 'content_type' should be set to OM_CTYPE_UNKNOWN.
>> It appears that the content type is only relevant (at the moment) as 
>> input from Target Discovery for existing partitions.  I only set it 
>> in om_create_partition() to be consistent.  I will change the type to 
>> OM_CTYPE_UNKNOWN for non-Solaris, but leave OM_CTYPE_SOLARIS for 
>> Solaris partitions.
>>
>> William
>

Reply via email to