)]}'
{"nova/objects/virt_device_metadata.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2c74e40c5f7952cb3fb54607542067af949ea7ea","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    fields \u003d {"},{"line_number":39,"context_line":"        \u0027address\u0027: fields.USBAddressField(),"},{"line_number":40,"context_line":"    }"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"5a18252c_f5a6d40e","side":"PARENT","line":39,"updated":"2016-04-13 14:36:32.000000000","message":"The versionning is probably not a matter here since this object is not used yet","commit_id":"0d54bc26a7becdea14ae96e2308a0827d26e6781"}],"nova/objects/virtual_interface.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2c74e40c5f7952cb3fb54607542067af949ea7ea","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    fields \u003d {"},{"line_number":31,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":32,"context_line":"        \u0027address\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":33,"context_line":"        \u0027network_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":34,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":36,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a18252c_553188eb","line":33,"updated":"2016-04-13 14:36:32.000000000","message":"Can we do that without to bump the version? So if an updated node is storing this value as NULL, how a old compute node will handle that. I have no idea :)","commit_id":"cc8c3b1a9fe09095141445278aaeaa93ea56f138"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"05a40fa5b5ad8b5fcb766b75d2cb71903025c294","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    fields \u003d {"},{"line_number":31,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":32,"context_line":"        \u0027address\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":33,"context_line":"        \u0027network_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":34,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":36,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a18252c_9ef4cff2","line":33,"in_reply_to":"5a18252c_553188eb","updated":"2016-04-13 19:22:28.000000000","message":"I\u0027m not sure, but I think it\u0027s not very much necessary because this object was used only by Nova network, which always has a not null network_id. So, only the metadata object can insert an nullable value, and it\u0027s new..","commit_id":"cc8c3b1a9fe09095141445278aaeaa93ea56f138"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3ee96d2d04dabc9759abf675e173cdcb095cdfbc","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    fields \u003d {"},{"line_number":31,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":32,"context_line":"        \u0027address\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":33,"context_line":"        \u0027network_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":34,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":36,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a18252c_c9368fe6","line":33,"in_reply_to":"5a18252c_9ef4cff2","updated":"2016-04-13 19:41:37.000000000","message":"You can\u0027t change this without a version bump, but on top of that, you can\u0027t change it without a plan for backporting. If we store this into the database or send it over RPC to a compute node from before this was nullable, we need a way to convert it a compatible format (i.e. fill a value for network_id that is not None).","commit_id":"cc8c3b1a9fe09095141445278aaeaa93ea56f138"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5abe74668d07a17b46bb8f1af22a7764558cb9f0","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    fields \u003d {"},{"line_number":31,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":32,"context_line":"        \u0027address\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":33,"context_line":"        \u0027network_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":34,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":36,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a18252c_296463e2","line":33,"updated":"2016-04-13 19:43:35.000000000","message":"You can\u0027t make this change without a bump and a backport plan (see comments on previous set).","commit_id":"d0a170482e9132a3fd0a893d43b33102dbb02e1a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"64818ad5ab1baa3f0b57cd206e83f8f75374d7d9","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    fields \u003d {"},{"line_number":31,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":32,"context_line":"        \u0027address\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":33,"context_line":"        \u0027network_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":34,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":36,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a122d0e_76a65c54","line":33,"in_reply_to":"1a122d0e_f3dc9048","updated":"2016-04-18 14:15:56.000000000","message":"I think that\u0027s reasonable, yeah, as long as it never leaks out of the API and isn\u0027t used anywhere else.\n\nHowever, maybe just don\u0027t set the field at all if you don\u0027t have a value for it? If nothing touches it then it won\u0027t matter, and if something does, you\u0027ll get a very recognizble traceback instead of just silently trying to use that (bogus) network_id...","commit_id":"d0a170482e9132a3fd0a893d43b33102dbb02e1a"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"010445abbdc7d5f1a34ea620dea79401e9f5ef12","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    fields \u003d {"},{"line_number":31,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":32,"context_line":"        \u0027address\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":33,"context_line":"        \u0027network_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":34,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":36,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":27,"id":"1a122d0e_f3dc9048","line":33,"in_reply_to":"5a18252c_296463e2","updated":"2016-04-15 09:17:48.000000000","message":"Ok. This object is not being used outside of nova network (besides metadata) and it\u0027s only the nova network that has network_id. Would it be enough to convert the null value to any static integer? (like 1?)","commit_id":"d0a170482e9132a3fd0a893d43b33102dbb02e1a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c00da0bac5da5464d7e7f728459a6b239edde961","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    fields \u003d {"},{"line_number":31,"context_line":"        \u0027id\u0027: fields.IntegerField(),"},{"line_number":32,"context_line":"        \u0027address\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":33,"context_line":"        \u0027network_id\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":34,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"},{"line_number":35,"context_line":"        \u0027uuid\u0027: fields.UUIDField(),"},{"line_number":36,"context_line":"        \u0027tag\u0027: fields.StringField(nullable\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":31,"id":"1a122d0e_f6470c75","line":33,"updated":"2016-04-18 14:16:22.000000000","message":"See my comment on a recent patch set.","commit_id":"f8d09e6e1b788cccb85e4a95ba3c51402f160cf0"}],"nova/tests/unit/objects/test_objects.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5abe74668d07a17b46bb8f1af22a7764558cb9f0","unresolved":false,"context_lines":[{"line_number":1200,"context_line":"    \u0027VirtCPUFeature\u0027: \u00271.0-3310718d8c72309259a6e39bdefe83ee\u0027,"},{"line_number":1201,"context_line":"    \u0027VirtCPUModel\u0027: \u00271.0-6a5cc9f322729fc70ddc6733bacd57d3\u0027,"},{"line_number":1202,"context_line":"    \u0027VirtCPUTopology\u0027: \u00271.0-fc694de72e20298f7c6bab1083fd4563\u0027,"},{"line_number":1203,"context_line":"    \u0027VirtualInterface\u0027: \u00271.1-cc6f02fe4f7e068e93ad1bfa0420e685\u0027,"},{"line_number":1204,"context_line":"    \u0027VirtualInterfaceList\u0027: \u00271.0-9750e2074437b3077e46359102779fc6\u0027,"},{"line_number":1205,"context_line":"    \u0027VolumeUsage\u0027: \u00271.0-6c8190c46ce1469bb3286a1f21c2e475\u0027,"},{"line_number":1206,"context_line":"    \u0027XenapiLiveMigrateData\u0027: \u00271.0-5f982bec68f066e194cd9ce53a24ac4c\u0027,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a18252c_a9cb53a4","line":1203,"updated":"2016-04-13 19:43:35.000000000","message":"This is almost never okay to do. There are a very few situations where we can allow a hash change without a bump, but this is not one of them :)","commit_id":"d0a170482e9132a3fd0a893d43b33102dbb02e1a"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fa07bc55cdc918226ddbfed2ecd362aa4b52dbe9","unresolved":false,"context_lines":[{"line_number":1434,"context_line":"                fake_block_device.FakeDbBlockDeviceDict("},{"line_number":1435,"context_line":"                    {\u0027id\u0027: 4,"},{"line_number":1436,"context_line":"                     \u0027source_type\u0027: \u0027volume\u0027, \u0027destination_type\u0027: \u0027volume\u0027,"},{"line_number":1437,"context_line":"                     \u0027device_name\u0027: \u0027/dev/vda\u0027, \u0027tag\u0027: \"nfvfunc3\"}),"},{"line_number":1438,"context_line":"            ]"},{"line_number":1439,"context_line":"        )"},{"line_number":1440,"context_line":"        vif \u003d obj_vif.VirtualInterface(context\u003dself.context)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_2eae6098","line":1437,"range":{"start_line":1437,"start_character":55,"end_line":1437,"end_character":65},"updated":"2016-06-28 13:55:47.000000000","message":"How about we have one of these with no tag to make sure it does the right thing in that case? Same for vif.","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a45ff00da4f3c11f23c1759e07ad6c5bd8afec2","unresolved":false,"context_lines":[{"line_number":1367,"context_line":"    @mock.patch.object(host.Host, \u0027get_connection\u0027)"},{"line_number":1368,"context_line":"    @mock.patch.object(nova.virt.libvirt.guest.Guest, \u0027get_xml_desc\u0027)"},{"line_number":1369,"context_line":"    def test_detach_pci_devices(self, mocked_get_xml_desc, mock_conn):"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        fake_domXML1_with_pci \u003d ("},{"line_number":1372,"context_line":"            \"\"\"\u003cdomain\u003e \u003cdevices\u003e"},{"line_number":1373,"context_line":"            \u003cdisk type\u003d\u0027file\u0027 device\u003d\u0027disk\u0027\u003e"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_82dbd11a","side":"PARENT","line":1370,"updated":"2016-06-28 20:20:55.000000000","message":"Unrelated whitespace change.","commit_id":"a5dae110fbeb0bd7ada607967fad9bb8cfba66ea"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":1367,"context_line":"    @mock.patch.object(host.Host, \u0027get_connection\u0027)"},{"line_number":1368,"context_line":"    @mock.patch.object(nova.virt.libvirt.guest.Guest, \u0027get_xml_desc\u0027)"},{"line_number":1369,"context_line":"    def test_detach_pci_devices(self, mocked_get_xml_desc, mock_conn):"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        fake_domXML1_with_pci \u003d ("},{"line_number":1372,"context_line":"            \"\"\"\u003cdomain\u003e \u003cdevices\u003e"},{"line_number":1373,"context_line":"            \u003cdisk type\u003d\u0027file\u0027 device\u003d\u0027disk\u0027\u003e"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_a8122f07","side":"PARENT","line":1370,"in_reply_to":"3aaa91ec_82dbd11a","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"a5dae110fbeb0bd7ada607967fad9bb8cfba66ea"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a45ff00da4f3c11f23c1759e07ad6c5bd8afec2","unresolved":false,"context_lines":[{"line_number":1370,"context_line":"        xml \u003d \"\"\""},{"line_number":1371,"context_line":"        \u003cdomain\u003e"},{"line_number":1372,"context_line":"          \u003cname\u003edummy\u003c/name\u003e"},{"line_number":1373,"context_line":"          \u003cuuid\u003ed4e13113-918e-42fe-9fc9-861693ffd432\u003c/uuid\u003e"},{"line_number":1374,"context_line":"            \u003cmemory\u003e1048576\u003c/memory\u003e"},{"line_number":1375,"context_line":"              \u003cvcpu\u003e1\u003c/vcpu\u003e"},{"line_number":1376,"context_line":"            \u003cos\u003e"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_621505e4","line":1373,"range":{"start_line":1373,"start_character":16,"end_line":1373,"end_character":52},"updated":"2016-06-28 20:20:55.000000000","message":"nit: technically 32dfcb37-5af1-552b-357c-be8c3aa38310","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":1370,"context_line":"        xml \u003d \"\"\""},{"line_number":1371,"context_line":"        \u003cdomain\u003e"},{"line_number":1372,"context_line":"          \u003cname\u003edummy\u003c/name\u003e"},{"line_number":1373,"context_line":"          \u003cuuid\u003ed4e13113-918e-42fe-9fc9-861693ffd432\u003c/uuid\u003e"},{"line_number":1374,"context_line":"            \u003cmemory\u003e1048576\u003c/memory\u003e"},{"line_number":1375,"context_line":"              \u003cvcpu\u003e1\u003c/vcpu\u003e"},{"line_number":1376,"context_line":"            \u003cos\u003e"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_885e53e7","line":1373,"range":{"start_line":1373,"start_character":16,"end_line":1373,"end_character":52},"in_reply_to":"3aaa91ec_621505e4","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a45ff00da4f3c11f23c1759e07ad6c5bd8afec2","unresolved":false,"context_lines":[{"line_number":1407,"context_line":"              \u003ctarget dev\u003d\u0027vda\u0027 bus\u003d\u0027virtio\u0027/\u003e"},{"line_number":1408,"context_line":"              \u003cboot order\u003d\u00271\u0027/\u003e"},{"line_number":1409,"context_line":"              \u003calias name\u003d\u0027virtio-disk0\u0027/\u003e"},{"line_number":1410,"context_line":"    \u003caddress type\u003d\u0027pci\u0027 domain\u003d\u00270x0000\u0027 bus\u003d\u00270x00\u0027 slot\u003d\u00270x09\u0027 function\u003d\u00270x0\u0027/\u003e"},{"line_number":1411,"context_line":"            \u003c/disk\u003e"},{"line_number":1412,"context_line":"            \u003cinterface type\u003d\u0027network\u0027\u003e"},{"line_number":1413,"context_line":"              \u003cmac address\u003d\u002752:54:00:f6:35:8f\u0027/\u003e"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_82f111d0","line":1410,"updated":"2016-06-28 20:20:55.000000000","message":"nit: alignment on these address elements is off.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a45ff00da4f3c11f23c1759e07ad6c5bd8afec2","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            mock.patch(\u0027nova.objects.BlockDeviceMappingList\u0027"},{"line_number":1474,"context_line":"                       \u0027.get_by_instance_uuid\u0027, return_value\u003dbdms),"},{"line_number":1475,"context_line":"            mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027,"},{"line_number":1476,"context_line":"                              return_value\u003dguest),"},{"line_number":1477,"context_line":"            mock.patch.object(nova.virt.libvirt.guest.Guest, \u0027get_xml_desc\u0027,"},{"line_number":1478,"context_line":"                              return_value\u003dxml)):"},{"line_number":1479,"context_line":"            metadata_obj \u003d drvr. _save_device_metadata(self.context,"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_4220c949","line":1476,"updated":"2016-06-28 20:20:55.000000000","message":"nit: alignment","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":1473,"context_line":"            mock.patch(\u0027nova.objects.BlockDeviceMappingList\u0027"},{"line_number":1474,"context_line":"                       \u0027.get_by_instance_uuid\u0027, return_value\u003dbdms),"},{"line_number":1475,"context_line":"            mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027,"},{"line_number":1476,"context_line":"                              return_value\u003dguest),"},{"line_number":1477,"context_line":"            mock.patch.object(nova.virt.libvirt.guest.Guest, \u0027get_xml_desc\u0027,"},{"line_number":1478,"context_line":"                              return_value\u003dxml)):"},{"line_number":1479,"context_line":"            metadata_obj \u003d drvr. _save_device_metadata(self.context,"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_c8585bfd","line":1476,"in_reply_to":"3aaa91ec_4220c949","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a45ff00da4f3c11f23c1759e07ad6c5bd8afec2","unresolved":false,"context_lines":[{"line_number":1476,"context_line":"                              return_value\u003dguest),"},{"line_number":1477,"context_line":"            mock.patch.object(nova.virt.libvirt.guest.Guest, \u0027get_xml_desc\u0027,"},{"line_number":1478,"context_line":"                              return_value\u003dxml)):"},{"line_number":1479,"context_line":"            metadata_obj \u003d drvr. _save_device_metadata(self.context,"},{"line_number":1480,"context_line":"                                                       instance_ref)"},{"line_number":1481,"context_line":"            metadata \u003d metadata_obj.devices"},{"line_number":1482,"context_line":"            self.assertEqual(5, len(metadata))"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_e284550c","line":1479,"range":{"start_line":1479,"start_character":32,"end_line":1479,"end_character":33},"updated":"2016-06-28 20:20:55.000000000","message":"wha?","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":1476,"context_line":"                              return_value\u003dguest),"},{"line_number":1477,"context_line":"            mock.patch.object(nova.virt.libvirt.guest.Guest, \u0027get_xml_desc\u0027,"},{"line_number":1478,"context_line":"                              return_value\u003dxml)):"},{"line_number":1479,"context_line":"            metadata_obj \u003d drvr. _save_device_metadata(self.context,"},{"line_number":1480,"context_line":"                                                       instance_ref)"},{"line_number":1481,"context_line":"            metadata \u003d metadata_obj.devices"},{"line_number":1482,"context_line":"            self.assertEqual(5, len(metadata))"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_08738381","line":1479,"range":{"start_line":1479,"start_character":32,"end_line":1479,"end_character":33},"in_reply_to":"3aaa91ec_e284550c","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a45ff00da4f3c11f23c1759e07ad6c5bd8afec2","unresolved":false,"context_lines":[{"line_number":1482,"context_line":"            self.assertEqual(5, len(metadata))"},{"line_number":1483,"context_line":"            self.assertIsInstance(metadata[0],"},{"line_number":1484,"context_line":"                                  objects.DiskMetadata)"},{"line_number":1485,"context_line":"            self.assertIsInstance(metadata[0].bus,"},{"line_number":1486,"context_line":"                                  objects.SCSIDeviceBus)"},{"line_number":1487,"context_line":"            self.assertEqual([\u0027nfvfunc1\u0027], metadata[1].tags)"},{"line_number":1488,"context_line":"            self.assertIsInstance(metadata[1],"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_c2dcb91a","line":1485,"range":{"start_line":1485,"start_character":34,"end_line":1485,"end_character":45},"updated":"2016-06-28 20:20:55.000000000","message":"You didn\u0027t assert the \u0027db\u0027 tag on this guy.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":1482,"context_line":"            self.assertEqual(5, len(metadata))"},{"line_number":1483,"context_line":"            self.assertIsInstance(metadata[0],"},{"line_number":1484,"context_line":"                                  objects.DiskMetadata)"},{"line_number":1485,"context_line":"            self.assertIsInstance(metadata[0].bus,"},{"line_number":1486,"context_line":"                                  objects.SCSIDeviceBus)"},{"line_number":1487,"context_line":"            self.assertEqual([\u0027nfvfunc1\u0027], metadata[1].tags)"},{"line_number":1488,"context_line":"            self.assertIsInstance(metadata[1],"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_e86d579a","line":1485,"range":{"start_line":1485,"start_character":34,"end_line":1485,"end_character":45},"in_reply_to":"3aaa91ec_22201db0","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"94d1208ba88868b32aaf59519221c1ba4823ee8f","unresolved":false,"context_lines":[{"line_number":1482,"context_line":"            self.assertEqual(5, len(metadata))"},{"line_number":1483,"context_line":"            self.assertIsInstance(metadata[0],"},{"line_number":1484,"context_line":"                                  objects.DiskMetadata)"},{"line_number":1485,"context_line":"            self.assertIsInstance(metadata[0].bus,"},{"line_number":1486,"context_line":"                                  objects.SCSIDeviceBus)"},{"line_number":1487,"context_line":"            self.assertEqual([\u0027nfvfunc1\u0027], metadata[1].tags)"},{"line_number":1488,"context_line":"            self.assertIsInstance(metadata[1],"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_22201db0","line":1485,"range":{"start_line":1485,"start_character":34,"end_line":1485,"end_character":45},"in_reply_to":"3aaa91ec_c2dcb91a","updated":"2016-06-28 20:30:35.000000000","message":"Good catch, I kinda glazed over this section after a while.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5a45ff00da4f3c11f23c1759e07ad6c5bd8afec2","unresolved":false,"context_lines":[{"line_number":1500,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1501,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1502,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1503,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1504,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1505,"context_line":"                                  objects.NetworkInterfaceMetadata)"},{"line_number":1506,"context_line":"            self.assertIsInstance(metadata[4].bus,"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_22cc9d60","line":1503,"range":{"start_line":1503,"start_character":45,"end_line":1503,"end_character":68},"updated":"2016-06-28 20:20:55.000000000","message":"Assert that bus.address is None on the other BDM devices?","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":1500,"context_line":"            self.assertIsInstance(metadata[3].bus,"},{"line_number":1501,"context_line":"                                  objects.PCIDeviceBus)"},{"line_number":1502,"context_line":"            self.assertEqual([\u0027nfvfunc3\u0027], metadata[3].tags)"},{"line_number":1503,"context_line":"            self.assertEqual(\u00270000:00:09.0\u0027, metadata[3].bus.address)"},{"line_number":1504,"context_line":"            self.assertIsInstance(metadata[4],"},{"line_number":1505,"context_line":"                                  objects.NetworkInterfaceMetadata)"},{"line_number":1506,"context_line":"            self.assertIsInstance(metadata[4].bus,"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_c80fbbdf","line":1503,"range":{"start_line":1503,"start_character":45,"end_line":1503,"end_character":68},"in_reply_to":"3aaa91ec_22cc9d60","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"}],"nova/virt/driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fa07bc55cdc918226ddbfed2ecd362aa4b52dbe9","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        \"supports_recreate\": False,"},{"line_number":138,"context_line":"        \"supports_migrate_to_same_host\": False,"},{"line_number":139,"context_line":"        \"supports_attach_interface\": False,"},{"line_number":140,"context_line":"        \"supports_device_tagging\": False"},{"line_number":141,"context_line":"    }"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def __init__(self, virtapi):"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_0ed3a439","line":140,"range":{"start_line":140,"start_character":39,"end_line":140,"end_character":40},"updated":"2016-06-28 13:55:47.000000000","message":"Can you add a trailing comma here so that the next patch doesn\u0027t have to touch both?","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"7fb81d69707f617c2b9006fa6916283db4663d90","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        \"supports_recreate\": False,"},{"line_number":138,"context_line":"        \"supports_migrate_to_same_host\": False,"},{"line_number":139,"context_line":"        \"supports_attach_interface\": False,"},{"line_number":140,"context_line":"        \"supports_device_tagging\": False"},{"line_number":141,"context_line":"    }"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def __init__(self, virtapi):"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_2412ff09","line":140,"range":{"start_line":140,"start_character":39,"end_line":140,"end_character":40},"in_reply_to":"3aaa91ec_0ed3a439","updated":"2016-06-28 16:29:35.000000000","message":"Done","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":16929,"name":"Giridhar Jayavelu","email":"gjayavelu@vmware.com","username":"gjayavelu"},"change_message_id":"ac7eba87dc65c249525a7e8d738eb9dc8bc8bb88","unresolved":false,"context_lines":[{"line_number":7159,"context_line":"                pci_bus \u003d int(address.attrib[\u0027bus\u0027], 16)"},{"line_number":7160,"context_line":"                pci_slot \u003d int(address.attrib[\u0027slot\u0027], 16)"},{"line_number":7161,"context_line":"                pci_function \u003d int(address.attrib[\u0027function\u0027], 16)"},{"line_number":7162,"context_line":"                device.address \u003d \u0027%d:%d:%d.%d\u0027 % (pci_domain, pci_bus,"},{"line_number":7163,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7164,"context_line":""},{"line_number":7165,"context_line":"            metadata.objects.append(device)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_d3cb69a2","line":7162,"updated":"2016-01-07 06:18:29.000000000","message":"please set the format to \"%04x:%02x:%02x.%1x\" to be consistent with\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4818","commit_id":"e19e4e53b7e1e67b445862cb0ed1f2192532c955"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"99d24f6a9e529682d104392a30b00f9be55662e2","unresolved":false,"context_lines":[{"line_number":7146,"context_line":""},{"line_number":7147,"context_line":"        for vif_tag in vif_tags:"},{"line_number":7148,"context_line":"            interface \u003d xml_dom.find("},{"line_number":7149,"context_line":"                \"./devices/interface/mac[@address\u003d\u0027%s\u0027]/..\" % vif_tag.address)"},{"line_number":7150,"context_line":""},{"line_number":7151,"context_line":"            device \u003d obj_metadata.NetworkInterfaceMetadata()"},{"line_number":7152,"context_line":"            device.type \u003d \u0027nic\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa69d971_ca57cac8","line":7149,"range":{"start_line":7149,"start_character":77,"end_line":7149,"end_character":78},"updated":"2016-01-08 11:28:13.000000000","message":"Please don\u0027t do custom XML parsing with xpath. Use the nova.virt.libvirt.config classes to parse XML and then just access the mac_addr attribute of the LibvirtConfigGuestInterface calss","commit_id":"359809c4a496dc48687c3f684058dc3d3bfe208c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"99d24f6a9e529682d104392a30b00f9be55662e2","unresolved":false,"context_lines":[{"line_number":7161,"context_line":"                pci_slot \u003d int(address.attrib[\u0027slot\u0027], 16)"},{"line_number":7162,"context_line":"                pci_function \u003d int(address.attrib[\u0027function\u0027], 16)"},{"line_number":7163,"context_line":"                device.address \u003d \u0027%d:%d:%d.%d\u0027 % (pci_domain, pci_bus,"},{"line_number":7164,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7165,"context_line":""},{"line_number":7166,"context_line":"            metadata.objects.append(device)"},{"line_number":7167,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa69d971_ea288e35","line":7164,"range":{"start_line":7164,"start_character":72,"end_line":7164,"end_character":73},"updated":"2016-01-08 11:28:13.000000000","message":"Likewise don\u0027t do this. We should introduce a LibvirtConfigGuestAddress class to parse libvirt \u003caddress\u003e elements, and have one subclass of it for each libvirt address. We can then have a helper method to convert from the libivirt config class to the metadat address type.","commit_id":"359809c4a496dc48687c3f684058dc3d3bfe208c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"99d24f6a9e529682d104392a30b00f9be55662e2","unresolved":false,"context_lines":[{"line_number":7164,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7165,"context_line":""},{"line_number":7166,"context_line":"            metadata.objects.append(device)"},{"line_number":7167,"context_line":""},{"line_number":7168,"context_line":"        return metadata"},{"line_number":7169,"context_line":""},{"line_number":7170,"context_line":"    def get_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa69d971_0ae9621a","line":7167,"updated":"2016-01-08 11:28:13.000000000","message":"We also need to provide disk tag info","commit_id":"359809c4a496dc48687c3f684058dc3d3bfe208c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"cc949214e629b6bcdc46cbed7d02e549b0b25b63","unresolved":false,"context_lines":[{"line_number":7164,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7165,"context_line":""},{"line_number":7166,"context_line":"            metadata.objects.append(device)"},{"line_number":7167,"context_line":""},{"line_number":7168,"context_line":"        return metadata"},{"line_number":7169,"context_line":""},{"line_number":7170,"context_line":"    def get_device_metadata(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa69d971_3414c442","line":7167,"in_reply_to":"fa69d971_0ae9621a","updated":"2016-01-11 19:02:42.000000000","message":"With this patch series I set out to implement only vNIC tagging to begin with. Are you insisting that the full spec get implemented in one go?","commit_id":"359809c4a496dc48687c3f684058dc3d3bfe208c"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"acd77511a02b1e4267309e6f903e24b3bd59e1d6","unresolved":false,"context_lines":[{"line_number":7168,"context_line":"        return metadata"},{"line_number":7169,"context_line":""},{"line_number":7170,"context_line":"    def get_device_metadata(self, context, instance):"},{"line_number":7171,"context_line":"        return self._get_vif_metadata(context, instance)"},{"line_number":7172,"context_line":""},{"line_number":7173,"context_line":"    def instance_on_disk(self, instance):"},{"line_number":7174,"context_line":"        # ensure directories exist and are writable"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa69d971_61c85875","line":7171,"range":{"start_line":7171,"start_character":8,"end_line":7171,"end_character":56},"updated":"2016-01-08 06:46:03.000000000","message":"does device_metadata only mean to vif_metadata? if not , it\u0027s better to\nappend vif_metadata to device metadata.\n\n  metadata \u003d obj_metadata.DeviceMetadataList()\n\n  metadata.apppend(self._get_vif_metadata(context, instance))\n  metadata.apppend(self._get_foo_metadata(context, instance))\n\n  return metatda\n\n(This syntax is wrong, just an example)","commit_id":"359809c4a496dc48687c3f684058dc3d3bfe208c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"cc949214e629b6bcdc46cbed7d02e549b0b25b63","unresolved":false,"context_lines":[{"line_number":7168,"context_line":"        return metadata"},{"line_number":7169,"context_line":""},{"line_number":7170,"context_line":"    def get_device_metadata(self, context, instance):"},{"line_number":7171,"context_line":"        return self._get_vif_metadata(context, instance)"},{"line_number":7172,"context_line":""},{"line_number":7173,"context_line":"    def instance_on_disk(self, instance):"},{"line_number":7174,"context_line":"        # ensure directories exist and are writable"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa69d971_b1b686da","line":7171,"range":{"start_line":7171,"start_character":8,"end_line":7171,"end_character":56},"in_reply_to":"fa69d971_61c85875","updated":"2016-01-11 19:02:42.000000000","message":"I agree this is confusing. This patch series only implements tagging for vNICs, not the full spec which calls for tagging on disks as well. I\u0027m tempted to keep the method signature as is but add a TODO to remind ourselves that eventually the DeviceMetadataList that\u0027s returned should also include disk metadata, not just vNIC metadata.","commit_id":"359809c4a496dc48687c3f684058dc3d3bfe208c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"bcdafc956e818d2c4418f987d17d7e347f686d00","unresolved":false,"context_lines":[{"line_number":7184,"context_line":""},{"line_number":7185,"context_line":"        for vif_tag in vif_tags:"},{"line_number":7186,"context_line":"            interface \u003d xml_dom.find("},{"line_number":7187,"context_line":"                \"./devices/interface/mac[@address\u003d\u0027%s\u0027]/..\" % vif_tag.address)"},{"line_number":7188,"context_line":""},{"line_number":7189,"context_line":"            device \u003d obj_metadata.NetworkInterfaceMetadata()"},{"line_number":7190,"context_line":"            device.type \u003d \u0027nic\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"da6ed579_2349338c","line":7187,"updated":"2016-01-12 10:49:06.000000000","message":"You\u0027ve ignored my previous comments - you should not be doing xpath queries to extract this - use the libvirt config classes.","commit_id":"c92456ac268f33ab78f0f209bc40c1f5c4a110a7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"bcdafc956e818d2c4418f987d17d7e347f686d00","unresolved":false,"context_lines":[{"line_number":7199,"context_line":"                pci_slot \u003d int(address.attrib[\u0027slot\u0027], 16)"},{"line_number":7200,"context_line":"                pci_function \u003d int(address.attrib[\u0027function\u0027], 16)"},{"line_number":7201,"context_line":"                device.address \u003d \u0027%d:%d:%d.%d\u0027 % (pci_domain, pci_bus,"},{"line_number":7202,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7203,"context_line":""},{"line_number":7204,"context_line":"            metadata.objects.append(device)"},{"line_number":7205,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"da6ed579_e35b1b53","line":7202,"range":{"start_line":7202,"start_character":72,"end_line":7202,"end_character":73},"updated":"2016-01-12 10:49:06.000000000","message":"Again, you\u0027ve ignored my previous comments - you should be handling all bus types not just pci","commit_id":"c92456ac268f33ab78f0f209bc40c1f5c4a110a7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1bcbfa10f81efb16d56bed3c95fd4edbd14f1bcf","unresolved":false,"context_lines":[{"line_number":7203,"context_line":""},{"line_number":7204,"context_line":"            metadata.objects.append(device)"},{"line_number":7205,"context_line":""},{"line_number":7206,"context_line":"        return metadata"},{"line_number":7207,"context_line":""},{"line_number":7208,"context_line":"    def get_device_metadata(self, context, instance):"},{"line_number":7209,"context_line":"        metdata \u003d self._get_vif_metadata(context, instance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da6ed579_89c939af","line":7206,"updated":"2016-01-11 22:08:03.000000000","message":"Where are the tests for this new method?","commit_id":"c92456ac268f33ab78f0f209bc40c1f5c4a110a7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1bcbfa10f81efb16d56bed3c95fd4edbd14f1bcf","unresolved":false,"context_lines":[{"line_number":7210,"context_line":"        # TODO(artom) Once disk metadata is implemented in a subsequent patch,"},{"line_number":7211,"context_line":"        # uncomment the following lines"},{"line_number":7212,"context_line":"        # disk_metadata \u003d self._get_disk_metadata(context, instance)"},{"line_number":7213,"context_line":"        # metadata.objects.append(disk_metadata.objects)"},{"line_number":7214,"context_line":"        return metadata"},{"line_number":7215,"context_line":""},{"line_number":7216,"context_line":"    def instance_on_disk(self, instance):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da6ed579_4913415d","line":7213,"updated":"2016-01-11 22:08:03.000000000","message":"Just add it in the patch when you need it, don\u0027t add it commented-out here.","commit_id":"c92456ac268f33ab78f0f209bc40c1f5c4a110a7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"bcdafc956e818d2c4418f987d17d7e347f686d00","unresolved":false,"context_lines":[{"line_number":7210,"context_line":"        # TODO(artom) Once disk metadata is implemented in a subsequent patch,"},{"line_number":7211,"context_line":"        # uncomment the following lines"},{"line_number":7212,"context_line":"        # disk_metadata \u003d self._get_disk_metadata(context, instance)"},{"line_number":7213,"context_line":"        # metadata.objects.append(disk_metadata.objects)"},{"line_number":7214,"context_line":"        return metadata"},{"line_number":7215,"context_line":""},{"line_number":7216,"context_line":"    def instance_on_disk(self, instance):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da6ed579_23bb7374","line":7213,"in_reply_to":"da6ed579_4913415d","updated":"2016-01-12 10:49:06.000000000","message":"This patch should really just add implement disk support right now IMHO. It is pointless postponing disk metadata support to a different patch series - most of the code you are adding already is generic support for device metdata - only a tiny bit is vNIC specific, so adding disk support right away is not adding to the burden in any significant way.","commit_id":"c92456ac268f33ab78f0f209bc40c1f5c4a110a7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"3add3aa5842a8dc65877719f48b691bfbfeb0e34","unresolved":false,"context_lines":[{"line_number":7665,"context_line":"                pci_slot \u003d int(address.attrib[\u0027slot\u0027], 16)"},{"line_number":7666,"context_line":"                pci_function \u003d int(address.attrib[\u0027function\u0027], 16)"},{"line_number":7667,"context_line":"                device.address \u003d \u0027%d:%d:%d.%d\u0027 % (pci_domain, pci_bus,"},{"line_number":7668,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7669,"context_line":""},{"line_number":7670,"context_line":"            metadata.objects.append(device)"},{"line_number":7671,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba0121b8_a9a6ed50","line":7668,"range":{"start_line":7668,"start_character":72,"end_line":7668,"end_character":73},"updated":"2016-03-29 17:52:04.000000000","message":"You should really factor handling of the \u003caddress\u003e element out into a separate method, since it\u0027ll be the same for disk devices too. eg _get_device_metadata_address, which delegates to _get_device_metadata_address_pci, _get_device_metadata_address_scsi, _get_device_metadata_address_usb, _get_device_metadata_address_ide methods","commit_id":"9ccd1204aab1660ca7c895d54c97e988f0c35b10"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"687378c45a7575e134b2f8eaec88ab3334907732","unresolved":false,"context_lines":[{"line_number":2752,"context_line":"                                            instance,"},{"line_number":2753,"context_line":"                                            image_meta,"},{"line_number":2754,"context_line":"                                            block_device_info)"},{"line_number":2755,"context_line":"        self._create_image(context, instance,"},{"line_number":2756,"context_line":"                           disk_info[\u0027mapping\u0027],"},{"line_number":2757,"context_line":"                           network_info\u003dnetwork_info,"},{"line_number":2758,"context_line":"                           block_device_info\u003dblock_device_info,"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a061dce_d46b5bb5","line":2755,"updated":"2016-04-07 15:58:44.000000000","message":"We create the configdrive here...","commit_id":"0915abd5c0e7b3dc978e156faccbbe4c08695db9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"687378c45a7575e134b2f8eaec88ab3334907732","unresolved":false,"context_lines":[{"line_number":2758,"context_line":"                           block_device_info\u003dblock_device_info,"},{"line_number":2759,"context_line":"                           files\u003dinjected_files,"},{"line_number":2760,"context_line":"                           admin_pass\u003dadmin_password)"},{"line_number":2761,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info,"},{"line_number":2762,"context_line":"                                  disk_info, image_meta,"},{"line_number":2763,"context_line":"                                  block_device_info\u003dblock_device_info,"},{"line_number":2764,"context_line":"                                  write_to_disk\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a061dce_94c9e3ae","line":2761,"updated":"2016-04-07 15:58:44.000000000","message":"But is this (or later) the place where we actually get the addresses assigned from libvirt, right? Doesn\u0027t that mean that we\u0027re not including the right stuff in the config drive?","commit_id":"0915abd5c0e7b3dc978e156faccbbe4c08695db9"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"60e47e6d87d205d0f175aa5fdf11b67f0d4ec5b5","unresolved":false,"context_lines":[{"line_number":2758,"context_line":"                           block_device_info\u003dblock_device_info,"},{"line_number":2759,"context_line":"                           files\u003dinjected_files,"},{"line_number":2760,"context_line":"                           admin_pass\u003dadmin_password)"},{"line_number":2761,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info,"},{"line_number":2762,"context_line":"                                  disk_info, image_meta,"},{"line_number":2763,"context_line":"                                  block_device_info\u003dblock_device_info,"},{"line_number":2764,"context_line":"                                  write_to_disk\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a061dce_8b807d8d","line":2761,"in_reply_to":"9a061dce_94c9e3ae","updated":"2016-04-08 08:53:49.000000000","message":"I was under the impression that this was a known limitation of the design we have currently and that we are OK with this info not being available in the config drive in the first iteration, but I can\u0027t seem to find it written down anywhere.","commit_id":"0915abd5c0e7b3dc978e156faccbbe4c08695db9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"0856a4df21e740eefcc1734932341227118c2ce4","unresolved":false,"context_lines":[{"line_number":7664,"context_line":"                pci_slot \u003d int(address.attrib[\u0027slot\u0027], 16)"},{"line_number":7665,"context_line":"                pci_function \u003d int(address.attrib[\u0027function\u0027], 16)"},{"line_number":7666,"context_line":"                device.address \u003d \u0027%d:%d:%d.%d\u0027 % (pci_domain, pci_bus,"},{"line_number":7667,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7668,"context_line":""},{"line_number":7669,"context_line":"            metadata.objects.append(device)"},{"line_number":7670,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9a061dce_a99c5f6a","line":7667,"range":{"start_line":7667,"start_character":72,"end_line":7667,"end_character":73},"updated":"2016-04-04 13:03:15.000000000","message":"My comments from earlier still apply here","commit_id":"0915abd5c0e7b3dc978e156faccbbe4c08695db9"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"72d5186d80b3785f8097f224ffefd4f48b7c07b9","unresolved":false,"context_lines":[{"line_number":7664,"context_line":"                pci_slot \u003d int(address.attrib[\u0027slot\u0027], 16)"},{"line_number":7665,"context_line":"                pci_function \u003d int(address.attrib[\u0027function\u0027], 16)"},{"line_number":7666,"context_line":"                device.address \u003d \u0027%d:%d:%d.%d\u0027 % (pci_domain, pci_bus,"},{"line_number":7667,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7668,"context_line":""},{"line_number":7669,"context_line":"            metadata.objects.append(device)"},{"line_number":7670,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9a061dce_78ddeb6f","line":7667,"range":{"start_line":7667,"start_character":72,"end_line":7667,"end_character":73},"in_reply_to":"9a061dce_7d895dca","updated":"2016-04-04 14:07:37.000000000","message":"Yeah that\u0027s probably a limitation of the libvirt qemu driver. For this kind of thing like create a USB device address object, but leave the address attribute unset. This at least indicates to the guest that the device they\u0027re looking for is on USB, even though we can\u0027t provide them the full info.","commit_id":"0915abd5c0e7b3dc978e156faccbbe4c08695db9"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"8f4fc1b3a9bfa8e4b5c73daa1dbcc5b2e04153b9","unresolved":false,"context_lines":[{"line_number":7664,"context_line":"                pci_slot \u003d int(address.attrib[\u0027slot\u0027], 16)"},{"line_number":7665,"context_line":"                pci_function \u003d int(address.attrib[\u0027function\u0027], 16)"},{"line_number":7666,"context_line":"                device.address \u003d \u0027%d:%d:%d.%d\u0027 % (pci_domain, pci_bus,"},{"line_number":7667,"context_line":"                                                  pci_slot, pci_function)"},{"line_number":7668,"context_line":""},{"line_number":7669,"context_line":"            metadata.objects.append(device)"},{"line_number":7670,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9a061dce_7d895dca","line":7667,"range":{"start_line":7667,"start_character":72,"end_line":7667,"end_character":73},"in_reply_to":"9a061dce_a99c5f6a","updated":"2016-04-04 14:01:17.000000000","message":"Yes, I\u0027m working on it.\n\nTBH, I ran into so issues with the disk implementation.\nFor example, USB wouldn\u0027t even have an address:\n\n\u003cdisk type\u003d\u0027file\u0027 device\u003d\u0027disk\u0027\u003e\n      \u003cdriver name\u003d\u0027qemu\u0027 type\u003d\u0027qcow2\u0027/\u003e\n      \u003csource file\u003d\u0027/var/lib/libvirt/images/aa1.qcow2\u0027/\u003e\n      \u003ctarget dev\u003d\u0027sdb\u0027 bus\u003d\u0027usb\u0027/\u003e\n    \u003c/disk\u003e\n\nEven have issues with converting (controller, bus, target, unit) to a correct scsi address..","commit_id":"0915abd5c0e7b3dc978e156faccbbe4c08695db9"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"60e47e6d87d205d0f175aa5fdf11b67f0d4ec5b5","unresolved":false,"context_lines":[{"line_number":7672,"context_line":""},{"line_number":7673,"context_line":"    def get_device_metadata(self, context, instance):"},{"line_number":7674,"context_line":"        metadata \u003d self._get_vif_metadata(context, instance)"},{"line_number":7675,"context_line":"        # TODO(artom) Once disk metadata is implemented in a subsequent patch,"},{"line_number":7676,"context_line":"        # uncomment the following lines"},{"line_number":7677,"context_line":"        # disk_metadata \u003d self._get_disk_metadata(context, instance)"},{"line_number":7678,"context_line":"        # metadata.objects.append(disk_metadata.objects)"},{"line_number":7679,"context_line":"        return metadata"},{"line_number":7680,"context_line":""},{"line_number":7681,"context_line":"    def instance_on_disk(self, instance):"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a061dce_8b2ebda3","line":7678,"range":{"start_line":7675,"start_character":8,"end_line":7678,"end_character":56},"updated":"2016-04-08 08:53:49.000000000","message":"This shouldn\u0027t be in this patch at all - just add it once it\u0027s actually done. Though IIUC we really want to have all of them done before we wire it in.","commit_id":"0915abd5c0e7b3dc978e156faccbbe4c08695db9"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2c74e40c5f7952cb3fb54607542067af949ea7ea","unresolved":false,"context_lines":[{"line_number":7665,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":7666,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":7667,"context_line":"        \"\"\""},{"line_number":7668,"context_line":"        vifs \u003d obj_vif.VirtualInterfaceList.get_by_instance_uuid("},{"line_number":7669,"context_line":"                                                        context, instance.uuid)"},{"line_number":7670,"context_line":"        bdm_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7671,"context_line":"                                                        context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a18252c_0a51ab62","line":7668,"updated":"2016-04-13 14:36:32.000000000","message":"Before to hit the DB for two scans, an optimization would be to check whether the domain XML is defining block and/or vif devices.","commit_id":"cc8c3b1a9fe09095141445278aaeaa93ea56f138"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"05a40fa5b5ad8b5fcb766b75d2cb71903025c294","unresolved":false,"context_lines":[{"line_number":7665,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":7666,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":7667,"context_line":"        \"\"\""},{"line_number":7668,"context_line":"        vifs \u003d obj_vif.VirtualInterfaceList.get_by_instance_uuid("},{"line_number":7669,"context_line":"                                                        context, instance.uuid)"},{"line_number":7670,"context_line":"        bdm_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7671,"context_line":"                                                        context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a18252c_bec0ab7d","line":7668,"in_reply_to":"5a18252c_0a51ab62","updated":"2016-04-13 19:22:28.000000000","message":"Good point. Done","commit_id":"cc8c3b1a9fe09095141445278aaeaa93ea56f138"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e07f2c3b571e355d111594f691c7c8c64f2397b7","unresolved":false,"context_lines":[{"line_number":7449,"context_line":"                   dev.device_addr else None)"},{"line_number":7450,"context_line":"        if address and isinstance(dev.device_addr,"},{"line_number":7451,"context_line":"                                  vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7452,"context_line":"            bus \u003d obj_device_metadata.PCIDeviceBus(address\u003daddress)"},{"line_number":7453,"context_line":"        elif dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7454,"context_line":"            bus \u003d obj_device_metadata.SCSIDeviceBus(address\u003daddress)"},{"line_number":7455,"context_line":"        elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":43,"id":"7aa08908_c6deaa0f","line":7452,"updated":"2016-06-16 08:59:31.000000000","message":"Oh, so this is why you made the \u0027address\u0027 field \"nullable\" in earlier patches. This is easily avoided.\n\n  bus \u003d obj_device_metadata.PCIDeviceBus()\n  if address is not None:\n     bus.address \u003d address","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"16b656314ab264267ab27252858fe08d0e9a435a","unresolved":false,"context_lines":[{"line_number":7449,"context_line":"                   dev.device_addr else None)"},{"line_number":7450,"context_line":"        if address and isinstance(dev.device_addr,"},{"line_number":7451,"context_line":"                                  vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7452,"context_line":"            bus \u003d obj_device_metadata.PCIDeviceBus(address\u003daddress)"},{"line_number":7453,"context_line":"        elif dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7454,"context_line":"            bus \u003d obj_device_metadata.SCSIDeviceBus(address\u003daddress)"},{"line_number":7455,"context_line":"        elif dev.target_bus \u003d\u003d \u0027ide\u0027:"}],"source_content_type":"text/x-python","patch_set":43,"id":"5a9d85d2_a99d115b","line":7452,"in_reply_to":"7aa08908_c6deaa0f","updated":"2016-06-21 14:08:35.000000000","message":"Done","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e07f2c3b571e355d111594f691c7c8c64f2397b7","unresolved":false,"context_lines":[{"line_number":7476,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestInterface):"},{"line_number":7477,"context_line":"                if not vifs:"},{"line_number":7478,"context_line":"                    vifs \u003d obj_vif.VirtualInterfaceList.get_by_instance_uuid("},{"line_number":7479,"context_line":"                                                        context, instance.uuid)"},{"line_number":7480,"context_line":"                for vif in vifs:"},{"line_number":7481,"context_line":"                    if not vif.address \u003d\u003d dev.mac_addr or vif.tag is None:"},{"line_number":7482,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":43,"id":"7aa08908_861ae2c4","line":7479,"updated":"2016-06-16 08:59:31.000000000","message":"Why not just initialize \u0027vifs\u0027 correctly at time of declaration. it will be extremely rare to have a guest with no network devices, so this isn\u0027t really helping efficiency in any meaningful way.","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"16b656314ab264267ab27252858fe08d0e9a435a","unresolved":false,"context_lines":[{"line_number":7476,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestInterface):"},{"line_number":7477,"context_line":"                if not vifs:"},{"line_number":7478,"context_line":"                    vifs \u003d obj_vif.VirtualInterfaceList.get_by_instance_uuid("},{"line_number":7479,"context_line":"                                                        context, instance.uuid)"},{"line_number":7480,"context_line":"                for vif in vifs:"},{"line_number":7481,"context_line":"                    if not vif.address \u003d\u003d dev.mac_addr or vif.tag is None:"},{"line_number":7482,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":43,"id":"5a9d85d2_0c3c33c7","line":7479,"in_reply_to":"7aa08908_861ae2c4","updated":"2016-06-21 14:08:35.000000000","message":"One of the previous reviews suggested that an optimization is needed. That\u0027s why it was here.","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"1d86ebd9247c159399c12c710f47ef4b54b19291","unresolved":false,"context_lines":[{"line_number":7484,"context_line":"                    device \u003d obj_device_metadata.NetworkInterfaceMetadata("},{"line_number":7485,"context_line":"                        mac\u003dvif.address,"},{"line_number":7486,"context_line":"                        bus\u003dbus,"},{"line_number":7487,"context_line":"                        tags\u003dvif.tag.split()"},{"line_number":7488,"context_line":"                    )"},{"line_number":7489,"context_line":"                    metadata.append(device)"},{"line_number":7490,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"7aa08908_8629429e","line":7487,"updated":"2016-06-16 09:04:19.000000000","message":"You shouldn\u0027t be calling split() here. The API is giving us a single tag - we shouldn\u0027t arbitrarily decide that it needs to be split on whitespace. If we want to allow multiple tags, then the public API \u0026 DB schema should directly represent multiple tags.  I don\u0027t think we need to do multiple tags though, so just remove the split(). Likewise for the BDM split()","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"16b656314ab264267ab27252858fe08d0e9a435a","unresolved":false,"context_lines":[{"line_number":7484,"context_line":"                    device \u003d obj_device_metadata.NetworkInterfaceMetadata("},{"line_number":7485,"context_line":"                        mac\u003dvif.address,"},{"line_number":7486,"context_line":"                        bus\u003dbus,"},{"line_number":7487,"context_line":"                        tags\u003dvif.tag.split()"},{"line_number":7488,"context_line":"                    )"},{"line_number":7489,"context_line":"                    metadata.append(device)"},{"line_number":7490,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"5a9d85d2_0cb1d347","line":7487,"in_reply_to":"7aa08908_8629429e","updated":"2016-06-21 14:08:35.000000000","message":"Right, the split was there just to make it a list, as it required by the schema.","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e07f2c3b571e355d111594f691c7c8c64f2397b7","unresolved":false,"context_lines":[{"line_number":7492,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestDisk):"},{"line_number":7493,"context_line":"                if not bdms:"},{"line_number":7494,"context_line":"                    bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7495,"context_line":"                                                        context, instance.uuid)"},{"line_number":7496,"context_line":"                for bdm in bdms:"},{"line_number":7497,"context_line":"                    device_name \u003d str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7498,"context_line":"                    if not device_name \u003d\u003d dev.target_dev or bdm.tag is None:"}],"source_content_type":"text/x-python","patch_set":43,"id":"7aa08908_c67a2af0","line":7495,"updated":"2016-06-16 08:59:31.000000000","message":"Again, just initialize this at time of declaration","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"16b656314ab264267ab27252858fe08d0e9a435a","unresolved":false,"context_lines":[{"line_number":7492,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestDisk):"},{"line_number":7493,"context_line":"                if not bdms:"},{"line_number":7494,"context_line":"                    bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7495,"context_line":"                                                        context, instance.uuid)"},{"line_number":7496,"context_line":"                for bdm in bdms:"},{"line_number":7497,"context_line":"                    device_name \u003d str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7498,"context_line":"                    if not device_name \u003d\u003d dev.target_dev or bdm.tag is None:"}],"source_content_type":"text/x-python","patch_set":43,"id":"5a9d85d2_cccb4ba4","line":7495,"in_reply_to":"7aa08908_c67a2af0","updated":"2016-06-21 14:08:35.000000000","message":"Done","commit_id":"91e396e6a347d55ff76f0fc2dc9dff50cd342717"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8a570adc0bb438fd17dbed664802ca2b9a7be212","unresolved":false,"context_lines":[{"line_number":7499,"context_line":"                    metadata.append(device)"},{"line_number":7500,"context_line":"        if metadata:"},{"line_number":7501,"context_line":"            dev_meta \u003d objects.InstanceDeviceMetadata("},{"line_number":7502,"context_line":"                                                   instance_uuid\u003dinstance.uuid,"},{"line_number":7503,"context_line":"                                                   devices\u003dmetadata)"},{"line_number":7504,"context_line":"            return dev_meta"},{"line_number":7505,"context_line":""}],"source_content_type":"text/x-python","patch_set":56,"id":"3aaa91ec_5b0fc187","line":7502,"updated":"2016-06-24 11:03:37.000000000","message":"The latest version of InstnaceDeviceMetadata patch does not have any \u0027instance_uuid\u0027 field.  IIRC, you\u0027re not seeing a failure because ovo Object just ignores unknown attributes in the constructor.","commit_id":"7719df87ec3f40f6b4f9e756f15dc4bc3353ba72"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"7876f7d426ef21e924ca9740e609c63fac480e9c","unresolved":false,"context_lines":[{"line_number":7499,"context_line":"                    metadata.append(device)"},{"line_number":7500,"context_line":"        if metadata:"},{"line_number":7501,"context_line":"            dev_meta \u003d objects.InstanceDeviceMetadata("},{"line_number":7502,"context_line":"                                                   instance_uuid\u003dinstance.uuid,"},{"line_number":7503,"context_line":"                                                   devices\u003dmetadata)"},{"line_number":7504,"context_line":"            return dev_meta"},{"line_number":7505,"context_line":""}],"source_content_type":"text/x-python","patch_set":56,"id":"3aaa91ec_dc560429","line":7502,"in_reply_to":"3aaa91ec_5b0fc187","updated":"2016-06-24 13:18:51.000000000","message":"Ah, absolutely, I\u0027ve missed it.. thanks.","commit_id":"7719df87ec3f40f6b4f9e756f15dc4bc3353ba72"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fa07bc55cdc918226ddbfed2ecd362aa4b52dbe9","unresolved":false,"context_lines":[{"line_number":3071,"context_line":"    def _create_configdrive(self, context, instance, admin_pass\u003dNone,"},{"line_number":3072,"context_line":"                            files\u003dNone, network_info\u003dNone, suffix\u003d\u0027\u0027):"},{"line_number":3073,"context_line":"        instance.device_metadata \u003d self._save_device_metadata(context,"},{"line_number":3074,"context_line":"                                                               instance)"},{"line_number":3075,"context_line":"        config_drive_image \u003d None"},{"line_number":3076,"context_line":"        if configdrive.required_by(instance):"},{"line_number":3077,"context_line":"            LOG.info(_LI(\u0027Using config drive\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_4e0f6ce1","line":3074,"range":{"start_line":3074,"start_character":63,"end_line":3074,"end_character":71},"updated":"2016-06-28 13:55:47.000000000","message":"Alignment nit","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"7fb81d69707f617c2b9006fa6916283db4663d90","unresolved":false,"context_lines":[{"line_number":3071,"context_line":"    def _create_configdrive(self, context, instance, admin_pass\u003dNone,"},{"line_number":3072,"context_line":"                            files\u003dNone, network_info\u003dNone, suffix\u003d\u0027\u0027):"},{"line_number":3073,"context_line":"        instance.device_metadata \u003d self._save_device_metadata(context,"},{"line_number":3074,"context_line":"                                                               instance)"},{"line_number":3075,"context_line":"        config_drive_image \u003d None"},{"line_number":3076,"context_line":"        if configdrive.required_by(instance):"},{"line_number":3077,"context_line":"            LOG.info(_LI(\u0027Using config drive\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_f591bf47","line":3074,"range":{"start_line":3074,"start_character":63,"end_line":3074,"end_character":71},"in_reply_to":"3aaa91ec_4e0f6ce1","updated":"2016-06-28 16:29:35.000000000","message":"Done","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fa07bc55cdc918226ddbfed2ecd362aa4b52dbe9","unresolved":false,"context_lines":[{"line_number":7473,"context_line":"        address \u003d (dev.device_addr.format_address() if"},{"line_number":7474,"context_line":"                   dev.device_addr else None)"},{"line_number":7475,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7476,"context_line":"                                  vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7477,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7478,"context_line":"        elif dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7479,"context_line":"            bus \u003d objects.SCSIDeviceBus()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_2ba2d24b","line":7476,"range":{"start_line":7476,"start_character":34,"end_line":7476,"end_character":78},"updated":"2016-06-28 13:55:47.000000000","message":"This is very strange alignment :/","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"7fb81d69707f617c2b9006fa6916283db4663d90","unresolved":false,"context_lines":[{"line_number":7473,"context_line":"        address \u003d (dev.device_addr.format_address() if"},{"line_number":7474,"context_line":"                   dev.device_addr else None)"},{"line_number":7475,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7476,"context_line":"                                  vconfig.LibvirtConfigGuestDeviceAddressPCI):"},{"line_number":7477,"context_line":"            bus \u003d objects.PCIDeviceBus()"},{"line_number":7478,"context_line":"        elif dev.target_bus \u003d\u003d \u0027scsi\u0027:"},{"line_number":7479,"context_line":"            bus \u003d objects.SCSIDeviceBus()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_359ce75c","line":7476,"range":{"start_line":7476,"start_character":34,"end_line":7476,"end_character":78},"in_reply_to":"3aaa91ec_2ba2d24b","updated":"2016-06-28 16:29:35.000000000","message":"Done","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fa07bc55cdc918226ddbfed2ecd362aa4b52dbe9","unresolved":false,"context_lines":[{"line_number":7492,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7493,"context_line":"                                                                 instance.uuid)"},{"line_number":7494,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid(context,"},{"line_number":7495,"context_line":"                                                                 instance.uuid)"},{"line_number":7496,"context_line":"        metadata \u003d []"},{"line_number":7497,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":7498,"context_line":"        xml \u003d guest.get_xml_desc()"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_abf7e257","line":7495,"range":{"start_line":7495,"start_character":65,"end_line":7495,"end_character":78},"updated":"2016-06-28 13:55:47.000000000","message":"Alignment nit here","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fa07bc55cdc918226ddbfed2ecd362aa4b52dbe9","unresolved":false,"context_lines":[{"line_number":7503,"context_line":"        for dev in guest_config.devices:"},{"line_number":7504,"context_line":"            # Build network intefaces related metedata"},{"line_number":7505,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestInterface):"},{"line_number":7506,"context_line":"                for vif in vifs:"},{"line_number":7507,"context_line":"                    if not vif.address \u003d\u003d dev.mac_addr or vif.tag is None:"},{"line_number":7508,"context_line":"                        continue"},{"line_number":7509,"context_line":"                    bus \u003d self._prepare_device_bus(dev)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_4e412c0b","line":7506,"range":{"start_line":7506,"start_character":16,"end_line":7506,"end_character":32},"updated":"2016-06-28 13:55:47.000000000","message":"This is getting pretty deeply nested at this point, and it also makes this O(n\u00262) which is kinda nasty. I know we likely won\u0027t have lots of devices, but...\n\nHow about we pre-create a dict of VIFs indexed by vif.address, and a dict of disks indexed by device name, and then we can rather easily march through the guest devices and pull out those devices, if present and tag them?","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"7fb81d69707f617c2b9006fa6916283db4663d90","unresolved":false,"context_lines":[{"line_number":7503,"context_line":"        for dev in guest_config.devices:"},{"line_number":7504,"context_line":"            # Build network intefaces related metedata"},{"line_number":7505,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestInterface):"},{"line_number":7506,"context_line":"                for vif in vifs:"},{"line_number":7507,"context_line":"                    if not vif.address \u003d\u003d dev.mac_addr or vif.tag is None:"},{"line_number":7508,"context_line":"                        continue"},{"line_number":7509,"context_line":"                    bus \u003d self._prepare_device_bus(dev)"}],"source_content_type":"text/x-python","patch_set":58,"id":"3aaa91ec_d5c48341","line":7506,"range":{"start_line":7506,"start_character":16,"end_line":7506,"end_character":32},"in_reply_to":"3aaa91ec_4e412c0b","updated":"2016-06-28 16:29:35.000000000","message":"Done","commit_id":"cb03b2d4a3c238fd03e162914e9391904f9c91e7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31deae3517a22469eb6c4877866b113ef1be0de9","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _create_configdrive(self, context, instance, admin_pass\u003dNone,"},{"line_number":3074,"context_line":"                            files\u003dNone, network_info\u003dNone, suffix\u003d\u0027\u0027):"},{"line_number":3075,"context_line":"        instance.device_metadata \u003d self._save_device_metadata(context,"},{"line_number":3076,"context_line":"                                                              instance)"},{"line_number":3077,"context_line":"        config_drive_image \u003d None"},{"line_number":3078,"context_line":"        if configdrive.required_by(instance):"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_4c412239","line":3075,"updated":"2016-06-28 19:31:33.000000000","message":"So, we do this here so it\u0027s available in the metadata service if configdrive.required_by(instance) is False, right? Otherwise it sort of looks weird, might be nice to add a comment to point that out.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cb92491936716e56d945efced8c48f50fcd404a8","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _create_configdrive(self, context, instance, admin_pass\u003dNone,"},{"line_number":3074,"context_line":"                            files\u003dNone, network_info\u003dNone, suffix\u003d\u0027\u0027):"},{"line_number":3075,"context_line":"        instance.device_metadata \u003d self._save_device_metadata(context,"},{"line_number":3076,"context_line":"                                                              instance)"},{"line_number":3077,"context_line":"        config_drive_image \u003d None"},{"line_number":3078,"context_line":"        if configdrive.required_by(instance):"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_cc04f2c7","line":3075,"in_reply_to":"3aaa91ec_4c412239","updated":"2016-06-28 19:38:35.000000000","message":"We need to do this sometime after we have built the XML but before we start the guest. This method is really \"create config drive if needed\", but gets called at the right part of the sequence because of that. So it could be called \"handle metadata related bits at the right time and maybe create config drive\" but otherwise it seems fine to me. A comment above about the importance of the _save_device_metadata() call being there would be fine of course if that helps.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _create_configdrive(self, context, instance, admin_pass\u003dNone,"},{"line_number":3074,"context_line":"                            files\u003dNone, network_info\u003dNone, suffix\u003d\u0027\u0027):"},{"line_number":3075,"context_line":"        instance.device_metadata \u003d self._save_device_metadata(context,"},{"line_number":3076,"context_line":"                                                              instance)"},{"line_number":3077,"context_line":"        config_drive_image \u003d None"},{"line_number":3078,"context_line":"        if configdrive.required_by(instance):"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_22c5bd3c","line":3075,"in_reply_to":"3aaa91ec_cc04f2c7","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31deae3517a22469eb6c4877866b113ef1be0de9","unresolved":false,"context_lines":[{"line_number":7472,"context_line":"    def _prepare_device_bus(dev):"},{"line_number":7473,"context_line":"        \"\"\"Determins the device bus and it\u0027s hypervisor assigned address"},{"line_number":7474,"context_line":"        \"\"\""},{"line_number":7475,"context_line":"        address \u003d (dev.device_addr.format_address() if"},{"line_number":7476,"context_line":"                   dev.device_addr else None)"},{"line_number":7477,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7478,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_31598d5c","line":7475,"range":{"start_line":7475,"start_character":35,"end_line":7475,"end_character":49},"updated":"2016-06-28 19:31:33.000000000","message":"This only returns something for LibvirtConfigGuestDeviceAddressPCI, is that what you expected? Seems to be pretty tightly coupled with how these things are parsed and modeled.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":7472,"context_line":"    def _prepare_device_bus(dev):"},{"line_number":7473,"context_line":"        \"\"\"Determins the device bus and it\u0027s hypervisor assigned address"},{"line_number":7474,"context_line":"        \"\"\""},{"line_number":7475,"context_line":"        address \u003d (dev.device_addr.format_address() if"},{"line_number":7476,"context_line":"                   dev.device_addr else None)"},{"line_number":7477,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7478,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_e7b8e777","line":7475,"range":{"start_line":7475,"start_character":35,"end_line":7475,"end_character":49},"in_reply_to":"3aaa91ec_31598d5c","updated":"2016-06-28 20:50:27.000000000","message":"Currently we can get a meaningful address only from PCI.\nThis is why it\u0027s build this way, however, we will change these to return addresses SCSI, IDE and USB once we will figure out a way to do it correctly.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cb92491936716e56d945efced8c48f50fcd404a8","unresolved":false,"context_lines":[{"line_number":7472,"context_line":"    def _prepare_device_bus(dev):"},{"line_number":7473,"context_line":"        \"\"\"Determins the device bus and it\u0027s hypervisor assigned address"},{"line_number":7474,"context_line":"        \"\"\""},{"line_number":7475,"context_line":"        address \u003d (dev.device_addr.format_address() if"},{"line_number":7476,"context_line":"                   dev.device_addr else None)"},{"line_number":7477,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7478,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_4cf922c8","line":7475,"range":{"start_line":7475,"start_character":35,"end_line":7475,"end_character":49},"in_reply_to":"3aaa91ec_31598d5c","updated":"2016-06-28 19:38:35.000000000","message":"Yeah, I think only PCI devices have a device_address at the moment. Other types of devices could later.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"77edb41574abfd4bd09fd0b23e0a4d8c5f6638cc","unresolved":false,"context_lines":[{"line_number":7472,"context_line":"    def _prepare_device_bus(dev):"},{"line_number":7473,"context_line":"        \"\"\"Determins the device bus and it\u0027s hypervisor assigned address"},{"line_number":7474,"context_line":"        \"\"\""},{"line_number":7475,"context_line":"        address \u003d (dev.device_addr.format_address() if"},{"line_number":7476,"context_line":"                   dev.device_addr else None)"},{"line_number":7477,"context_line":"        if isinstance(dev.device_addr,"},{"line_number":7478,"context_line":"                      vconfig.LibvirtConfigGuestDeviceAddressPCI):"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_678b5769","line":7475,"range":{"start_line":7475,"start_character":35,"end_line":7475,"end_character":49},"in_reply_to":"3aaa91ec_4cf922c8","updated":"2016-06-28 19:57:36.000000000","message":"I was looking at https://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L1149 which would also have LibvirtConfigGuestDeviceAddressDrive which returns None:\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L1172","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d84fe10e38647bb2985a44311bd4ea6698de7c3d","unresolved":false,"context_lines":[{"line_number":7487,"context_line":"            bus.address \u003d address"},{"line_number":7488,"context_line":"        return bus"},{"line_number":7489,"context_line":""},{"line_number":7490,"context_line":"    def _save_device_metadata(self, context, instance):"},{"line_number":7491,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":7492,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":7493,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_2cbf76fd","line":7490,"range":{"start_line":7490,"start_character":9,"end_line":7490,"end_character":13},"updated":"2016-06-28 19:34:09.000000000","message":"nit: we dont actually save anything here, i.e. we don\u0027t update the database right? So this is really more like _build_device_metadata or something.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":7487,"context_line":"            bus.address \u003d address"},{"line_number":7488,"context_line":"        return bus"},{"line_number":7489,"context_line":""},{"line_number":7490,"context_line":"    def _save_device_metadata(self, context, instance):"},{"line_number":7491,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":7492,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":7493,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_677cf7b5","line":7490,"range":{"start_line":7490,"start_character":9,"end_line":7490,"end_character":13},"in_reply_to":"3aaa91ec_2cbf76fd","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31deae3517a22469eb6c4877866b113ef1be0de9","unresolved":false,"context_lines":[{"line_number":7491,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":7492,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":7493,"context_line":"        \"\"\""},{"line_number":7494,"context_line":"        def _get_device_name(bdm):"},{"line_number":7495,"context_line":"            return str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7496,"context_line":""},{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_71edd5b6","line":7494,"updated":"2016-06-28 19:31:33.000000000","message":"Seems we should reuse nova.block_device.strip_dev here?","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":7491,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":7492,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":7493,"context_line":"        \"\"\""},{"line_number":7494,"context_line":"        def _get_device_name(bdm):"},{"line_number":7495,"context_line":"            return str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7496,"context_line":""},{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_e83a7733","line":7494,"in_reply_to":"3aaa91ec_71edd5b6","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31deae3517a22469eb6c4877866b113ef1be0de9","unresolved":false,"context_lines":[{"line_number":7494,"context_line":"        def _get_device_name(bdm):"},{"line_number":7495,"context_line":"            return str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7496,"context_line":""},{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7498,"context_line":"                                                                 instance.uuid)"},{"line_number":7499,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7500,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_911de168","line":7497,"updated":"2016-06-28 19:31:33.000000000","message":"At this point in the series these don\u0027t exist yet, they get created in the compute manager here:\n\nhttps://review.openstack.org/#/c/264017/62/nova/compute/manager.py\n\nWhich comes after this, but I guess everyone else already knew that, and as Dan pointed out in IRC it doesn\u0027t matter until you get through the API service version check anyway as this code will be deployed with the compute manager code at the same time.\n\n--\n\nIt also sucks that we have to do a DB lookup every time we build a config drive, it would be nicer if the compute manager passed this information down to the virt driver, but we also do BDM lookups from the driver at times, so this isn\u0027t anything new. And it sounds like Dan has plans for doing some kind of bulk query too.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"146b791e02d84af8e3801c501ba46c7d99f7a3d2","unresolved":false,"context_lines":[{"line_number":7494,"context_line":"        def _get_device_name(bdm):"},{"line_number":7495,"context_line":"            return str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7496,"context_line":""},{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7498,"context_line":"                                                                 instance.uuid)"},{"line_number":7499,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7500,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_e76f47fa","line":7497,"in_reply_to":"3aaa91ec_6c15fee3","updated":"2016-06-28 20:02:36.000000000","message":"Umm, doesn\u0027t the network_info that is passed into spawn() also have the MAC address information that we can use? That\u0027s what\u0027s used to create the VIFs in the DB in the compute manager in the next patch.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cb92491936716e56d945efced8c48f50fcd404a8","unresolved":false,"context_lines":[{"line_number":7494,"context_line":"        def _get_device_name(bdm):"},{"line_number":7495,"context_line":"            return str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7496,"context_line":""},{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7498,"context_line":"                                                                 instance.uuid)"},{"line_number":7499,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7500,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_6c15fee3","line":7497,"in_reply_to":"3aaa91ec_911de168","updated":"2016-06-28 19:38:35.000000000","message":"Yep","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"30b9c0e1b4f63d3a55847a067f6fe164d3405fa0","unresolved":false,"context_lines":[{"line_number":7494,"context_line":"        def _get_device_name(bdm):"},{"line_number":7495,"context_line":"            return str(bdm.device_name.rsplit(\u0027/\u0027, 1)[1])"},{"line_number":7496,"context_line":""},{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7498,"context_line":"                                                                 instance.uuid)"},{"line_number":7499,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7500,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_47257bc0","line":7497,"in_reply_to":"3aaa91ec_e76f47fa","updated":"2016-06-28 20:04:53.000000000","message":"Nevermind, we don\u0027t have requested_networks in spawn() so we don\u0027t have enough info to correlate which networks are tagged.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31deae3517a22469eb6c4877866b113ef1be0de9","unresolved":false,"context_lines":[{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7498,"context_line":"                                                                 instance.uuid)"},{"line_number":7499,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7500,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7501,"context_line":"            context, instance.uuid)"},{"line_number":7502,"context_line":"        tagged_bdms \u003d {_get_device_name(bdm): bdm for bdm in bdms if bdm.tag}"},{"line_number":7503,"context_line":""}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_11b0d1dc","line":7500,"updated":"2016-06-28 19:31:33.000000000","message":"Similar comment on doing the DB lookups, don\u0027t we already have block_device_info[\u0027block_device_mapping\u0027] in a lot of cases by the time we create the config drive? That might require changes to nova.virt.block_device.DriverBlockDevice to proxy the bdm.tag attribute call, but it would avoid the lookup if we had the BDMs already.\n\nThis could probably be a TODO for a later optimization also if that works.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cb92491936716e56d945efced8c48f50fcd404a8","unresolved":false,"context_lines":[{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7498,"context_line":"                                                                 instance.uuid)"},{"line_number":7499,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7500,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7501,"context_line":"            context, instance.uuid)"},{"line_number":7502,"context_line":"        tagged_bdms \u003d {_get_device_name(bdm): bdm for bdm in bdms if bdm.tag}"},{"line_number":7503,"context_line":""}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_2c90164e","line":7500,"in_reply_to":"3aaa91ec_11b0d1dc","updated":"2016-06-28 19:38:35.000000000","message":"Not that I see in the chain it\u0027s being called from unless they can do more plumbing to get them in here.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"77edb41574abfd4bd09fd0b23e0a4d8c5f6638cc","unresolved":false,"context_lines":[{"line_number":7497,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7498,"context_line":"                                                                 instance.uuid)"},{"line_number":7499,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7500,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7501,"context_line":"            context, instance.uuid)"},{"line_number":7502,"context_line":"        tagged_bdms \u003d {_get_device_name(bdm): bdm for bdm in bdms if bdm.tag}"},{"line_number":7503,"context_line":""}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_879f635e","line":7500,"in_reply_to":"3aaa91ec_2c90164e","updated":"2016-06-28 19:57:36.000000000","message":"I was looking at _prep_block_device in the compute manager which calls into the virt layer here:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1562\n\nwhich converts the BDMs to DriverBlockDevice objects here:\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/driver.py#L69\n\nwhich gives you a list of DriverBlockDevice objects, which are wrappers around the BlockDeviceMapping object, which has the tag in it - but as noted, we\u0027d have to modify DriverBlockDevice to access the tag via the _proxy_as_attr variable.\n\nBut at least for spawn(), I think we have the block_device_info[\u0027block_device_mapping\u0027] at this point which is the list of DriverBlockDevice wrapper objects so we don\u0027t need to perform this lookup to the DB since it\u0027s passed in from the compute manager.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31deae3517a22469eb6c4877866b113ef1be0de9","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"            context, instance.uuid)"},{"line_number":7502,"context_line":"        tagged_bdms \u003d {_get_device_name(bdm): bdm for bdm in bdms if bdm.tag}"},{"line_number":7503,"context_line":""},{"line_number":7504,"context_line":"        metadata \u003d []"},{"line_number":7505,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":7506,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":7507,"context_line":"        xml_dom \u003d etree.fromstring(xml)"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_f10ee58a","line":7504,"range":{"start_line":7504,"start_character":8,"end_line":7504,"end_character":16},"updated":"2016-06-28 19:31:33.000000000","message":"nit: I\u0027d probably call this devices so it\u0027s not confused with instance.metadata.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"            context, instance.uuid)"},{"line_number":7502,"context_line":"        tagged_bdms \u003d {_get_device_name(bdm): bdm for bdm in bdms if bdm.tag}"},{"line_number":7503,"context_line":""},{"line_number":7504,"context_line":"        metadata \u003d []"},{"line_number":7505,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":7506,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":7507,"context_line":"        xml_dom \u003d etree.fromstring(xml)"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_e259f536","line":7504,"range":{"start_line":7504,"start_character":8,"end_line":7504,"end_character":16},"in_reply_to":"3aaa91ec_f10ee58a","updated":"2016-06-28 20:50:27.000000000","message":"Done","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"be8a8a831e3610be9d3aad39a0e20f1562cba52a","unresolved":false,"context_lines":[{"line_number":7511,"context_line":"        for dev in guest_config.devices:"},{"line_number":7512,"context_line":"            # Build network intefaces related metedata"},{"line_number":7513,"context_line":"            if isinstance(dev, vconfig.LibvirtConfigGuestInterface):"},{"line_number":7514,"context_line":"                vif \u003d tagged_vifs.get(dev.mac_addr)"},{"line_number":7515,"context_line":"                if not vif:"},{"line_number":7516,"context_line":"                    continue"},{"line_number":7517,"context_line":"                bus \u003d self._prepare_device_bus(dev)"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_d39ad1c4","line":7514,"updated":"2016-06-28 18:24:54.000000000","message":"Much nicer, thanks :)","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31deae3517a22469eb6c4877866b113ef1be0de9","unresolved":false,"context_lines":[{"line_number":7528,"context_line":"                if not bdm:"},{"line_number":7529,"context_line":"                    continue"},{"line_number":7530,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7531,"context_line":"                device \u003d objects.DiskMetadata(bus\u003dbus, tags\u003d[bdm.tag])"},{"line_number":7532,"context_line":"                metadata.append(device)"},{"line_number":7533,"context_line":"        if metadata:"},{"line_number":7534,"context_line":"            dev_meta \u003d objects.InstanceDeviceMetadata(devices\u003dmetadata)"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_ec228ea2","line":7531,"range":{"start_line":7531,"start_character":33,"end_line":7531,"end_character":45},"updated":"2016-06-28 19:31:33.000000000","message":"We don\u0027t care about setting serial or path on this?","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":8802,"name":"Vladik Romanovsky","email":"vromanso@redhat.com","username":"vladikr"},"change_message_id":"e1900732e6cb5f9f3572040065ab587aa5ae3455","unresolved":false,"context_lines":[{"line_number":7528,"context_line":"                if not bdm:"},{"line_number":7529,"context_line":"                    continue"},{"line_number":7530,"context_line":"                bus \u003d self._prepare_device_bus(dev)"},{"line_number":7531,"context_line":"                device \u003d objects.DiskMetadata(bus\u003dbus, tags\u003d[bdm.tag])"},{"line_number":7532,"context_line":"                metadata.append(device)"},{"line_number":7533,"context_line":"        if metadata:"},{"line_number":7534,"context_line":"            dev_meta \u003d objects.InstanceDeviceMetadata(devices\u003dmetadata)"}],"source_content_type":"text/x-python","patch_set":59,"id":"3aaa91ec_0709b32c","line":7531,"range":{"start_line":7531,"start_character":33,"end_line":7531,"end_character":45},"in_reply_to":"3aaa91ec_ec228ea2","updated":"2016-06-28 20:50:27.000000000","message":"Path is more for containers use (physical path to a device), but at the moment we don\u0027t implement that.\nAs for serial, we don\u0027t currently set nor retrieve it.\nWe might set those when we will have an actual use case.","commit_id":"1461a3e5e3de5b3eabfdbe4fc9abe9069b6c13a3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fe3578f5b08f7510157ce3f462d149045cd66d77","unresolved":false,"context_lines":[{"line_number":7501,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":7502,"context_line":"                                                                 instance.uuid)"},{"line_number":7503,"context_line":"        tagged_vifs \u003d {vif.address: vif for vif in vifs if vif.tag}"},{"line_number":7504,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":7505,"context_line":"            context, instance.uuid)"},{"line_number":7506,"context_line":"        tagged_bdms \u003d {_get_device_name(bdm): bdm for bdm in bdms if bdm.tag}"},{"line_number":7507,"context_line":""}],"source_content_type":"text/x-python","patch_set":60,"id":"3aaa91ec_ef41d63a","line":7504,"updated":"2016-06-29 01:40:08.000000000","message":"If you respin, I\u0027d like a TODO on this since I think we can use the block_device_info[\u0027block_device_mapping\u0027] that\u0027s passed into spawn, reboot and finish_migration so we don\u0027t actually have to do another DB query here - the compute manager already did that work and is passing it to the virt driver. In the case of rescue it\u0027s on there, but we could make the block_device_info parameter optional in this method for rescue.","commit_id":"b466c382eaecb95566357b473e599f749baeaf6f"}]}