Hi Jack,
Thanks your quick response. my comments:
(1)I'm not very clear about class devscan_data, what its main function?
so far I think it's no more different with a list type. The main
scanning function was implemented by ddu_devscan(). If devscan_data only
provide like append function, I suggest just use a list but not this
additional class.
(2)Which class will in charge of store "Device compatible name" string
used by ddu_package_lookup()? I did not put it in class dev_data because
I think this class only store general device information, while "Device
compatible name" is not a general information especially for that device
which already has a working driver. "Device compatible name" will be
only useful for fetching driver information. Shall we add a new function
to generate "Device compatible name"? This function's argument can be
pci path and output is "Device compatible name".
(3)function ddu_devscan() and ddu_package_lookup() will take a little
long time when it running, also during this time these two functions
will fork some child processes. Should we provide a mechanism to stop
these functions during it running then they can kill all their child
processes? This situation usually happens in GUI environment.
Also thanks Keith's review, I agree with his comments.
Thank,
Zhongyuan
?2009?12?17??07:02?Jack Schwartz???
> Hi Zhongyuan.
>
> Thanks for your feedback. Here's my assessment of requirements, based
> on both of our emails, and a new proposal which I believe addresses
> both
> of our needs and concerns. Hopefully it is not too long...
>
> A) You need a way to easily restrict the output to a given device
> type.
> I assume this why you proposed a dictionary keyed to the device type,
> at the top level. I didn't account for that in my original proposal.
>
> B) My proposal placed more of an emphasis on readability and clarity.
> This is why I originally proposed to store a device's attributes in a
> dictionary: attributes could be extracted by name.
>
> Additional requirements, as I see things:
>
> C) We don't want to make the data structures too complicated from a
> data
> access perspective. For example a dictionary of lists of dictionaries
> makes the code hard to follow.
>
> D) We don't want to introduce unnecessary overhead. If we can use
> something more lightweight than a dictionary and still have the
> functionality we need, then I would favor not using the dictionary.
>
>
> ddu_devscan() will be returning a group of items corresponding to
> devices, so it is appropriate to use a collection of some kind for the
> top level data structure it returns. At first I agreed with you that
> a
> dictionary would be best at the top level:
> - its items are named (via a key),
> - it can filter out unwanted items,
> - it doesn't have to keep a place for items it doesn't need.
> However, after thinking more, this could prove restrictive in the
> future. We return lots of information for each device, any of which
> could potentially be used as a filter for returning data. If the
> dictionary is keyed on device type and later we decide we need to
> filter on something else (or perhaps multiple items), say vendor ID
> and/or device ID, we're not set up for that.
>
> Instead, I propose a new class, call it devscan_data, which stores the
> full list of items returned by ddu_devscan() and has a method for
> searching and returning only items of a particular device type. Then
> in the future we could add other methods to support returning items
> based on some other filter criteria. It is also lightweight and
> provides only the functionality needed.
>
> What are the devscan_data list items? They would store the different
> fields of data per device. The list could point to dictionaries as I
> had originally proposed, to tuples as you have proposed, or to
> something
> else. I have changed my mind about dictionaries, and think they are
> the
> wrong tool; the number and type of fields will not change from one
> device to the next. I also still don't like tuples as the fields are
> hard to keep track of. Named tuples are better, but more heavyweight
> than if we defined a new class to hold the data. So I propose a new
> class, which would have attributes and get() methods for each field we
> want to return. Let's call it dev_data for now.
>
> To summarize:
>
> ddu_devscan() would return a devscan_data object defined like this:
>
> class devscan_data():
>
> def __init__()
> # Holds objects of dev_data.
> self.dev_data_list = []
>
> def add_dev_data_object(dev_data):
> self.dev_data_list.append(dev_data)
>
> def get_list_by_devtype(device_type):
> # loop through instance list data to find and return matches
> on
> # device_type
>
> Lists returned by get_list_by_devtype() point to objects of type
> dev_data.
> There is one of these objects per device returned.
>
> class dev_data:
>
> def __init__(...):
> self.device_type = ...
> self.description = ...
> self.driver_name
> self.instance_ID
> self.item_ID
> self.parent_item_ID
> self.attach_status
> self.vendor_id
> self.device_id
> self.class_code
> self.pci_path
>
> def get_device_type():
> return self.device_type
>
> def get_description():
> return self.description
>
> ...
> ...
>
> def get_pci_path:
> return self.pci_path
>
> Does this work for you?
>
> Thanks,
> Jack
>
> On 12/15/09 21:56, zhongyuan sun - Sun Microsystems - Beijing China
> wrote:
> > Hi Jack,
> >
> > It's OK ddu_devscan return dictionary type instead of a list of tuples.
> > I suggest use device type as this dictionary's key and the dictionary's
> > value would be a list of tuples.
> >
> > ddu_devscan description would be as following:
> >
> > Arguments:
> > - return_missing_only: Boolean; default value is True
> > - When True, ddu_devscan returns only the devices which are missing
> > drivers.
> > - When False, all devices found are returned.
> > - device_type: Type of devices to scan for. "All" to check all.
> > - Possible types
> > are:"audio"/"battery"/"cpu"/"cd_dvd"/"memory"/"network"/"storage"/"usb"/"video"/"others"
> >
> >
> > In each tuple of the list:
> > - ItemID:
> > - Parent Item ID:
> > /*these two fields(ItemID and Parent Item ID) used for device tree to
> > identify a device and the controller it attached to. Of course
> > for a
> > controller we do not need to set its parent Item ID.
> > */
> > - Device description:Device name
> > - Device ID:
> > - Classcode:
> > - PCI physical path:This field used to get its detail information.
> > - Driver name:
> > - Instance ID:
> > - Attachment status:
> > /*These two fields(Instance ID and Attachment status) used for
> > determine whether a driver
> > has been attached abnormally.
> > */
> > - Vendor ID
> >
> >
> > Here is a short example of ddu_devscan return value:
> >
> > > > > {'Audio': [(200,"","nVidia Corporation MCP55 High Definition
> > > > >
> > Audio","0x0371","00040300","[0,6,1]","unknown",-1,"Detached","0x10de")],'Network':[(0,"","nVidia
> > Corporation MCP55
> > Ethernet","0x0373","00068000","[80,8,0]","nge",0,"Attached","0x10de"),(1,"","nVidia
> > Corporation MCP55
> > Ethernet","0x0373","00068000","[80,9,0]","nge",1,"Attached","0x10de")],'DVD':[(400,"","nVidia
> > Corporation MCP55
> > IDE","0x036e","0001018a","[0,4,0]","pci-ide",0,"Attached","0x10de"),(401,400,"MATSHITA
> > DVD-RAM UJ-85JS","","","/pci at 0,0/pci-ide at 4/ide at 0/sd at
> > 0,0","sd",0,"Attached","")]}
> >
> >
> > I also would like to discuss with you a issue during scan process. When
> > user kill GUI during scan process, some backround scripts will still be
> > running even GUI thread has been killed, these scripts was created when
> > performing scanning. Our solution was send a SIGUSR2 signal when GUI
> > want to kill its child processes. While if we wrapped our scripts into
> > ddu_devscan, how can this function send a signal to its child processes?
> > can we add a argument for this?
> >
> >
> > Thanks,
> > Zhongyuan
> >
> > ?2009?12?15??06:43?Jack Schwartz???
> >
> > > Hi Zhongyuan.
> > >
> > > Thanks for reviewing the interface spec. Moving to public forum.
> > >
> > > > I've some comments on DDU library API--ddu_devscan() function.
> > > > You've expanded this function to involve all devices found, it's
> > > > good! While for the Return value, currently it only includes
> > > > "compatible name" and "device descriptor" strings, It's not
> > > > sufficient especially for GUI part:
> > > >
> > > > *For non-mission items
> > > > (1)If the driver works fine,we need a "driver name" field.
> > > >
> > > OK
> > >
> > > > (2)If the driver dos not works fine, wee need the driver instance
> > > > ID and attachment status for this driver.
> > > >
> > > > Instance ID is a number get from system when driver attached. If
> > > > you have 2 e1000g nic cards and they all have been attached
> > > > properly, then they will get instance 0 and 1, while if the driver
> > > > have not been attached properly or device have been installed
> > > > properly, the instance ID will be <0 and attachment status will be
> > > > "detached".
> > > >
> > > > DDU GUI version used "instance ID" and attachment status to give
> > > > user a notification "driver missconfigured".
> > >
> > > OK
> > >
> > > >
> > > > BTW, for both non-missing and missing device, I hope we can add
> > > > "device category" argument to ddu_devscan function to indicate what
> > > > type of category will be looked for(like "Audio"/"Network" etc). By
> > > > default this argument can be set to "All" to look for all types of >
> > > device.
> > >
> > > OK.
> > >
> > > > For both missing and non-missing items, we also need these fields
> > > > at least:
> > > > (1)"Vendor id"/"device id"/"classcode" to identify which category
> > > > this device belongs to.
> > >
> > > OK.
> > >
> > > > (2)pci path to indicate this device's detail information.
> > > > Hopefully this can be involved into ddu_devscan function.
> > >
> > > OK.
> > >
> > > These are a lot of changes for ddu_devscan() for the GUI. I propose
> > > that ddu_devscan returns a list of dictionaries rather than a list of
> > > tuples (as before). Dictionaries because there are a lot of items
> > > returned. Also, not all fields will be used for all devices. (For
> > > example, the driver_name field won't be used when a device is missing).
> > >
> > > So this would translate into adding a signature of ddu_devscan() as
> > > follows:
> > >
> > > Arguments:
> > > - return_missing_only: Boolean; default value is True
> > > - When True, ddu_devscan returns only the devices which are missing
> > > drivers.
> > > - When False, all devices found are returned.
> > > - device_type: Type of devices to scan for. "All" to check all.
> > >
> > > ===> Please send the list of device types which are valid. "Audio" and
> > > "Network" were mentioned earlier.
> > >
> > > Returns:
> > > A list of dictionaries, each dictionary representing a device. List can
> > > be empty (possible only when Return_missing_only is True) to represent a
> > > system with no missing drivers.
> > >
> > > Items in each dictionary are:
> > >
> > > - compatible: device compatible name string, a space-separated string of
> > > all compatible names for a device. Compatible names in the string are
> > > ordered from the most specific name to the most general name (i.e. the
> > > "pciclass" definitions are listed last).
> > > - It is OK if two list items have one or more compatible
> > > names in common, but each list item should be unique.
> > >
> > > - description: Device description. can be used for error messages and
> > > status reporting.
> > >
> > > - device_type: Type of device (e.g. "Audio", "Network")
> > >
> > > - driver_name: Name of the driver operating the device. (Can be omitted
> > > if driver is missing.)
> > >
> > > - instance_ID: Instance ID assigned to the device. (Always present.
> > > Will be <0 if no driver is attached to the device.)
> > >
> > > - attach_status: Status of whether or not a driver is attached to the
> > > device. (Always present. Will be "detached" if driver is not attached,
> > > "attached" if driver is attached.
> > >
> > > ===> Any other statuses possible?
> > >
> > > - vendor_id: Device's vendor ID. (Always present.)
> > >
> > > - device_id: Device's device ID (Always present.)
> > >
> > > - class_code: Device's class code (Always present.)
> > >
> > > - pci_path: Device path (Always present.)
> > >
> > > Please let me know by today COB (PRC time) the device types and if any
> > > corrections are needed.
> > >
> > > Thanks,
> > > Jack
> > >
> > > On 12/04/09 09:20, Jack Schwartz wrote:
> > >
> > > > Hi everyone.
> > > >
> > > > FYI: Driver Update functional and programming interface specs have had
> > > > a minor update. The items addressed deal either with subtleties
> > > > missed during the main review but which came up during development, or
> > > > other things we discovered were needed during development.
> > > >
> > > > Functional spec:
> > > > - Clarify that any repo can be specified, not only repos which a
> > > > system already
> > > > knows about.
> > > > - Clarify that it is an error if a publisher name is already mapped to
> > > > a repo,
> > > > and another repo with the same publisher name is requested.
> > > > - Add publisher to AI searchall capability
> > > >
> > > > Interface spec:
> > > > - Add arg to ddu_devscan() so that the function can return either all
> > > > devices
> > > > on a system, or only devices which are missing their driver.
> > > >
> > > > Links on the Driver Update page have been updated. Previous spec
> > > > versions have been retained on that page for comparison purposes.
> > > >
> > > > Driver update page is at:
> > > > http://hub.opensolaris.org/bin/view/Project+caiman/Driver_Update
> > > >
> > > > If there are any questions or comments please send them my way.
> > > >
> > > > Thanks,
> > > > Jack
> > > >
> > > >
> > > >
> >
>
