Hi William,
these additional changes look good to me.
Thank you,
Jan
On 02/18/09 18:57, William Schumann wrote:
> 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
>>