)]}'
{"nova/network/linux_net.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5c5ce7acec6da6b90e490aa04faf460ce5c32733","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                raise ValueError(_(\u0027tunctl does not support creation \u0027"},{"line_number":1325,"context_line":"                                   \u0027of multi-queue tap devices\u0027))"},{"line_number":1326,"context_line":"            # Second option: tunctl"},{"line_number":1327,"context_line":"            utils.execute(\u0027tunctl\u0027, \u0027-b\u0027, \u0027-t\u0027, dev, run_as_root\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_a2fbdb45","line":1324,"updated":"2016-11-03 17:15:29.000000000","message":"What is going to catch that exception ? The compute only catch exception like VirtualInterfacePlugException. We should probably use it here.","commit_id":"6180dfd9c2f3f7de94ca51b85bae45d1fc55485e"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"8a7de38380755017184823fbb92cd36c38fa3f57","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                raise ValueError(_(\u0027tunctl does not support creation \u0027"},{"line_number":1325,"context_line":"                                   \u0027of multi-queue tap devices\u0027))"},{"line_number":1326,"context_line":"            # Second option: tunctl"},{"line_number":1327,"context_line":"            utils.execute(\u0027tunctl\u0027, \u0027-b\u0027, \u0027-t\u0027, dev, run_as_root\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_cd02a909","line":1324,"in_reply_to":"ba5da102_a2fbdb45","updated":"2016-11-04 10:54:48.000000000","message":"Ah, good point: I originally thought VIF plugging error must be fatal, but looks like it\u0027s always the case, depending on the VIF type.\n\nBased on http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/vif.py callers except processutils.ProcessExecutionError, so I probably should just re-raise it here.","commit_id":"6180dfd9c2f3f7de94ca51b85bae45d1fc55485e"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"7a2fda57f5f52017d3c216b85637b311d17eae82","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.error(_LE(\u0027tunctl does not support creation \u0027"},{"line_number":1325,"context_line":"                              \u0027of multi-queue tap devices\u0027))"},{"line_number":1326,"context_line":"                raise"},{"line_number":1327,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_b503f7bd","line":1324,"updated":"2016-11-04 11:24:14.000000000","message":"I would say it\u0027s \u0027ip tuntap\u0027 which does not support creation of muti-queued tap devices.","commit_id":"0bba1f4cc0781cbe7b7996a8c12255af9b6bfdbc"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"11a40adc5d5023a8491b89c10037da13ae56e643","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.error(_LE(\u0027tunctl does not support creation \u0027"},{"line_number":1325,"context_line":"                              \u0027of multi-queue tap devices\u0027))"},{"line_number":1326,"context_line":"                raise"},{"line_number":1327,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_e99e7ab7","line":1324,"in_reply_to":"ba5da102_b503f7bd","updated":"2016-11-04 13:26:21.000000000","message":"It may have failed for another reason :( I guess what I intended to do here is to signal why we won\u0027t give tunctl a try. Maybe the following will be more clear: log a debug message instead, fallback to tunctl is an implementation detail anyway.","commit_id":"0bba1f4cc0781cbe7b7996a8c12255af9b6bfdbc"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"7a2fda57f5f52017d3c216b85637b311d17eae82","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"                LOG.error(_LE(\u0027tunctl does not support creation \u0027"},{"line_number":1325,"context_line":"                              \u0027of multi-queue tap devices\u0027))"},{"line_number":1326,"context_line":"                raise"},{"line_number":1327,"context_line":""},{"line_number":1328,"context_line":"            # Second option: tunctl"},{"line_number":1329,"context_line":"            utils.execute(\u0027tunctl\u0027, \u0027-b\u0027, \u0027-t\u0027, dev, run_as_root\u003dTrue)"},{"line_number":1330,"context_line":"        if mac_address:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_95c43360","line":1327,"updated":"2016-11-04 11:24:14.000000000","message":"Looking at that, we should log an info to indicate that \u0027ip tuntap\u0027 failed and so we are going to try with \u0027tunctl\u0027. (but probably in a second patch)","commit_id":"0bba1f4cc0781cbe7b7996a8c12255af9b6bfdbc"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"4453b8102df83c7dbaf66f109dcaf79598831d4f","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"                LOG.error(_LE(\u0027tunctl does not support creation \u0027"},{"line_number":1325,"context_line":"                              \u0027of multi-queue tap devices\u0027))"},{"line_number":1326,"context_line":"                raise"},{"line_number":1327,"context_line":""},{"line_number":1328,"context_line":"            # Second option: tunctl"},{"line_number":1329,"context_line":"            utils.execute(\u0027tunctl\u0027, \u0027-b\u0027, \u0027-t\u0027, dev, run_as_root\u003dTrue)"},{"line_number":1330,"context_line":"        if mac_address:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_64644fe5","line":1327,"in_reply_to":"ba5da102_898fbe74","updated":"2016-11-04 13:30:07.000000000","message":"Well we are going to see the two commands that are executed in the log but we don\u0027t know that we execute the second one because the first one is failling. That why i wanted that log, but it\u0027s not something really related to your patch","commit_id":"0bba1f4cc0781cbe7b7996a8c12255af9b6bfdbc"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"11a40adc5d5023a8491b89c10037da13ae56e643","unresolved":false,"context_lines":[{"line_number":1324,"context_line":"                LOG.error(_LE(\u0027tunctl does not support creation \u0027"},{"line_number":1325,"context_line":"                              \u0027of multi-queue tap devices\u0027))"},{"line_number":1326,"context_line":"                raise"},{"line_number":1327,"context_line":""},{"line_number":1328,"context_line":"            # Second option: tunctl"},{"line_number":1329,"context_line":"            utils.execute(\u0027tunctl\u0027, \u0027-b\u0027, \u0027-t\u0027, dev, run_as_root\u003dTrue)"},{"line_number":1330,"context_line":"        if mac_address:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_898fbe74","line":1327,"in_reply_to":"ba5da102_95c43360","updated":"2016-11-04 13:26:21.000000000","message":"We could do that as a debug log, I think, but we already log the commands that are executed. So I\u0027m not sure that will add more value.","commit_id":"0bba1f4cc0781cbe7b7996a8c12255af9b6bfdbc"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"e001d0559af12d35bee1a040899b9f0a844287ce","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.debug(\u0027Failed to create a tap device with ip tuntap. \u0027"},{"line_number":1325,"context_line":"                          \u0027tunctl does not support creation of multi-queue \u0027"},{"line_number":1326,"context_line":"                          \u0027enabled devices, skipping fallback.\u0027)"},{"line_number":1327,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_ee3b95e9","line":1324,"range":{"start_line":1324,"start_character":16,"end_line":1324,"end_character":25},"updated":"2016-11-24 11:30:37.000000000","message":"I\u0027d prefer to see LOG.warning here","commit_id":"fbab48c6b7d4476a7c5932de6022aeb878c7503c"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8ef438b15f492f7dd846c5728bda2ee1a9f9d0aa","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.debug(\u0027Failed to create a tap device with ip tuntap. \u0027"},{"line_number":1325,"context_line":"                          \u0027tunctl does not support creation of multi-queue \u0027"},{"line_number":1326,"context_line":"                          \u0027enabled devices, skipping fallback.\u0027)"},{"line_number":1327,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_99e95be9","line":1324,"in_reply_to":"5a74a57a_7aff3faa","updated":"2016-11-25 15:04:28.000000000","message":"Hum an error is going to be written in the log anyway, not sure if that really make sense to see a warning: \"something went wrong\" and then an error \"that similar thing went wrong\".\n\nI would say debug was enough and now I think we should just remove that log.\n\nLet\u0027s see what other contributors think about that.","commit_id":"fbab48c6b7d4476a7c5932de6022aeb878c7503c"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"064458bc93a4690c86fa5df52a2a30b416ace142","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.debug(\u0027Failed to create a tap device with ip tuntap. \u0027"},{"line_number":1325,"context_line":"                          \u0027tunctl does not support creation of multi-queue \u0027"},{"line_number":1326,"context_line":"                          \u0027enabled devices, skipping fallback.\u0027)"},{"line_number":1327,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a74a57a_7aff3faa","line":1324,"range":{"start_line":1324,"start_character":16,"end_line":1324,"end_character":25},"in_reply_to":"5a74a57a_ee3b95e9","updated":"2016-11-24 12:36:00.000000000","message":"I guess I don\u0027t have a strong preference here, although, a fallback is an implementation detail and failure here is fatal anyway - we re-raise the exception.\n\nWe had a short discussion in the 2nd patchset - https://review.openstack.org/#/c/392185/2/nova/network/linux_net.py","commit_id":"fbab48c6b7d4476a7c5932de6022aeb878c7503c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"04e2def3e979732f0d420bcd352efee91304cecc","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.warning("},{"line_number":1325,"context_line":"                    _LW(\u0027Failed to create a tap device with ip tuntap. \u0027"},{"line_number":1326,"context_line":"                        \u0027tunctl does not support creation of multi-queue \u0027"},{"line_number":1327,"context_line":"                        \u0027enabled devices, skipping fallback.\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_7284835d","line":1324,"updated":"2016-11-26 15:20:05.000000000","message":"I tend to agree with sahid in PS4 that the warning here isn\u0027t really necessary as the exception is handled here:\n\nhttps://review.openstack.org/#/c/393416/5/nova/virt/libvirt/vif.py@782\n\nAnd that\u0027s logging a traceback and ERROR. I\u0027m fine with that being an error because to get into that state the image meta used to boot the instance tells us that we can support multiqueue, but the host isn\u0027t setup to support it, so it\u0027s an operations problem.\n\nHaving said that, if I don\u0027t have debug logging enabled and this fails, I get the ERROR message but no details on why it failed, so maybe having warning here is fine for additional details on the error. So I\u0027m fine with leaving this in here.","commit_id":"41d601851dc01c5ed1058ffedd23e7856200b628"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"ea2cb337949fd5e270ce69f23961d55146f85705","unresolved":false,"context_lines":[{"line_number":1321,"context_line":"            utils.execute(*cmd, run_as_root\u003dTrue, check_exit_code\u003d[0, 2, 254])"},{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.warning("},{"line_number":1325,"context_line":"                    _LW(\u0027Failed to create a tap device with ip tuntap. \u0027"},{"line_number":1326,"context_line":"                        \u0027tunctl does not support creation of multi-queue \u0027"},{"line_number":1327,"context_line":"                        \u0027enabled devices, skipping fallback.\u0027))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_26d0fb1e","line":1324,"in_reply_to":"5a74a57a_7284835d","updated":"2016-11-28 08:30:58.000000000","message":"\u003e\u003e\u003e Having said that, if I don\u0027t have debug logging enabled and this fails, I get the ERROR message but no details on why it failed, so maybe having warning here is fine for additional details on the error. So I\u0027m fine with leaving this in here.\n\nYeah, that\u0027s exactly how Timofei made me change it from debug() to warning() :)","commit_id":"41d601851dc01c5ed1058ffedd23e7856200b628"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"04e2def3e979732f0d420bcd352efee91304cecc","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.warning("},{"line_number":1325,"context_line":"                    _LW(\u0027Failed to create a tap device with ip tuntap. \u0027"},{"line_number":1326,"context_line":"                        \u0027tunctl does not support creation of multi-queue \u0027"},{"line_number":1327,"context_line":"                        \u0027enabled devices, skipping fallback.\u0027))"},{"line_number":1328,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_32b84b2a","line":1325,"range":{"start_line":1325,"start_character":44,"end_line":1325,"end_character":47},"updated":"2016-11-26 15:20:05.000000000","message":"nit: s/tap/multi-queue tap/ for context. I know you point it out later so not a huge issue.","commit_id":"41d601851dc01c5ed1058ffedd23e7856200b628"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"ea2cb337949fd5e270ce69f23961d55146f85705","unresolved":false,"context_lines":[{"line_number":1322,"context_line":"        except processutils.ProcessExecutionError:"},{"line_number":1323,"context_line":"            if multiqueue:"},{"line_number":1324,"context_line":"                LOG.warning("},{"line_number":1325,"context_line":"                    _LW(\u0027Failed to create a tap device with ip tuntap. \u0027"},{"line_number":1326,"context_line":"                        \u0027tunctl does not support creation of multi-queue \u0027"},{"line_number":1327,"context_line":"                        \u0027enabled devices, skipping fallback.\u0027))"},{"line_number":1328,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a74a57a_e6a8c38e","line":1325,"range":{"start_line":1325,"start_character":44,"end_line":1325,"end_character":47},"in_reply_to":"5a74a57a_32b84b2a","updated":"2016-11-28 08:30:58.000000000","message":"Ok, will fix this one, if there are more comments from reviewers.","commit_id":"41d601851dc01c5ed1058ffedd23e7856200b628"}],"nova/tests/unit/network/test_linux_net.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"84952eb1626b9847791867b74e75efde65f93f5f","unresolved":false,"context_lines":[{"line_number":1482,"context_line":""},{"line_number":1483,"context_line":"    @mock.patch(\u0027nova.utils.execute\u0027)"},{"line_number":1484,"context_line":"    def test_create_tap_dev_multiqueue_tunctl_raises(self, mock_execute):"},{"line_number":1485,"context_line":"        # ip failed, fall back to tunctl"},{"line_number":1486,"context_line":"        mock_execute.side_effect \u003d processutils.ProcessExecutionError"},{"line_number":1487,"context_line":"        # tunctl can\u0027t create multiqueue taps"},{"line_number":1488,"context_line":"        self.assertRaises(processutils.ProcessExecutionError,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_36aee485","line":1485,"range":{"start_line":1485,"start_character":8,"end_line":1485,"end_character":40},"updated":"2016-11-23 22:18:31.000000000","message":"This is wrong as you don\u0027t actually fallback. I realize it\u0027s a copy/paste error...","commit_id":"00eb04769a255af31d1001c3ada2efa7e3a57cd2"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"9475b545047291682b58a4f7628d2246c365f7b2","unresolved":false,"context_lines":[{"line_number":1482,"context_line":""},{"line_number":1483,"context_line":"    @mock.patch(\u0027nova.utils.execute\u0027)"},{"line_number":1484,"context_line":"    def test_create_tap_dev_multiqueue_tunctl_raises(self, mock_execute):"},{"line_number":1485,"context_line":"        # ip failed, fall back to tunctl"},{"line_number":1486,"context_line":"        mock_execute.side_effect \u003d processutils.ProcessExecutionError"},{"line_number":1487,"context_line":"        # tunctl can\u0027t create multiqueue taps"},{"line_number":1488,"context_line":"        self.assertRaises(processutils.ProcessExecutionError,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_0b19a488","line":1485,"range":{"start_line":1485,"start_character":8,"end_line":1485,"end_character":40},"in_reply_to":"5a74a57a_36aee485","updated":"2016-11-24 10:46:31.000000000","message":"Rephrased it in the new patch set.","commit_id":"00eb04769a255af31d1001c3ada2efa7e3a57cd2"}]}
