)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"dc704faffb4c3c99fe7a89141a56bedcbeb11c20","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Sets instance\u0027s devices_metadata during spawn."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"NOTE: can\u0027t have multiple parent commits with git. This"},{"line_number":12,"context_line":"should be on top of the virt-device-role-tagging blueprint."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Partially-Implements: blueprint virt-device-role-tagging"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3aaa91ec_4be0ef96","line":11,"updated":"2016-06-28 18:12:03.000000000","message":"If the goal is to merge this in Newton we should probably change this commit message :)","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"}],"nova/tests/unit/virt/hyperv/test_block_device_manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":86,"context_line":"            [mock.call(bus\u003dmock_get_device_bus.return_value, tags\u003d[bdm.tag]),"},{"line_number":87,"context_line":"             mock.call(bus\u003dmock_get_device_bus.return_value, tags\u003d[eph.tag])],"},{"line_number":88,"context_line":"            any_order\u003dTrue)"},{"line_number":89,"context_line":"        self.assertEqual([mock_DiskMetadata.return_value] * 2, bdm_metadata)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @mock.patch(\u0027nova.virt.configdrive.required_by\u0027)"},{"line_number":92,"context_line":"    def test_init_controller_slot_counter_gen1_no_configdrive("}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_03e0d9f9","line":89,"range":{"start_line":89,"start_character":8,"end_line":89,"end_character":76},"updated":"2016-07-13 21:45:28.000000000","message":"This confuses me...","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"}],"nova/tests/unit/virt/hyperv/test_vmops.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":341,"context_line":"        mock_create_dynamic_vhd.assert_called_once_with(\u0027fake_eph_path\u0027,"},{"line_number":342,"context_line":"                                                        10 * units.Gi)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    @mock.patch.object(vmops.objects, \u0027PCIDeviceBus\u0027)"},{"line_number":345,"context_line":"    @mock.patch.object(vmops.objects, \u0027NetworkInterfaceMetadata\u0027)"},{"line_number":346,"context_line":"    @mock.patch.object(vmops.objects.VirtualInterfaceList,"},{"line_number":347,"context_line":"                       \u0027get_by_instance_uuid\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_4dae9030","line":344,"updated":"2016-07-13 21:45:28.000000000","message":"It seems weird that you are mocking this out when all the code does is construct it but doesn\u0027t call anything on that object.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":347,"context_line":"                       \u0027get_by_instance_uuid\u0027)"},{"line_number":348,"context_line":"    def test_get_vif_metadata(self, mock_get_by_inst_uuid,"},{"line_number":349,"context_line":"                              mock_NetworkInterfaceMetadata, mock_PCIDevBus):"},{"line_number":350,"context_line":"        mock_vif \u003d mock.MagicMock(tag\u003d\u0027taggy\u0027)"},{"line_number":351,"context_line":"        mock_vif.__contains__.side_effect \u003d ("},{"line_number":352,"context_line":"            lambda attr: getattr(mock_vif, attr, None) is not None)"},{"line_number":353,"context_line":"        mock_get_by_inst_uuid.return_value \u003d [mock_vif,"},{"line_number":354,"context_line":"                                              mock.MagicMock(tag\u003dNone)]"},{"line_number":355,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_4d5cd04c","line":352,"range":{"start_line":350,"start_character":8,"end_line":352,"end_character":67},"updated":"2016-07-13 21:45:28.000000000","message":"I would just create a fake VirtualInterface object, there isn\u0027t a reason to mock that class out.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":351,"context_line":"        mock_vif.__contains__.side_effect \u003d ("},{"line_number":352,"context_line":"            lambda attr: getattr(mock_vif, attr, None) is not None)"},{"line_number":353,"context_line":"        mock_get_by_inst_uuid.return_value \u003d [mock_vif,"},{"line_number":354,"context_line":"                                              mock.MagicMock(tag\u003dNone)]"},{"line_number":355,"context_line":""},{"line_number":356,"context_line":"        vif_metadata \u003d self._vmops._get_vif_metadata(self.context,"},{"line_number":357,"context_line":"                                                     mock.sentinel.instance_id)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_e8fc8a0e","line":354,"range":{"start_line":354,"start_character":46,"end_line":354,"end_character":70},"updated":"2016-07-13 21:45:28.000000000","message":"ditto, I\u0027d just have an empty VirtualInterface object.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":358,"context_line":""},{"line_number":359,"context_line":"        mock_get_by_inst_uuid.assert_called_once_with("},{"line_number":360,"context_line":"            self.context, mock.sentinel.instance_id)"},{"line_number":361,"context_line":"        mock_NetworkInterfaceMetadata.assert_called_once_with("},{"line_number":362,"context_line":"            mac\u003dmock_vif.address,"},{"line_number":363,"context_line":"            bus\u003dmock_PCIDevBus.return_value,"},{"line_number":364,"context_line":"            tags\u003d[mock_vif.tag])"},{"line_number":365,"context_line":"        self.assertEqual([mock_NetworkInterfaceMetadata.return_value],"},{"line_number":366,"context_line":"                         vif_metadata)"},{"line_number":367,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_286cd2cf","line":364,"range":{"start_line":361,"start_character":8,"end_line":364,"end_character":32},"updated":"2016-07-13 21:45:28.000000000","message":"Same comment, I would have just asserted that the thing we get back in the list from the method contains a single entry and we know it\u0027s values. But anyway...","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"}],"nova/virt/hyperv/block_device_manager.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c7c48b601864c0efdbde195a5d12a9b1ee845bf8","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                                str(bdm[\u0027drive_addr\u0027])])"},{"line_number":60,"context_line":"            bus \u003d objects.IDEDeviceBus(address\u003daddress)"},{"line_number":61,"context_line":"        else:"},{"line_number":62,"context_line":"            raise exception.InvalidBDMBusType(bus_type\u003dbdm[\u0027disk_bus\u0027])"},{"line_number":63,"context_line":"        return bus"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def get_bdm_metadata(self, context, instance, block_device_info):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_bc49fb8d","line":62,"updated":"2016-06-22 12:19:17.000000000","message":"I\u0027m not sure I\u0027d raise here, as we don\u0027t necessarily want to fail if the bus type is unknown, especially since this is called on instance spawn. Maybe log it, but we can still present metadata and tags for whatever devices were were able to process.","commit_id":"d4f5beaa9113b100e6265660167d6dd42dff9cb3"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e9657e7bcfdd1f4c1cff8ac29968d12e803c4a09","unresolved":false,"context_lines":[{"line_number":59,"context_line":"                                str(bdm[\u0027drive_addr\u0027])])"},{"line_number":60,"context_line":"            bus \u003d objects.IDEDeviceBus(address\u003daddress)"},{"line_number":61,"context_line":"        else:"},{"line_number":62,"context_line":"            raise exception.InvalidBDMBusType(bus_type\u003dbdm[\u0027disk_bus\u0027])"},{"line_number":63,"context_line":"        return bus"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def get_bdm_metadata(self, context, instance, block_device_info):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_8c120eb3","line":62,"in_reply_to":"3aaa91ec_bc49fb8d","updated":"2016-06-22 12:56:03.000000000","message":"hm, we apparently set in within this module. it can\u0027t be anything else other than ide or scsi, it seems. will remove it.","commit_id":"d4f5beaa9113b100e6265660167d6dd42dff9cb3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c7c48b601864c0efdbde195a5d12a9b1ee845bf8","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                    [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            bdm_obj \u003d bdm_obj_map.get(bdm[\u0027mount_device\u0027])"},{"line_number":80,"context_line":"            if not bdm_obj:"},{"line_number":81,"context_line":"                raise exception.InvalidBDM(_("},{"line_number":82,"context_line":"                    \u0027block_device_info contains unknown BDM: %s\u0027) % bdm)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if not bdm_obj.tag:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_97ffb800","line":81,"updated":"2016-06-22 12:19:17.000000000","message":"Same qualm about raising, though I\u0027m less sure it\u0027s a valid concern here. Under what conditions would this code be reached?","commit_id":"d4f5beaa9113b100e6265660167d6dd42dff9cb3"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"e9657e7bcfdd1f4c1cff8ac29968d12e803c4a09","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                    [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            bdm_obj \u003d bdm_obj_map.get(bdm[\u0027mount_device\u0027])"},{"line_number":80,"context_line":"            if not bdm_obj:"},{"line_number":81,"context_line":"                raise exception.InvalidBDM(_("},{"line_number":82,"context_line":"                    \u0027block_device_info contains unknown BDM: %s\u0027) % bdm)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if not bdm_obj.tag:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_acb9caeb","line":81,"in_reply_to":"3aaa91ec_97ffb800","updated":"2016-06-22 12:56:03.000000000","message":"hopefully, never. this would happen if the block_device_info given at spawn and objects.BlockDeviceMappingList.get_by_instance_uuid return different things. But that\u0027s probably just me being a bit paranoid.","commit_id":"d4f5beaa9113b100e6265660167d6dd42dff9cb3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"dc704faffb4c3c99fe7a89141a56bedcbeb11c20","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                    [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":76,"context_line":"            bdm_obj \u003d bdm_obj_map.get(bdm[\u0027mount_device\u0027])"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"            if not bdm_obj or not bdm_obj.tag:"},{"line_number":79,"context_line":"                # bdm is not tagged, no need to create metadata"},{"line_number":80,"context_line":"                continue"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_bd39d6a5","line":78,"updated":"2016-06-28 18:12:03.000000000","message":"I think we may run into issues if we check for bdm_obj.tag directly, maybe it\u0027s better to use obj_attr_is_set to be safer.","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"1ce0206cc77ff152446941a92a85a3ebd9ae4920","unresolved":false,"context_lines":[{"line_number":75,"context_line":"                    [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":76,"context_line":"            bdm_obj \u003d bdm_obj_map.get(bdm[\u0027mount_device\u0027])"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"            if not bdm_obj or not bdm_obj.tag:"},{"line_number":79,"context_line":"                # bdm is not tagged, no need to create metadata"},{"line_number":80,"context_line":"                continue"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_8f41230e","line":78,"in_reply_to":"3aaa91ec_bd39d6a5","updated":"2016-06-29 09:32:40.000000000","message":"Done","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f98e68ceee3358758e5ebe62aae7611dd8121a8b","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":66,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        # block_device_info does not contain tags information."},{"line_number":69,"context_line":"        bdm_obj_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":70,"context_line":"            context, instance.uuid)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_6f429b10","line":68,"updated":"2016-06-30 18:21:21.000000000","message":"I have a TODO in the libvirt driver for this, I think block_device_info[\u0027block_device_mapping\u0027] would have the tags because it\u0027s the list of DriverBlockDevice objects that wrap the BDM objects which have the tag, but DriverBlockDevice doesn\u0027t expose a passthrough getattr for the tag, so we have to update that.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bd64a4b0d62f3d11182b8373e2b696bc423313e5","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":66,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        # block_device_info does not contain tags information."},{"line_number":69,"context_line":"        bdm_obj_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":70,"context_line":"            context, instance.uuid)"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_b57970ca","line":68,"in_reply_to":"3aaa91ec_6f429b10","updated":"2016-06-30 19:06:58.000000000","message":"sure, I can change this as soon as block_device_info contains tags. it will simplify the logic a lot, IMO.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a7588e334650db98a9ef1d8cf37eb568df342b4a","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        bdm_obj_map \u003d {bdm.device_name: bdm for bdm in bdm_obj_list}"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        bdm_metadata \u003d []"},{"line_number":76,"context_line":"        for bdm in itertools.chain(block_device_info[\u0027block_device_mapping\u0027],"},{"line_number":77,"context_line":"                                   block_device_info[\u0027ephemerals\u0027],"},{"line_number":78,"context_line":"                                   [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            # NOTE(claudiub): ephemerals have device_name instead of"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_3a28cf85","line":76,"updated":"2016-06-30 18:26:47.000000000","message":"What about swaps, I think those can be tagged?","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bd64a4b0d62f3d11182b8373e2b696bc423313e5","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        bdm_obj_map \u003d {bdm.device_name: bdm for bdm in bdm_obj_list}"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        bdm_metadata \u003d []"},{"line_number":76,"context_line":"        for bdm in itertools.chain(block_device_info[\u0027block_device_mapping\u0027],"},{"line_number":77,"context_line":"                                   block_device_info[\u0027ephemerals\u0027],"},{"line_number":78,"context_line":"                                   [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            # NOTE(claudiub): ephemerals have device_name instead of"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_5513ec64","line":76,"in_reply_to":"3aaa91ec_3a28cf85","updated":"2016-06-30 19:06:58.000000000","message":"it seems we don\u0027t currently support swaps, at the moment..\n\nBut, adding support for it shouldn\u0027t be bigger than 15-20 LOC (w/o unit tests). so I can do that, if it can still be accepted until FF.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f98e68ceee3358758e5ebe62aae7611dd8121a8b","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        bdm_metadata \u003d []"},{"line_number":76,"context_line":"        for bdm in itertools.chain(block_device_info[\u0027block_device_mapping\u0027],"},{"line_number":77,"context_line":"                                   block_device_info[\u0027ephemerals\u0027],"},{"line_number":78,"context_line":"                                   [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            # NOTE(claudiub): ephemerals have device_name instead of"},{"line_number":80,"context_line":"            # mount_device."},{"line_number":81,"context_line":"            device_name \u003d bdm.get(\u0027mount_device\u0027) or bdm.get(\u0027device_name\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_6fde1b3f","line":78,"range":{"start_line":78,"start_character":55,"end_line":78,"end_character":64},"updated":"2016-06-30 18:21:21.000000000","message":"Where does this get set?","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bd64a4b0d62f3d11182b8373e2b696bc423313e5","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        bdm_metadata \u003d []"},{"line_number":76,"context_line":"        for bdm in itertools.chain(block_device_info[\u0027block_device_mapping\u0027],"},{"line_number":77,"context_line":"                                   block_device_info[\u0027ephemerals\u0027],"},{"line_number":78,"context_line":"                                   [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            # NOTE(claudiub): ephemerals have device_name instead of"},{"line_number":80,"context_line":"            # mount_device."},{"line_number":81,"context_line":"            device_name \u003d bdm.get(\u0027mount_device\u0027) or bdm.get(\u0027device_name\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_a010104f","line":78,"range":{"start_line":78,"start_character":55,"end_line":78,"end_character":64},"in_reply_to":"3aaa91ec_6fde1b3f","updated":"2016-06-30 19:06:58.000000000","message":"L157.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"417619d3ed4858859b17d2c2f78762939d839b3d","unresolved":false,"context_lines":[{"line_number":81,"context_line":"            device_name \u003d bdm.get(\u0027mount_device\u0027) or bdm.get(\u0027device_name\u0027)"},{"line_number":82,"context_line":"            bdm_obj \u003d bdm_obj_map.get(device_name)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if not bdm_obj or \u0027tag\u0027 not in bdm_obj or not bdm_obj.tag:"},{"line_number":85,"context_line":"                # bdm is not tagged, no need to create metadata"},{"line_number":86,"context_line":"                continue"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_0e7fa715","line":84,"updated":"2016-06-30 14:37:30.000000000","message":"Ok, am I the only one who finds this negative logic harder to wrap my head around? Wouldn\u0027t it be clearer:\n\nif bdm_obj and \u0027tag\u0027 in bdm_obj and bdm_obj.tag:\n    # tagging stuff\nelse:\n    continue # not tagged","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"19b2d1c362bd14a36f9dd784d78c907da75abcd0","unresolved":false,"context_lines":[{"line_number":81,"context_line":"            device_name \u003d bdm.get(\u0027mount_device\u0027) or bdm.get(\u0027device_name\u0027)"},{"line_number":82,"context_line":"            bdm_obj \u003d bdm_obj_map.get(device_name)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if not bdm_obj or \u0027tag\u0027 not in bdm_obj or not bdm_obj.tag:"},{"line_number":85,"context_line":"                # bdm is not tagged, no need to create metadata"},{"line_number":86,"context_line":"                continue"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_60b8ef2a","line":84,"in_reply_to":"3aaa91ec_0e7fa715","updated":"2016-06-30 16:51:39.000000000","message":"Everyone needs a bit of negativity in their lifes. :)\n\nHonestly, I prefer the negatives in code. It allows me to avoid indenting to infinity in code. Persoanlly, I hate indenting too much. :)","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f98e68ceee3358758e5ebe62aae7611dd8121a8b","unresolved":false,"context_lines":[{"line_number":81,"context_line":"            device_name \u003d bdm.get(\u0027mount_device\u0027) or bdm.get(\u0027device_name\u0027)"},{"line_number":82,"context_line":"            bdm_obj \u003d bdm_obj_map.get(device_name)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if not bdm_obj or \u0027tag\u0027 not in bdm_obj or not bdm_obj.tag:"},{"line_number":85,"context_line":"                # bdm is not tagged, no need to create metadata"},{"line_number":86,"context_line":"                continue"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_2fa6d3c0","line":84,"in_reply_to":"3aaa91ec_60b8ef2a","updated":"2016-06-30 18:21:21.000000000","message":"I also dislike negative logic like this and prefer Artom\u0027s suggestion.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f98e68ceee3358758e5ebe62aae7611dd8121a8b","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                # bdm is not tagged, no need to create metadata"},{"line_number":86,"context_line":"                continue"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"            bus \u003d self._get_device_bus(bdm)"},{"line_number":89,"context_line":"            device \u003d objects.DiskMetadata(bus\u003dbus,"},{"line_number":90,"context_line":"                                          tags\u003d[bdm_obj.tag])"},{"line_number":91,"context_line":"            bdm_metadata.append(device)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_cf7e2f18","line":88,"range":{"start_line":88,"start_character":12,"end_line":88,"end_character":15},"updated":"2016-06-30 18:21:21.000000000","message":"Can this be None?","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bd64a4b0d62f3d11182b8373e2b696bc423313e5","unresolved":false,"context_lines":[{"line_number":85,"context_line":"                # bdm is not tagged, no need to create metadata"},{"line_number":86,"context_line":"                continue"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"            bus \u003d self._get_device_bus(bdm)"},{"line_number":89,"context_line":"            device \u003d objects.DiskMetadata(bus\u003dbus,"},{"line_number":90,"context_line":"                                          tags\u003d[bdm_obj.tag])"},{"line_number":91,"context_line":"            bdm_metadata.append(device)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_c098bcc2","line":88,"range":{"start_line":88,"start_character":12,"end_line":88,"end_character":15},"in_reply_to":"3aaa91ec_cf7e2f18","updated":"2016-06-30 19:06:58.000000000","message":"no. all BDMs are validated, if they need a disk_bus other than the ones found in _VALID_BUS, an exception is raised during validation.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":66,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        # block_device_info does not contain tags information."},{"line_number":69,"context_line":"        bdm_obj_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":70,"context_line":"            context, instance.uuid)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        # create a map between BDM object and its device name."},{"line_number":73,"context_line":"        bdm_obj_map \u003d {bdm.device_name: bdm for bdm in bdm_obj_list}"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_e8348aad","line":70,"range":{"start_line":69,"start_character":8,"end_line":70,"end_character":35},"updated":"2016-07-13 21:45:28.000000000","message":"Similar to the TODO in the libvirt driver, I think block_device_info[\u0027block_device_mapping\u0027] has what we need, but we might need to tweak nova.virt.block_device.DriverBlockDevice to pass through the getattr for the tag field.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"80e7465fc288bfbb92a568e640e120d87144a3d4","unresolved":false,"context_lines":[{"line_number":66,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        # block_device_info does not contain tags information."},{"line_number":69,"context_line":"        bdm_obj_list \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":70,"context_line":"            context, instance.uuid)"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        # create a map between BDM object and its device name."},{"line_number":73,"context_line":"        bdm_obj_map \u003d {bdm.device_name: bdm for bdm in bdm_obj_list}"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_eb9d92bc","line":70,"range":{"start_line":69,"start_character":8,"end_line":70,"end_character":35},"in_reply_to":"1aa78d24_e8348aad","updated":"2016-07-14 08:03:24.000000000","message":"yeah, that would make things easier.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        bdm_metadata \u003d []"},{"line_number":76,"context_line":"        for bdm in itertools.chain(block_device_info[\u0027block_device_mapping\u0027],"},{"line_number":77,"context_line":"                                   block_device_info[\u0027ephemerals\u0027],"},{"line_number":78,"context_line":"                                   [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            # NOTE(claudiub): ephemerals have device_name instead of"},{"line_number":80,"context_line":"            # mount_device."},{"line_number":81,"context_line":"            device_name \u003d bdm.get(\u0027mount_device\u0027) or bdm.get(\u0027device_name\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_e885aa0a","line":78,"range":{"start_line":78,"start_character":55,"end_line":78,"end_character":64},"updated":"2016-07-13 21:45:28.000000000","message":"Where is this set? Because looking at:\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/driver.py#L64\n\nThe only keys in the block_device_info dict are block_device_mapping, ephemerals, swap and root_device_name.\n\n--\n\nI see it\u0027s set on L154 below.\n\nWhat about swap devices? Do we not tag those? I guess the hyperv driver doesn\u0027t support swap devices.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"80e7465fc288bfbb92a568e640e120d87144a3d4","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        bdm_metadata \u003d []"},{"line_number":76,"context_line":"        for bdm in itertools.chain(block_device_info[\u0027block_device_mapping\u0027],"},{"line_number":77,"context_line":"                                   block_device_info[\u0027ephemerals\u0027],"},{"line_number":78,"context_line":"                                   [block_device_info[\u0027root_disk\u0027]]):"},{"line_number":79,"context_line":"            # NOTE(claudiub): ephemerals have device_name instead of"},{"line_number":80,"context_line":"            # mount_device."},{"line_number":81,"context_line":"            device_name \u003d bdm.get(\u0027mount_device\u0027) or bdm.get(\u0027device_name\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_6b6362de","line":78,"range":{"start_line":78,"start_character":55,"end_line":78,"end_character":64},"in_reply_to":"1aa78d24_e885aa0a","updated":"2016-07-14 08:03:24.000000000","message":"I think I\u0027ve mentioned this before, but we don\u0027t support swap devices in Hyper-V at the moment.\n\nIt can be easily done, IMO, won\u0027t require more than 20 LOC without unit tests. I can do it, if it can still merge in newton.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            bdm_obj \u003d bdm_obj_map.get(device_name)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if bdm_obj and \u0027tag\u0027 in bdm_obj and bdm_obj.tag:"},{"line_number":85,"context_line":"                bus \u003d self._get_device_bus(bdm)"},{"line_number":86,"context_line":"                device \u003d objects.DiskMetadata(bus\u003dbus,"},{"line_number":87,"context_line":"                                              tags\u003d[bdm_obj.tag])"},{"line_number":88,"context_line":"                bdm_metadata.append(device)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_23f19502","line":85,"range":{"start_line":85,"start_character":16,"end_line":85,"end_character":47},"updated":"2016-07-13 21:45:28.000000000","message":"Technically bus here could be None...","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"80e7465fc288bfbb92a568e640e120d87144a3d4","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            bdm_obj \u003d bdm_obj_map.get(device_name)"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"            if bdm_obj and \u0027tag\u0027 in bdm_obj and bdm_obj.tag:"},{"line_number":85,"context_line":"                bus \u003d self._get_device_bus(bdm)"},{"line_number":86,"context_line":"                device \u003d objects.DiskMetadata(bus\u003dbus,"},{"line_number":87,"context_line":"                                              tags\u003d[bdm_obj.tag])"},{"line_number":88,"context_line":"                bdm_metadata.append(device)"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_b0013709","line":85,"range":{"start_line":85,"start_character":16,"end_line":85,"end_character":47},"in_reply_to":"1aa78d24_23f19502","updated":"2016-07-14 08:03:24.000000000","message":"We don\u0027t allow any unsuported bus_types at this moment. If a bdm requires an unsupported bus_type, an exception is raised duting bdm validation. L205.\n\nso, only IDE and SCSI bus_types will be here.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        root_disk[\u0027boot_index\u0027] \u003d 0"},{"line_number":152,"context_line":"        root_disk[\u0027mount_device\u0027] \u003d root_device"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        block_device_info[\u0027root_disk\u0027] \u003d root_disk"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def _get_available_controller_slot(self, controller_type, slot_map):"},{"line_number":157,"context_line":"        max_slots \u003d (os_win_const.IDE_CONTROLLER_SLOTS_NUMBER if"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_2897b23b","line":154,"range":{"start_line":154,"start_character":8,"end_line":154,"end_character":50},"updated":"2016-07-13 21:45:28.000000000","message":"OK I guess it\u0027s set here which is called from validate_and_update_bdi which is called from vmops.spawn before we get to the device meta code above.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"}],"nova/virt/hyperv/driver.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"417619d3ed4858859b17d2c2f78762939d839b3d","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        \"supports_recreate\": False,"},{"line_number":99,"context_line":"        \"supports_migrate_to_same_host\": True,"},{"line_number":100,"context_line":"        \"supports_attach_interface\": True,"},{"line_number":101,"context_line":"        \"supports_device_tagging\": True,"},{"line_number":102,"context_line":"    }"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def __init__(self, virtapi):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_6e310ba2","line":101,"updated":"2016-06-30 14:37:30.000000000","message":"Nice! I was going to leave another review on a previous patch reminding you to do this - it\u0027s a fairly recent addition to our own libvirt stuff - but you found it yourself.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"19b2d1c362bd14a36f9dd784d78c907da75abcd0","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        \"supports_recreate\": False,"},{"line_number":99,"context_line":"        \"supports_migrate_to_same_host\": True,"},{"line_number":100,"context_line":"        \"supports_attach_interface\": True,"},{"line_number":101,"context_line":"        \"supports_device_tagging\": True,"},{"line_number":102,"context_line":"    }"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def __init__(self, virtapi):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_e0c85faa","line":101,"in_reply_to":"3aaa91ec_6e310ba2","updated":"2016-06-30 16:51:39.000000000","message":"yeah, the spawn is actually failing because of this. And I\u0027d like to remind you as well, that instead of self.driver.capabilities[\u0027supports_device_tagging\u0027], you should use self.driver.capabilities.get(\u0027supports_device_tagging\u0027), otherwise, you would break the other drivers.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f98e68ceee3358758e5ebe62aae7611dd8121a8b","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        \"supports_recreate\": False,"},{"line_number":99,"context_line":"        \"supports_migrate_to_same_host\": True,"},{"line_number":100,"context_line":"        \"supports_attach_interface\": True,"},{"line_number":101,"context_line":"        \"supports_device_tagging\": True,"},{"line_number":102,"context_line":"    }"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    def __init__(self, virtapi):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_0ca96524","line":101,"in_reply_to":"3aaa91ec_e0c85faa","updated":"2016-06-30 18:21:21.000000000","message":"That\u0027s what the compute manager is using now for this very reason.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"}],"nova/virt/hyperv/vmops.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c7c48b601864c0efdbde195a5d12a9b1ee845bf8","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"            device \u003d objects.NetworkInterfaceMetadata("},{"line_number":270,"context_line":"                mac\u003dvif.address,"},{"line_number":271,"context_line":"                bus\u003dobjects.DeviceBus(),"},{"line_number":272,"context_line":"                tags\u003dvif.tag.split())"},{"line_number":273,"context_line":"            vif_metadata.append(device)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_37bc0405","line":271,"updated":"2016-06-22 12:19:17.000000000","message":"It\u0027s missing the address here, no?","commit_id":"d4f5beaa9113b100e6265660167d6dd42dff9cb3"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"090d819b45ac66237dc27686de1281173468e772","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"            device \u003d objects.NetworkInterfaceMetadata("},{"line_number":270,"context_line":"                mac\u003dvif.address,"},{"line_number":271,"context_line":"                bus\u003dobjects.DeviceBus(),"},{"line_number":272,"context_line":"                tags\u003dvif.tag.split())"},{"line_number":273,"context_line":"            vif_metadata.append(device)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_cce8967f","line":271,"in_reply_to":"3aaa91ec_37bc0405","updated":"2016-06-22 12:58:13.000000000","message":"DeviceBus has no \u0027address\u0027 field. Actually, it doesn\u0027t have any fields.","commit_id":"d4f5beaa9113b100e6265660167d6dd42dff9cb3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b767df308032da60dbf619dca3f0328b8a88369e","unresolved":false,"context_lines":[{"line_number":268,"context_line":""},{"line_number":269,"context_line":"            device \u003d objects.NetworkInterfaceMetadata("},{"line_number":270,"context_line":"                mac\u003dvif.address,"},{"line_number":271,"context_line":"                bus\u003dobjects.DeviceBus(),"},{"line_number":272,"context_line":"                tags\u003dvif.tag.split())"},{"line_number":273,"context_line":"            vif_metadata.append(device)"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_4263cddc","line":271,"in_reply_to":"3aaa91ec_cce8967f","updated":"2016-06-22 13:20:59.000000000","message":"DeviceBus is an abstract class that exists because we have a \u0027bus\u0027 field that needs to have a type [1]. The actual children classes that have address fields are the BlahDeviceBus classes, starting with PCIDeviceBus [2].\n\n\n[1] https://github.com/openstack/nova/blob/master/nova/objects/virt_device_metadata.py#L66\n[2] https://github.com/openstack/nova/blob/master/nova/objects/virt_device_metadata.py#L26","commit_id":"d4f5beaa9113b100e6265660167d6dd42dff9cb3"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"dc704faffb4c3c99fe7a89141a56bedcbeb11c20","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                                                                 instance_id)"},{"line_number":253,"context_line":"        vif_metadata \u003d []"},{"line_number":254,"context_line":"        for vif in vifs:"},{"line_number":255,"context_line":"            if not vif.tag:"},{"line_number":256,"context_line":"                # vif is not tagged, no need to create metadata."},{"line_number":257,"context_line":"                continue"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_3d2166fd","line":255,"updated":"2016-06-28 18:12:03.000000000","message":"Same thing about using obj_attr_is_set.","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea221a7b97b804e77b76d404c57617cd812041e4","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                                                                 instance_id)"},{"line_number":253,"context_line":"        vif_metadata \u003d []"},{"line_number":254,"context_line":"        for vif in vifs:"},{"line_number":255,"context_line":"            if not vif.tag:"},{"line_number":256,"context_line":"                # vif is not tagged, no need to create metadata."},{"line_number":257,"context_line":"                continue"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_d325b1ae","line":255,"in_reply_to":"3aaa91ec_3d2166fd","updated":"2016-06-28 18:22:29.000000000","message":"You can use the nicer syntax of:\n\n if \u0027tag\u0027 not in vif:\n     ...\n\nwhich is the same as obj_attr_is_set()","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"1ce0206cc77ff152446941a92a85a3ebd9ae4920","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                                                                 instance_id)"},{"line_number":253,"context_line":"        vif_metadata \u003d []"},{"line_number":254,"context_line":"        for vif in vifs:"},{"line_number":255,"context_line":"            if not vif.tag:"},{"line_number":256,"context_line":"                # vif is not tagged, no need to create metadata."},{"line_number":257,"context_line":"                continue"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_4f373bb0","line":255,"in_reply_to":"3aaa91ec_d325b1ae","updated":"2016-06-29 09:32:40.000000000","message":"Done","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"dc704faffb4c3c99fe7a89141a56bedcbeb11c20","unresolved":false,"context_lines":[{"line_number":259,"context_line":"            device \u003d objects.NetworkInterfaceMetadata("},{"line_number":260,"context_line":"                mac\u003dvif.address,"},{"line_number":261,"context_line":"                bus\u003dobjects.PCIDeviceBus(),"},{"line_number":262,"context_line":"                tags\u003dvif.tag.split())"},{"line_number":263,"context_line":"            vif_metadata.append(device)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        return vif_metadata"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_ddc66215","line":262,"updated":"2016-06-28 18:12:03.000000000","message":"Don\u0027t do split() here, it\u0027s a single tag, even if it has whitespace in it.","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"1ce0206cc77ff152446941a92a85a3ebd9ae4920","unresolved":false,"context_lines":[{"line_number":259,"context_line":"            device \u003d objects.NetworkInterfaceMetadata("},{"line_number":260,"context_line":"                mac\u003dvif.address,"},{"line_number":261,"context_line":"                bus\u003dobjects.PCIDeviceBus(),"},{"line_number":262,"context_line":"                tags\u003dvif.tag.split())"},{"line_number":263,"context_line":"            vif_metadata.append(device)"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        return vif_metadata"}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_0f91b399","line":262,"in_reply_to":"3aaa91ec_ddc66215","updated":"2016-06-29 09:32:40.000000000","message":"Done","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"dc704faffb4c3c99fe7a89141a56bedcbeb11c20","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        return vif_metadata"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def _save_device_metadata(self, context, instance, block_device_info\u003dNone):"},{"line_number":268,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":269,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":270,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_ddbb0274","line":267,"updated":"2016-06-28 18:12:03.000000000","message":"Do we need the default None here? Will this ever be called without block_device_info?","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"1ce0206cc77ff152446941a92a85a3ebd9ae4920","unresolved":false,"context_lines":[{"line_number":264,"context_line":""},{"line_number":265,"context_line":"        return vif_metadata"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def _save_device_metadata(self, context, instance, block_device_info\u003dNone):"},{"line_number":268,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":269,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":270,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"3aaa91ec_af633fdc","line":267,"in_reply_to":"3aaa91ec_ddbb0274","updated":"2016-06-29 09:32:40.000000000","message":"Done","commit_id":"e4819782302a0c0c10d6f9095ff08f300a148d98"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"417619d3ed4858859b17d2c2f78762939d839b3d","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                                                                 instance_id)"},{"line_number":253,"context_line":"        vif_metadata \u003d []"},{"line_number":254,"context_line":"        for vif in vifs:"},{"line_number":255,"context_line":"            if \u0027tag\u0027 not in vif or not vif.tag:"},{"line_number":256,"context_line":"                # vif is not tagged, no need to create metadata."},{"line_number":257,"context_line":"                continue"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_aeff7364","line":255,"updated":"2016-06-30 14:37:30.000000000","message":"Same thing about negative logic.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f98e68ceee3358758e5ebe62aae7611dd8121a8b","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                                                                 instance_id)"},{"line_number":253,"context_line":"        vif_metadata \u003d []"},{"line_number":254,"context_line":"        for vif in vifs:"},{"line_number":255,"context_line":"            if \u0027tag\u0027 not in vif or not vif.tag:"},{"line_number":256,"context_line":"                # vif is not tagged, no need to create metadata."},{"line_number":257,"context_line":"                continue"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_efbdebb1","line":255,"in_reply_to":"3aaa91ec_803cfb93","updated":"2016-06-30 18:21:21.000000000","message":"Agree with Artom.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"19b2d1c362bd14a36f9dd784d78c907da75abcd0","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                                                                 instance_id)"},{"line_number":253,"context_line":"        vif_metadata \u003d []"},{"line_number":254,"context_line":"        for vif in vifs:"},{"line_number":255,"context_line":"            if \u0027tag\u0027 not in vif or not vif.tag:"},{"line_number":256,"context_line":"                # vif is not tagged, no need to create metadata."},{"line_number":257,"context_line":"                continue"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_803cfb93","line":255,"in_reply_to":"3aaa91ec_aeff7364","updated":"2016-06-30 16:51:39.000000000","message":"ditto.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f98e68ceee3358758e5ebe62aae7611dd8121a8b","unresolved":false,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"            device \u003d objects.NetworkInterfaceMetadata("},{"line_number":260,"context_line":"                mac\u003dvif.address,"},{"line_number":261,"context_line":"                bus\u003dobjects.PCIDeviceBus(),"},{"line_number":262,"context_line":"                tags\u003d[vif.tag])"},{"line_number":263,"context_line":"            vif_metadata.append(device)"},{"line_number":264,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_2fbb73c5","line":261,"range":{"start_line":261,"start_character":20,"end_line":261,"end_character":42},"updated":"2016-06-30 18:21:21.000000000","message":"This is the only type that we can get here? Probably, I think that\u0027s true in the libvirt driver also, but it\u0027s not as explicit.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bd64a4b0d62f3d11182b8373e2b696bc423313e5","unresolved":false,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"            device \u003d objects.NetworkInterfaceMetadata("},{"line_number":260,"context_line":"                mac\u003dvif.address,"},{"line_number":261,"context_line":"                bus\u003dobjects.PCIDeviceBus(),"},{"line_number":262,"context_line":"                tags\u003d[vif.tag])"},{"line_number":263,"context_line":"            vif_metadata.append(device)"},{"line_number":264,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3aaa91ec_00e64443","line":261,"range":{"start_line":261,"start_character":20,"end_line":261,"end_character":42},"in_reply_to":"3aaa91ec_2fbb73c5","updated":"2016-06-30 19:06:58.000000000","message":"indeed, it is the only one. currently, a simple DeviceBus is ignored and there won\u0027t be any metadata generated.","commit_id":"8a52b285cbd4e342dd2b09959821f7b072a486ae"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"77facd54c0e4c4014ae3df214c71a9f1fb06495d","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        metadata \u003d []"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        metadata.extend(self._get_vif_metadata(context, instance.uuid))"},{"line_number":271,"context_line":"        if block_device_info:"},{"line_number":272,"context_line":"            metadata.extend(self._block_dev_man.get_bdm_metadata("},{"line_number":273,"context_line":"                context, instance, block_device_info))"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_1797773b","line":271,"updated":"2016-07-13 20:20:07.000000000","message":"When would this happen? If we do a basic boot? Again, I don\u0027t think this is covered in unit tests, but definitely is by tempest.","commit_id":"c005105517da7ca738a4e1aae02950918ed0d9f8"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"5340f5bb7e27a5018a058f96d92abbf0fb8df19f","unresolved":false,"context_lines":[{"line_number":268,"context_line":"        metadata \u003d []"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        metadata.extend(self._get_vif_metadata(context, instance.uuid))"},{"line_number":271,"context_line":"        if block_device_info:"},{"line_number":272,"context_line":"            metadata.extend(self._block_dev_man.get_bdm_metadata("},{"line_number":273,"context_line":"                context, instance, block_device_info))"},{"line_number":274,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_778ebbd1","line":271,"in_reply_to":"1aa78d24_1797773b","updated":"2016-07-13 20:29:11.000000000","message":"I wanted to be sure. spawn method has a block_device_info\u003dNone argument. It made the most sense for me to have an if, just in case.","commit_id":"c005105517da7ca738a4e1aae02950918ed0d9f8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"77facd54c0e4c4014ae3df214c71a9f1fb06495d","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            metadata.extend(self._block_dev_man.get_bdm_metadata("},{"line_number":273,"context_line":"                context, instance, block_device_info))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        if metadata:"},{"line_number":276,"context_line":"            instance.device_metadata \u003d objects.InstanceDeviceMetadata("},{"line_number":277,"context_line":"                devices\u003dmetadata)"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_577a9f26","line":275,"updated":"2016-07-13 20:20:07.000000000","message":"I think the case where this is false isn\u0027t tested, but.. probably not a huge deal as it\u0027s tested many times by tempest.","commit_id":"c005105517da7ca738a4e1aae02950918ed0d9f8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"40a03154b6d22fff5fa4cf689f725391234eba7a","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            metadata.extend(self._block_dev_man.get_bdm_metadata("},{"line_number":273,"context_line":"                context, instance, block_device_info))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        if metadata:"},{"line_number":276,"context_line":"            instance.device_metadata \u003d objects.InstanceDeviceMetadata("},{"line_number":277,"context_line":"                devices\u003dmetadata)"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_8aa85e22","line":275,"in_reply_to":"1aa78d24_0a074e19","updated":"2016-07-13 20:46:13.000000000","message":"Yeah, but it affects the outcome of the method because it leaves instance.device_metadata unset. But looking at instance.save() it looks like it will be handled properly, so that\u0027s fine.","commit_id":"c005105517da7ca738a4e1aae02950918ed0d9f8"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"5340f5bb7e27a5018a058f96d92abbf0fb8df19f","unresolved":false,"context_lines":[{"line_number":272,"context_line":"            metadata.extend(self._block_dev_man.get_bdm_metadata("},{"line_number":273,"context_line":"                context, instance, block_device_info))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        if metadata:"},{"line_number":276,"context_line":"            instance.device_metadata \u003d objects.InstanceDeviceMetadata("},{"line_number":277,"context_line":"                devices\u003dmetadata)"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_0a074e19","line":275,"in_reply_to":"1aa78d24_577a9f26","updated":"2016-07-13 20:29:11.000000000","message":"Typically, when I add unit tests, I try to cover every code branch. I didn\u0027t think it was necessary to add a negative case, as it woudldn\u0027t add any coverage.","commit_id":"c005105517da7ca738a4e1aae02950918ed0d9f8"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"77facd54c0e4c4014ae3df214c71a9f1fb06495d","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        self._block_dev_man.validate_and_update_bdi("},{"line_number":295,"context_line":"            instance, image_meta, vm_gen, block_device_info)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        root_device \u003d block_device_info[\u0027root_disk\u0027]"},{"line_number":298,"context_line":"        self._create_root_device(context, instance, root_device, vm_gen)"},{"line_number":299,"context_line":"        self._create_ephemerals(instance, block_device_info[\u0027ephemerals\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_170db7e8","line":296,"updated":"2016-07-13 20:20:07.000000000","message":"Random whitespace :/","commit_id":"c005105517da7ca738a4e1aae02950918ed0d9f8"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"5340f5bb7e27a5018a058f96d92abbf0fb8df19f","unresolved":false,"context_lines":[{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        self._block_dev_man.validate_and_update_bdi("},{"line_number":295,"context_line":"            instance, image_meta, vm_gen, block_device_info)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        root_device \u003d block_device_info[\u0027root_disk\u0027]"},{"line_number":298,"context_line":"        self._create_root_device(context, instance, root_device, vm_gen)"},{"line_number":299,"context_line":"        self._create_ephemerals(instance, block_device_info[\u0027ephemerals\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_176bf7f2","line":296,"in_reply_to":"1aa78d24_170db7e8","updated":"2016-07-13 20:29:11.000000000","message":"Done","commit_id":"c005105517da7ca738a4e1aae02950918ed0d9f8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                                          eph_info[\u0027size\u0027] * units.Gi)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    @staticmethod"},{"line_number":250,"context_line":"    def _get_vif_metadata(context, instance_id):"},{"line_number":251,"context_line":"        vifs \u003d objects.VirtualInterfaceList.get_by_instance_uuid(context,"},{"line_number":252,"context_line":"                                                                 instance_id)"},{"line_number":253,"context_line":"        vif_metadata \u003d []"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_2dcce460","line":250,"range":{"start_line":250,"start_character":35,"end_line":250,"end_character":46},"updated":"2016-07-13 21:45:28.000000000","message":"nit: I know this is a thing that\u0027s all over the hyperv driver, but I really dislike this naming convention since I don\u0027t know if it\u0027s an instance.id or instance.uuid without looking at the caller. But anyway, I had to say something.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            if \u0027tag\u0027 in vif and vif.tag:"},{"line_number":256,"context_line":"                device \u003d objects.NetworkInterfaceMetadata("},{"line_number":257,"context_line":"                    mac\u003dvif.address,"},{"line_number":258,"context_line":"                    bus\u003dobjects.PCIDeviceBus(),"},{"line_number":259,"context_line":"                    tags\u003d[vif.tag])"},{"line_number":260,"context_line":"                vif_metadata.append(device)"},{"line_number":261,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_ed6f7cfa","line":258,"range":{"start_line":258,"start_character":32,"end_line":258,"end_character":44},"updated":"2016-07-13 21:45:28.000000000","message":"Is there an address to set on this?","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4d207c0feb77ec8e04ba21825d9770d8d8c69647","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            if \u0027tag\u0027 in vif and vif.tag:"},{"line_number":256,"context_line":"                device \u003d objects.NetworkInterfaceMetadata("},{"line_number":257,"context_line":"                    mac\u003dvif.address,"},{"line_number":258,"context_line":"                    bus\u003dobjects.PCIDeviceBus(),"},{"line_number":259,"context_line":"                    tags\u003d[vif.tag])"},{"line_number":260,"context_line":"                vif_metadata.append(device)"},{"line_number":261,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_236b55f3","line":258,"range":{"start_line":258,"start_character":32,"end_line":258,"end_character":44},"in_reply_to":"1aa78d24_834969ba","updated":"2016-07-13 21:57:51.000000000","message":"Yeah, for libvirt we actually get the address for pci devices, but not the disk devices, which is why I asked.\n\nIt is optional in the metadata handler, so yeah, I won\u0027t block on this - just something to think about.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cac37e30dbe094c313fb0b7a104fc46698622b74","unresolved":false,"context_lines":[{"line_number":255,"context_line":"            if \u0027tag\u0027 in vif and vif.tag:"},{"line_number":256,"context_line":"                device \u003d objects.NetworkInterfaceMetadata("},{"line_number":257,"context_line":"                    mac\u003dvif.address,"},{"line_number":258,"context_line":"                    bus\u003dobjects.PCIDeviceBus(),"},{"line_number":259,"context_line":"                    tags\u003d[vif.tag])"},{"line_number":260,"context_line":"                vif_metadata.append(device)"},{"line_number":261,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_834969ba","line":258,"range":{"start_line":258,"start_character":32,"end_line":258,"end_character":44},"in_reply_to":"1aa78d24_ed6f7cfa","updated":"2016-07-13 21:53:17.000000000","message":"It\u0027s optional in the metadata handler. Something about libvirt not being able to give us this for all kinds of devices I think.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"80d8b0119b1b4ff16d9bb49379e483010cbaeaac","unresolved":false,"context_lines":[{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        return vif_metadata"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def _save_device_metadata(self, context, instance, block_device_info):"},{"line_number":265,"context_line":"        \"\"\"Builds a metadata object for instance devices, that maps the user"},{"line_number":266,"context_line":"           provided tag to the hypervisor assigned device address."},{"line_number":267,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_adb8d4c8","line":264,"range":{"start_line":264,"start_character":55,"end_line":264,"end_character":72},"updated":"2016-07-13 21:45:28.000000000","message":"nit: I\u0027d probably default this to None like in spawn.","commit_id":"d47b5f993ece3aa52b486a8b6add2d96e9381599"}]}