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
>>>
>>>
>>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20091216/aa97ef88/attachment.html>
