)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0eb34e66dd91464dda5a6d25a0ef4b829237f7f6","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2019-02-06 21:59:51 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP: Get resolved Cyborg ARQs and add PCI BDFs to VM\u0027s domain XML."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I08cd4787ab4c9539574237e26ba5bf6d4246b32e"},{"line_number":10,"context_line":"Blueprint: nova-cyborg-interaction"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fdfeff1_5208eed7","line":8,"updated":"2019-02-09 00:15:24.000000000","message":"Words. Like:\n\nAugments libvirt\u0027s spawn method to use resolved Cyborg ARQ (Accelerator ReQuest) information to determine the specific PCI devices to attach to the VM.\n\n(As mentioned inline, I think the await-and-retrieve-resolved-arqs-from-cyborg bit should probably be outside of spawn. That should possibly be done in a separate patch between the previous one and this one.)","commit_id":"1e4d7bce757f4db1d36e693c93813a5e6b9eae05"}],"nova/accelerator/cyborg.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6b57712804521fa549583fca608cf97a8e45ee2","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        data \u003d {\"device_profile_name\": dp_name}"},{"line_number":104,"context_line":"        r \u003d self._client.post(url, json\u003ddata)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"        LOG.warning(\u0027NSS arq ret code %s\u0027, r.status_code)"},{"line_number":107,"context_line":"        LOG.warning(\u0027NSS arq ret %s\u0027, r)"},{"line_number":108,"context_line":"        return r.json()"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def bind_arqs(self, bindings):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_32768aa6","side":"PARENT","line":107,"range":{"start_line":106,"start_character":0,"end_line":107,"end_character":40},"updated":"2019-02-09 00:05:34.000000000","message":"if yer gonna remove these, do it in the prior patch where they were introduced.","commit_id":"369cf0f2a4d890f341d72d06be451cecdee04ea4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6b57712804521fa549583fca608cf97a8e45ee2","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        # this instance is not resolved. But that\u0027s not working yet."},{"line_number":126,"context_line":"        # So, we check for unresolved ARQs and raise an exception"},{"line_number":127,"context_line":"        # that clients can catch."},{"line_number":128,"context_line":"        RETRY_LIMIT \u003d 10"},{"line_number":129,"context_line":"        RETRY_INTERVAL \u003d 0.5  # in seconds"},{"line_number":130,"context_line":"        VALID_STATES \u003d [\u0027Bound\u0027, \u0027BindFailed\u0027]"},{"line_number":131,"context_line":"        for i in range(RETRY_LIMIT):"},{"line_number":132,"context_line":"            r \u003d self._client.get(url, params\u003dquery).json()"},{"line_number":133,"context_line":"            arqs \u003d r[\u0027arqs\u0027]"},{"line_number":134,"context_line":"            unresolved \u003d False"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_d264dee7","line":131,"range":{"start_line":128,"start_character":0,"end_line":131,"end_character":36},"updated":"2019-02-09 00:05:34.000000000","message":"Use a retry decorator like @retrying.retry. See examples in SchedulerReportClient or ResourceTracker.","commit_id":"1e4d7bce757f4db1d36e693c93813a5e6b9eae05"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6b57712804521fa549583fca608cf97a8e45ee2","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        # Convert attach_info JSON to a dictonary."},{"line_number":148,"context_line":"        for arq in arqs:"},{"line_number":149,"context_line":"            attach_json \u003d arq[\u0027attach_handle_info\u0027]"},{"line_number":150,"context_line":"            attach_dict \u003d jsonutils.loads(attach_json)"},{"line_number":151,"context_line":"            arq[\u0027attach_handle_info\u0027] \u003d attach_dict"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        return arqs"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_529eaeb8","line":150,"range":{"start_line":150,"start_character":36,"end_line":150,"end_character":41},"updated":"2019-02-09 00:05:34.000000000","message":"ugh, again with the double encode","commit_id":"1e4d7bce757f4db1d36e693c93813a5e6b9eae05"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0629fbd3427cca66a5166444e6eeaed0d1114f5e","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from nova import service_auth"},{"line_number":18,"context_line":"from nova import utils"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":"   Note on object relationships:"},{"line_number":23,"context_line":"   1 device profile (DP) has D \u003e\u003d 1 request groups (just as a flavor"}],"source_content_type":"text/x-python","patch_set":36,"id":"3fa7e38b_29e1d0d0","line":20,"updated":"2019-11-01 14:25:19.000000000","message":"Random whitespace damage. Please clean things like this up.","commit_id":"91ee75db2ba9498aae368eab4520918625308df0"}],"nova/api/openstack/compute/schemas/server_external_events.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"39af5ee0f5ba09c5833ec7a04b3e05a4655318de","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                            \u0027network-vif-plugged\u0027,"},{"line_number":34,"context_line":"                            \u0027network-vif-unplugged\u0027,"},{"line_number":35,"context_line":"                            \u0027network-vif-deleted\u0027,"},{"line_number":36,"context_line":"                            \u0027accelerator-requests-bound\u0027,"},{"line_number":37,"context_line":"                        ],"},{"line_number":38,"context_line":"                    },"},{"line_number":39,"context_line":"                    \u0027status\u0027: {"}],"source_content_type":"text/x-python","patch_set":36,"id":"3fa7e38b_69510830","line":36,"updated":"2019-11-01 14:47:51.000000000","message":"This is a schema change to a versioned API. You can\u0027t just add this here. It has to be a new microversion. See how 2.51 and 2.76 work below.","commit_id":"91ee75db2ba9498aae368eab4520918625308df0"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a1ae1b9ba6cbe792b01f54327764fe961a5b7da6","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                            \u0027network-vif-plugged\u0027,"},{"line_number":34,"context_line":"                            \u0027network-vif-unplugged\u0027,"},{"line_number":35,"context_line":"                            \u0027network-vif-deleted\u0027,"},{"line_number":36,"context_line":"                            \u0027accelerator-requests-bound\u0027,"},{"line_number":37,"context_line":"                        ],"},{"line_number":38,"context_line":"                    },"},{"line_number":39,"context_line":"                    \u0027status\u0027: {"}],"source_content_type":"text/x-python","patch_set":36,"id":"3fa7e38b_57cbc11c","line":36,"in_reply_to":"3fa7e38b_69510830","updated":"2019-11-19 06:08:37.000000000","message":"Done","commit_id":"91ee75db2ba9498aae368eab4520918625308df0"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f039927a7a5178e647f806fdf4c50bbad3efae9e","unresolved":false,"context_lines":[{"line_number":2492,"context_line":"                            task_states.BLOCK_DEVICE_MAPPING)"},{"line_number":2493,"context_line":"                    block_device_info \u003d resources[\u0027block_device_info\u0027]"},{"line_number":2494,"context_line":"                    network_info \u003d resources[\u0027network_info\u0027]"},{"line_number":2495,"context_line":"                    arqs \u003d resources[\u0027accel_info\u0027]"},{"line_number":2496,"context_line":"                    LOG.debug(\u0027Start spawning the instance on the hypervisor.\u0027,"},{"line_number":2497,"context_line":"                              instance\u003dinstance)"},{"line_number":2498,"context_line":"                    with timeutils.StopWatch() as timer:"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_88cce98a","line":2495,"range":{"start_line":2495,"start_character":20,"end_line":2495,"end_character":24},"updated":"2019-12-04 15:13:15.000000000","message":"accel_info","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"01709a7d213c2547068700ef4c74f7fc9ba4d770","unresolved":false,"context_lines":[{"line_number":2492,"context_line":"                            task_states.BLOCK_DEVICE_MAPPING)"},{"line_number":2493,"context_line":"                    block_device_info \u003d resources[\u0027block_device_info\u0027]"},{"line_number":2494,"context_line":"                    network_info \u003d resources[\u0027network_info\u0027]"},{"line_number":2495,"context_line":"                    arqs \u003d resources[\u0027accel_info\u0027]"},{"line_number":2496,"context_line":"                    LOG.debug(\u0027Start spawning the instance on the hypervisor.\u0027,"},{"line_number":2497,"context_line":"                              instance\u003dinstance)"},{"line_number":2498,"context_line":"                    with timeutils.StopWatch() as timer:"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_fc3414fc","line":2495,"range":{"start_line":2495,"start_character":20,"end_line":2495,"end_character":24},"in_reply_to":"3fa7e38b_88cce98a","updated":"2019-12-09 08:27:46.000000000","message":"Done","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f039927a7a5178e647f806fdf4c50bbad3efae9e","unresolved":false,"context_lines":[{"line_number":2500,"context_line":"                                          injected_files, admin_password,"},{"line_number":2501,"context_line":"                                          allocs, network_info\u003dnetwork_info,"},{"line_number":2502,"context_line":"                                          block_device_info\u003dblock_device_info,"},{"line_number":2503,"context_line":"                                          arqs\u003darqs)"},{"line_number":2504,"context_line":"                    LOG.info(\u0027Took %0.2f seconds to spawn the instance on \u0027"},{"line_number":2505,"context_line":"                             \u0027the hypervisor.\u0027, timer.elapsed(),"},{"line_number":2506,"context_line":"                             instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_089eb968","line":2503,"range":{"start_line":2503,"start_character":42,"end_line":2503,"end_character":46},"updated":"2019-12-04 15:13:15.000000000","message":"accel_info","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"01709a7d213c2547068700ef4c74f7fc9ba4d770","unresolved":false,"context_lines":[{"line_number":2500,"context_line":"                                          injected_files, admin_password,"},{"line_number":2501,"context_line":"                                          allocs, network_info\u003dnetwork_info,"},{"line_number":2502,"context_line":"                                          block_device_info\u003dblock_device_info,"},{"line_number":2503,"context_line":"                                          arqs\u003darqs)"},{"line_number":2504,"context_line":"                    LOG.info(\u0027Took %0.2f seconds to spawn the instance on \u0027"},{"line_number":2505,"context_line":"                             \u0027the hypervisor.\u0027, timer.elapsed(),"},{"line_number":2506,"context_line":"                             instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_1c3a50e5","line":2503,"range":{"start_line":2503,"start_character":42,"end_line":2503,"end_character":46},"in_reply_to":"3fa7e38b_089eb968","updated":"2019-12-09 08:27:46.000000000","message":"Done","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"}],"nova/objects/external_event.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b606bbadd8e9201034182895c02b5bcb8cbf8c4a","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    # Version 1.2: adds volume-extended event"},{"line_number":53,"context_line":"    # Version 1.3: adds power-update event"},{"line_number":54,"context_line":"    # Version 1.4: adds accelerator-requests-bound event"},{"line_number":55,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    fields \u003d {"},{"line_number":58,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"}],"source_content_type":"text/x-python","patch_set":36,"id":"3fa7e38b_a9adc031","line":55,"updated":"2019-11-01 14:45:23.000000000","message":"Why is this change in this patch? It used to be in a separate one, but it\u0027s here now and isn\u0027t used by anything in this patch that I can see. Am I missing it?","commit_id":"91ee75db2ba9498aae368eab4520918625308df0"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"a1ae1b9ba6cbe792b01f54327764fe961a5b7da6","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    # Version 1.2: adds volume-extended event"},{"line_number":53,"context_line":"    # Version 1.3: adds power-update event"},{"line_number":54,"context_line":"    # Version 1.4: adds accelerator-requests-bound event"},{"line_number":55,"context_line":"    VERSION \u003d \u00271.4\u0027"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    fields \u003d {"},{"line_number":58,"context_line":"        \u0027instance_uuid\u0027: fields.UUIDField(),"}],"source_content_type":"text/x-python","patch_set":36,"id":"3fa7e38b_9747f9a3","line":55,"in_reply_to":"3fa7e38b_a9adc031","updated":"2019-11-19 06:08:37.000000000","message":"Done","commit_id":"91ee75db2ba9498aae368eab4520918625308df0"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9c2aedc9f1faffe0b2dafc7fa4919e2ea36aa779","unresolved":false,"context_lines":[{"line_number":2393,"context_line":"        ) as ("},{"line_number":2394,"context_line":"            debug_mock, conf_mock"},{"line_number":2395,"context_line":"        ):"},{"line_number":2396,"context_line":"            instance \u003d objects.Instance(**self.test_instance)"},{"line_number":2397,"context_line":"            drvr._get_guest_xml(self.context, instance,"},{"line_number":2398,"context_line":"                                network_info\u003d{}, disk_info\u003d{},"},{"line_number":2399,"context_line":"                                image_meta\u003d{}, block_device_info\u003dbdi)"}],"source_content_type":"text/x-python","patch_set":55,"id":"3fa7e38b_4ac8851f","line":2396,"updated":"2020-02-18 10:01:03.000000000","message":"this feels like an unnecessary change. I reverted it and the test still passes","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"6ed5b40063e27c84c4d6f724e32b182678ec0e7a","unresolved":false,"context_lines":[{"line_number":2393,"context_line":"        ) as ("},{"line_number":2394,"context_line":"            debug_mock, conf_mock"},{"line_number":2395,"context_line":"        ):"},{"line_number":2396,"context_line":"            instance \u003d objects.Instance(**self.test_instance)"},{"line_number":2397,"context_line":"            drvr._get_guest_xml(self.context, instance,"},{"line_number":2398,"context_line":"                                network_info\u003d{}, disk_info\u003d{},"},{"line_number":2399,"context_line":"                                image_meta\u003d{}, block_device_info\u003dbdi)"}],"source_content_type":"text/x-python","patch_set":55,"id":"3fa7e38b_478125a1","line":2396,"in_reply_to":"3fa7e38b_4ac8851f","updated":"2020-02-23 08:29:33.000000000","message":"Done","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9c2aedc9f1faffe0b2dafc7fa4919e2ea36aa779","unresolved":false,"context_lines":[{"line_number":13886,"context_line":"        accel_info \u003d []"},{"line_number":13887,"context_line":"        self._test_spawn_accels(accel_info)"},{"line_number":13888,"context_line":"        mock_get_guest_xml.assert_called_once()"},{"line_number":13889,"context_line":"        self.assertIn(accel_info, mock_get_guest_xml.call_args.args)"},{"line_number":13890,"context_line":""},{"line_number":13891,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver, \u0027_get_guest_xml\u0027)"},{"line_number":13892,"context_line":"    def test_spawn_accels_with_accel_info(self, mock_get_guest_xml):"}],"source_content_type":"text/x-python","patch_set":55,"id":"3fa7e38b_ca50f530","line":13889,"updated":"2020-02-18 10:01:03.000000000","message":"This is a pretty weak assert. If there will be a new param introduced later to _get_guest_xml with a default value of [] then this assert can have false positive result. Could you please use mock_get_guest_xml.assert_called_once_with() call?\n\n// later\n\nActually it is not even assert what you think it asserts. The accel_info is passed in as kwargs and you are checking if [] is in args.","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"6ed5b40063e27c84c4d6f724e32b182678ec0e7a","unresolved":false,"context_lines":[{"line_number":13886,"context_line":"        accel_info \u003d []"},{"line_number":13887,"context_line":"        self._test_spawn_accels(accel_info)"},{"line_number":13888,"context_line":"        mock_get_guest_xml.assert_called_once()"},{"line_number":13889,"context_line":"        self.assertIn(accel_info, mock_get_guest_xml.call_args.args)"},{"line_number":13890,"context_line":""},{"line_number":13891,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver, \u0027_get_guest_xml\u0027)"},{"line_number":13892,"context_line":"    def test_spawn_accels_with_accel_info(self, mock_get_guest_xml):"}],"source_content_type":"text/x-python","patch_set":55,"id":"1fa4df85_46d2d436","line":13889,"in_reply_to":"3fa7e38b_ca50f530","updated":"2020-02-23 08:29:33.000000000","message":"_test_spawn_accels() passes accel_info as positional argument, so it shows up in args. But I changed that to kwargs and changed this too.","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9c2aedc9f1faffe0b2dafc7fa4919e2ea36aa779","unresolved":false,"context_lines":[{"line_number":13894,"context_line":"        accel_info \u003d nova_fixtures.CyborgFixture.bound_arq_list"},{"line_number":13895,"context_line":"        self._test_spawn_accels(accel_info)"},{"line_number":13896,"context_line":"        mock_get_guest_xml.assert_called_once()"},{"line_number":13897,"context_line":"        self.assertIn(accel_info, mock_get_guest_xml.call_args.args)"},{"line_number":13898,"context_line":""},{"line_number":13899,"context_line":"    # Methods called directly by spawn()"},{"line_number":13900,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver, \u0027_get_guest_xml\u0027)"}],"source_content_type":"text/x-python","patch_set":55,"id":"3fa7e38b_a5728aae","line":13897,"updated":"2020-02-18 10:01:03.000000000","message":"ditto","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"6ed5b40063e27c84c4d6f724e32b182678ec0e7a","unresolved":false,"context_lines":[{"line_number":13894,"context_line":"        accel_info \u003d nova_fixtures.CyborgFixture.bound_arq_list"},{"line_number":13895,"context_line":"        self._test_spawn_accels(accel_info)"},{"line_number":13896,"context_line":"        mock_get_guest_xml.assert_called_once()"},{"line_number":13897,"context_line":"        self.assertIn(accel_info, mock_get_guest_xml.call_args.args)"},{"line_number":13898,"context_line":""},{"line_number":13899,"context_line":"    # Methods called directly by spawn()"},{"line_number":13900,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver, \u0027_get_guest_xml\u0027)"}],"source_content_type":"text/x-python","patch_set":55,"id":"1fa4df85_66d7d025","line":13897,"in_reply_to":"3fa7e38b_a5728aae","updated":"2020-02-23 08:29:33.000000000","message":"Same as above","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9c2aedc9f1faffe0b2dafc7fa4919e2ea36aa779","unresolved":false,"context_lines":[{"line_number":19195,"context_line":""},{"line_number":19196,"context_line":"        drvr._get_guest_config(instance, network_info\u003d[],"},{"line_number":19197,"context_line":"            image_meta\u003dimage_meta, disk_info\u003ddisk_info, accel_info\u003daccel_info)"},{"line_number":19198,"context_line":"        mock_add_accel.assert_called_once_with(mock.ANY, [])"},{"line_number":19199,"context_line":""},{"line_number":19200,"context_line":"    def test_get_guest_disk_config_rbd_older_config_drive_fall_back(self):"},{"line_number":19201,"context_line":"        # New config drives are stored in rbd but existing instances have"}],"source_content_type":"text/x-python","patch_set":55,"id":"3fa7e38b_258c5a80","line":19198,"updated":"2020-02-18 10:01:03.000000000","message":"I would assert the logging as well.","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"6ed5b40063e27c84c4d6f724e32b182678ec0e7a","unresolved":false,"context_lines":[{"line_number":19195,"context_line":""},{"line_number":19196,"context_line":"        drvr._get_guest_config(instance, network_info\u003d[],"},{"line_number":19197,"context_line":"            image_meta\u003dimage_meta, disk_info\u003ddisk_info, accel_info\u003daccel_info)"},{"line_number":19198,"context_line":"        mock_add_accel.assert_called_once_with(mock.ANY, [])"},{"line_number":19199,"context_line":""},{"line_number":19200,"context_line":"    def test_get_guest_disk_config_rbd_older_config_drive_fall_back(self):"},{"line_number":19201,"context_line":"        # New config drives are stored in rbd but existing instances have"}],"source_content_type":"text/x-python","patch_set":55,"id":"3fa7e38b_02155b3e","line":19198,"in_reply_to":"3fa7e38b_258c5a80","updated":"2020-02-23 08:29:33.000000000","message":"Done","commit_id":"604a6b75d9fabf167b0cd1e98a0deb92223d6247"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"379389cd92d23af4b460095c8083115a66f38dbb","unresolved":false,"context_lines":[{"line_number":13856,"context_line":"        accel_info \u003d []"},{"line_number":13857,"context_line":"        instance \u003d self._test_spawn_accels(accel_info)"},{"line_number":13858,"context_line":"        mock_get_guest_xml.assert_called_once_with("},{"line_number":13859,"context_line":"            self.context, instance, mock.ANY, mock.ANY, mock.ANY,"},{"line_number":13860,"context_line":"            block_device_info\u003dNone, mdevs\u003dmock.ANY,"},{"line_number":13861,"context_line":"            accel_info\u003d[])"},{"line_number":13862,"context_line":""}],"source_content_type":"text/x-python","patch_set":61,"id":"1fa4df85_30276f67","line":13859,"range":{"start_line":13859,"start_character":56,"end_line":13859,"end_character":64},"updated":"2020-03-17 15:15:41.000000000","message":"this is the image_meta you created in _test_spawn_accels","commit_id":"d1fbb72665d536524243f852c57d8241f3b00a23"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6b57712804521fa549583fca608cf97a8e45ee2","unresolved":false,"context_lines":[{"line_number":3132,"context_line":"        if instance.flavor.device_profile_name is not None:"},{"line_number":3133,"context_line":"            cyclient \u003d cyborg.get_client()"},{"line_number":3134,"context_line":"            try:"},{"line_number":3135,"context_line":"                arqs \u003d cyclient.get_resolved_arqs_for_instance(instance.uuid)"},{"line_number":3136,"context_line":"            except Exception as e:"},{"line_number":3137,"context_line":"                # TODO(Sundar) Use CONF var to decide if this is fatal."},{"line_number":3138,"context_line":"                LOG.warning(\"Cyborg returned error %s\", e)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_f2cb22a2","line":3135,"range":{"start_line":3135,"start_character":16,"end_line":3135,"end_character":77},"updated":"2019-02-09 00:05:34.000000000","message":"hmph. kind of thinking this should be done before spawn is called, and the arqs (which are guaranteed to be resolved at that point) are just passed in.\n\nMaybe *this* is the bit that gets done in/around instance_claim...","commit_id":"1e4d7bce757f4db1d36e693c93813a5e6b9eae05"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6b57712804521fa549583fca608cf97a8e45ee2","unresolved":false,"context_lines":[{"line_number":3134,"context_line":"            try:"},{"line_number":3135,"context_line":"                arqs \u003d cyclient.get_resolved_arqs_for_instance(instance.uuid)"},{"line_number":3136,"context_line":"            except Exception as e:"},{"line_number":3137,"context_line":"                # TODO(Sundar) Use CONF var to decide if this is fatal."},{"line_number":3138,"context_line":"                LOG.warning(\"Cyborg returned error %s\", e)"},{"line_number":3139,"context_line":""},{"line_number":3140,"context_line":"        xml \u003d self._get_guest_xml(context, instance, network_info,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_92e1762d","line":3137,"range":{"start_line":3137,"start_character":16,"end_line":3137,"end_character":71},"updated":"2019-02-09 00:05:34.000000000","message":"Again, this should just be fatal (reschedulable). What\u0027s the alternative? Spawning without the requested device? Naw.","commit_id":"1e4d7bce757f4db1d36e693c93813a5e6b9eae05"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e6b57712804521fa549583fca608cf97a8e45ee2","unresolved":false,"context_lines":[{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"        self._guest_add_pci_devices(guest, instance)"},{"line_number":5364,"context_line":""},{"line_number":5365,"context_line":"        if arqs is not None and len(arqs) \u003e 0 and \\"},{"line_number":5366,"context_line":"                arqs[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027:"},{"line_number":5367,"context_line":"            # All ARQs are expected to have same handle type."},{"line_number":5368,"context_line":"            # So we check the first alone."}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_12cfc6b0","line":5365,"range":{"start_line":5365,"start_character":50,"end_line":5365,"end_character":51},"updated":"2019-02-09 00:05:34.000000000","message":"style: use parens instead of backslashes","commit_id":"1e4d7bce757f4db1d36e693c93813a5e6b9eae05"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"11570c4ed375e7a37007ff5f9a9c2b59f82a5505","unresolved":false,"context_lines":[{"line_number":6056,"context_line":"        if dp_name is not None:"},{"line_number":6057,"context_line":"            cyclient \u003d cyborg.get_client(context)"},{"line_number":6058,"context_line":"            arqs \u003d cyclient.get_resolved_arqs_for_instance("},{"line_number":6059,"context_line":"                instance.uuid)"},{"line_number":6060,"context_line":""},{"line_number":6061,"context_line":"        # NOTE(mriedem): block_device_info can contain auth_password so we"},{"line_number":6062,"context_line":"        # need to sanitize the password in the message."}],"source_content_type":"text/x-python","patch_set":39,"id":"3fa7e38b_f95e0a43","line":6059,"updated":"2019-11-25 19:31:50.000000000","message":"It is not reasonable, IMHO, to call to an external service in this method. We\u0027re supposed to be generating the guest XML here, which should be doable with nothing more than our database information and a connection to libvirt. If that means you need to store some more information, then do that.","commit_id":"f6f19b04a8e954e7f8593a33a92d2f63c3066782"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"e97581976f890cc56db80ba74610af88f8418a6a","unresolved":false,"context_lines":[{"line_number":6056,"context_line":"        if dp_name is not None:"},{"line_number":6057,"context_line":"            cyclient \u003d cyborg.get_client(context)"},{"line_number":6058,"context_line":"            arqs \u003d cyclient.get_resolved_arqs_for_instance("},{"line_number":6059,"context_line":"                instance.uuid)"},{"line_number":6060,"context_line":""},{"line_number":6061,"context_line":"        # NOTE(mriedem): block_device_info can contain auth_password so we"},{"line_number":6062,"context_line":"        # need to sanitize the password in the message."}],"source_content_type":"text/x-python","patch_set":39,"id":"3fa7e38b_0b41da0a","line":6059,"in_reply_to":"3fa7e38b_3541ef64","updated":"2019-12-04 09:15:01.000000000","message":"Done","commit_id":"f6f19b04a8e954e7f8593a33a92d2f63c3066782"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bcc88ef8faa170202274751bf901a267ea66bed9","unresolved":false,"context_lines":[{"line_number":6056,"context_line":"        if dp_name is not None:"},{"line_number":6057,"context_line":"            cyclient \u003d cyborg.get_client(context)"},{"line_number":6058,"context_line":"            arqs \u003d cyclient.get_resolved_arqs_for_instance("},{"line_number":6059,"context_line":"                instance.uuid)"},{"line_number":6060,"context_line":""},{"line_number":6061,"context_line":"        # NOTE(mriedem): block_device_info can contain auth_password so we"},{"line_number":6062,"context_line":"        # need to sanitize the password in the message."}],"source_content_type":"text/x-python","patch_set":39,"id":"3fa7e38b_b5a63f54","line":6059,"in_reply_to":"3fa7e38b_55b82b74","updated":"2019-11-26 23:14:15.000000000","message":"\u003e Agree. Make whatever call to cyborg that you need in compute and\n \u003e pass it down to the driver.spawn method. The _build_resources\n \u003e method in conductor that yields the block_device_info and\n \u003e network_info (that are passed to spawn()) could return the arqs or\n \u003e whatever the same way. Using existing patterns is good for\n \u003e maintainability and eventually allowing other compute drivers to\n \u003e implement support as well.\n\n*typo - I meant _build_resources method in compute (ComputeManager), not conductor.","commit_id":"f6f19b04a8e954e7f8593a33a92d2f63c3066782"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1a240e84ec9edb89e40e1eb08ad456410d3caf97","unresolved":false,"context_lines":[{"line_number":6056,"context_line":"        if dp_name is not None:"},{"line_number":6057,"context_line":"            cyclient \u003d cyborg.get_client(context)"},{"line_number":6058,"context_line":"            arqs \u003d cyclient.get_resolved_arqs_for_instance("},{"line_number":6059,"context_line":"                instance.uuid)"},{"line_number":6060,"context_line":""},{"line_number":6061,"context_line":"        # NOTE(mriedem): block_device_info can contain auth_password so we"},{"line_number":6062,"context_line":"        # need to sanitize the password in the message."}],"source_content_type":"text/x-python","patch_set":39,"id":"3fa7e38b_3541ef64","line":6059,"in_reply_to":"3fa7e38b_55b82b74","updated":"2019-11-26 23:21:18.000000000","message":"\u003e Agree. Make whatever call to cyborg that you need in compute and\n \u003e pass it down to the driver.spawn method. The _build_resources\n \u003e method in conductor that yields the block_device_info and\n \u003e network_info (that are passed to spawn()) could return the arqs or\n \u003e whatever the same way. Using existing patterns is good for\n \u003e maintainability and eventually allowing other compute drivers to\n \u003e implement support as well.\n\nRight, and that also avoids the repetitive check here for the flavor having an accelerator. We pass the ARQs down if the instance has one. The signal that an instance has one is kept in resource management code, and the virt driver doesn\u0027t need to be so tightly coupled to how and where.","commit_id":"f6f19b04a8e954e7f8593a33a92d2f63c3066782"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"21f781ef77d483fea65c2c05da011190109e2c6d","unresolved":false,"context_lines":[{"line_number":6056,"context_line":"        if dp_name is not None:"},{"line_number":6057,"context_line":"            cyclient \u003d cyborg.get_client(context)"},{"line_number":6058,"context_line":"            arqs \u003d cyclient.get_resolved_arqs_for_instance("},{"line_number":6059,"context_line":"                instance.uuid)"},{"line_number":6060,"context_line":""},{"line_number":6061,"context_line":"        # NOTE(mriedem): block_device_info can contain auth_password so we"},{"line_number":6062,"context_line":"        # need to sanitize the password in the message."}],"source_content_type":"text/x-python","patch_set":39,"id":"3fa7e38b_55b82b74","line":6059,"in_reply_to":"3fa7e38b_f95e0a43","updated":"2019-11-26 23:13:26.000000000","message":"Agree. Make whatever call to cyborg that you need in compute and pass it down to the driver.spawn method. The _build_resources method in conductor that yields the block_device_info and network_info (that are passed to spawn()) could return the arqs or whatever the same way. Using existing patterns is good for maintainability and eventually allowing other compute drivers to implement support as well.","commit_id":"f6f19b04a8e954e7f8593a33a92d2f63c3066782"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f039927a7a5178e647f806fdf4c50bbad3efae9e","unresolved":false,"context_lines":[{"line_number":5726,"context_line":""},{"line_number":5727,"context_line":"        self._guest_add_pci_devices(guest, instance)"},{"line_number":5728,"context_line":""},{"line_number":5729,"context_line":"        if (arqs and len(arqs) \u003e 0 and"},{"line_number":5730,"context_line":"                arqs[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027):"},{"line_number":5731,"context_line":"            # All ARQs are expected to have same handle type."},{"line_number":5732,"context_line":"            # So we check the first alone."}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_48a9117a","line":5729,"updated":"2019-12-04 15:13:15.000000000","message":"If you fix the thing in resources like I suggested, then you can remove this entire conditional, always call self._get_add_accel_pci_devices() (maybe with a filter generator to select only the PCI ones).","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"01709a7d213c2547068700ef4c74f7fc9ba4d770","unresolved":false,"context_lines":[{"line_number":5726,"context_line":""},{"line_number":5727,"context_line":"        self._guest_add_pci_devices(guest, instance)"},{"line_number":5728,"context_line":""},{"line_number":5729,"context_line":"        if (arqs and len(arqs) \u003e 0 and"},{"line_number":5730,"context_line":"                arqs[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027):"},{"line_number":5731,"context_line":"            # All ARQs are expected to have same handle type."},{"line_number":5732,"context_line":"            # So we check the first alone."}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_b7e4750f","line":5729,"in_reply_to":"3fa7e38b_48a9117a","updated":"2019-12-09 08:27:46.000000000","message":"Done","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f039927a7a5178e647f806fdf4c50bbad3efae9e","unresolved":false,"context_lines":[{"line_number":5727,"context_line":"        self._guest_add_pci_devices(guest, instance)"},{"line_number":5728,"context_line":""},{"line_number":5729,"context_line":"        if (arqs and len(arqs) \u003e 0 and"},{"line_number":5730,"context_line":"                arqs[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027):"},{"line_number":5731,"context_line":"            # All ARQs are expected to have same handle type."},{"line_number":5732,"context_line":"            # So we check the first alone."},{"line_number":5733,"context_line":"            self._guest_add_accel_pci_devices(guest, arqs)"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_28b855c5","line":5730,"updated":"2019-12-04 15:13:15.000000000","message":"Presumably in the future we could have a mix of accelerators and this will be waiting to bite. Why not just pass a filtered-for-pci-devices copy of this to the PCI handler?","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"01709a7d213c2547068700ef4c74f7fc9ba4d770","unresolved":false,"context_lines":[{"line_number":5727,"context_line":"        self._guest_add_pci_devices(guest, instance)"},{"line_number":5728,"context_line":""},{"line_number":5729,"context_line":"        if (arqs and len(arqs) \u003e 0 and"},{"line_number":5730,"context_line":"                arqs[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027):"},{"line_number":5731,"context_line":"            # All ARQs are expected to have same handle type."},{"line_number":5732,"context_line":"            # So we check the first alone."},{"line_number":5733,"context_line":"            self._guest_add_accel_pci_devices(guest, arqs)"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_d7e1f1fd","line":5730,"in_reply_to":"3fa7e38b_28b855c5","updated":"2019-12-09 08:27:46.000000000","message":"Done","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f039927a7a5178e647f806fdf4c50bbad3efae9e","unresolved":false,"context_lines":[{"line_number":6038,"context_line":"               {\u0027network_info\u0027: network_info_str, \u0027disk_info\u0027: disk_info,"},{"line_number":6039,"context_line":"                \u0027image_meta\u0027: image_meta, \u0027rescue\u0027: rescue,"},{"line_number":6040,"context_line":"                \u0027block_device_info\u0027: block_device_info})"},{"line_number":6041,"context_line":""},{"line_number":6042,"context_line":"        # NOTE(mriedem): block_device_info can contain auth_password so we"},{"line_number":6043,"context_line":"        # need to sanitize the password in the message."},{"line_number":6044,"context_line":"        LOG.debug(strutils.mask_password(msg), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_e898fd69","line":6041,"updated":"2019-12-04 15:13:15.000000000","message":"Whitespace damage","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"01709a7d213c2547068700ef4c74f7fc9ba4d770","unresolved":false,"context_lines":[{"line_number":6038,"context_line":"               {\u0027network_info\u0027: network_info_str, \u0027disk_info\u0027: disk_info,"},{"line_number":6039,"context_line":"                \u0027image_meta\u0027: image_meta, \u0027rescue\u0027: rescue,"},{"line_number":6040,"context_line":"                \u0027block_device_info\u0027: block_device_info})"},{"line_number":6041,"context_line":""},{"line_number":6042,"context_line":"        # NOTE(mriedem): block_device_info can contain auth_password so we"},{"line_number":6043,"context_line":"        # need to sanitize the password in the message."},{"line_number":6044,"context_line":"        LOG.debug(strutils.mask_password(msg), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":40,"id":"3fa7e38b_37bac5e0","line":6041,"in_reply_to":"3fa7e38b_e898fd69","updated":"2019-12-09 08:27:46.000000000","message":"Done","commit_id":"db7798dc5418de4516f67d768e47a974edaaa745"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"444eea575beabc9a9f59d020fe89b0328298d9e9","unresolved":false,"context_lines":[{"line_number":5729,"context_line":"        pci_arq_list \u003d []"},{"line_number":5730,"context_line":"        if accel_info:"},{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_10169e4b","line":5732,"updated":"2019-12-09 16:49:28.000000000","message":"So, this is why the test at the end of this series works, but doesn\u0027t actually put anything into the XML right? Because the type is TEST_PCI?\n\nI think that we should at *least* LOG.warning() here for each accelerator we\u0027re ignoring because the type is unsupported. I would tend to argue that we should *fail* in that case so that people don\u0027t spin up instances with expensive accelerators (which billing will be charging for) and that we\u0027re silently ignoring.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"9d7405db81c31e3be3d041f6063a8101c375e471","unresolved":false,"context_lines":[{"line_number":5729,"context_line":"        pci_arq_list \u003d []"},{"line_number":5730,"context_line":"        if accel_info:"},{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_f79cff5b","line":5732,"in_reply_to":"3fa7e38b_10169e4b","updated":"2019-12-11 19:55:04.000000000","message":"\u003e So, this is why the test at the end of this series works, but\n \u003e doesn\u0027t actually put anything into the XML right? Because the type\n \u003e is TEST_PCI?\n\nYes.\n\n \u003e I think that we should at *least* LOG.warning() here for each\n \u003e accelerator we\u0027re ignoring because the type is unsupported. I would\n \u003e tend to argue that we should *fail* in that case so that people\n \u003e don\u0027t spin up instances with expensive accelerators (which billing\n \u003e will be charging for) and that we\u0027re silently ignoring.\n\nAll the devices that Cyborg supports now and in the near future are going to be with PCI passthrough. This case is going to be hit for the fake driver only. Warnings here will be distracting IMHO, but a LOG.info wouldn\u0027t hurt.\n\nFurther more, Cyborg documentation states which drivers/devices are supported. If we add a device with a different attach handle type, it will be considered unsupported till both Cyborg and Nova changes are done to enable it. In fact, PCI devices are listed as unsupported today since this patch series has not merged.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"d58677456f2227c3ad6a54ede3ec91e624548852","unresolved":false,"context_lines":[{"line_number":5729,"context_line":"        pci_arq_list \u003d []"},{"line_number":5730,"context_line":"        if accel_info:"},{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_89c48493","line":5732,"in_reply_to":"3fa7e38b_2e8c7633","updated":"2019-12-11 20:53:48.000000000","message":"\u003e My point is exactly for the case where everyone assumes PCI until\n \u003e they don\u0027t. \n\nJust curious: what non-PCI device are you concerned about? If you are referring to PCI passthrough vs mediated devices, yes, that will require changes, but Cyborg will need changes too.\n\nMy concern is with putting checks/warnings now in anticipation of a future which may be a long while away. IMHO, it would be better to make the changes as we need them.\n\nThat said, please look at my next update, which should address some of your concerns at least.\n\n \u003e \u003e Further more, Cyborg documentation states which drivers/devices\n \u003e are\n \u003e \u003e supported. If we add a device with a different attach handle\n \u003e type,\n \u003e \u003e it will be considered unsupported till both Cyborg and Nova\n \u003e changes\n \u003e \u003e are done to enable it. In fact, PCI devices are listed as\n \u003e \u003e unsupported today since this patch series has not merged.\n \u003e \n \u003e This argument holds no weight for me at all. Silently ignoring\n \u003e things is always bad.\n\nPlease see my rework of this logic in the next patch.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4a143b3b7481e4665b01e3b852bddd45e4fb55c","unresolved":false,"context_lines":[{"line_number":5729,"context_line":"        pci_arq_list \u003d []"},{"line_number":5730,"context_line":"        if accel_info:"},{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_29d5501b","line":5732,"in_reply_to":"3fa7e38b_89c48493","updated":"2019-12-11 21:03:48.000000000","message":"\u003e Just curious: what non-PCI device are you concerned about? If you\n \u003e are referring to PCI passthrough vs mediated devices, yes, that\n \u003e will require changes, but Cyborg will need changes too.\n\nI\u0027m worried about things we don\u0027t support now, but will in the future. I want this code to be defensive and not just ignore things it doesn\u0027t know about. In some future someone will try to make changes to nova or cyborg in isolation, or run a newer cyborg with an older nova, etc. This code shouldn\u0027t just ignore things it is instructed to handle, but doesn\u0027t know about.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2719a8902362ab65b7d42295fb51458de61c74ba","unresolved":false,"context_lines":[{"line_number":5729,"context_line":"        pci_arq_list \u003d []"},{"line_number":5730,"context_line":"        if accel_info:"},{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_2e8c7633","line":5732,"in_reply_to":"3fa7e38b_f79cff5b","updated":"2019-12-11 20:00:51.000000000","message":"\u003e All the devices that Cyborg supports now and in the near future are\n \u003e going to be with PCI passthrough. This case is going to be hit for\n \u003e the fake driver only. Warnings here will be distracting IMHO, but a\n \u003e LOG.info wouldn\u0027t hurt.\n\nMy point is exactly for the case where everyone assumes PCI until they don\u0027t. I really feel like we should fail here, were it not for the TEST_PCI example. I think logging a warning in the TEST_PCI case is fine, and is the *minimum* we should do to highlight the mismatch on a real system.\n\n \u003e Further more, Cyborg documentation states which drivers/devices are\n \u003e supported. If we add a device with a different attach handle type,\n \u003e it will be considered unsupported till both Cyborg and Nova changes\n \u003e are done to enable it. In fact, PCI devices are listed as\n \u003e unsupported today since this patch series has not merged.\n\nThis argument holds no weight for me at all. Silently ignoring things is always bad.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"444eea575beabc9a9f59d020fe89b0328298d9e9","unresolved":false,"context_lines":[{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"},{"line_number":5736,"context_line":""},{"line_number":5737,"context_line":"        self._guest_add_watchdog_action(guest, flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_d0fc860d","line":5734,"updated":"2019-12-09 16:49:28.000000000","message":"Surely this won\u0027t be the case forever, right? Can we not just assert that they are all the same? Combined with the above, we can probably build a list of things we\u0027re not going to attach (so we can warn/fail) and then just check that that list is empty.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2719a8902362ab65b7d42295fb51458de61c74ba","unresolved":false,"context_lines":[{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"},{"line_number":5736,"context_line":""},{"line_number":5737,"context_line":"        self._guest_add_watchdog_action(guest, flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_ee817efb","line":5734,"in_reply_to":"3fa7e38b_97d58be8","updated":"2019-12-11 20:00:51.000000000","message":"Then there is zero problem with making the assertion that they\u0027re all PCI right?","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"9d7405db81c31e3be3d041f6063a8101c375e471","unresolved":false,"context_lines":[{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"},{"line_number":5736,"context_line":""},{"line_number":5737,"context_line":"        self._guest_add_watchdog_action(guest, flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_97d58be8","line":5734,"in_reply_to":"3fa7e38b_d0fc860d","updated":"2019-12-11 19:55:04.000000000","message":"Please see above. All the devices we plan to support, for the foreseeable future, are PCI passthrough. If we get something else, it is going to require multiple changes and appropriate documentation.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"d58677456f2227c3ad6a54ede3ec91e624548852","unresolved":false,"context_lines":[{"line_number":5731,"context_line":"            pci_arq_list \u003d [arq for arq in accel_info if"},{"line_number":5732,"context_line":"                            accel_info[0][\u0027attach_handle_type\u0027] \u003d\u003d \u0027PCI\u0027]"},{"line_number":5733,"context_line":"        # All ARQs are expected to have same handle type."},{"line_number":5734,"context_line":"        # So we check the first alone."},{"line_number":5735,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"},{"line_number":5736,"context_line":""},{"line_number":5737,"context_line":"        self._guest_add_watchdog_action(guest, flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3fa7e38b_69b58818","line":5734,"in_reply_to":"3fa7e38b_ee817efb","updated":"2019-12-11 20:53:48.000000000","message":"I made this change in the next patchset.","commit_id":"0ff009d4a97d968df4b8aa27ef38ad2f753f5a13"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"be0ce2c77ab156b33bcfd0183739aeb5b6f12c7f","unresolved":false,"context_lines":[{"line_number":5754,"context_line":"            if first_ah_type \u003d\u003d \u0027PCI\u0027 and all_ah_have_same_type:"},{"line_number":5755,"context_line":"                pci_arq_list \u003d accel_info"},{"line_number":5756,"context_line":"            else:"},{"line_number":5757,"context_line":"                LOG.info(\u0027Ignoring accelerator requests for \u0027"},{"line_number":5758,"context_line":"                         \u0027instance %s. Attach handle types %s.\u0027,"},{"line_number":5759,"context_line":"                         instance.uuid, ah_type_list)"},{"line_number":5760,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"}],"source_content_type":"text/x-python","patch_set":44,"id":"3fa7e38b_094c1474","line":5757,"range":{"start_line":5757,"start_character":20,"end_line":5757,"end_character":24},"updated":"2019-12-11 20:46:29.000000000","message":"warning","commit_id":"e88aaf0d35310446899d677f9b9e8e800475a561"},{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"ca760f64d13079e7f7b8280d140d2b04b1173f06","unresolved":false,"context_lines":[{"line_number":5749,"context_line":"            # expect a mixture of different attach handles for the same"},{"line_number":5750,"context_line":"            # instance; but that case also gets ignored by this logic."},{"line_number":5751,"context_line":"            ah_type_list \u003d [arq[\u0027attach_handle_type\u0027] for arq in accel_info]"},{"line_number":5752,"context_line":"            first_ah_type \u003d ah_type_list[0]"},{"line_number":5753,"context_line":"            all_ah_have_same_type \u003d (len(set(ah_type_list)) \u003d\u003d 1)"},{"line_number":5754,"context_line":"            if first_ah_type \u003d\u003d \u0027PCI\u0027 and all_ah_have_same_type:"},{"line_number":5755,"context_line":"                pci_arq_list \u003d accel_info"},{"line_number":5756,"context_line":"            else:"},{"line_number":5757,"context_line":"                LOG.info(\u0027Ignoring accelerator requests for \u0027"}],"source_content_type":"text/x-python","patch_set":46,"id":"3fa7e38b_3d896ac6","line":5754,"range":{"start_line":5752,"start_character":0,"end_line":5754,"end_character":64},"updated":"2019-12-24 22:36:10.000000000","message":"If we used a set at the beginning\n\n ah_type_list \u003d {arq[\u0027attach_handle_type\u0027] for arq in accel_info}\n\nThen we could just\n\n if len(ah_type_list) \u003d\u003d 1 and \u0027PCI\u0027 in ah_type_list:\n ...\n\nThat would save creation of two variables, remove an index lookup, and also remove casting of the list to set. The only downside to this that I can see is that we will only have the unique values when logging.","commit_id":"8ae7a4aacfe1094cb7a6b82da27d23693fcb3ac0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4c7d16e0ea34f9f348193e69a2a4cd31d3790b6b","unresolved":false,"context_lines":[{"line_number":5749,"context_line":"            # expect a mixture of different attach handles for the same"},{"line_number":5750,"context_line":"            # instance; but that case also gets ignored by this logic."},{"line_number":5751,"context_line":"            ah_type_list \u003d [arq[\u0027attach_handle_type\u0027] for arq in accel_info]"},{"line_number":5752,"context_line":"            first_ah_type \u003d ah_type_list[0]"},{"line_number":5753,"context_line":"            all_ah_have_same_type \u003d (len(set(ah_type_list)) \u003d\u003d 1)"},{"line_number":5754,"context_line":"            if first_ah_type \u003d\u003d \u0027PCI\u0027 and all_ah_have_same_type:"},{"line_number":5755,"context_line":"                pci_arq_list \u003d accel_info"},{"line_number":5756,"context_line":"            else:"},{"line_number":5757,"context_line":"                LOG.info(\u0027Ignoring accelerator requests for \u0027"}],"source_content_type":"text/x-python","patch_set":46,"id":"3fa7e38b_2ae9eda9","line":5754,"range":{"start_line":5752,"start_character":0,"end_line":5754,"end_character":64},"in_reply_to":"3fa7e38b_1d558e1e","updated":"2020-01-06 15:40:21.000000000","message":"Aside from hating the new set syntax (which I think was a terrible design decision), I agree that comparing the supported types (just PCI) to the set of requested ones would be cleaner than \"if first is expected and all are the same\" logic.","commit_id":"8ae7a4aacfe1094cb7a6b82da27d23693fcb3ac0"},{"author":{"_account_id":21672,"name":"Sundar Nadathur","email":"sundar.nadathur@intel.com","username":"nsundar"},"change_message_id":"b917bf67d1811866b9eb4c09b70aa50bdf9b6d65","unresolved":false,"context_lines":[{"line_number":5749,"context_line":"            # expect a mixture of different attach handles for the same"},{"line_number":5750,"context_line":"            # instance; but that case also gets ignored by this logic."},{"line_number":5751,"context_line":"            ah_type_list \u003d [arq[\u0027attach_handle_type\u0027] for arq in accel_info]"},{"line_number":5752,"context_line":"            first_ah_type \u003d ah_type_list[0]"},{"line_number":5753,"context_line":"            all_ah_have_same_type \u003d (len(set(ah_type_list)) \u003d\u003d 1)"},{"line_number":5754,"context_line":"            if first_ah_type \u003d\u003d \u0027PCI\u0027 and all_ah_have_same_type:"},{"line_number":5755,"context_line":"                pci_arq_list \u003d accel_info"},{"line_number":5756,"context_line":"            else:"},{"line_number":5757,"context_line":"                LOG.info(\u0027Ignoring accelerator requests for \u0027"}],"source_content_type":"text/x-python","patch_set":46,"id":"3fa7e38b_335a7282","line":5754,"range":{"start_line":5752,"start_character":0,"end_line":5754,"end_character":64},"in_reply_to":"3fa7e38b_2ae9eda9","updated":"2020-01-15 06:42:37.000000000","message":"Done","commit_id":"8ae7a4aacfe1094cb7a6b82da27d23693fcb3ac0"},{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"0b314b4ea094fbdfa661b8980240ae88cfdb676d","unresolved":false,"context_lines":[{"line_number":5749,"context_line":"            # expect a mixture of different attach handles for the same"},{"line_number":5750,"context_line":"            # instance; but that case also gets ignored by this logic."},{"line_number":5751,"context_line":"            ah_type_list \u003d [arq[\u0027attach_handle_type\u0027] for arq in accel_info]"},{"line_number":5752,"context_line":"            first_ah_type \u003d ah_type_list[0]"},{"line_number":5753,"context_line":"            all_ah_have_same_type \u003d (len(set(ah_type_list)) \u003d\u003d 1)"},{"line_number":5754,"context_line":"            if first_ah_type \u003d\u003d \u0027PCI\u0027 and all_ah_have_same_type:"},{"line_number":5755,"context_line":"                pci_arq_list \u003d accel_info"},{"line_number":5756,"context_line":"            else:"},{"line_number":5757,"context_line":"                LOG.info(\u0027Ignoring accelerator requests for \u0027"}],"source_content_type":"text/x-python","patch_set":46,"id":"3fa7e38b_9d417e66","line":5754,"range":{"start_line":5752,"start_character":0,"end_line":5754,"end_character":64},"in_reply_to":"3fa7e38b_3d896ac6","updated":"2019-12-24 23:09:28.000000000","message":"Or even\n if len(ah_type_list) \u003d\u003d {\u0027PCI\u0027}:\n ...\n\nBut that might be harder to extend in the future if we needed to support more than one type on an instance..","commit_id":"8ae7a4aacfe1094cb7a6b82da27d23693fcb3ac0"},{"author":{"_account_id":29745,"name":"Dustin Cowles","email":"cowlesd@gmail.com","username":"dustinc","status":"inactive"},"change_message_id":"f96d8a14ca57785e20034a2202e9940b89444e7c","unresolved":false,"context_lines":[{"line_number":5749,"context_line":"            # expect a mixture of different attach handles for the same"},{"line_number":5750,"context_line":"            # instance; but that case also gets ignored by this logic."},{"line_number":5751,"context_line":"            ah_type_list \u003d [arq[\u0027attach_handle_type\u0027] for arq in accel_info]"},{"line_number":5752,"context_line":"            first_ah_type \u003d ah_type_list[0]"},{"line_number":5753,"context_line":"            all_ah_have_same_type \u003d (len(set(ah_type_list)) \u003d\u003d 1)"},{"line_number":5754,"context_line":"            if first_ah_type \u003d\u003d \u0027PCI\u0027 and all_ah_have_same_type:"},{"line_number":5755,"context_line":"                pci_arq_list \u003d accel_info"},{"line_number":5756,"context_line":"            else:"},{"line_number":5757,"context_line":"                LOG.info(\u0027Ignoring accelerator requests for \u0027"}],"source_content_type":"text/x-python","patch_set":46,"id":"3fa7e38b_1d558e1e","line":5754,"range":{"start_line":5752,"start_character":0,"end_line":5754,"end_character":64},"in_reply_to":"3fa7e38b_9d417e66","updated":"2019-12-24 23:10:36.000000000","message":"oops, I meant..\n if ah_type_list \u003d\u003d {\u0027PCI\u0027}:\n ...","commit_id":"8ae7a4aacfe1094cb7a6b82da27d23693fcb3ac0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73ce0d921a2e289ef2e4ea044878222c9853f451","unresolved":false,"context_lines":[{"line_number":4589,"context_line":"        dbsf \u003d pci_utils.parse_address(pci_device.address)"},{"line_number":4590,"context_line":"        dev \u003d vconfig.LibvirtConfigGuestHostdevPCI()"},{"line_number":4591,"context_line":"        dev.domain, dev.bus, dev.slot, dev.function \u003d dbsf"},{"line_number":4592,"context_line":""},{"line_number":4593,"context_line":"        # only kvm support managed mode"},{"line_number":4594,"context_line":"        if CONF.libvirt.virt_type in (\u0027xen\u0027, \u0027parallels\u0027,):"},{"line_number":4595,"context_line":"            dev.managed \u003d \u0027no\u0027"},{"line_number":4596,"context_line":"        if CONF.libvirt.virt_type in (\u0027kvm\u0027, \u0027qemu\u0027):"},{"line_number":4597,"context_line":"            dev.managed \u003d \u0027yes\u0027"},{"line_number":4598,"context_line":""},{"line_number":4599,"context_line":"        return dev"},{"line_number":4600,"context_line":""}],"source_content_type":"text/x-python","patch_set":62,"id":"df33271e_1da2509b","side":"PARENT","line":4597,"range":{"start_line":4592,"start_character":0,"end_line":4597,"end_character":31},"updated":"2020-03-23 19:17:11.000000000","message":"this is factored out into a function as this logic now need to be reused for cyborg requests.\n+1","commit_id":"1ff60fa52d698716b03225a3863dec1219c34640"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"73ce0d921a2e289ef2e4ea044878222c9853f451","unresolved":false,"context_lines":[{"line_number":5790,"context_line":"            if ah_types_set \u003d\u003d supported_types_set:"},{"line_number":5791,"context_line":"                pci_arq_list \u003d accel_info"},{"line_number":5792,"context_line":"            else:"},{"line_number":5793,"context_line":"                LOG.info(\u0027Ignoring accelerator requests for instance %s. \u0027"},{"line_number":5794,"context_line":"                         \u0027Supported Attach handle types: %s. \u0027"},{"line_number":5795,"context_line":"                         \u0027But got these unsupported types: %s.\u0027,"},{"line_number":5796,"context_line":"                         instance.uuid, supported_types_set,"},{"line_number":5797,"context_line":"                         ah_types_set.difference(supported_types_set))"},{"line_number":5798,"context_line":"        self._guest_add_accel_pci_devices(guest, pci_arq_list)"},{"line_number":5799,"context_line":""},{"line_number":5800,"context_line":"        self._guest_add_watchdog_action(guest, flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":62,"id":"df33271e_dd34684d","line":5797,"range":{"start_line":5793,"start_character":16,"end_line":5797,"end_character":70},"updated":"2020-03-23 19:17:11.000000000","message":"this makes sense but it might be nice to add something to the xml to make testing with the fake driver eaiser.\n\nwhat im actully thinking about is modifying howe we generate the instnace metadata in the xml.\n\ni have been wanting to add the falvor extra spces can image metadata for a while. but im now thinking it might also be nice to add some cyborg info so that we can correctly tell if the device are added or removed by looking at the metadata.\n\nthat is out of scope of this change but i would like to see if we can make debugging the fake driver easier in the future. im not sure it would be right to do it here or not but basically i would like to be able to assert the correct behavir for things like resize or other operations so that we can look at the xml when using the TEST_PCI types form the fake driver. even soemthing as simple as an xml commnet would work.\n\nanyway thats just somthing i have been thinking about.","commit_id":"faeb39e02d54e87212fabb6b09d004cad3ad3d24"}]}