on 2014/04/24/ 16:49, Mark Wu wrote:
On 04/23/2014 05:48 PM, Zhou Zheng Sheng wrote:
+def _xpath(xml_str): + def get_first_text(the_path): + r = xpath_get_text(xml_str, the_path) + if r == []: + return None + r = r[0] + if r is None: + return '' + return r + return get_first_text It can serve for general purpose. So do you think it's better move to utils.
Agree. Thanks.
+ + +def get_all_host_devices(libvirt_conn): Any reason to keep an argument for libvirt connection? I mean you can get the connection inside this function.
Nice suggestion, thanks. Generally I used to not use singleton and global variable in the first draft. I like having the caller insert dependencies via the parameters, rather than have the callee find the dependency. This is because if we have two libvirt connections to different remote hosts, it's better to let the caller to specify which connection to use. Currently this is not the case so it's OK to use the global singleton here.
+def _get_pci_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'domain': xpath('/device/capability/domain'), + 'bus': xpath('/device/capability/bus'), + 'slot': xpath('/device/capability/slot'), + 'function': xpath('/device/capability/function'), + 'product': { + 'id': xpath('/device/capability/product/@id'), + 'description': xpath('/device/capability/product')}, + 'vendor': { + 'id': xpath('/device/capability/vendor/@id'), + 'name': xpath('/device/capability/vendor')}} the xml path of device info can be categorized into a few patterns, then you can use helper functions to get device info for all kinds of devices. It will make the code look clean.
Get it. Thanks.
+def _get_system_dev_info(xml_str): + xpath = _xpath(xml_str) + info = { + 'hardware': { + 'vendor': xpath('/device/capability/hardware/vendor'), + 'version': xpath('/device/capability/hardware/version'), + 'serial': xpath('/device/capability/hardware/serial'), + 'uuid': xpath('/device/capability/hardware/uuid')}, + 'firmware': { + 'vendor': xpath('/device/capability/firmware/vendor'), + 'version': xpath('/device/capability/firmware/version'), + 'release_date': xpath('/device/capability/firmware/version')}} should be release_date, not version.
Nice catch! Thanks.