Sarah, Sundar,
Finally got a chance to go through the design, I've got some general
comments below:
3.0:
Should hostname be listed here, since the GUI is going to prompt for
this?
4.1:
om_content_type_t enum:
- why only three types? ~or~ if this enum is used to delineate
between the Solaris and LinuxSwap parts that share the same ID,
why is Linux in here?
SliceInfo_t struct:
- the mountPoint member doesn't seem to belong in this struct.
These Disk/Partition/Slice structures all seem to hold data
describing physical attributes. Knowledge of where a slice is
mounted should be at the software or OS level. Is there a
reason why you choose to put this here?
upgradeInfo struct:
- don't we need a type specifier the specify the type of union?
- nit: zonesConfigured should be changed to zonesInstalled
- Would it make more sense to make the "whyUpgradeNotAllowed"
and "incorrectZoneList" members be integer error codes instead
string pointers?
4.1.2:
om_initiateTargetDiscovery()
- The sentence beneath the FALSE return value description is
superfluous. The fatality of the error should be a design
decision decided on by the consumer.
om_validateDiskPartition() and om_validateUpgradeTarget()
- should set string pointers instead of message id pointers.
Otherwise you'd have to expose another interface for the
consumer to get the error message:
get_<blah_type>_err_message(messageID).
om_releaseTargetData()
- does this function release the *copies* of the cache data
which were returned to the caller in the various om_get***()
functions? If so, this seems dangerous. The consumer is
allowed to make modifications to these copies that were
returned, but then a call to om_releaseTargetData() free's them.
The consumer may have handles to these objects or their members
floating around.
Would it make more sense to have om_releaseTargetData() release
just the cache data, and then provide additional free'ing
functions for the consumer to call to free their copies of the
objects?
4.2
Will upgrade analysis (space analysis) take any significant amount of
time in dwarf? i.e. would it need a progress reporting as well?
4.3.2
om_getInstallLanguages()
- Error Handling. I'm unclear as to what is being returned in
the error case. We're returning a non-NULL list of on_locale_t
objects with just the 'language' parameter set to NULL. Is the
'locale' member valid? If so, what for and why not just set the
passed 'locales' parameter to NULL for the error case?
om_getKeyboardTypes()
- the overloading of this functions return code is assuming what
the consumer is intending to do with the list of keyboard types
returned. Can we instead separate this out into two functions:
om_isSelfIDKeyboard()
om_getKeyboardTypes()
om_free()
- How do we know what type of object is being passed in to
free?
5.1
Why do we need to set dummy values for timezone, timeserver, and locale?
This indicates to me that the sysidtools are going to run before the
dwarf GUI runs. Why? The individual sysidtools don't run outside of
the GUI. The orchestrator is going to dictate which sysidtools run and
when. So we could:
a) fully populate a sysidcfg with the data gathered from the
GUI and the data from the default sysidcfg file, and then run
all the sysidtools to configure the apporiate files.
b) not run the sysidtools at all, and create new functions in
the orchestrator to configure the system with the data.
c) a mixture of both.
Sarah Jelinek wrote:
>
>
> Ethan Quach wrote:
>>
>>
>> Dave Miner wrote:
>>> Niall Power wrote:
>>>> More comments/questions after having spent more timing examining the
>>>> disk layout modelling:)
>>>>
>>>> 4.1.1:
>>>> The data model assumes that only one VTOC (DiskSlices_t) can exist
>>>> on disk. On X86, the DiskSlice_t is associated with a parition.
>>>> Will the assumption on X86 that only one partition can contain a
>>>> VTOC hold true
>>>> in the future? Will ZFS make this irrelevant?
>>>>
>>>
>>> It's likely to hold true for a while, at least; there aren't any firm
>>> plans that I'm aware of which would change this.
>>
>> But the configuration exists nonetheless. People have hacked their
>> way into creating a disk with multiple solaris fdisk partitions, each
>> with an instance of solaris installed.
>>
>> I suppose how we want to handle this in dwarf is the question. Right
>> now, the existence of two solaris fdisk partitions on a disk is
>> detectable, but only one VTOC is mapped by the driver.
>>
>> The current GUI installer doesn't handle this very gracefully (makes
>> you delete one.) Are we doing the same here in dwarf?
>
> Actually, we will be talking about just this issue in todays meeting. It
> is supposed to be only 1 Solaris fdisk partition, but as you note people
> have hacked there way in to other configurations.
>
> sarah
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://opensolaris.org/mailman/listinfo/caiman-discuss
