)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"da2d689045d28044ef3c0abfaf415601776a2e3e","unresolved":false,"context_lines":[{"line_number":10,"context_line":"has the address \"virtio-mmio\", not \"PCI\" or \"Drive\", so it can\u0027t"},{"line_number":11,"context_line":"be assigned a object such as \"LibvirtConfigGuestDeviceAddres**\"."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Close-Bug: #1598370"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: I10004c2060ff8f5a60e065b237311fe5b9fd1876"},{"line_number":16,"context_line":"Signed-off-by: Kevin Zhao \u003ckevin.zhao@linaro.org\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1aa78d24_87995571","line":13,"range":{"start_line":13,"start_character":0,"end_line":13,"end_character":5},"updated":"2016-07-04 11:40:51.000000000","message":"nit: That should be \"Closes\" to notify Launchpad correctly about the status of this bug.","commit_id":"b6bb845008aa9a39fec6f1ed0e15de9fec8bf261"}],"nova/tests/unit/virt/libvirt/test_config.py":[{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"11ffd2de30ffad2e4e8e807dce10c4292c10de9f","unresolved":false,"context_lines":[{"line_number":1700,"context_line":""},{"line_number":1701,"context_line":"        self.assertIsInstance(obj.device_addr,"},{"line_number":1702,"context_line":"                              config.LibvirtConfigGuestDeviceAddress)"},{"line_number":1703,"context_line":"        self.assertEqual(\u0027virtio-mmio\u0027, obj.device_addr.type)"},{"line_number":1704,"context_line":""},{"line_number":1705,"context_line":""},{"line_number":1706,"context_line":"class LibvirtConfigGuestFeatureTest(LibvirtConfigBaseTest):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_e0ad9532","line":1703,"updated":"2016-07-04 15:36:09.000000000","message":"I also think that there should be a tests that verifies that \n_build_device_metadata in the driver is working properly with the change.\nI think you can add the device to this test: \nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_driver.py#L1580","commit_id":"3edd43e795416e63fc9f56a3fdcb1438a3053df5"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"aaf25241e61fa6a87192e15c99ea89e421b5b4c6","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"                                                obj.device_addr.unit))"},{"line_number":1003,"context_line":"        self.assertIsNone(obj.device_addr.format_address())"},{"line_number":1004,"context_line":""},{"line_number":1005,"context_line":"    def test_config_disk_device_address_type_virtio_mmio(self):"},{"line_number":1006,"context_line":"        xml \u003d \"\"\""},{"line_number":1007,"context_line":"            \u003cdisk type\u003d\u0027file\u0027 device\u003d\u0027disk\u0027\u003e"},{"line_number":1008,"context_line":"                \u003cdriver name\u003d\u0027qemu\u0027 type\u003d\u0027qcow2\u0027 cache\u003d\u0027none\u0027/\u003e"},{"line_number":1009,"context_line":"                \u003csource file\u003d\u0027/var/lib/libvirt/images/generic.qcow2\u0027/\u003e"}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_e1ff33ff","line":1006,"range":{"start_line":1005,"start_character":1,"end_line":1006,"end_character":17},"updated":"2016-07-19 13:11:03.000000000","message":"Add test case here, I am not sure whether it is redundant. Since it is almost the same as the interface","commit_id":"c29f1f7a11c933ea5d3f874adb97b639eb3206a6"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"aaf25241e61fa6a87192e15c99ea89e421b5b4c6","unresolved":false,"context_lines":[{"line_number":1611,"context_line":"              \u003caddress type\u003d\u0027pci\u0027 domain\u003d\u00270x0000\u0027 bus\u003d\u00270x00\u0027 slot\u003d\u00270x09\u0027"},{"line_number":1612,"context_line":"              function\u003d\u00270x0\u0027/\u003e"},{"line_number":1613,"context_line":"            \u003c/disk\u003e"},{"line_number":1614,"context_line":"            \u003cdisk type\u003d\u0027file\u0027 device\u003d\u0027disk\u0027\u003e"},{"line_number":1615,"context_line":"                \u003cdriver name\u003d\u0027qemu\u0027 type\u003d\u0027qcow2\u0027 cache\u003d\u0027none\u0027/\u003e"},{"line_number":1616,"context_line":"                \u003csource file\u003d\u0027/var/lib/libvirt/images/generic.qcow2\u0027/\u003e"},{"line_number":1617,"context_line":"                \u003ctarget dev\u003d\u0027vdb\u0027 bus\u003d\u0027virtio\u0027/\u003e"},{"line_number":1618,"context_line":"                \u003caddress type\u003d\u0027virtio-mmio\u0027/\u003e"},{"line_number":1619,"context_line":"            \u003c/disk\u003e"},{"line_number":1620,"context_line":"            \u003cdisk type\u003d\u0027file\u0027 device\u003d\u0027disk\u0027\u003e"},{"line_number":1621,"context_line":"                \u003cdriver name\u003d\u0027qemu\u0027 type\u003d\u0027qcow2\u0027/\u003e"},{"line_number":1622,"context_line":"                \u003csource file\u003d\u0027/var/lib/libvirt/images/test.qcow2\u0027/\u003e"},{"line_number":1623,"context_line":"                \u003cbackingStore/\u003e"},{"line_number":1624,"context_line":"                \u003ctarget dev\u003d\u0027vdc\u0027 bus\u003d\u0027virtio\u0027/\u003e"},{"line_number":1625,"context_line":"                \u003calias name\u003d\u0027virtio-disk1\u0027/\u003e"},{"line_number":1626,"context_line":"                \u003caddress type\u003d\u0027ccw\u0027 cssid\u003d\u00270xfe\u0027 ssid\u003d\u00270x0\u0027 devno\u003d\u00270x0000\u0027/\u003e"},{"line_number":1627,"context_line":"            \u003c/disk\u003e"},{"line_number":1628,"context_line":"            \u003cinterface type\u003d\u0027network\u0027\u003e"},{"line_number":1629,"context_line":"              \u003cmac address\u003d\u002752:54:00:f6:35:8f\u0027/\u003e"},{"line_number":1630,"context_line":"              \u003csource network\u003d\u0027default\u0027/\u003e"}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_5e26dece","line":1627,"range":{"start_line":1614,"start_character":0,"end_line":1627,"end_character":19},"updated":"2016-07-19 13:11:03.000000000","message":"Add test case here, I am not sure whether it is redundant.Since it is almost the same as the interface","commit_id":"c29f1f7a11c933ea5d3f874adb97b639eb3206a6"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e83bedd3abf2cd655c6e2ea45a68589d5f721d66","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                            self.vif_outbound_peak \u003d int(sub.get(\u0027peak\u0027))"},{"line_number":1400,"context_line":"            elif c.tag \u003d\u003d \u0027address\u0027:"},{"line_number":1401,"context_line":"                obj \u003d LibvirtConfigGuestDeviceAddress.factory(c)"},{"line_number":1402,"context_line":"                if obj is not None:"},{"line_number":1403,"context_line":"                    obj.parse_dom(c)"},{"line_number":1404,"context_line":"                    self.device_addr \u003d obj"},{"line_number":1405,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_5056fcd2","line":1402,"range":{"start_line":1402,"start_character":0,"end_line":1402,"end_character":35},"updated":"2016-07-04 13:48:05.000000000","message":"I don\u0027t think this is not the right way to go.\nI think it would be better for the factory to return a default object that will return None from parse_dom and \nformat_address.\nOtherwise, if device_addr is None, we will fail here:\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7524\n\nAlso, it won\u0027t be enough just to handle the address here, you will need to fix the same on line 895 as well. So I think having a default object will be a better approach.","commit_id":"109a53782168053fd75ede007e2f718acb027656"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"955e8bf154b264686cdecaeeee03795d36b7fe9d","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"                            self.vif_outbound_peak \u003d int(sub.get(\u0027peak\u0027))"},{"line_number":1400,"context_line":"            elif c.tag \u003d\u003d \u0027address\u0027:"},{"line_number":1401,"context_line":"                obj \u003d LibvirtConfigGuestDeviceAddress.factory(c)"},{"line_number":1402,"context_line":"                if obj is not None:"},{"line_number":1403,"context_line":"                    obj.parse_dom(c)"},{"line_number":1404,"context_line":"                    self.device_addr \u003d obj"},{"line_number":1405,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_708df830","line":1402,"range":{"start_line":1402,"start_character":0,"end_line":1402,"end_character":35},"in_reply_to":"1aa78d24_5056fcd2","updated":"2016-07-04 13:52:11.000000000","message":"Maybe, to make it look like:\nhttps://paste.fedoraproject.org/387843/40282146/","commit_id":"109a53782168053fd75ede007e2f718acb027656"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"11ffd2de30ffad2e4e8e807dce10c4292c10de9f","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        else:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddress(addr_type)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_ad49c68d","line":1156,"range":{"start_line":1156,"start_character":19,"end_line":1156,"end_character":50},"updated":"2016-07-04 15:36:09.000000000","message":"I think you still need to have a format_address() method in the LibvirtConfigGuestDeviceAddress class,\notherwise this will break, no? https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7524","commit_id":"3edd43e795416e63fc9f56a3fdcb1438a3053df5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7006c2503f51140727661669c80ac0c3a53c5484","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        else:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddress(addr_type)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":"    def format_address(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_445e2213","line":1155,"range":{"start_line":1155,"start_character":8,"end_line":1155,"end_character":13},"updated":"2016-07-05 08:54:33.000000000","message":"Should this not check for \"virtio-mmio\"?","commit_id":"1b966fd4f02e47ebeb9d0e6ea157348ef11d80c3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"239b6c07256a3542b21843e955b3a6db3c3d639c","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        else:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddress(addr_type)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":"    def format_address(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_103c46c7","line":1155,"range":{"start_line":1155,"start_character":8,"end_line":1155,"end_character":13},"in_reply_to":"1aa78d24_445e2213","updated":"2016-07-05 13:23:06.000000000","message":"I don\u0027t think we should do it in this patch.\nThis patch should just introduce a default class that is not PCI or Drive type. Overall, at this stage we can only support PCI devices, disk or network, so we just need to reply with an object that return None for format_address and parse_dom()\nLater you can add a specific support for anything else.","commit_id":"1b966fd4f02e47ebeb9d0e6ea157348ef11d80c3"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e724fd30a25185d5c8cfdc66528624ca8aa4277d","unresolved":false,"context_lines":[{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddress(addr_type)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":"    def format_address(self):"},{"line_number":1159,"context_line":"        return None"},{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":""},{"line_number":1162,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_125bf82f","line":1159,"updated":"2016-07-08 09:16:59.000000000","message":"This is wrong - we shouldn\u0027t be adding a dummy address object here if the XML does not contain an address.  If you want to support the mmio address type, then create a suitable subclass for it","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"dc9eb577576aa5a897089cc240d9c983a68622d4","unresolved":false,"context_lines":[{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddress(addr_type)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":"    def format_address(self):"},{"line_number":1159,"context_line":"        return None"},{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":""},{"line_number":1162,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_51728bae","line":1159,"in_reply_to":"1aa78d24_125bf82f","updated":"2016-07-08 15:11:02.000000000","message":"Hi Daniel,\n   First thanks for your attention to my patch :-) \n   For virtio-mmio, the xml is : \n\u003caddress type\u003d\u0027virtio-mmio\u0027/\u003e.\n so does it means that I shouldn\u0027t use an address object and  here just return None? Or I should add a subclass just like :LibvirtConfigGuestDeviceAddressVirtiommio?","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"8995d50a2858e8d69aec302902a83f1ac46b862a","unresolved":false,"context_lines":[{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddress(addr_type)"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":"    def format_address(self):"},{"line_number":1159,"context_line":"        return None"},{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":""},{"line_number":1162,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_224acac5","line":1159,"in_reply_to":"1aa78d24_51728bae","updated":"2016-07-11 13:52:35.000000000","message":"Done","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"d8efb931d04a14d7cde365a05b69dcd80b00a063","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddressVirtiommio()"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_9e49d362","line":1156,"updated":"2016-07-13 15:05:17.000000000","message":"This will still fail on libvirt-kvm on s390x architecture. We use the \"ccw\" type, like in this domain XML snippet:\n\n    \u003cinterface type\u003d\u0027bridge\u0027\u003e\n      \u003cmac address\u003d\u0027fa:10:3e:14:af:af\u0027/\u003e\n      \u003csource bridge\u003d\u0027br-int\u0027/\u003e\n      \u003cvirtualport type\u003d\u0027openvswitch\u0027\u003e\n        \u003cparameters interfaceid\u003d\u00271edbb893-06ac-4990-bed4-c5ca4edb8e34\u0027/\u003e\n      \u003c/virtualport\u003e\n      \u003ctarget dev\u003d\u0027tap1edbb478-11\u0027/\u003e\n      \u003cmodel type\u003d\u0027virtio\u0027/\u003e\n      \u003calias name\u003d\u0027net0\u0027/\u003e\n      \u003caddress type\u003d\u0027ccw\u0027 cssid\u003d\u00270xfe\u0027 ssid\u003d\u00270x0\u0027 devno\u003d\u00270x0001\u0027/\u003e\n    \u003c/interface\u003e\n\nI\u0027m going to have a better look at this tomorrow.","commit_id":"bbe1b16892caa0724b377600ece4d9c7e5846ef1"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"d3992dabc80564500262c5605a8099ae3df52a27","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddressVirtiommio()"},{"line_number":1157,"context_line":""},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_f71d9562","line":1156,"in_reply_to":"1aa78d24_9e49d362","updated":"2016-07-13 17:04:55.000000000","message":"Hi Markus, \n   Sorry I am not sure about s390x address type . I was a Blue IBMer last year and I have worked on s390x of virt-manager last year by chance :-) Today I have got a xml file of s390x from an IBMer friend ,the xml of interface is like this:\n   \u003cinterface type\u003d\u0027network\u0027\u003e\n      \u003cmac address\u003d\u002752:54:00:14:6f:50\u0027/\u003e\n      \u003csource network\u003d\u0027default\u0027 bridge\u003d\u0027virbr0\u0027/\u003e\n      \u003ctarget dev\u003d\u0027vnet0\u0027/\u003e\n      \u003cmodel type\u003d\u0027virtio\u0027/\u003e\n      \u003calias name\u003d\u0027net0\u0027/\u003e\n      \u003caddress type\u003d\u0027ccw\u0027 cssid\u003d\u00270xfe\u0027 ssid\u003d\u00270x0\u0027 devno\u003d\u00270x0001\u0027/\u003e\n    \u003c/interface\u003e\n     \n     I can see the address type is ccw. So I add a subclass for ccw. Since it is generated by libvirt, we don\u0027t need to assign an address manually just like PCI, so I have deal with it just like virtio-mmio.\n     But now I don\u0027t have a s390x machine,so maybe you can test this patch if it can works ,thanks~","commit_id":"bbe1b16892caa0724b377600ece4d9c7e5846ef1"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"2359e64a936396d38fc359cf3ea87f414817395d","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"                __init__(type\u003d\u0027virtio-mmio\u0027, **kwargs)"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    def format_address(self):"},{"line_number":1210,"context_line":"            return None"},{"line_number":1211,"context_line":""},{"line_number":1212,"context_line":""},{"line_number":1213,"context_line":"class LibvirtConfigGuestDeviceAddressCcw \\"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_f07bbff2","line":1210,"range":{"start_line":1210,"start_character":8,"end_line":1210,"end_character":12},"updated":"2016-07-14 08:00:40.000000000","message":"I think these spaces should be removed. It\u0027s odd that pep8 doesn\u0027t complain about this.","commit_id":"39fa02ccc7e16168511c4fa7fbb518efb2cde40d"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"9c4463999608c71990b37d3c87e9641cd9c23f8f","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"                __init__(type\u003d\u0027virtio-mmio\u0027, **kwargs)"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    def format_address(self):"},{"line_number":1210,"context_line":"            return None"},{"line_number":1211,"context_line":""},{"line_number":1212,"context_line":""},{"line_number":1213,"context_line":"class LibvirtConfigGuestDeviceAddressCcw \\"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_40762907","line":1210,"range":{"start_line":1210,"start_character":8,"end_line":1210,"end_character":12},"in_reply_to":"1aa78d24_f07bbff2","updated":"2016-07-14 16:54:21.000000000","message":"Thanks markus_z.\nI have removed it now :-)","commit_id":"39fa02ccc7e16168511c4fa7fbb518efb2cde40d"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"f35238c83ed75e98f82966ae67e8953f34f76a2b","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddressVirtiommio()"},{"line_number":1157,"context_line":"        elif addr_type \u003d\u003d \u0027ccw\u0027:"},{"line_number":1158,"context_line":"            return LibvirtConfigGuestDeviceAddressCcw()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_fbc87b77","line":1158,"range":{"start_line":1155,"start_character":8,"end_line":1158,"end_character":55},"updated":"2016-07-15 12:05:33.000000000","message":"That\u0027s exactly the reason why I just wanted to return a default object for anything we don\u0027t know how to present it\u0027s address. It\u0027s a bit weird to have multiple classes that do nothing and we will probably add more of these later..","commit_id":"4a4cade29d042bb8baa6ec04c25961ef79eea066"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"7513f5b24779916c5dc00d84c1eecbdde8b79358","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddressVirtiommio()"},{"line_number":1157,"context_line":"        elif addr_type \u003d\u003d \u0027ccw\u0027:"},{"line_number":1158,"context_line":"            return LibvirtConfigGuestDeviceAddressCcw()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_25f873e5","line":1158,"range":{"start_line":1155,"start_character":8,"end_line":1158,"end_character":55},"in_reply_to":"1aa78d24_fbc87b77","updated":"2016-07-18 13:25:11.000000000","message":"If possible I\u0027d like to get this working solution merged and make it clean with a follow up patch. The bug which gets fixed by this blocks our (not yet public) CI.","commit_id":"4a4cade29d042bb8baa6ec04c25961ef79eea066"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2e578677ba934815a87c20c9f2903250d6ea094b","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddressVirtiommio()"},{"line_number":1157,"context_line":"        elif addr_type \u003d\u003d \u0027ccw\u0027:"},{"line_number":1158,"context_line":"            return LibvirtConfigGuestDeviceAddressCcw()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_bac75944","line":1158,"range":{"start_line":1155,"start_character":8,"end_line":1158,"end_character":55},"in_reply_to":"dada55a8_25f873e5","updated":"2016-07-18 15:39:30.000000000","message":"If we\u0027re not going to bother parsing the address, this should just return None and the callers should handle that. Creating dummy subclasses which do nothing (as below) is just wrong\n\nWe should fix the places which call factory() to cope with a return value of None. Only add support for extra device types here, if you\u0027re actually going to do it properly","commit_id":"4a4cade29d042bb8baa6ec04c25961ef79eea066"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"0f52d3b1fb020ff3335bac0897cb7b718e732375","unresolved":false,"context_lines":[{"line_number":1152,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1153,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1154,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1155,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027:"},{"line_number":1156,"context_line":"            return LibvirtConfigGuestDeviceAddressVirtiommio()"},{"line_number":1157,"context_line":"        elif addr_type \u003d\u003d \u0027ccw\u0027:"},{"line_number":1158,"context_line":"            return LibvirtConfigGuestDeviceAddressCcw()"},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":""},{"line_number":1161,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_de2c8e4c","line":1158,"range":{"start_line":1155,"start_character":8,"end_line":1158,"end_character":55},"in_reply_to":"dada55a8_bac75944","updated":"2016-07-19 13:06:25.000000000","message":"Hi Daniel,\n   Follow your advice , I have modified here , remove the dummy subclass. Today I also find that for the disk device which has the virtio-mmio address type also meet the problem as the interface device, so I also add the fixing for disk device in Patch set 12.","commit_id":"4a4cade29d042bb8baa6ec04c25961ef79eea066"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2e578677ba934815a87c20c9f2903250d6ea094b","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"                __init__(type\u003d\u0027virtio-mmio\u0027, **kwargs)"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    def format_address(self):"},{"line_number":1210,"context_line":"        return None"},{"line_number":1211,"context_line":""},{"line_number":1212,"context_line":""},{"line_number":1213,"context_line":"class LibvirtConfigGuestDeviceAddressCcw \\"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_5a8f3d23","line":1210,"range":{"start_line":1210,"start_character":15,"end_line":1210,"end_character":19},"updated":"2016-07-18 15:39:30.000000000","message":"This is bogus - its not actually parsing or formatting the addresses","commit_id":"4a4cade29d042bb8baa6ec04c25961ef79eea066"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"0f52d3b1fb020ff3335bac0897cb7b718e732375","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"                __init__(type\u003d\u0027virtio-mmio\u0027, **kwargs)"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":"    def format_address(self):"},{"line_number":1210,"context_line":"        return None"},{"line_number":1211,"context_line":""},{"line_number":1212,"context_line":""},{"line_number":1213,"context_line":"class LibvirtConfigGuestDeviceAddressCcw \\"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_fe299238","line":1210,"range":{"start_line":1210,"start_character":15,"end_line":1210,"end_character":19},"in_reply_to":"dada55a8_5a8f3d23","updated":"2016-07-19 13:06:25.000000000","message":"Thanks~ I have removed here.","commit_id":"4a4cade29d042bb8baa6ec04c25961ef79eea066"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"aaf25241e61fa6a87192e15c99ea89e421b5b4c6","unresolved":false,"context_lines":[{"line_number":893,"context_line":"                self.shareable \u003d True"},{"line_number":894,"context_line":"            elif c.tag \u003d\u003d \u0027address\u0027:"},{"line_number":895,"context_line":"                obj \u003d LibvirtConfigGuestDeviceAddress.factory(c)"},{"line_number":896,"context_line":"                if obj is not None:"},{"line_number":897,"context_line":"                    obj.parse_dom(c)"},{"line_number":898,"context_line":"                    self.device_addr \u003d obj"},{"line_number":899,"context_line":"            elif c.tag \u003d\u003d \u0027boot\u0027:"},{"line_number":900,"context_line":"                self.boot_order \u003d c.get(\u0027order\u0027)"},{"line_number":901,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_3ec68a49","line":898,"range":{"start_line":896,"start_character":9,"end_line":898,"end_character":42},"updated":"2016-07-19 13:11:03.000000000","message":"The virtio disk in AArch64 also met the problem. Also I think s390x which has ccw met too. So I add the code here.","commit_id":"c29f1f7a11c933ea5d3f874adb97b639eb3206a6"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"aaf25241e61fa6a87192e15c99ea89e421b5b4c6","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1154,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1155,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1156,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027 or addr_type \u003d\u003d \u0027ccw\u0027:"},{"line_number":1157,"context_line":"            return None"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_7ed762ed","line":1157,"range":{"start_line":1156,"start_character":0,"end_line":1157,"end_character":23},"updated":"2016-07-19 13:11:03.000000000","message":"Return None if the addr_type is in virtio-mmio or ccw","commit_id":"c29f1f7a11c933ea5d3f874adb97b639eb3206a6"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"072a725f1c16befabe607bb6b18b007e53af7960","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1154,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1155,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1156,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027 or addr_type \u003d\u003d \u0027ccw\u0027:"},{"line_number":1157,"context_line":"            return None"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_815e3710","line":1157,"range":{"start_line":1156,"start_character":0,"end_line":1157,"end_character":23},"in_reply_to":"dada55a8_7ed762ed","updated":"2016-07-19 13:13:06.000000000","message":"This last \u0027elif\u0027 is pointless - just change it to \u0027else\u0027","commit_id":"c29f1f7a11c933ea5d3f874adb97b639eb3206a6"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"d09abe4f4143b381f5f6c01f6ded0c10939c6110","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"            return LibvirtConfigGuestDeviceAddressPCI()"},{"line_number":1154,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1155,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1156,"context_line":"        elif addr_type \u003d\u003d \u0027virtio-mmio\u0027 or addr_type \u003d\u003d \u0027ccw\u0027:"},{"line_number":1157,"context_line":"            return None"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":12,"id":"dada55a8_1c0aae21","line":1157,"range":{"start_line":1156,"start_character":0,"end_line":1157,"end_character":23},"in_reply_to":"dada55a8_815e3710","updated":"2016-07-19 13:24:37.000000000","message":"Thanks Daniel ~ I will modify now","commit_id":"c29f1f7a11c933ea5d3f874adb97b639eb3206a6"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"1f50a4cce7b1478ee054a08d71e3e3915966ff12","unresolved":false,"context_lines":[{"line_number":1154,"context_line":"        elif addr_type \u003d\u003d \u0027drive\u0027:"},{"line_number":1155,"context_line":"            return LibvirtConfigGuestDeviceAddressDrive()"},{"line_number":1156,"context_line":"        else:"},{"line_number":1157,"context_line":"            return None"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":""},{"line_number":1160,"context_line":"class LibvirtConfigGuestDeviceAddressDrive(LibvirtConfigGuestDeviceAddress):"}],"source_content_type":"text/x-python","patch_set":13,"id":"dada55a8_37d7cdc0","line":1157,"updated":"2016-07-19 13:48:48.000000000","message":"Looking at this again, I think it\u0027d be  better if this method actually called \u0027parse_dom\u0027, since we\u0027re already passing the dom in.\n\nThis would mean that callers would not need to check the value - they can just blindly assign it to dev_addr.","commit_id":"5be30f16721cc6dfd1916346dfec30e5f30cc825"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"239b6c07256a3542b21843e955b3a6db3c3d639c","unresolved":false,"context_lines":[{"line_number":7566,"context_line":"                vif \u003d tagged_vifs.get(dev.mac_addr)"},{"line_number":7567,"context_line":"                if not vif:"},{"line_number":7568,"context_line":"                    continue"},{"line_number":7569,"context_line":"                if isinstance(dev.device_addr,"},{"line_number":7570,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI) or \\"},{"line_number":7571,"context_line":"                        isinstance(dev.device_addr,"},{"line_number":7572,"context_line":"                            vconfig.LibvirtConfigGuestDeviceAddressDrive):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_0d043148","line":7569,"range":{"start_line":7569,"start_character":19,"end_line":7569,"end_character":29},"updated":"2016-07-05 13:23:06.000000000","message":"This is what _prepare_device_bus() should settle.\n\nI think, that all you need instead, is just to add the bus after initialization:\n\ndevice \u003d objects.NetworkInterfaceMetadata(\n                        mac\u003dvif.address,\n                        tags\u003d[vif.tag]\n                    )\nif bus:\n    device.bus \u003d bus","commit_id":"1b966fd4f02e47ebeb9d0e6ea157348ef11d80c3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"560d7be55f80294d6f7fb6ddc0270461422951eb","unresolved":false,"context_lines":[{"line_number":7526,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7527,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7528,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7529,"context_line":"        elif hasattr(dev, \u0027target_bus\u0027):"},{"line_number":7530,"context_line":"            if dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7531,"context_line":"                bus \u003d objects.SCSIDeviceBus()"},{"line_number":7532,"context_line":"            elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_1f6e0595","line":7529,"range":{"start_line":7529,"start_character":13,"end_line":7529,"end_character":40},"updated":"2016-07-05 17:14:49.000000000","message":"I would check the object type instead:\nelif isinstance(dev.device_addr,\n                             vconfig.LibvirtConfigGuestDeviceAddressDrive):","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"c899662d2a77225e09696270727b4637630b8897","unresolved":false,"context_lines":[{"line_number":7526,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7527,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7528,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7529,"context_line":"        elif hasattr(dev, \u0027target_bus\u0027):"},{"line_number":7530,"context_line":"            if dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7531,"context_line":"                bus \u003d objects.SCSIDeviceBus()"},{"line_number":7532,"context_line":"            elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_6a38173e","line":7529,"range":{"start_line":7529,"start_character":13,"end_line":7529,"end_character":40},"in_reply_to":"1aa78d24_1f6e0595","updated":"2016-07-06 03:24:27.000000000","message":"Thanks~\nFor USB device , it\u0027s device.addr is None, not LibvirtConfigGuestDeviceAddressDrive. It would cause unit tests error.\nSo may not change here tomporarily.","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"560d7be55f80294d6f7fb6ddc0270461422951eb","unresolved":false,"context_lines":[{"line_number":7533,"context_line":"                bus \u003d objects.IDEDeviceBus()"},{"line_number":7534,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7535,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7536,"context_line":"        else:"},{"line_number":7537,"context_line":"            bus \u003d None"},{"line_number":7538,"context_line":"        if address is not None and bus is not None:"},{"line_number":7539,"context_line":"            bus.address \u003d address"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_3f8ca134","line":7536,"range":{"start_line":7536,"start_character":8,"end_line":7536,"end_character":12},"updated":"2016-07-05 17:14:49.000000000","message":"Maybe just declare it on top as None","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"c899662d2a77225e09696270727b4637630b8897","unresolved":false,"context_lines":[{"line_number":7533,"context_line":"                bus \u003d objects.IDEDeviceBus()"},{"line_number":7534,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7535,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7536,"context_line":"        else:"},{"line_number":7537,"context_line":"            bus \u003d None"},{"line_number":7538,"context_line":"        if address is not None and bus is not None:"},{"line_number":7539,"context_line":"            bus.address \u003d address"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_ca566b5b","line":7536,"range":{"start_line":7536,"start_character":8,"end_line":7536,"end_character":12},"in_reply_to":"1aa78d24_3f8ca134","updated":"2016-07-06 03:24:27.000000000","message":"Done","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"560d7be55f80294d6f7fb6ddc0270461422951eb","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7536,"context_line":"        else:"},{"line_number":7537,"context_line":"            bus \u003d None"},{"line_number":7538,"context_line":"        if address is not None and bus is not None:"},{"line_number":7539,"context_line":"            bus.address \u003d address"},{"line_number":7540,"context_line":"        return bus"},{"line_number":7541,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_3f9f81b5","line":7538,"range":{"start_line":7538,"start_character":35,"end_line":7538,"end_character":38},"updated":"2016-07-05 17:14:49.000000000","message":"I think this is not needed, as it\u0027s unlikely to have an address and not have they bus.","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"c899662d2a77225e09696270727b4637630b8897","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7536,"context_line":"        else:"},{"line_number":7537,"context_line":"            bus \u003d None"},{"line_number":7538,"context_line":"        if address is not None and bus is not None:"},{"line_number":7539,"context_line":"            bus.address \u003d address"},{"line_number":7540,"context_line":"        return bus"},{"line_number":7541,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_aa531f6a","line":7538,"range":{"start_line":7538,"start_character":35,"end_line":7538,"end_character":38},"in_reply_to":"1aa78d24_3f9f81b5","updated":"2016-07-06 03:24:27.000000000","message":"Done","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"560d7be55f80294d6f7fb6ddc0270461422951eb","unresolved":false,"context_lines":[{"line_number":7584,"context_line":"                if not bdm:"},{"line_number":7585,"context_line":"                    continue"},{"line_number":7586,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7587,"context_line":"                device \u003d objects.DiskMetadata(bus\u003dbus, tags\u003d[bdm.tag])"},{"line_number":7588,"context_line":"                devices.append(device)"},{"line_number":7589,"context_line":"        if devices:"},{"line_number":7590,"context_line":"            dev_meta \u003d objects.InstanceDeviceMetadata(devices\u003ddevices)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_5fb7edd7","line":7587,"range":{"start_line":7587,"start_character":46,"end_line":7587,"end_character":49},"updated":"2016-07-05 17:14:49.000000000","message":"It might be better to make the same change here as well.","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"c899662d2a77225e09696270727b4637630b8897","unresolved":false,"context_lines":[{"line_number":7584,"context_line":"                if not bdm:"},{"line_number":7585,"context_line":"                    continue"},{"line_number":7586,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7587,"context_line":"                device \u003d objects.DiskMetadata(bus\u003dbus, tags\u003d[bdm.tag])"},{"line_number":7588,"context_line":"                devices.append(device)"},{"line_number":7589,"context_line":"        if devices:"},{"line_number":7590,"context_line":"            dev_meta \u003d objects.InstanceDeviceMetadata(devices\u003ddevices)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_0a4d13c4","line":7587,"range":{"start_line":7587,"start_character":46,"end_line":7587,"end_character":49},"in_reply_to":"1aa78d24_5fb7edd7","updated":"2016-07-06 03:24:27.000000000","message":"Done","commit_id":"97d6b384a87da18231116c75b4e5c0c210142cb5"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"481b8c860773093a3ad16c04639ee64502a064dd","unresolved":false,"context_lines":[{"line_number":7527,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7528,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7529,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7530,"context_line":"        elif hasattr(dev, \u0027target_bus\u0027):"},{"line_number":7531,"context_line":"            if dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7532,"context_line":"                bus \u003d objects.SCSIDeviceBus()"},{"line_number":7533,"context_line":"            elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_4bb6a1f9","line":7530,"range":{"start_line":7530,"start_character":27,"end_line":7530,"end_character":37},"updated":"2016-07-08 13:44:02.000000000","message":"Agree, it\u0027s better to check the instance.","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7e5d06d9f2059f02cca6d2fea743adb3d8a5b2fe","unresolved":false,"context_lines":[{"line_number":7527,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7528,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7529,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7530,"context_line":"        elif hasattr(dev, \u0027target_bus\u0027):"},{"line_number":7531,"context_line":"            if dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7532,"context_line":"                bus \u003d objects.SCSIDeviceBus()"},{"line_number":7533,"context_line":"            elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_172aca2c","line":7530,"updated":"2016-07-08 09:08:11.000000000","message":"I saw comments on earlier PSes about why you need to add this. ++","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"8995d50a2858e8d69aec302902a83f1ac46b862a","unresolved":false,"context_lines":[{"line_number":7527,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7528,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7529,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7530,"context_line":"        elif hasattr(dev, \u0027target_bus\u0027):"},{"line_number":7531,"context_line":"            if dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7532,"context_line":"                bus \u003d objects.SCSIDeviceBus()"},{"line_number":7533,"context_line":"            elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_02eeaeb3","line":7530,"in_reply_to":"1aa78d24_172aca2c","updated":"2016-07-11 13:52:35.000000000","message":"Done","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e724fd30a25185d5c8cfdc66528624ca8aa4277d","unresolved":false,"context_lines":[{"line_number":7527,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7528,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7529,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7530,"context_line":"        elif hasattr(dev, \u0027target_bus\u0027):"},{"line_number":7531,"context_line":"            if dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7532,"context_line":"                bus \u003d objects.SCSIDeviceBus()"},{"line_number":7533,"context_line":"            elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_b2b8242c","line":7530,"in_reply_to":"1aa78d24_172aca2c","updated":"2016-07-08 09:16:59.000000000","message":"You should really be checking that the \u0027dev\u0027 is an instance of LibvirtConfigGuestDeviceDisk here, since that\u0027s ultimately what the code is assuming.","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"dc9eb577576aa5a897089cc240d9c983a68622d4","unresolved":false,"context_lines":[{"line_number":7527,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7528,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7529,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7530,"context_line":"        elif hasattr(dev, \u0027target_bus\u0027):"},{"line_number":7531,"context_line":"            if dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7532,"context_line":"                bus \u003d objects.SCSIDeviceBus()"},{"line_number":7533,"context_line":"            elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_11a5a3b4","line":7530,"in_reply_to":"1aa78d24_b2b8242c","updated":"2016-07-08 15:11:02.000000000","message":"OK I will modify here","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e724fd30a25185d5c8cfdc66528624ca8aa4277d","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7536,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7537,"context_line":"        if address is not None:"},{"line_number":7538,"context_line":"            bus.address \u003d address"},{"line_number":7539,"context_line":"        return bus"},{"line_number":7540,"context_line":""},{"line_number":7541,"context_line":"    def _build_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_97e19a9d","line":7538,"updated":"2016-07-08 09:16:59.000000000","message":"The elif block above has the potential to leave \u0027bus\u0027 as None. So you could end up trying to set  None.address here which will fail in an explody way.","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e1c43ec6d027d8c74a9cca0df505d0e4bb4d10ac","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7536,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7537,"context_line":"        if address is not None:"},{"line_number":7538,"context_line":"            bus.address \u003d address"},{"line_number":7539,"context_line":"        return bus"},{"line_number":7540,"context_line":""},{"line_number":7541,"context_line":"    def _build_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_29be7dff","line":7538,"in_reply_to":"1aa78d24_1edbf51c","updated":"2016-07-08 13:54:21.000000000","message":"That\u0027s irrelevant as it runs after this code. \n\nHere you have possibility of \u0027address\u0027 being non-None, and \u0027bus\u0027 being None. In that case this code crashes.","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"20a933f60910cd73c91bdf63e7e69fc4f1114d5e","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7536,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7537,"context_line":"        if address is not None:"},{"line_number":7538,"context_line":"            bus.address \u003d address"},{"line_number":7539,"context_line":"        return bus"},{"line_number":7540,"context_line":""},{"line_number":7541,"context_line":"    def _build_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_78116c5d","line":7538,"in_reply_to":"1aa78d24_29be7dff","updated":"2016-07-08 14:29:47.000000000","message":"Agree, I just don\u0027t see a possibility how we\u0027ve managed to get an address but not the bus.. but you are right, it\u0027s better to be safe.","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"dc9eb577576aa5a897089cc240d9c983a68622d4","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7536,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7537,"context_line":"        if address is not None:"},{"line_number":7538,"context_line":"            bus.address \u003d address"},{"line_number":7539,"context_line":"        return bus"},{"line_number":7540,"context_line":""},{"line_number":7541,"context_line":"    def _build_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_718187fb","line":7538,"in_reply_to":"1aa78d24_29be7dff","updated":"2016-07-08 15:11:02.000000000","message":"Thanks~I will add the judgement.","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":22076,"name":"Kevin Zhao","email":"kevin.zhao@linaro.org","username":"KevinZhao"},"change_message_id":"8995d50a2858e8d69aec302902a83f1ac46b862a","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7536,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7537,"context_line":"        if address is not None:"},{"line_number":7538,"context_line":"            bus.address \u003d address"},{"line_number":7539,"context_line":"        return bus"},{"line_number":7540,"context_line":""},{"line_number":7541,"context_line":"    def _build_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_22f3ea9f","line":7538,"in_reply_to":"1aa78d24_718187fb","updated":"2016-07-11 13:52:35.000000000","message":"Done","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"481b8c860773093a3ad16c04639ee64502a064dd","unresolved":false,"context_lines":[{"line_number":7535,"context_line":"            elif dev.target_bus \u003d\u003d \u0027usb\u0027:"},{"line_number":7536,"context_line":"                bus \u003d objects.USBDeviceBus()"},{"line_number":7537,"context_line":"        if address is not None:"},{"line_number":7538,"context_line":"            bus.address \u003d address"},{"line_number":7539,"context_line":"        return bus"},{"line_number":7540,"context_line":""},{"line_number":7541,"context_line":"    def _build_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_1edbf51c","line":7538,"in_reply_to":"1aa78d24_97e19a9d","updated":"2016-07-08 13:44:02.000000000","message":"There is a check on like 7576 and 7587 that sets bus only if it\u0027s not None","commit_id":"545b2f8e7787c93685672fe6e2f624d8d3bf1ee7"}]}