Comments on Dwarf Caiman Orchestrator Design Document v0.3:
4.1 : 3 VTOC info:
Is this data inteneded for consumption by the GUI? I was under the
impression that
the Dwarf Caiman GUI wouldn't involve itself with VTOC layouts, and
just use a
default VTOC layout provided by the orchestrator. I'm not suggesting
it be removed,
I just want to clarify on it's target usage/consumer.
4.1.1 om_disk_type_t data has no enumerations for the disk bus types:
SATA & IEEE1394/Firewire,
om_content_type_t has no enumerations for the content types:
Primary DOS and "Unknown"
struct diskinfo:
I see Dave has already commented on this. I think capacity types
should be unsigned.
Also, if sizes are going to be represented in MB rather than GB
then what is
the definition for GB: 1024MB or 1000MB. The GUI is going to
present sizes
in GB. Who really needs a MB level granularity with modern disk
sizes?
om_paritition_type: I can't see any definition for this data type.?
UpgradeInfo_t {
....
char *whyUpgradeNotAllowed;
....
}
whyUpgradeNotAllowed is something that will have to be presented
in the GUI as
a text message. In order to use a char * type, the Orchestrator
will need to localise
this message correctly for the installation locale. The
alternative might be an
enumeration type, which the GUI can match against it's own set
of localised strings.
4.1.2 om_initiateTargetDiscocery ()
Wouldn't it be useful to return some kind of session/discovery handle
for the consumer
to use for subsequent calls? This would serve a few purposes:
- If om_initiateTargetDiscovery () gets recalled, other discovery calls
can be associated
with the correct target discovery process and previous handles can be
invalidated if
necessary.
- more multi-thread friendly.
- Prevents accidental calling of other target discovery related
functions
(it discourages calling of these functions in cases where target
discovery has not been
initiated because it explicitly requires you to have a discovery
handle first);
om_DiskPartitionInfo (char *diskname, PartitionInfo *pinfo)
How does the caller indicate which parition ID it wants partition
info for?
The PartitionInfo structure definition is a per partition
structure.
4.2.1 In addition/combination with Dave's comments about the callback function
type:
To reclarify, the callback mechanism that both GTK and Gnome use is
based on
the Glib/Gobject type mechanism. Glib and GObject have no GUI features
or functionality,
and therefore the callback mechanism is not tied to the GNOME desktop,
or even
X-windows. The same callback mechanism could equally be used by a
command line
installer.
4.3.1 om_locale_t: if you are using a linked list, you might be interested in
Glib's pre built
list types (single and double linked) and accessor/modifier functions:
http://developer.gnome.org/doc/API/2.0/glib/glib-Singly-Linked-Lists.html
http://developer.gnome.org/doc/API/2.0/glib/glib-Doubly-Linked-Lists.html
This also applies more generally to the usage of linked list structures
in the design
document.
4.3.2 void om_getKeyBoardTypes (char **kbd_types):
- A return type of void prevents a return value for the caller
(OM_SUCCESS, etc.)
- if a NULL list is returned - which "English" is the default
layout: english-US,
english-UK? I'm guessing english-US right?
4.4 om_perform_install()
The additional data provided to the GUI's registred callback function
via it's function
parameters needs to be specified. I know.. this is TBD :)
5.1 Sysid tools: network-interface
- Shoulnd't this be DHCP instead of non-networked? At least for the
case where there
is a working NIC detected....
Other stuff:
I don't see an API for specifying the system timezone for the target
installation.
The function naming conventions seem a bit mixed/confusing when trying
to
identify namespaces. Just underscores gets my vote:
eg. om_initiate_target_discovery () instead of
om_initiateTargetDiscovery () or
OmInitiateTargetDiscovery.
I think underscores make the function names easier to read than upper
cases
personally.
Thanks! :)
Niall.
--
This message posted from opensolaris.org
