)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"ed5f5720bef8c234c6571327992340ad7d36b004","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     David Edery \u003cdavid.edery@alcatel-lucent.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-03-15 17:50:05 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix failed snapshot on VMs with SRIOV ports"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"After taking a snapshot of a VM that contains SRIOV port/s, if there is"},{"line_number":10,"context_line":"a problem with the PF (for example, it\u0027s offline for some reason), the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a122d0e_fa5fcf3e","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":43},"updated":"2016-04-19 07:15:16.000000000","message":"you patch only handle snapshot, can this happens on suspend/resume scenario?","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d14534d6727eccf836f4036d588034084f34f2bb","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     David Edery \u003cdavid.edery@alcatel-lucent.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-03-15 17:50:05 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix failed snapshot on VMs with SRIOV ports"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"After taking a snapshot of a VM that contains SRIOV port/s, if there is"},{"line_number":10,"context_line":"a problem with the PF (for example, it\u0027s offline for some reason), the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a122d0e_1f30b09d","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":43},"in_reply_to":"1a122d0e_fa5fcf3e","updated":"2016-04-19 10:49:56.000000000","message":"I guess so. will check and handle in the appropriate methods (according to your comments on driver.py)","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ea1878aa7ace8cdd41356852a1e770a2774ee14e","unresolved":false,"context_lines":[{"line_number":14,"context_line":"This is due to an exception thrown by the driver\u0027s _attach_sriov_ports."},{"line_number":15,"context_line":"This fix catches all of the exceptions that are raised by"},{"line_number":16,"context_line":"_attach_sriov_ports when it is called from _snapshot_domain,"},{"line_number":17,"context_line":"logs them as an error and continues as if nothing happened."},{"line_number":18,"context_line":"Thus, both the VM is resumed (as before) and the snapshot is"},{"line_number":19,"context_line":"not deleted (\u003d\u003dthe expected behavior of taking a snapshot of a"},{"line_number":20,"context_line":"suboptimal VM)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a122d0e_196f4793","line":17,"range":{"start_line":17,"start_character":13,"end_line":17,"end_character":21},"updated":"2016-04-18 21:05:00.000000000","message":"a warning","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99785205c94ac449d2988f0efd036cb1dceb583f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"1ac06dbe_7d0337b9","updated":"2016-09-02 16:35:51.000000000","message":"Excellent commit message","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"}],"nova/network/linux_net.py":[{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"3f7c53b17a3d6fbb5023b77e7454f5eeda581709","unresolved":false,"context_lines":[{"line_number":1912,"context_line":"    result \u003d _execute(\u0027ip\u0027, \u0027link\u0027, \u0027show\u0027, pf_ifname)"},{"line_number":1913,"context_line":"    if result:"},{"line_number":1914,"context_line":"        out, err \u003d result"},{"line_number":1915,"context_line":"        lines \u003d out.split(\u0027\\n\u0027)"},{"line_number":1916,"context_line":"        return lines and \" state UP \" in lines[0]"},{"line_number":1917,"context_line":""},{"line_number":1918,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_dabd47b2","line":1915,"range":{"start_line":1915,"start_character":20,"end_line":1915,"end_character":31},"updated":"2016-07-13 04:48:46.000000000","message":"you can use splitlines","commit_id":"454886d6bca372e6617c4c36911665e26e4dd2a4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"14536217e02b823ad22599fb1a14935bd573c7dc","unresolved":false,"context_lines":[{"line_number":1912,"context_line":"    result \u003d _execute(\u0027ip\u0027, \u0027link\u0027, \u0027show\u0027, pf_ifname)"},{"line_number":1913,"context_line":"    if result:"},{"line_number":1914,"context_line":"        out, err \u003d result"},{"line_number":1915,"context_line":"        lines \u003d out.split(\u0027\\n\u0027)"},{"line_number":1916,"context_line":"        return lines and \" state UP \" in lines[0]"},{"line_number":1917,"context_line":""},{"line_number":1918,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_2e1ff0b7","line":1915,"range":{"start_line":1915,"start_character":20,"end_line":1915,"end_character":31},"in_reply_to":"1aa78d24_dabd47b2","updated":"2016-07-13 05:24:48.000000000","message":"Done","commit_id":"454886d6bca372e6617c4c36911665e26e4dd2a4"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"62edd1ebb64d32964773668d358a04c3b16c7f4f","unresolved":false,"context_lines":[{"line_number":1909,"context_line":"    pf_ifname \u003d pci_utils.get_ifname_by_pci_address(pci_addr,"},{"line_number":1910,"context_line":"                                                    pf_interface)"},{"line_number":1911,"context_line":""},{"line_number":1912,"context_line":"    result \u003d _execute(\u0027ip\u0027, \u0027link\u0027, \u0027show\u0027, pf_ifname)"},{"line_number":1913,"context_line":"    if result:"},{"line_number":1914,"context_line":"        out, err \u003d result"},{"line_number":1915,"context_line":"        lines \u003d out.splitlines()"}],"source_content_type":"text/x-python","patch_set":11,"id":"fad55976_aa30c49f","line":1912,"updated":"2016-07-15 17:54:26.000000000","message":"Nit: For consistency, maybe use \"utils.execute\" (see above).","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"28abe61a99ddf7daee64f39768b84d04c5892b9b","unresolved":false,"context_lines":[{"line_number":1909,"context_line":"    pf_ifname \u003d pci_utils.get_ifname_by_pci_address(pci_addr,"},{"line_number":1910,"context_line":"                                                    pf_interface)"},{"line_number":1911,"context_line":""},{"line_number":1912,"context_line":"    result \u003d _execute(\u0027ip\u0027, \u0027link\u0027, \u0027show\u0027, pf_ifname)"},{"line_number":1913,"context_line":"    if result:"},{"line_number":1914,"context_line":"        out, err \u003d result"},{"line_number":1915,"context_line":"        lines \u003d out.splitlines()"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_a969b0c0","line":1912,"in_reply_to":"fad55976_aa30c49f","updated":"2016-07-19 09:04:10.000000000","message":"thank you. will change it","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"8fffdb75f7991825e57375e04ebd523a9052b100","unresolved":false,"context_lines":[{"line_number":16722,"context_line":""},{"line_number":16723,"context_line":"            if sriov or pci:"},{"line_number":16724,"context_line":"                info_called \u003d False"},{"line_number":16725,"context_line":"                for msg in mock_log.info.call_args_list:"},{"line_number":16726,"context_line":"                    info_called \u003d \\"},{"line_number":16727,"context_line":"                        info_called or \\"},{"line_number":16728,"context_line":"                        (\"Could not attach %(device_type)s\" in msg[0][0])"},{"line_number":16729,"context_line":"                self.assertTrue(info_called)"},{"line_number":16730,"context_line":""},{"line_number":16731,"context_line":"            return snapshot"},{"line_number":16732,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5aef4532_f958008c","line":16729,"range":{"start_line":16725,"start_character":16,"end_line":16729,"end_character":44},"updated":"2016-03-11 00:59:05.000000000","message":"I didn\u0027t find a better way to check if the info log message was called (there are at least 3 info log messages called during the snapshot process).\nIf there\u0027s another way - i\u0027d be happy to change the code","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"4f1bd0dac18c03c2f0c6a24a2a60c7bc88e5f638","unresolved":false,"context_lines":[{"line_number":16726,"context_line":"                    info_called \u003d \\"},{"line_number":16727,"context_line":"                        info_called or \\"},{"line_number":16728,"context_line":"                        (\"Could not attach %(device_type)s\" in msg[0][0])"},{"line_number":16729,"context_line":"                self.assertTrue(info_called)"},{"line_number":16730,"context_line":""},{"line_number":16731,"context_line":"            return snapshot"},{"line_number":16732,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3afc51ec_a72c564b","line":16729,"in_reply_to":"3afc51ec_3ce4060c","updated":"2016-03-13 10:29:53.000000000","message":"See the comment at driver.py","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"62c136628ef3b56b75c67c7a4426d43e3dfb0f80","unresolved":false,"context_lines":[{"line_number":16726,"context_line":"                    info_called \u003d \\"},{"line_number":16727,"context_line":"                        info_called or \\"},{"line_number":16728,"context_line":"                        (\"Could not attach %(device_type)s\" in msg[0][0])"},{"line_number":16729,"context_line":"                self.assertTrue(info_called)"},{"line_number":16730,"context_line":""},{"line_number":16731,"context_line":"            return snapshot"},{"line_number":16732,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3afc51ec_3ce4060c","line":16729,"in_reply_to":"5aef4532_f958008c","updated":"2016-03-11 14:10:10.000000000","message":"You\u0027ll probably increase the level to warn or error so it should be easier to test it here ;)","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"26086a1e4df34a7106ef488e41b3c4f8f3a094c0","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        if uuidstr is None:"},{"line_number":299,"context_line":"            uuidstr \u003d str(uuid.uuid4())"},{"line_number":300,"context_line":"        self.uuidstr \u003d uuidstr"},{"line_number":301,"context_line":"        if id is None:"},{"line_number":302,"context_line":"            id \u003d random.randint(0, 1000)"},{"line_number":303,"context_line":"        self.id \u003d id"},{"line_number":304,"context_line":"        self.domname \u003d name"},{"line_number":305,"context_line":"        self._info \u003d [power_state.RUNNING, 2048 * units.Mi, 1234 * units.Mi,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_1f886170","line":302,"range":{"start_line":301,"start_character":0,"end_line":302,"end_character":40},"updated":"2016-07-07 09:25:46.000000000","message":"In case of an error, mock.assert_called_with._error_message fails on _format_mock_failure_message due to:\n...\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 948, in assert_called_once_with\n        return self.assert_called_with(*args, **kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 937, in assert_called_with\n        six.raise_from(AssertionError(_error_message(cause)), cause)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 928, in _error_message\n        msg \u003d self._format_mock_failure_message(args, kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 860, in _format_mock_failure_message\n        expected_string \u003d self._format_mock_call_signature(args, kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 855, in _format_mock_call_signature\n        return _format_call_signature(name, args, kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 2067, in _format_call_signature\n        args_string \u003d \u0027, \u0027.join([repr(arg) for arg in args])\n      File \"nova/virt/libvirt/guest.py\", line 92, in __repr__\n        \u0027uuid\u0027: self.uuid\n    TypeError: %d format: a number is required, not NoneType","commit_id":"63f250c5938251602c67ab8e255b21a10e47c75b"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"26086a1e4df34a7106ef488e41b3c4f8f3a094c0","unresolved":false,"context_lines":[{"line_number":18077,"context_line":""},{"line_number":18078,"context_line":"    def test_qcow2_and_kvm_pci(self):"},{"line_number":18079,"context_line":"        self.flags(virt_type\u003d\u0027kvm\u0027, group\u003d\u0027libvirt\u0027)"},{"line_number":18080,"context_line":"        self._test_snapshot(disk_format\u003d\u0027qcow2\u0027, pci\u003dTrue)"},{"line_number":18081,"context_line":""},{"line_number":18082,"context_line":"    @mock.patch.object(fake_libvirt_utils, \u0027disk_type\u0027, new\u003d\u0027ploop\u0027)"},{"line_number":18083,"context_line":"    @mock.patch.object(libvirt_driver.imagebackend.images,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_5fcca9ba","line":18080,"range":{"start_line":18080,"start_character":49,"end_line":18080,"end_character":57},"updated":"2016-07-07 09:25:46.000000000","message":"In the next patch-set I will add the offline-pci test (change in _snapshot)","commit_id":"63f250c5938251602c67ab8e255b21a10e47c75b"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"3f7c53b17a3d6fbb5023b77e7454f5eeda581709","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        if uuidstr is None:"},{"line_number":299,"context_line":"            uuidstr \u003d str(uuid.uuid4())"},{"line_number":300,"context_line":"        self.uuidstr \u003d uuidstr"},{"line_number":301,"context_line":"        if id is None:"},{"line_number":302,"context_line":"            id \u003d random.randint(0, 1000)"},{"line_number":303,"context_line":"        self.id \u003d id"},{"line_number":304,"context_line":"        self.domname \u003d name"},{"line_number":305,"context_line":"        self._info \u003d [power_state.RUNNING, 2048 * units.Mi, 1234 * units.Mi,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_faf00393","line":302,"range":{"start_line":301,"start_character":8,"end_line":302,"end_character":40},"updated":"2016-07-13 04:48:46.000000000","message":"is this change related to the patch","commit_id":"454886d6bca372e6617c4c36911665e26e4dd2a4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"0daf77c571334e28b4c7605df94365df3ae60f9c","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        if uuidstr is None:"},{"line_number":299,"context_line":"            uuidstr \u003d str(uuid.uuid4())"},{"line_number":300,"context_line":"        self.uuidstr \u003d uuidstr"},{"line_number":301,"context_line":"        if id is None:"},{"line_number":302,"context_line":"            id \u003d random.randint(0, 1000)"},{"line_number":303,"context_line":"        self.id \u003d id"},{"line_number":304,"context_line":"        self.domname \u003d name"},{"line_number":305,"context_line":"        self._info \u003d [power_state.RUNNING, 2048 * units.Mi, 1234 * units.Mi,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_ae7ac074","line":302,"range":{"start_line":301,"start_character":8,"end_line":302,"end_character":40},"in_reply_to":"1aa78d24_faf00393","updated":"2016-07-13 05:02:54.000000000","message":"Yes.\nI\u0027ve encountered it when a test failed and the logging operation of the guest failed due to the following exception of mock.assert_called_with._error_message that fails on _format_mock_failure_message:\n...\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 948, in assert_called_once_with\n        return self.assert_called_with(*args, **kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 937, in assert_called_with\n        six.raise_from(AssertionError(_error_message(cause)), cause)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 928, in _error_message\n        msg \u003d self._format_mock_failure_message(args, kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 860, in _format_mock_failure_message\n        expected_string \u003d self._format_mock_call_signature(args, kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 855, in _format_mock_call_signature\n        return _format_call_signature(name, args, kwargs)\n      File \"/opt/stack/nova/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py\", line 2067, in _format_call_signature\n        args_string \u003d \u0027, \u0027.join([repr(arg) for arg in args])\n      File \"nova/virt/libvirt/guest.py\", line 92, in __repr__\n        \u0027uuid\u0027: self.uuid\n    TypeError: %d format: a number is required, not NoneType","commit_id":"454886d6bca372e6617c4c36911665e26e4dd2a4"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"32e7de10ae2bdf49cf70bd5611c4fa7cc0eb11ce","unresolved":false,"context_lines":[{"line_number":1761,"context_line":"                guest \u003d self._create_domain(domain\u003dvirt_dom, pause\u003dTrue)"},{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                self._attach_pci_devices("},{"line_number":1765,"context_line":"                    guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1766,"context_line":"                # NOTE(dedery): no need to fail the snapshot creation in case"},{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aef4532_be773539","line":1764,"range":{"start_line":1764,"start_character":21,"end_line":1764,"end_character":40},"updated":"2016-03-08 08:19:09.000000000","message":"point for discussion #1: \n\nthe solution is probably missing the PCI-PT use-case which may be the same (e.g. when the physical NIC is down, the vNIC is down and the snapshot will fail due to in-ability to attach to the offline physical NIC - but this was not checked so I don\u0027t know what\u0027s the real behavior of libVirt\u0027s PCI-PT attach mechanism)","commit_id":"e706d6ef4750da1d0a573cfb0e802f7a99e21e81"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"32e7de10ae2bdf49cf70bd5611c4fa7cc0eb11ce","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"},{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1770,"context_line":"                except Exception as ex:"},{"line_number":1771,"context_line":"                    LOG.error(_LE(\"Could not attach SRIOV ports to VM: \""},{"line_number":1772,"context_line":"                                  \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1773,"context_line":"                              {\u0027instance\u0027: instance, \"error\": ex})"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aef4532_5ec6f149","line":1770,"range":{"start_line":1770,"start_character":16,"end_line":1770,"end_character":32},"updated":"2016-03-08 08:19:09.000000000","message":"point for discussion #2:\n\nshould any exception be caught and ignored? (I think that yes of course... no reason to fail the snapshot if you can\u0027t attach the port for any reason)","commit_id":"e706d6ef4750da1d0a573cfb0e802f7a99e21e81"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8b7946a33802d002dd210260709af62bd5bfa3dd","unresolved":false,"context_lines":[{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1770,"context_line":"                except Exception as ex:"},{"line_number":1771,"context_line":"                    LOG.error(_LE(\"Could not attach SRIOV ports to VM: \""},{"line_number":1772,"context_line":"                                  \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1773,"context_line":"                              {\u0027instance\u0027: instance, \"error\": ex})"},{"line_number":1774,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aef4532_b10a6c88","line":1771,"range":{"start_line":1771,"start_character":24,"end_line":1771,"end_character":29},"updated":"2016-03-09 16:41:45.000000000","message":"If you\u0027re going to ignore the error, then it\u0027s not an error and shouldn\u0027t be logged as one, log as INFO probably if it\u0027s OK to ignore it.","commit_id":"e706d6ef4750da1d0a573cfb0e802f7a99e21e81"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"39d48d65624e8a115cb30ae85c16b6f8d5e5da74","unresolved":false,"context_lines":[{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1770,"context_line":"                except Exception as ex:"},{"line_number":1771,"context_line":"                    LOG.error(_LE(\"Could not attach SRIOV ports to VM: \""},{"line_number":1772,"context_line":"                                  \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1773,"context_line":"                              {\u0027instance\u0027: instance, \"error\": ex})"},{"line_number":1774,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"5aef4532_34c92a5a","line":1771,"range":{"start_line":1771,"start_character":24,"end_line":1771,"end_character":29},"in_reply_to":"5aef4532_b10a6c88","updated":"2016-03-09 16:50:34.000000000","message":"Done","commit_id":"e706d6ef4750da1d0a573cfb0e802f7a99e21e81"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"c96013dc6365470fe5cd21bca38103f963fc27e2","unresolved":false,"context_lines":[{"line_number":1761,"context_line":"                guest \u003d self._create_domain(domain\u003dvirt_dom, pause\u003dTrue)"},{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                self._attach_pci_devices("},{"line_number":1765,"context_line":"                    guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1766,"context_line":"                # NOTE(dedery): no need to fail the snapshot creation in case"},{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_f48502cf","line":1764,"range":{"start_line":1764,"start_character":21,"end_line":1764,"end_character":40},"updated":"2016-03-09 16:52:55.000000000","message":"point for discussion #1: \nthe solution is probably missing the PCI-PT use-case which may be the same (e.g. when the physical NIC is down, the vNIC is down and the snapshot will fail due to in-ability to attach to the offline physical NIC - but this was not checked so I don\u0027t know what\u0027s the real behavior of libVirt\u0027s PCI-PT attach mechanism)","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"9860346bdb018e59c29c777996a1ac132e21d9b1","unresolved":false,"context_lines":[{"line_number":1761,"context_line":"                guest \u003d self._create_domain(domain\u003dvirt_dom, pause\u003dTrue)"},{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                self._attach_pci_devices("},{"line_number":1765,"context_line":"                    guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1766,"context_line":"                # NOTE(dedery): no need to fail the snapshot creation in case"},{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_59a774a6","line":1764,"in_reply_to":"5aef4532_a09170f3","updated":"2016-03-11 00:55:45.000000000","message":"Done","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8b3bca468d0fc5f8905f54d335c41b871501fa82","unresolved":false,"context_lines":[{"line_number":1761,"context_line":"                guest \u003d self._create_domain(domain\u003dvirt_dom, pause\u003dTrue)"},{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                self._attach_pci_devices("},{"line_number":1765,"context_line":"                    guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1766,"context_line":"                # NOTE(dedery): no need to fail the snapshot creation in case"},{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_a09170f3","line":1764,"in_reply_to":"5aef4532_f48502cf","updated":"2016-03-09 17:40:13.000000000","message":"Yes you could probably add something to catch any exception which can happen when attaching PCI.","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8b3bca468d0fc5f8905f54d335c41b871501fa82","unresolved":false,"context_lines":[{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                self._attach_pci_devices("},{"line_number":1765,"context_line":"                    guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1766,"context_line":"                # NOTE(dedery): no need to fail the snapshot creation in case"},{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"},{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_2b5541f9","line":1766,"updated":"2016-03-09 17:40:13.000000000","message":"Please remove that NOTE(dedery) It\u0027s really just a comment in code (-1 because of that).","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"9860346bdb018e59c29c777996a1ac132e21d9b1","unresolved":false,"context_lines":[{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                self._attach_pci_devices("},{"line_number":1765,"context_line":"                    guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1766,"context_line":"                # NOTE(dedery): no need to fail the snapshot creation in case"},{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"},{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_99bbbcc3","line":1766,"in_reply_to":"5aef4532_2b5541f9","updated":"2016-03-11 00:55:45.000000000","message":"Done","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"c96013dc6365470fe5cd21bca38103f963fc27e2","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"},{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1770,"context_line":"                except Exception as ex:"},{"line_number":1771,"context_line":"                    LOG.info(_LI(\"Could not attach SRIOV ports to VM: \""},{"line_number":1772,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1773,"context_line":"                             {\u0027instance\u0027: instance, \"error\": ex})"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_d4c8c624","line":1770,"range":{"start_line":1770,"start_character":23,"end_line":1770,"end_character":32},"updated":"2016-03-09 16:52:55.000000000","message":"point for discussion #2:\nshould any exception be caught and ignored? (I think that yes of course... no reason to fail the snapshot if you can\u0027t attach the port for any reason)","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"9860346bdb018e59c29c777996a1ac132e21d9b1","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"},{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1770,"context_line":"                except Exception as ex:"},{"line_number":1771,"context_line":"                    LOG.info(_LI(\"Could not attach SRIOV ports to VM: \""},{"line_number":1772,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1773,"context_line":"                             {\u0027instance\u0027: instance, \"error\": ex})"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_f933e05f","line":1770,"in_reply_to":"5aef4532_2b1ee15e","updated":"2016-03-11 00:55:45.000000000","message":"It happened while playing with the devices (came from our QA). I didn\u0027t see it in a running environment. Still, the role of protection use-cases is important and valid and manually shutting the devices down is the closest we can get to the actual scenario without opening/zapping/breaking something","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8b3bca468d0fc5f8905f54d335c41b871501fa82","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                #               of failed sriov attach (i.e. due to offline PF)"},{"line_number":1768,"context_line":"                try:"},{"line_number":1769,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1770,"context_line":"                except Exception as ex:"},{"line_number":1771,"context_line":"                    LOG.info(_LI(\"Could not attach SRIOV ports to VM: \""},{"line_number":1772,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1773,"context_line":"                             {\u0027instance\u0027: instance, \"error\": ex})"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_2b1ee15e","line":1770,"in_reply_to":"5aef4532_d4c8c624","updated":"2016-03-09 17:40:13.000000000","message":"Hum actually even for PCI Passthought I\u0027m not sure to see how this can happen in real life ? On your bug report you manually down the device. Is this really happened in a running environment or did you get that error when playing with devices ?\n\nBut at least I\u0027m agree with you we should to ensure this point.","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"4072829a15b536ba009a7efef9e11e6ea438ec71","unresolved":false,"context_lines":[{"line_number":1775,"context_line":""},{"line_number":1776,"context_line":"    def _can_set_admin_password(self, image_meta):"},{"line_number":1777,"context_line":"        if (CONF.libvirt.virt_type not in (\u0027kvm\u0027, \u0027qemu\u0027) or"},{"line_number":1778,"context_line":"           not self._host.has_min_version(MIN_LIBVIRT_SET_ADMIN_PASSWD)):"},{"line_number":1779,"context_line":"            raise exception.SetAdminPasswdNotSupported()"},{"line_number":1780,"context_line":""},{"line_number":1781,"context_line":"        hw_qga \u003d image_meta.properties.get(\u0027hw_qemu_guest_agent\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_eb835954","line":1778,"updated":"2016-03-09 17:40:51.000000000","message":"This change is not necessary please remove it too","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"9860346bdb018e59c29c777996a1ac132e21d9b1","unresolved":false,"context_lines":[{"line_number":1775,"context_line":""},{"line_number":1776,"context_line":"    def _can_set_admin_password(self, image_meta):"},{"line_number":1777,"context_line":"        if (CONF.libvirt.virt_type not in (\u0027kvm\u0027, \u0027qemu\u0027) or"},{"line_number":1778,"context_line":"           not self._host.has_min_version(MIN_LIBVIRT_SET_ADMIN_PASSWD)):"},{"line_number":1779,"context_line":"            raise exception.SetAdminPasswdNotSupported()"},{"line_number":1780,"context_line":""},{"line_number":1781,"context_line":"        hw_qga \u003d image_meta.properties.get(\u0027hw_qemu_guest_agent\u0027, \u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5aef4532_d9b1c4e1","line":1778,"in_reply_to":"5aef4532_eb835954","updated":"2016-03-11 00:55:45.000000000","message":"Done","commit_id":"b866f8a0c3bd001f30e9638faa65aba75b7221f0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"62c136628ef3b56b75c67c7a4426d43e3dfb0f80","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"                # no need to fail the snapshot creation in case"},{"line_number":1765,"context_line":"                # of failed sriov/pci attach (i.e. due to offline PF/device)"},{"line_number":1766,"context_line":"                device_type \u003d \"\""},{"line_number":1767,"context_line":"                try:"},{"line_number":1768,"context_line":"                    device_type \u003d \"PCI devices\""},{"line_number":1769,"context_line":"                    self._attach_pci_devices("},{"line_number":1770,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afc51ec_9c90ba97","line":1767,"updated":"2016-03-11 14:10:10.000000000","message":"I do not think you can put both methods in same try ... except, that because if something wrong happens for _attach_pci_devices, it might be worth to try _attach_sriov_ports, no ?","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"82cf898f080662ee0eeb06ec6e28256fceecb675","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"                # no need to fail the snapshot creation in case"},{"line_number":1765,"context_line":"                # of failed sriov/pci attach (i.e. due to offline PF/device)"},{"line_number":1766,"context_line":"                device_type \u003d \"\""},{"line_number":1767,"context_line":"                try:"},{"line_number":1768,"context_line":"                    device_type \u003d \"PCI devices\""},{"line_number":1769,"context_line":"                    self._attach_pci_devices("},{"line_number":1770,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"}],"source_content_type":"text/x-python","patch_set":5,"id":"1af94dfe_4ecb1673","line":1767,"in_reply_to":"1af94dfe_1d266668","updated":"2016-03-15 15:18:35.000000000","message":"Done","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"15649d00da6ba89eb1547947a422326fee5b92c2","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"                # no need to fail the snapshot creation in case"},{"line_number":1765,"context_line":"                # of failed sriov/pci attach (i.e. due to offline PF/device)"},{"line_number":1766,"context_line":"                device_type \u003d \"\""},{"line_number":1767,"context_line":"                try:"},{"line_number":1768,"context_line":"                    device_type \u003d \"PCI devices\""},{"line_number":1769,"context_line":"                    self._attach_pci_devices("},{"line_number":1770,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"}],"source_content_type":"text/x-python","patch_set":5,"id":"1af94dfe_1d266668","line":1767,"in_reply_to":"3afc51ec_27fa06be","updated":"2016-03-15 14:28:21.000000000","message":"You can have a helper, but actually i think it\u0027s OK to have two times a try..except even if that feel horrible, so it\u0027s up to you really.","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"4f1bd0dac18c03c2f0c6a24a2a60c7bc88e5f638","unresolved":false,"context_lines":[{"line_number":1764,"context_line":"                # no need to fail the snapshot creation in case"},{"line_number":1765,"context_line":"                # of failed sriov/pci attach (i.e. due to offline PF/device)"},{"line_number":1766,"context_line":"                device_type \u003d \"\""},{"line_number":1767,"context_line":"                try:"},{"line_number":1768,"context_line":"                    device_type \u003d \"PCI devices\""},{"line_number":1769,"context_line":"                    self._attach_pci_devices("},{"line_number":1770,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afc51ec_27fa06be","line":1767,"in_reply_to":"3afc51ec_9c90ba97","updated":"2016-03-13 10:29:53.000000000","message":"So I gave it some thought before I did this change and it seemed like it changes the attachment logic too much compared to the original state (which is the same as it is here - if the attach_pci_devices fail, we won\u0027t get to the sriov ports).\nRethinking of this I guess that there was no *real* intention to have this logic (if pci fails don\u0027t do sriov) so I tend to agree (though the code will look horrible... is there a nice way to solve the\ntry\n  attach1\nexcept\n  log\n  pass\n\ntimes two?\n(maybe helper function - safe_call(method name, **kargs)?)","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"62c136628ef3b56b75c67c7a4426d43e3dfb0f80","unresolved":false,"context_lines":[{"line_number":1772,"context_line":"                    device_type \u003d \"SRIOV ports\""},{"line_number":1773,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1774,"context_line":"                except Exception as ex:"},{"line_number":1775,"context_line":"                    LOG.info(_LI(\"Could not attach %(device_type)s to VM: \""},{"line_number":1776,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1777,"context_line":"                             {\u0027device_type\u0027: device_type,"},{"line_number":1778,"context_line":"                              \u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afc51ec_fc457e17","line":1775,"updated":"2016-03-11 14:10:10.000000000","message":"I guess you can increase the error level to \u0027warning\u0027, even more; \u0027error\u0027, no?.","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"82cf898f080662ee0eeb06ec6e28256fceecb675","unresolved":false,"context_lines":[{"line_number":1772,"context_line":"                    device_type \u003d \"SRIOV ports\""},{"line_number":1773,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1774,"context_line":"                except Exception as ex:"},{"line_number":1775,"context_line":"                    LOG.info(_LI(\"Could not attach %(device_type)s to VM: \""},{"line_number":1776,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1777,"context_line":"                             {\u0027device_type\u0027: device_type,"},{"line_number":1778,"context_line":"                              \u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1af94dfe_8ec51e5d","line":1775,"in_reply_to":"1af94dfe_fdce1ab3","updated":"2016-03-15 15:18:35.000000000","message":"Done","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"15649d00da6ba89eb1547947a422326fee5b92c2","unresolved":false,"context_lines":[{"line_number":1772,"context_line":"                    device_type \u003d \"SRIOV ports\""},{"line_number":1773,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1774,"context_line":"                except Exception as ex:"},{"line_number":1775,"context_line":"                    LOG.info(_LI(\"Could not attach %(device_type)s to VM: \""},{"line_number":1776,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1777,"context_line":"                             {\u0027device_type\u0027: device_type,"},{"line_number":1778,"context_line":"                              \u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1af94dfe_fdce1ab3","line":1775,"in_reply_to":"3afc51ec_ec9f6501","updated":"2016-03-15 14:28:21.000000000","message":"Ok for \u0027warning\u0027, it\u0027s good deal even If we should never be in that situation actually.","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"4f1bd0dac18c03c2f0c6a24a2a60c7bc88e5f638","unresolved":false,"context_lines":[{"line_number":1772,"context_line":"                    device_type \u003d \"SRIOV ports\""},{"line_number":1773,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"},{"line_number":1774,"context_line":"                except Exception as ex:"},{"line_number":1775,"context_line":"                    LOG.info(_LI(\"Could not attach %(device_type)s to VM: \""},{"line_number":1776,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1777,"context_line":"                             {\u0027device_type\u0027: device_type,"},{"line_number":1778,"context_line":"                              \u0027instance\u0027: instance,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afc51ec_ec9f6501","line":1775,"in_reply_to":"3afc51ec_fc457e17","updated":"2016-03-13 10:29:53.000000000","message":"Originally it was \"error\" but I\u0027ve lowered it to \"info\" due to mriedem\u0027s comment:\nhttps://review.openstack.org/#/c/287515/3/nova/virt/libvirt/driver.py\n\nRethinking about it, it might be better to mark it as warning - since the VM is suboptimal and it\u0027s irreversible automatically. In the case of SRIOV, before the snapshot the port which is connected to the offline PF is marked as DOWN and if the PF comes back up the port will be UP again.\nHowever, after the snapshot, if the PF is still down, the VM comes up WITHOUT the interface and even if the PF comes up the VM will still be suboptimal. What do you say? Worth a \"warning\"?","commit_id":"025b6d84a02ae6ee25cb5fcd1c53b2a5f37b8d39"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"98aa2977108ab7c5f2261d174d239f3c916559f0","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                    self._attach_pci_devices("},{"line_number":1768,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1769,"context_line":"                except Exception as ex:"},{"line_number":1770,"context_line":"                    LOG.warn(_LW(\"Could not attach PCI devices to VM: \""},{"line_number":1771,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1772,"context_line":"                             {\u0027instance\u0027: instance,"},{"line_number":1773,"context_line":"                              \"error\": ex})"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af94dfe_50a57002","line":1770,"updated":"2016-03-15 15:43:07.000000000","message":"Based on that commit [1], I think you should to use LOG.warning instead.\n\n[1] https://review.openstack.org/#/c/264133/","commit_id":"eee6ce61c238da78ec85ce9d67dc30e1b4ab5dbd"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"a96e1a8f79616a34f0ec00f7cd151d4f0901b02a","unresolved":false,"context_lines":[{"line_number":1767,"context_line":"                    self._attach_pci_devices("},{"line_number":1768,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1769,"context_line":"                except Exception as ex:"},{"line_number":1770,"context_line":"                    LOG.warn(_LW(\"Could not attach PCI devices to VM: \""},{"line_number":1771,"context_line":"                                 \"%(instance)s due to: %(error)s. Ignoring\"),"},{"line_number":1772,"context_line":"                             {\u0027instance\u0027: instance,"},{"line_number":1773,"context_line":"                              \"error\": ex})"}],"source_content_type":"text/x-python","patch_set":6,"id":"1af94dfe_503bb0eb","line":1770,"in_reply_to":"1af94dfe_50a57002","updated":"2016-03-15 15:51:11.000000000","message":"Done","commit_id":"eee6ce61c238da78ec85ce9d67dc30e1b4ab5dbd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d22b7614971f09a9c392cbb7859a8bf8fb492a12","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"        # NOTE(dkang): managedSave does not work for LXC"},{"line_number":1668,"context_line":"        if CONF.libvirt.virt_type !\u003d \u0027lxc\u0027 and not live_snapshot:"},{"line_number":1669,"context_line":"            if state \u003d\u003d power_state.RUNNING or state \u003d\u003d power_state.PAUSED:"},{"line_number":1670,"context_line":"                self._detach_pci_devices(guest,"},{"line_number":1671,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1672,"context_line":"                self._detach_sriov_ports(context, instance, guest)"},{"line_number":1673,"context_line":"                guest.save_memory_state()"},{"line_number":1674,"context_line":""},{"line_number":1675,"context_line":"        snapshot_backend \u003d self.image_backend.snapshot(instance,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_14370ee4","line":1672,"range":{"start_line":1670,"start_character":16,"end_line":1672,"end_character":66},"updated":"2016-04-18 21:27:09.000000000","message":"OHHHHHHHHKKKK, now I get it. I was missing this context because this code is so grossly tightly coupled. We detach the pci/sriov devices before the snapshot is taken, and then attempt to attach them afterward, which I guess fails. Yikes.","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"40996599186222b5fe4717ca682839ad5fe7bba2","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"        # NOTE(dkang): managedSave does not work for LXC"},{"line_number":1668,"context_line":"        if CONF.libvirt.virt_type !\u003d \u0027lxc\u0027 and not live_snapshot:"},{"line_number":1669,"context_line":"            if state \u003d\u003d power_state.RUNNING or state \u003d\u003d power_state.PAUSED:"},{"line_number":1670,"context_line":"                self._detach_pci_devices(guest,"},{"line_number":1671,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1672,"context_line":"                self._detach_sriov_ports(context, instance, guest)"},{"line_number":1673,"context_line":"                guest.save_memory_state()"},{"line_number":1674,"context_line":""},{"line_number":1675,"context_line":"        snapshot_backend \u003d self.image_backend.snapshot(instance,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_34220a53","line":1672,"range":{"start_line":1670,"start_character":16,"end_line":1672,"end_character":66},"in_reply_to":"1a122d0e_14370ee4","updated":"2016-04-18 21:37:35.000000000","message":"If the problem is that the attach is going to fail to because something is down which is going to prevent the re-attach, we should detect that before detaching the devices in the first place and fail fast, rather than detach them and shoot ourselves in the foot with this instance, since you can\u0027t re-attach the ports later via the nova API.","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d4f4bab7db137f94d1fa721bbd4efe8fa39f9018","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"        # NOTE(dkang): managedSave does not work for LXC"},{"line_number":1668,"context_line":"        if CONF.libvirt.virt_type !\u003d \u0027lxc\u0027 and not live_snapshot:"},{"line_number":1669,"context_line":"            if state \u003d\u003d power_state.RUNNING or state \u003d\u003d power_state.PAUSED:"},{"line_number":1670,"context_line":"                self._detach_pci_devices(guest,"},{"line_number":1671,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1672,"context_line":"                self._detach_sriov_ports(context, instance, guest)"},{"line_number":1673,"context_line":"                guest.save_memory_state()"},{"line_number":1674,"context_line":""},{"line_number":1675,"context_line":"        snapshot_backend \u003d self.image_backend.snapshot(instance,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_ff5475ca","line":1672,"range":{"start_line":1670,"start_character":16,"end_line":1672,"end_character":66},"in_reply_to":"1a122d0e_34220a53","updated":"2016-07-07 09:31:37.000000000","message":"Done","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d14534d6727eccf836f4036d588034084f34f2bb","unresolved":false,"context_lines":[{"line_number":1667,"context_line":"        # NOTE(dkang): managedSave does not work for LXC"},{"line_number":1668,"context_line":"        if CONF.libvirt.virt_type !\u003d \u0027lxc\u0027 and not live_snapshot:"},{"line_number":1669,"context_line":"            if state \u003d\u003d power_state.RUNNING or state \u003d\u003d power_state.PAUSED:"},{"line_number":1670,"context_line":"                self._detach_pci_devices(guest,"},{"line_number":1671,"context_line":"                    pci_manager.get_instance_pci_devs(instance))"},{"line_number":1672,"context_line":"                self._detach_sriov_ports(context, instance, guest)"},{"line_number":1673,"context_line":"                guest.save_memory_state()"},{"line_number":1674,"context_line":""},{"line_number":1675,"context_line":"        snapshot_backend \u003d self.image_backend.snapshot(instance,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_9f47a01c","line":1672,"range":{"start_line":1670,"start_character":16,"end_line":1672,"end_character":66},"in_reply_to":"1a122d0e_34220a53","updated":"2016-04-19 10:49:56.000000000","message":"I think that it should be a mix of both (to avoid the race). Pre port/pci device detach (and before the snapshot) the code should check the re-attach-ability of the port/pci device and fail fast in case that there\u0027s a potential problem (by that, failing the snapshot operation before it started with a meaningful error message). However, i think that we should consider a \"snapshot --force\" flag that will enable the desperate user to take a snapshot even if the VM will die because of it - but that\u0027s another discussion.\n\nEven with the change described above there\u0027s a quite big time window during the snapshot operation in which the PF/PCI device can go down and the VM will get back up without the port/device (this we can\u0027t avoid) and the snapshot will fail (due to the exception raised in the attach operation as described in the bug report). Adding proper exception handling (which basically ignores the exceptions - as suggested below by moshele) will prevent the snapshot failure.","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"48c14c2208850e4e28a1b5d7f19965d40fb6f1c3","unresolved":false,"context_lines":[{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                # no need to fail the snapshot creation in case"},{"line_number":1765,"context_line":"                # of failed sriov/pci attach (i.e. due to offline PF/device)"},{"line_number":1766,"context_line":"                try:"},{"line_number":1767,"context_line":"                    self._attach_pci_devices("},{"line_number":1768,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_d9ce6fb3","line":1765,"range":{"start_line":1765,"start_character":45,"end_line":1765,"end_character":76},"updated":"2016-04-18 21:04:33.000000000","message":"How do you know that\u0027s why it failed? This is why catching a blanket Exception worries me.","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d4f4bab7db137f94d1fa721bbd4efe8fa39f9018","unresolved":false,"context_lines":[{"line_number":1762,"context_line":""},{"line_number":1763,"context_line":"            if guest is not None:"},{"line_number":1764,"context_line":"                # no need to fail the snapshot creation in case"},{"line_number":1765,"context_line":"                # of failed sriov/pci attach (i.e. due to offline PF/device)"},{"line_number":1766,"context_line":"                try:"},{"line_number":1767,"context_line":"                    self._attach_pci_devices("},{"line_number":1768,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_9f1e110a","line":1765,"range":{"start_line":1765,"start_character":45,"end_line":1765,"end_character":76},"in_reply_to":"1a122d0e_d9ce6fb3","updated":"2016-07-07 09:31:37.000000000","message":"Done","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"48c14c2208850e4e28a1b5d7f19965d40fb6f1c3","unresolved":false,"context_lines":[{"line_number":1766,"context_line":"                try:"},{"line_number":1767,"context_line":"                    self._attach_pci_devices("},{"line_number":1768,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1769,"context_line":"                except Exception as ex:"},{"line_number":1770,"context_line":"                    LOG.warning(_LW(\"Could not attach PCI devices to VM: \""},{"line_number":1771,"context_line":"                                    \"%(instance)s due to: %(error)s. \""},{"line_number":1772,"context_line":"                                    \"Ignoring\"),"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_59e41f3a","line":1769,"range":{"start_line":1769,"start_character":23,"end_line":1769,"end_character":32},"updated":"2016-04-18 21:04:33.000000000","message":"There isn\u0027t something more specific we can handle for this?","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d4f4bab7db137f94d1fa721bbd4efe8fa39f9018","unresolved":false,"context_lines":[{"line_number":1766,"context_line":"                try:"},{"line_number":1767,"context_line":"                    self._attach_pci_devices("},{"line_number":1768,"context_line":"                        guest, pci_manager.get_instance_pci_devs(instance))"},{"line_number":1769,"context_line":"                except Exception as ex:"},{"line_number":1770,"context_line":"                    LOG.warning(_LW(\"Could not attach PCI devices to VM: \""},{"line_number":1771,"context_line":"                                    \"%(instance)s due to: %(error)s. \""},{"line_number":1772,"context_line":"                                    \"Ignoring\"),"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_df1419ea","line":1769,"range":{"start_line":1769,"start_character":23,"end_line":1769,"end_character":32},"in_reply_to":"1a122d0e_59e41f3a","updated":"2016-07-07 09:31:37.000000000","message":"Done","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"48c14c2208850e4e28a1b5d7f19965d40fb6f1c3","unresolved":false,"context_lines":[{"line_number":1770,"context_line":"                    LOG.warning(_LW(\"Could not attach PCI devices to VM: \""},{"line_number":1771,"context_line":"                                    \"%(instance)s due to: %(error)s. \""},{"line_number":1772,"context_line":"                                    \"Ignoring\"),"},{"line_number":1773,"context_line":"                                {\u0027instance\u0027: instance, \"error\": ex})"},{"line_number":1774,"context_line":"                    pass"},{"line_number":1775,"context_line":""},{"line_number":1776,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_39dfa36a","line":1773,"range":{"start_line":1773,"start_character":33,"end_line":1773,"end_character":53},"updated":"2016-04-18 21:04:33.000000000","message":"Use the instance\u003dinstance kwarg in LOG.warning so we get the syntax highlighting with oslo.log.\n\nThen you could reword the warning as:\n\nLOG.warning(_LW(\"Could not attach PCI devices to instance due to error: %s\"), ex, instance\u003dinstance)","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"459bcb069436466c5ebb2790eb6a35581d07186f","unresolved":false,"context_lines":[{"line_number":1771,"context_line":"                                    \"%(instance)s due to: %(error)s. \""},{"line_number":1772,"context_line":"                                    \"Ignoring\"),"},{"line_number":1773,"context_line":"                                {\u0027instance\u0027: instance, \"error\": ex})"},{"line_number":1774,"context_line":"                    pass"},{"line_number":1775,"context_line":""},{"line_number":1776,"context_line":"                try:"},{"line_number":1777,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa0719c6_564fa16a","line":1774,"updated":"2016-03-23 10:57:48.000000000","message":"nit: pass statement is redundant here as you already have the LOG.warning() call","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d4f4bab7db137f94d1fa721bbd4efe8fa39f9018","unresolved":false,"context_lines":[{"line_number":1771,"context_line":"                                    \"%(instance)s due to: %(error)s. \""},{"line_number":1772,"context_line":"                                    \"Ignoring\"),"},{"line_number":1773,"context_line":"                                {\u0027instance\u0027: instance, \"error\": ex})"},{"line_number":1774,"context_line":"                    pass"},{"line_number":1775,"context_line":""},{"line_number":1776,"context_line":"                try:"},{"line_number":1777,"context_line":"                    self._attach_sriov_ports(context, instance, guest)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_5fdf8939","line":1774,"in_reply_to":"fa0719c6_564fa16a","updated":"2016-07-07 09:31:37.000000000","message":"Done","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"48c14c2208850e4e28a1b5d7f19965d40fb6f1c3","unresolved":false,"context_lines":[{"line_number":1779,"context_line":"                    LOG.warning(_LW(\"Could not attach SRIOV devices to VM: \""},{"line_number":1780,"context_line":"                                    \"%(instance)s due to: %(error)s. \""},{"line_number":1781,"context_line":"                                    \"Ignoring\"),"},{"line_number":1782,"context_line":"                                {\u0027instance\u0027: instance, \"error\": ex})"},{"line_number":1783,"context_line":"                    pass"},{"line_number":1784,"context_line":""},{"line_number":1785,"context_line":"    def _can_set_admin_password(self, image_meta):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_19daa778","line":1782,"updated":"2016-04-18 21:04:33.000000000","message":"ditto on the kwarg usage for instance.","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"459bcb069436466c5ebb2790eb6a35581d07186f","unresolved":false,"context_lines":[{"line_number":1780,"context_line":"                                    \"%(instance)s due to: %(error)s. \""},{"line_number":1781,"context_line":"                                    \"Ignoring\"),"},{"line_number":1782,"context_line":"                                {\u0027instance\u0027: instance, \"error\": ex})"},{"line_number":1783,"context_line":"                    pass"},{"line_number":1784,"context_line":""},{"line_number":1785,"context_line":"    def _can_set_admin_password(self, image_meta):"},{"line_number":1786,"context_line":"        if (CONF.libvirt.virt_type not in (\u0027kvm\u0027, \u0027qemu\u0027) or"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa0719c6_3654adfd","line":1783,"updated":"2016-03-23 10:57:48.000000000","message":"nit: ditto","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"ed5f5720bef8c234c6571327992340ad7d36b004","unresolved":false,"context_lines":[{"line_number":3385,"context_line":"        except libvirt.libvirtError:"},{"line_number":3386,"context_line":"            LOG.error(_LE(\u0027Attaching PCI devices %(dev)s to %(dom)s failed.\u0027),"},{"line_number":3387,"context_line":"                      {\u0027dev\u0027: pci_devs, \u0027dom\u0027: guest.id})"},{"line_number":3388,"context_line":"            raise"},{"line_number":3389,"context_line":""},{"line_number":3390,"context_line":"    @staticmethod"},{"line_number":3391,"context_line":"    def _has_sriov_port(network_info):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_4578a88f","line":3388,"range":{"start_line":3388,"start_character":8,"end_line":3388,"end_character":17},"updated":"2016-04-19 07:15:16.000000000","message":"instead of doing try except in snapshot you can just delete the raise here and this will handle snapshot and suspend resume. So here you can pass-through PF or VF but standard PCI so the validation needs to be more careful","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d4f4bab7db137f94d1fa721bbd4efe8fa39f9018","unresolved":false,"context_lines":[{"line_number":3385,"context_line":"        except libvirt.libvirtError:"},{"line_number":3386,"context_line":"            LOG.error(_LE(\u0027Attaching PCI devices %(dev)s to %(dom)s failed.\u0027),"},{"line_number":3387,"context_line":"                      {\u0027dev\u0027: pci_devs, \u0027dom\u0027: guest.id})"},{"line_number":3388,"context_line":"            raise"},{"line_number":3389,"context_line":""},{"line_number":3390,"context_line":"    @staticmethod"},{"line_number":3391,"context_line":"    def _has_sriov_port(network_info):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1aa78d24_dfeb79da","line":3388,"range":{"start_line":3388,"start_character":8,"end_line":3388,"end_character":17},"in_reply_to":"1a122d0e_4578a88f","updated":"2016-07-07 09:31:37.000000000","message":"Done","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d14534d6727eccf836f4036d588034084f34f2bb","unresolved":false,"context_lines":[{"line_number":3385,"context_line":"        except libvirt.libvirtError:"},{"line_number":3386,"context_line":"            LOG.error(_LE(\u0027Attaching PCI devices %(dev)s to %(dom)s failed.\u0027),"},{"line_number":3387,"context_line":"                      {\u0027dev\u0027: pci_devs, \u0027dom\u0027: guest.id})"},{"line_number":3388,"context_line":"            raise"},{"line_number":3389,"context_line":""},{"line_number":3390,"context_line":"    @staticmethod"},{"line_number":3391,"context_line":"    def _has_sriov_port(network_info):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_a43b7537","line":3388,"range":{"start_line":3388,"start_character":8,"end_line":3388,"end_character":17},"in_reply_to":"1a122d0e_4578a88f","updated":"2016-04-19 10:49:56.000000000","message":"Removing the raise also requires the for loop to be outside of the try-except isn\u0027t it? (or else it\u0027ll just stop and won\u0027t best-effort over all of the devices)","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"ed5f5720bef8c234c6571327992340ad7d36b004","unresolved":false,"context_lines":[{"line_number":3412,"context_line":"                    LOG.debug(\u0027Attaching SR-IOV port %(port)s to %(dom)s\u0027,"},{"line_number":3413,"context_line":"                              {\u0027port\u0027: vif, \u0027dom\u0027: guest.id},"},{"line_number":3414,"context_line":"                              instance\u003dinstance)"},{"line_number":3415,"context_line":"                    guest.attach_device(cfg)"},{"line_number":3416,"context_line":""},{"line_number":3417,"context_line":"    def _detach_sriov_ports(self, context, instance, guest):"},{"line_number":3418,"context_line":"        network_info \u003d instance.info_cache.network_info"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_85a920e2","line":3415,"range":{"start_line":3415,"start_character":20,"end_line":3415,"end_character":44},"updated":"2016-04-19 07:15:16.000000000","message":"you can add try catch here as we have in the _attach_pci_devices and here you can add the validation that the PF is up. Please note the pci can be either PF or VF \nonce this will be merged https://review.openstack.org/#/c/300890/","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"d14534d6727eccf836f4036d588034084f34f2bb","unresolved":false,"context_lines":[{"line_number":3412,"context_line":"                    LOG.debug(\u0027Attaching SR-IOV port %(port)s to %(dom)s\u0027,"},{"line_number":3413,"context_line":"                              {\u0027port\u0027: vif, \u0027dom\u0027: guest.id},"},{"line_number":3414,"context_line":"                              instance\u003dinstance)"},{"line_number":3415,"context_line":"                    guest.attach_device(cfg)"},{"line_number":3416,"context_line":""},{"line_number":3417,"context_line":"    def _detach_sriov_ports(self, context, instance, guest):"},{"line_number":3418,"context_line":"        network_info \u003d instance.info_cache.network_info"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a122d0e_e4f8cd4b","line":3415,"range":{"start_line":3415,"start_character":20,"end_line":3415,"end_character":44},"in_reply_to":"1a122d0e_85a920e2","updated":"2016-04-19 10:49:56.000000000","message":"This method will be called post-snapshot so if by \"add the validation that the PF is up\" you\u0027re referring to mriedem\u0027s comments i think that this is too late in the code (his comment was about failing fast - before the snapshot - in case that the port couldn\u0027t be re-attached post-snapshot). Failing fast should be a pre-detach code.\nSo if i got your comment correctly basically there\u0027s no need here to check if the PF is up or not. Just try to attach and swallow the exception if raised. Am i missing something?","commit_id":"280c7475fedfc1da36f9f91c7af76028e288a1b4"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"9141f82e2ef25709bf8d0f77ae4b1a69bcb61f06","unresolved":false,"context_lines":[{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)"},{"line_number":2521,"context_line":"        sriov_devs \u003d self._get_sriov_devs(context, instance)"},{"line_number":2522,"context_line":"        pci_devs \u003d set(["},{"line_number":2523,"context_line":"            sriov_dev.extra_info[\u0027phys_function\u0027]"},{"line_number":2524,"context_line":"            for sriov_dev in sriov_devs"},{"line_number":2525,"context_line":"        ])"},{"line_number":2526,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_7e4e4bc2","line":2523,"range":{"start_line":2523,"start_character":12,"end_line":2523,"end_character":49},"updated":"2016-07-10 09:37:13.000000000","message":"this will throw keyError exception if pci device is PF.","commit_id":"63f250c5938251602c67ab8e255b21a10e47c75b"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"389cb17bc147a17e7259f6c06c96937d1aa0cac0","unresolved":false,"context_lines":[{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)"},{"line_number":2521,"context_line":"        sriov_devs \u003d self._get_sriov_devs(context, instance)"},{"line_number":2522,"context_line":"        pci_devs \u003d set(["},{"line_number":2523,"context_line":"            sriov_dev.extra_info[\u0027phys_function\u0027]"},{"line_number":2524,"context_line":"            for sriov_dev in sriov_devs"},{"line_number":2525,"context_line":"        ])"},{"line_number":2526,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_6f905796","line":2523,"range":{"start_line":2523,"start_character":12,"end_line":2523,"end_character":49},"in_reply_to":"1aa78d24_7e4e4bc2","updated":"2016-07-10 14:52:41.000000000","message":"Done","commit_id":"63f250c5938251602c67ab8e255b21a10e47c75b"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"17f770f2a65ce146714c3edbc251eb54833440e8","unresolved":false,"context_lines":[{"line_number":3310,"context_line":"                                     \u0027to %(instance)s failed due to \u0027"},{"line_number":3311,"context_line":"                                     \u0027%(error)s.\u0027),"},{"line_number":3312,"context_line":"                                 {\u0027cfg\u0027: cfg,"},{"line_number":3313,"context_line":"                                  \u0027instance\u0027: instance,"},{"line_number":3314,"context_line":"                                  \u0027error\u0027: e})"},{"line_number":3315,"context_line":""},{"line_number":3316,"context_line":"    def _get_sriov_devs(self, context, instance):"}],"source_content_type":"text/x-python","patch_set":9,"id":"1aa78d24_5fe949b6","line":3313,"range":{"start_line":3313,"start_character":34,"end_line":3313,"end_character":54},"updated":"2016-07-07 09:34:54.000000000","message":"next patch-set will address the instance\u003dinstance kwarg in LOG.warning so we get the syntax highlighting with oslo.log","commit_id":"63f250c5938251602c67ab8e255b21a10e47c75b"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"3f7c53b17a3d6fbb5023b77e7454f5eeda581709","unresolved":false,"context_lines":[{"line_number":2516,"context_line":"        \"\"\"Suspend the specified instance.\"\"\""},{"line_number":2517,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"        # Before detaching the PCI devices/sriov ports, check that they could"},{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)"},{"line_number":2521,"context_line":"        sriov_devs \u003d self._get_sriov_devs(context, instance)"},{"line_number":2522,"context_line":"        pci_devs \u003d set(["},{"line_number":2523,"context_line":"            sriov_dev.extra_info[\u0027phys_function\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_da4267aa","line":2520,"range":{"start_line":2519,"start_character":8,"end_line":2520,"end_character":71},"updated":"2016-07-13 04:48:46.000000000","message":"can you update the comment on why we are doing it only for VF?","commit_id":"454886d6bca372e6617c4c36911665e26e4dd2a4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"14536217e02b823ad22599fb1a14935bd573c7dc","unresolved":false,"context_lines":[{"line_number":2516,"context_line":"        \"\"\"Suspend the specified instance.\"\"\""},{"line_number":2517,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"        # Before detaching the PCI devices/sriov ports, check that they could"},{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)"},{"line_number":2521,"context_line":"        sriov_devs \u003d self._get_sriov_devs(context, instance)"},{"line_number":2522,"context_line":"        pci_devs \u003d set(["},{"line_number":2523,"context_line":"            sriov_dev.extra_info[\u0027phys_function\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_8e8424d3","line":2520,"range":{"start_line":2519,"start_character":8,"end_line":2520,"end_character":71},"in_reply_to":"1aa78d24_0e8af463","updated":"2016-07-13 05:24:48.000000000","message":"Done","commit_id":"454886d6bca372e6617c4c36911665e26e4dd2a4"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"0daf77c571334e28b4c7605df94365df3ae60f9c","unresolved":false,"context_lines":[{"line_number":2516,"context_line":"        \"\"\"Suspend the specified instance.\"\"\""},{"line_number":2517,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"        # Before detaching the PCI devices/sriov ports, check that they could"},{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)"},{"line_number":2521,"context_line":"        sriov_devs \u003d self._get_sriov_devs(context, instance)"},{"line_number":2522,"context_line":"        pci_devs \u003d set(["},{"line_number":2523,"context_line":"            sriov_dev.extra_info[\u0027phys_function\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"1aa78d24_0e8af463","line":2520,"range":{"start_line":2519,"start_character":8,"end_line":2520,"end_character":71},"in_reply_to":"1aa78d24_da4267aa","updated":"2016-07-13 05:02:54.000000000","message":"sure","commit_id":"454886d6bca372e6617c4c36911665e26e4dd2a4"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"62edd1ebb64d32964773668d358a04c3b16c7f4f","unresolved":false,"context_lines":[{"line_number":2517,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"        # Before detaching sriov ports, check that they could"},{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)."},{"line_number":2521,"context_line":"        # The reason for which this is not done with PCI-PT devices is that"},{"line_number":2522,"context_line":"        # they are not visible to the host (ip link show will not show them"},{"line_number":2523,"context_line":"        # hence there\u0027s no way to check their availability)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fad55976_0a6d9004","line":2520,"updated":"2016-07-15 17:54:26.000000000","message":"\"re-attached at the end\" do you mean re-attached when the instance will resume ?","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"9f1e5ef33616d0ff97f3d6c4564bca98c70920b9","unresolved":false,"context_lines":[{"line_number":2517,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"        # Before detaching sriov ports, check that they could"},{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)."},{"line_number":2521,"context_line":"        # The reason for which this is not done with PCI-PT devices is that"},{"line_number":2522,"context_line":"        # they are not visible to the host (ip link show will not show them"},{"line_number":2523,"context_line":"        # hence there\u0027s no way to check their availability)"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_4afd6639","line":2520,"in_reply_to":"dada55a8_0e08c290","updated":"2016-07-25 18:18:50.000000000","message":"Ok so maybe just change the comment, maybe something like \"check that they could be re-attached on resume\"","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99785205c94ac449d2988f0efd036cb1dceb583f","unresolved":false,"context_lines":[{"line_number":2517,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"        # Before detaching sriov ports, check that they could"},{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)."},{"line_number":2521,"context_line":"        # The reason for which this is not done with PCI-PT devices is that"},{"line_number":2522,"context_line":"        # they are not visible to the host (ip link show will not show them"},{"line_number":2523,"context_line":"        # hence there\u0027s no way to check their availability)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ac06dbe_dd9d0b5b","line":2520,"in_reply_to":"dada55a8_4afd6639","updated":"2016-09-02 16:35:51.000000000","message":"+1","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"28abe61a99ddf7daee64f39768b84d04c5892b9b","unresolved":false,"context_lines":[{"line_number":2517,"context_line":"        guest \u003d self._host.get_guest(instance)"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"        # Before detaching sriov ports, check that they could"},{"line_number":2520,"context_line":"        # be re-attached at the end (\u003d\u003dthe physical resource is online)."},{"line_number":2521,"context_line":"        # The reason for which this is not done with PCI-PT devices is that"},{"line_number":2522,"context_line":"        # they are not visible to the host (ip link show will not show them"},{"line_number":2523,"context_line":"        # hence there\u0027s no way to check their availability)"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_0e08c290","line":2520,"in_reply_to":"fad55976_0a6d9004","updated":"2016-07-19 09:04:10.000000000","message":"exactly","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"62edd1ebb64d32964773668d358a04c3b16c7f4f","unresolved":false,"context_lines":[{"line_number":2528,"context_line":"            if sriov_dev.dev_type \u003d\u003d fields.PciDeviceType.SRIOV_VF"},{"line_number":2529,"context_line":"        ])"},{"line_number":2530,"context_line":""},{"line_number":2531,"context_line":"        for pci_dev in pci_devs:"},{"line_number":2532,"context_line":"            if not linux_net.is_pf_online(pci_dev):"},{"line_number":2533,"context_line":"                msg \u003d (_(\u0027Could not suspend VM: %(vm)s due to an \u0027"},{"line_number":2534,"context_line":"                         \u0027error in a physical device (address: \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"fad55976_2a517463","line":2531,"updated":"2016-07-15 17:54:26.000000000","message":"Do we want to do this check only if we are executing suspend in the scope of a snapshot ?  Or we want this behaviour in all cases ?","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"28abe61a99ddf7daee64f39768b84d04c5892b9b","unresolved":false,"context_lines":[{"line_number":2528,"context_line":"            if sriov_dev.dev_type \u003d\u003d fields.PciDeviceType.SRIOV_VF"},{"line_number":2529,"context_line":"        ])"},{"line_number":2530,"context_line":""},{"line_number":2531,"context_line":"        for pci_dev in pci_devs:"},{"line_number":2532,"context_line":"            if not linux_net.is_pf_online(pci_dev):"},{"line_number":2533,"context_line":"                msg \u003d (_(\u0027Could not suspend VM: %(vm)s due to an \u0027"},{"line_number":2534,"context_line":"                         \u0027error in a physical device (address: \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_4e875ad7","line":2531,"in_reply_to":"fad55976_2a517463","updated":"2016-07-19 09:04:10.000000000","message":"The relevant flow is any time that an automatic process detaches the devices and then tries to attach them again. Suspend qualifies as such flow. Did you have something else in mind? Migration maybe?","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"99785205c94ac449d2988f0efd036cb1dceb583f","unresolved":false,"context_lines":[{"line_number":2530,"context_line":""},{"line_number":2531,"context_line":"        for pci_dev in pci_devs:"},{"line_number":2532,"context_line":"            if not linux_net.is_pf_online(pci_dev):"},{"line_number":2533,"context_line":"                msg \u003d (_(\u0027Could not suspend VM: %(vm)s due to an \u0027"},{"line_number":2534,"context_line":"                         \u0027error in a physical device (address: \u0027"},{"line_number":2535,"context_line":"                         \u0027%(addr)s) which will modify the VM after \u0027"},{"line_number":2536,"context_line":"                         \u0027the VM resumes\u0027)"},{"line_number":2537,"context_line":"                       % {\u0027vm\u0027: instance, \u0027addr\u0027: pci_dev})"},{"line_number":2538,"context_line":"                raise exception.NovaException(msg)"},{"line_number":2539,"context_line":""},{"line_number":2540,"context_line":"        self._detach_pci_devices("},{"line_number":2541,"context_line":"            guest,"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ac06dbe_7ddb371c","line":2538,"range":{"start_line":2533,"start_character":0,"end_line":2538,"end_character":50},"updated":"2016-09-02 16:35:51.000000000","message":"Any idea what the requirements for creating a new exception are?","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"62edd1ebb64d32964773668d358a04c3b16c7f4f","unresolved":false,"context_lines":[{"line_number":3271,"context_line":""},{"line_number":3272,"context_line":"        # for loop is outside since we now don\u0027t raise an exception"},{"line_number":3273,"context_line":"        # so we try to attach all of the pci devices and continue"},{"line_number":3274,"context_line":"        # even if one/more failed"},{"line_number":3275,"context_line":"        for dev in pci_devs:"},{"line_number":3276,"context_line":"            try:"},{"line_number":3277,"context_line":"                LOG.debug(\u0027Attaching PCI device %(dev)s to %(dom)s\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"fad55976_6568bdbb","line":3274,"updated":"2016-07-15 17:54:26.000000000","message":"I\u0027m not sure about this.  We want to raise an exception if one or more PCI devices have failed to attached on resume.  It\u0027s kind of a breach of contract, we can\u0027t resume an instance that is missing PCI devices.  The instance is expecting all the PCI devices to be online.  Else it should go in error.","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"9f1e5ef33616d0ff97f3d6c4564bca98c70920b9","unresolved":false,"context_lines":[{"line_number":3271,"context_line":""},{"line_number":3272,"context_line":"        # for loop is outside since we now don\u0027t raise an exception"},{"line_number":3273,"context_line":"        # so we try to attach all of the pci devices and continue"},{"line_number":3274,"context_line":"        # even if one/more failed"},{"line_number":3275,"context_line":"        for dev in pci_devs:"},{"line_number":3276,"context_line":"            try:"},{"line_number":3277,"context_line":"                LOG.debug(\u0027Attaching PCI device %(dev)s to %(dom)s\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_804a9f81","line":3274,"in_reply_to":"dada55a8_c947c449","updated":"2016-07-25 18:18:50.000000000","message":"Sounds fair.  I\u0027m just worried that the guest expects all the PCI devices to be present on resume but there could be cases where they are not all there.\n\nI won\u0027t block your patch and let cores comment on it.","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"},{"author":{"_account_id":15427,"name":"David Edery","email":"david.edery@gmail.com","username":"dedery"},"change_message_id":"28abe61a99ddf7daee64f39768b84d04c5892b9b","unresolved":false,"context_lines":[{"line_number":3271,"context_line":""},{"line_number":3272,"context_line":"        # for loop is outside since we now don\u0027t raise an exception"},{"line_number":3273,"context_line":"        # so we try to attach all of the pci devices and continue"},{"line_number":3274,"context_line":"        # even if one/more failed"},{"line_number":3275,"context_line":"        for dev in pci_devs:"},{"line_number":3276,"context_line":"            try:"},{"line_number":3277,"context_line":"                LOG.debug(\u0027Attaching PCI device %(dev)s to %(dom)s\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"dada55a8_c947c449","line":3274,"in_reply_to":"fad55976_6568bdbb","updated":"2016-07-19 09:04:10.000000000","message":"So the current state of things (before my patch) is different than the one you describe. During resume, after the snapshot, while trying to attach the device - and failing, the raised exception fails the snapshot operation (and it\u0027s deleted) but not the resume of the VM. \nBasically, implementing what you\u0027ve described in the comment means that potentially, the behavior is:\nVM works great -\u003e PF goes offline for some reason -\u003e VM loses one/more ports (they go down) but VM is alive -\u003e user does snapshot -\u003e snapshot created -\u003e VM doesn\u0027t resume due to failure to re-attach the PCI devices.\n\nSo the VM went from a sub-optimal state pre-snapshot to failed state post-snapshot.\n\nI\u0027m not sure that this is the desired behavior. At least not from my conversations with the cores (mriedem \u0026 ndipanov).","commit_id":"9a025510da109018394c8a9c8e75e2c4e5339893"}]}