)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4f81ecfb54b4ccefb00d9b9b6788b1a50c9bd39e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sudipta Biswas \u003csbiswas7@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-18 17:22:21 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow discrete blocks of online pCPUs for pinning"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, the check for cpu pinning is a little broad and"},{"line_number":10,"context_line":"it doesn\u0027t allow pinning for discrete online pcpu sets if the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"dac4157f_c25738c9","line":7,"updated":"2015-04-19 01:03:16.000000000","message":"This message is confused. This patch just correct the number of vcpus?","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"2430fa98a18b3d749aa7475c588312934d87319d","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sudipta Biswas \u003csbiswas7@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-18 17:22:21 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow discrete blocks of online pCPUs for pinning"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, the check for cpu pinning is a little broad and"},{"line_number":10,"context_line":"it doesn\u0027t allow pinning for discrete online pcpu sets if the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"dac4157f_2d1705ef","line":7,"in_reply_to":"dac4157f_c25738c9","updated":"2015-04-19 05:09:14.000000000","message":"This patch will allow operators to specify discrete pcpus blocks. For example - if you have specified 8, 16, 24, 32 as the CPU pin set, they are not continuous. The total online pcpus on the system are therefore - 4. But if we stick with the older behavior - it would fail to allow such a configuration (32 \u003e 4 - hence raise an exception).\nI didn\u0027t find a better way to contain the explanation within 50 characters - any better suggestion?","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"600a18dbde8bbcd06b9966bef184b4d5da06ab2a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sudipta Biswas \u003csbiswas7@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-22 20:50:31 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow discrete blocks of online pCPUs for pinning"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, the check for cpu pinning is a little broad and"},{"line_number":10,"context_line":"it doesn\u0027t allow pinning for discrete online pcpu sets if the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"dac4157f_32b7e1f6","line":7,"updated":"2015-04-22 16:10:14.000000000","message":"Please prefix with libvirt:","commit_id":"2bb14343cfc0d6622155e58e445b3c9bb2790486"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"325a818e4e1c11d059c0280262d93115e5d2d895","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sudipta Biswas \u003csbiswas7@in.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-04-22 20:50:31 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow discrete blocks of online pCPUs for pinning"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, the check for cpu pinning is a little broad and"},{"line_number":10,"context_line":"it doesn\u0027t allow pinning for discrete online pcpu sets if the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"dac4157f_f5539369","line":7,"in_reply_to":"dac4157f_32b7e1f6","updated":"2015-04-22 16:38:00.000000000","message":"Done","commit_id":"2bb14343cfc0d6622155e58e445b3c9bb2790486"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"8f067b3ab7072e691bd6251582d174d051928ef4","unresolved":false,"context_lines":[{"line_number":4330,"context_line":"    def test_get_host_vcpus(self):"},{"line_number":4331,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4332,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4333,"context_line":"        with contextlib.nested("},{"line_number":4334,"context_line":"                mock.patch.object("},{"line_number":4335,"context_line":"                    hardware, \u0027get_vcpu_pin_set\u0027,"},{"line_number":4336,"context_line":"                    return_value\u003dset([4, 8])),"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_57589456","line":4333,"updated":"2015-04-15 22:03:07.000000000","message":"PEP8 should complain IMHO\n\nyou no longer need to use contextlib.nested, as you can have a context manager using more than one context method","commit_id":"172b44150765ed00f621862c26706d54d10b5d75"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"acfaee7395ae0465dee1c753a07371cd48ef392a","unresolved":false,"context_lines":[{"line_number":4330,"context_line":"    def test_get_host_vcpus(self):"},{"line_number":4331,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4332,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4333,"context_line":"        with contextlib.nested("},{"line_number":4334,"context_line":"                mock.patch.object("},{"line_number":4335,"context_line":"                    hardware, \u0027get_vcpu_pin_set\u0027,"},{"line_number":4336,"context_line":"                    return_value\u003dset([4, 8])),"}],"source_content_type":"text/x-python","patch_set":7,"id":"5aa745d1_1711d9ac","line":4333,"in_reply_to":"5aa745d1_57589456","updated":"2015-04-16 11:13:11.000000000","message":"Done","commit_id":"172b44150765ed00f621862c26706d54d10b5d75"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b06e559a5f7315fff246a9f63bcb7c964d3c8c25","unresolved":false,"context_lines":[{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4335,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5])"},{"line_number":4336,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4337,"context_line":"        drvr._get_vcpu_total()"},{"line_number":4338,"context_line":""},{"line_number":4339,"context_line":"    @mock.patch(\u0027nova.virt.hardware.get_vcpu_pin_set\u0027)"},{"line_number":4340,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5aa745d1_b018e285","line":4337,"updated":"2015-04-16 15:20:21.000000000","message":"you should assert at least the result from the method","commit_id":"0d06e888b95471eb6e1537ecf9efd30c13af3e47"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"69695aa48e2a98bdc1f1c21cc9446539a46af771","unresolved":false,"context_lines":[{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4335,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5])"},{"line_number":4336,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4337,"context_line":"        drvr._get_vcpu_total()"},{"line_number":4338,"context_line":""},{"line_number":4339,"context_line":"    @mock.patch(\u0027nova.virt.hardware.get_vcpu_pin_set\u0027)"},{"line_number":4340,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5aa745d1_6fe82be3","line":4337,"in_reply_to":"5aa745d1_b018e285","updated":"2015-04-16 19:13:02.000000000","message":"Done","commit_id":"0d06e888b95471eb6e1537ecf9efd30c13af3e47"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b06e559a5f7315fff246a9f63bcb7c964d3c8c25","unresolved":false,"context_lines":[{"line_number":4344,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4345,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5, 6])"},{"line_number":4346,"context_line":"        get_online_cpus.return_value \u003d set([4, 5])"},{"line_number":4347,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"},{"line_number":4348,"context_line":""},{"line_number":4349,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":4350,"context_line":"    def test_quiesce(self, mock_has_min_version):"}],"source_content_type":"text/x-python","patch_set":12,"id":"5aa745d1_f058aa58","line":4347,"updated":"2015-04-16 15:20:21.000000000","message":"IMHO, you should add a test for making sure it won\u0027t raise the libvirt exception if it\u0027s going there","commit_id":"0d06e888b95471eb6e1537ecf9efd30c13af3e47"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"69695aa48e2a98bdc1f1c21cc9446539a46af771","unresolved":false,"context_lines":[{"line_number":4344,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4345,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5, 6])"},{"line_number":4346,"context_line":"        get_online_cpus.return_value \u003d set([4, 5])"},{"line_number":4347,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"},{"line_number":4348,"context_line":""},{"line_number":4349,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":4350,"context_line":"    def test_quiesce(self, mock_has_min_version):"}],"source_content_type":"text/x-python","patch_set":12,"id":"5aa745d1_cfe29fd5","line":4347,"in_reply_to":"5aa745d1_f058aa58","updated":"2015-04-16 19:13:02.000000000","message":"Nah, this isn\u0027t raising the Libvirt error. Do you want me to add a test that would lead to a Libvirt error?","commit_id":"0d06e888b95471eb6e1537ecf9efd30c13af3e47"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e96e49744dfeaf8e005d4ceeab3ae3faa7bb70a6","unresolved":false,"context_lines":[{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4335,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5])"},{"line_number":4336,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4337,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_82f44587","line":4334,"updated":"2015-04-21 18:55:56.000000000","message":"Should this value be \"4-5\"?","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"9b608653ecea90cc5fb3190ae3ce31a90d18e7f4","unresolved":false,"context_lines":[{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4335,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5])"},{"line_number":4336,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4337,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_9f2c8903","line":4334,"in_reply_to":"dac4157f_7da31350","updated":"2015-04-22 08:46:26.000000000","message":"The host.get_online_cpus is operating off the data initialized in the fakelibvirt driver.\nIt appeared to me that the data is coming from the fake Connection class which is initlalizing the HostInfo class - that by default returns only 2 active CPUS -  online_cpus \u003d set([0, 1]) - Do you want me to override this behavior and ask this to return explicit online set ids? Any examples on how I could do that?","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14b18d6b1d43061693df7dd5c75fdb085635c7f5","unresolved":false,"context_lines":[{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4335,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5])"},{"line_number":4336,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4337,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_7da31350","line":4334,"in_reply_to":"dac4157f_82f44587","updated":"2015-04-21 20:04:07.000000000","message":"Actually, since this is just to pass the is None check in the driver for the config option, and we mock out host.get_online_cpus, I almost think it would be better to *not* mock out the call to host.get_online_cpus since we can actually parse the config value you\u0027re using which makes the test more robust - even though it\u0027s using something from another module not being directly tested here.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78dc5754ccd2335ca6b7c412819c50e52f31ed05","unresolved":false,"context_lines":[{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4335,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5])"},{"line_number":4336,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4337,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_e3eae20e","line":4334,"in_reply_to":"dac4157f_82f44587","updated":"2015-04-21 20:44:08.000000000","message":"We actually don\u0027t care about the value as the method is mocked, but we just need to make sure that it\u0027s not None.\n\nSo, we can just do :\n\n  CONF.set_override(\"vcpu_pin_set\", \"anything_else_but_None\")","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afa67099cf2ce297ac3a75bae8b9cd5e7fa86531","unresolved":false,"context_lines":[{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4335,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5])"},{"line_number":4336,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4337,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_51515861","line":4334,"in_reply_to":"dac4157f_e3eae20e","updated":"2015-04-21 21:52:22.000000000","message":"Yeah, I know, and that kind of grosses me out, why not just actually use the utility mehod in the hosts module rather than mock it out?  Normally I\u0027m against unit testing things outside the target module, but in this case I think it makes sense given how much the driver code depends on the results of the parsing in the hosts utility.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e96e49744dfeaf8e005d4ceeab3ae3faa7bb70a6","unresolved":false,"context_lines":[{"line_number":4342,"context_line":"    def test_get_host_vcpus_out_of_range(self, get_online_cpus,"},{"line_number":4343,"context_line":"                                         get_vcpu_pin_set):"},{"line_number":4344,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4345,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4346,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5, 6])"},{"line_number":4347,"context_line":"        get_online_cpus.return_value \u003d set([4, 5])"},{"line_number":4348,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_022f15f7","line":4345,"updated":"2015-04-21 18:55:56.000000000","message":"Shouldn\u0027t this be \"4-6\"?","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e96e49744dfeaf8e005d4ceeab3ae3faa7bb70a6","unresolved":false,"context_lines":[{"line_number":4367,"context_line":"            \u0027this function is not supported by the connection driver:\u0027"},{"line_number":4368,"context_line":"            \u0027 virNodeNumOfDevices\u0027,"},{"line_number":4369,"context_line":"            error_code\u003dfakelibvirt.VIR_ERR_NO_SUPPORT)"},{"line_number":4370,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4371,"context_line":"        get_vcpu_pin_set.return_value \u003d set([4, 5, 6])"},{"line_number":4372,"context_line":"        get_online_cpus.side_effect \u003d not_supported_exc"},{"line_number":4373,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_622131fb","line":4370,"updated":"2015-04-21 18:55:56.000000000","message":"\"4-6\"?","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e96e49744dfeaf8e005d4ceeab3ae3faa7bb70a6","unresolved":false,"context_lines":[{"line_number":4386,"context_line":"        get_vcpu_pin_set.return_value \u003d set([1])"},{"line_number":4387,"context_line":"        get_online_cpus.side_effect \u003d not_supported_exc"},{"line_number":4388,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"},{"line_number":4389,"context_line":"        self.assertEqual(vcpus, 1)"},{"line_number":4390,"context_line":""},{"line_number":4391,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":4392,"context_line":"    def test_quiesce(self, mock_has_min_version):"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_22efb988","line":4389,"updated":"2015-04-21 18:55:56.000000000","message":"args should be: assertEqual(expected, observed),\n\nso assertEqual(1, vcpus)\n\nThis is better when it fails, otherwise the mismatch error is confusing.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffb16e6339fc3cf09948618a5fecc69b19b66363","unresolved":false,"context_lines":[{"line_number":4386,"context_line":"        get_vcpu_pin_set.return_value \u003d set([1])"},{"line_number":4387,"context_line":"        get_online_cpus.side_effect \u003d not_supported_exc"},{"line_number":4388,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"},{"line_number":4389,"context_line":"        self.assertEqual(vcpus, 1)"},{"line_number":4390,"context_line":""},{"line_number":4391,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":4392,"context_line":"    def test_quiesce(self, mock_has_min_version):"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_9d7828c6","line":4389,"in_reply_to":"dac4157f_22efb988","updated":"2015-04-21 19:34:17.000000000","message":"Damn, I usually catch these.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"9b608653ecea90cc5fb3190ae3ce31a90d18e7f4","unresolved":false,"context_lines":[{"line_number":4386,"context_line":"        get_vcpu_pin_set.return_value \u003d set([1])"},{"line_number":4387,"context_line":"        get_online_cpus.side_effect \u003d not_supported_exc"},{"line_number":4388,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"},{"line_number":4389,"context_line":"        self.assertEqual(vcpus, 1)"},{"line_number":4390,"context_line":""},{"line_number":4391,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":4392,"context_line":"    def test_quiesce(self, mock_has_min_version):"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_f44f2cbb","line":4389,"in_reply_to":"dac4157f_22efb988","updated":"2015-04-22 08:46:26.000000000","message":"Done","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78dc5754ccd2335ca6b7c412819c50e52f31ed05","unresolved":false,"context_lines":[{"line_number":4386,"context_line":"        get_vcpu_pin_set.return_value \u003d set([1])"},{"line_number":4387,"context_line":"        get_online_cpus.side_effect \u003d not_supported_exc"},{"line_number":4388,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"},{"line_number":4389,"context_line":"        self.assertEqual(vcpus, 1)"},{"line_number":4390,"context_line":""},{"line_number":4391,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":4392,"context_line":"    def test_quiesce(self, mock_has_min_version):"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_bd2acb7c","line":4389,"in_reply_to":"dac4157f_9d7828c6","updated":"2015-04-21 20:44:08.000000000","message":"*facepalm*","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9dfedeb98159eb9704236a7102af1e069bb39866","unresolved":false,"context_lines":[{"line_number":4328,"context_line":"        mock_list.assert_called_with()"},{"line_number":4329,"context_line":""},{"line_number":4330,"context_line":"    @mock.patch(\u0027nova.virt.hardware.get_vcpu_pin_set\u0027)"},{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"4-5\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_326c86cf","line":4331,"updated":"2015-04-22 14:23:03.000000000","message":"As mentioned previously, I think we should actually not mock this out and let the utility code run through for us since the driver code is tightly coupled to how that utility works and returns the list of available pcpus.  It\u0027s not like we\u0027re hitting the database or something in that utility so mocking it is a bit overkill in my opinion and actually using it would make these tests more robust.","commit_id":"0fae2430dea33b8d151ef409a4190cc35114fc37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"03490206b5ebfd20fdc2c62655867ec2fd503b72","unresolved":false,"context_lines":[{"line_number":4328,"context_line":"        mock_list.assert_called_with()"},{"line_number":4329,"context_line":""},{"line_number":4330,"context_line":"    @mock.patch(\u0027nova.virt.hardware.get_vcpu_pin_set\u0027)"},{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"4-5\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_d2242ac6","line":4331,"in_reply_to":"dac4157f_326c86cf","updated":"2015-04-22 14:42:10.000000000","message":"Nevermind, it\u0027s not get_online_cpus that I care about (that defaults to 2 in the fakelibvirt.HostInfo class, we could override that or just mock it out, mocking is fine).\n\nI care more about hardware.get_vcpu_pin_set since that\u0027s the utility method that does the parsing of the config option and we care about the form of that given the test data.","commit_id":"0fae2430dea33b8d151ef409a4190cc35114fc37"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"77be970fddd851a573767576118fb0b7f7764d4d","unresolved":false,"context_lines":[{"line_number":4328,"context_line":"        mock_list.assert_called_with()"},{"line_number":4329,"context_line":""},{"line_number":4330,"context_line":"    @mock.patch(\u0027nova.virt.hardware.get_vcpu_pin_set\u0027)"},{"line_number":4331,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4332,"context_line":"    def test_get_host_vcpus(self, get_online_cpus, get_vcpu_pin_set):"},{"line_number":4333,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4334,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"4-5\")"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_1d5d1379","line":4331,"in_reply_to":"dac4157f_d2242ac6","updated":"2015-04-22 14:59:24.000000000","message":"Fair enough","commit_id":"0fae2430dea33b8d151ef409a4190cc35114fc37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"71977d045c42db072942d2cf0c87eca3e315967b","unresolved":false,"context_lines":[{"line_number":4344,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"},{"line_number":4345,"context_line":""},{"line_number":4346,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4347,"context_line":"    def test_get_host_vcpus_disjoint_range(self, get_online_cpus):"},{"line_number":4348,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4349,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"7-8\")"},{"line_number":4350,"context_line":"        get_online_cpus.return_value \u003d set([4, 5])"}],"source_content_type":"text/x-python","patch_set":19,"id":"dac4157f_1812c109","line":4347,"updated":"2015-04-22 15:15:01.000000000","message":"Isn\u0027t this basically the same as test_get_host_vcpus_out_of_range now?","commit_id":"27e91fa80b39071080cd5169e66159f71130ca40"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"d05845cd8585f2644f2d05ca737fe04d6f409208","unresolved":false,"context_lines":[{"line_number":4344,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"},{"line_number":4345,"context_line":""},{"line_number":4346,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4347,"context_line":"    def test_get_host_vcpus_disjoint_range(self, get_online_cpus):"},{"line_number":4348,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4349,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"7-8\")"},{"line_number":4350,"context_line":"        get_online_cpus.return_value \u003d set([4, 5])"}],"source_content_type":"text/x-python","patch_set":19,"id":"dac4157f_f856ed5a","line":4347,"in_reply_to":"dac4157f_1812c109","updated":"2015-04-22 15:21:33.000000000","message":"Yeah - made this change when I had to prove that \u003e\u003d is not the exact opposite of \u003c for sets :( Done now.","commit_id":"27e91fa80b39071080cd5169e66159f71130ca40"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"600a18dbde8bbcd06b9966bef184b4d5da06ab2a","unresolved":false,"context_lines":[{"line_number":4330,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4331,"context_line":"    def test_get_host_vcpus(self, get_online_cpus):"},{"line_number":4332,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4333,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"4-5\")"},{"line_number":4334,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4335,"context_line":"        expected_vcpus \u003d 2"},{"line_number":4336,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":20,"id":"dac4157f_12e05d28","line":4333,"range":{"start_line":4333,"start_character":27,"end_line":4333,"end_character":39},"updated":"2015-04-22 16:10:14.000000000","message":"consider using self.flags(\u0027vcpu_pin_set\u0027, \"4-5\")","commit_id":"2bb14343cfc0d6622155e58e445b3c9bb2790486"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"325a818e4e1c11d059c0280262d93115e5d2d895","unresolved":false,"context_lines":[{"line_number":4330,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4331,"context_line":"    def test_get_host_vcpus(self, get_online_cpus):"},{"line_number":4332,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4333,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"4-5\")"},{"line_number":4334,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4335,"context_line":"        expected_vcpus \u003d 2"},{"line_number":4336,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":20,"id":"dac4157f_75ce632f","line":4333,"in_reply_to":"dac4157f_12e05d28","updated":"2015-04-22 16:38:00.000000000","message":"Done for all.","commit_id":"2bb14343cfc0d6622155e58e445b3c9bb2790486"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"600a18dbde8bbcd06b9966bef184b4d5da06ab2a","unresolved":false,"context_lines":[{"line_number":4339,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4340,"context_line":"    def test_get_host_vcpus_out_of_range(self, get_online_cpus):"},{"line_number":4341,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":4342,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"4-6\")"},{"line_number":4343,"context_line":"        get_online_cpus.return_value \u003d set([4, 5])"},{"line_number":4344,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"},{"line_number":4345,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"dac4157f_52c815a8","line":4342,"updated":"2015-04-22 16:10:14.000000000","message":"ditto","commit_id":"2bb14343cfc0d6622155e58e445b3c9bb2790486"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"600a18dbde8bbcd06b9966bef184b4d5da06ab2a","unresolved":false,"context_lines":[{"line_number":4351,"context_line":"            \u0027this function is not supported by the connection driver:\u0027"},{"line_number":4352,"context_line":"            \u0027 virNodeNumOfDevices\u0027,"},{"line_number":4353,"context_line":"            error_code\u003dfakelibvirt.VIR_ERR_NO_SUPPORT)"},{"line_number":4354,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"4-6\")"},{"line_number":4355,"context_line":"        get_online_cpus.side_effect \u003d not_supported_exc"},{"line_number":4356,"context_line":"        self.assertRaises(exception.Invalid, drvr._get_vcpu_total)"},{"line_number":4357,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"dac4157f_32cda1b7","line":4354,"updated":"2015-04-22 16:10:14.000000000","message":"ditto","commit_id":"2bb14343cfc0d6622155e58e445b3c9bb2790486"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"600a18dbde8bbcd06b9966bef184b4d5da06ab2a","unresolved":false,"context_lines":[{"line_number":4363,"context_line":"            \u0027this function is not supported by the connection driver:\u0027"},{"line_number":4364,"context_line":"            \u0027 virNodeNumOfDevices\u0027,"},{"line_number":4365,"context_line":"            error_code\u003dfakelibvirt.VIR_ERR_NO_SUPPORT)"},{"line_number":4366,"context_line":"        CONF.set_override(\u0027vcpu_pin_set\u0027, \"1\")"},{"line_number":4367,"context_line":"        get_online_cpus.side_effect \u003d not_supported_exc"},{"line_number":4368,"context_line":"        expected_vcpus \u003d 1"},{"line_number":4369,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"}],"source_content_type":"text/x-python","patch_set":20,"id":"dac4157f_f2e60914","line":4366,"updated":"2015-04-22 16:10:14.000000000","message":"ditto","commit_id":"2bb14343cfc0d6622155e58e445b3c9bb2790486"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"bce4dacd438e97491db3cb2a0902168bf0c00da6","unresolved":false,"context_lines":[{"line_number":4334,"context_line":"        get_online_cpus.return_value \u003d set([4, 5, 6])"},{"line_number":4335,"context_line":"        expected_vcpus \u003d 2"},{"line_number":4336,"context_line":"        vcpus \u003d drvr._get_vcpu_total()"},{"line_number":4337,"context_line":"        self.assertEqual(expected_vcpus, vcpus)"},{"line_number":4338,"context_line":""},{"line_number":4339,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_online_cpus\u0027)"},{"line_number":4340,"context_line":"    def test_get_host_vcpus_out_of_range(self, get_online_cpus):"}],"source_content_type":"text/x-python","patch_set":21,"id":"bab921fb_36f3bb75","line":4337,"updated":"2015-04-23 10:55:22.000000000","message":"micro nit, I prefer having the value 2 in here rather than the variable name, not sure it ads anything in this case, but its personal preference.","commit_id":"bead2f51f8ad651cc46f8436cb1b0c3f8a7c93a9"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e9d4188c5d368f8c2ed21c596bfdc76552c72316","unresolved":false,"context_lines":[{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._get_host_online_pcpus()"},{"line_number":4481,"context_line":"        if online_pcpus:"},{"line_number":4482,"context_line":"            if available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"},{"line_number":4485,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_d8e007cf","line":4482,"updated":"2015-04-14 12:33:05.000000000","message":"nit: only one conditional is needed :\n\n   if online_pcpus and available_ids \u003c\u003d online_pcpus:","commit_id":"5191d3ae732332b83dbbe6a5bcb468c11a68f0bc"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"e7a597e24b0b9df39394babaabaf440e385f9b03","unresolved":false,"context_lines":[{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._get_host_online_pcpus()"},{"line_number":4481,"context_line":"        if online_pcpus:"},{"line_number":4482,"context_line":"            if available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"},{"line_number":4485,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_5db5e5eb","line":4482,"in_reply_to":"7aaa499b_d8e007cf","updated":"2015-04-15 07:32:28.000000000","message":"Done","commit_id":"5191d3ae732332b83dbbe6a5bcb468c11a68f0bc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e9d4188c5d368f8c2ed21c596bfdc76552c72316","unresolved":false,"context_lines":[{"line_number":4503,"context_line":"            for cpu in cell.cpus:"},{"line_number":4504,"context_line":"                online_cpus.append(int(cpu.id))"},{"line_number":4505,"context_line":"        return set(online_cpus)"},{"line_number":4506,"context_line":""},{"line_number":4507,"context_line":"    def _get_memory_mb_total(self):"},{"line_number":4508,"context_line":"        \"\"\"Get the total memory size(MB) of physical computer."},{"line_number":4509,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_589f5750","line":4506,"updated":"2015-04-14 12:33:05.000000000","message":"you\u0027re missing unittests for both the new method and the new conditional you added.","commit_id":"5191d3ae732332b83dbbe6a5bcb468c11a68f0bc"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"e7a597e24b0b9df39394babaabaf440e385f9b03","unresolved":false,"context_lines":[{"line_number":4503,"context_line":"            for cpu in cell.cpus:"},{"line_number":4504,"context_line":"                online_cpus.append(int(cpu.id))"},{"line_number":4505,"context_line":"        return set(online_cpus)"},{"line_number":4506,"context_line":""},{"line_number":4507,"context_line":"    def _get_memory_mb_total(self):"},{"line_number":4508,"context_line":"        \"\"\"Get the total memory size(MB) of physical computer."},{"line_number":4509,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7aaa499b_5dacc59f","line":4506,"in_reply_to":"7aaa499b_589f5750","updated":"2015-04-15 07:32:28.000000000","message":"Removed this function entirely. There was an alternate function available which is doing the exact same thing in host.py called get_online_cpus(). So used that one instead. \n\nAlso didn\u0027t see any UTs written for the parent method, should I add one for it?","commit_id":"5191d3ae732332b83dbbe6a5bcb468c11a68f0bc"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"1a98358340cc0f0778ed42c2b2d63a37b1709c38","unresolved":false,"context_lines":[{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_91761e3d","line":4480,"updated":"2015-04-16 08:20:22.000000000","message":"This method can raise an exception. You should make it in a block try...catch","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"1a245be1b45f80330994426a70b6921daec9067b","unresolved":false,"context_lines":[{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_cd651ee2","line":4480,"in_reply_to":"5aa745d1_854ed4bf","updated":"2015-04-16 11:57:07.000000000","message":"This method explicitly raise an exception and it makes sense to catch it since that will break the process and that it is not what we want here. I do not have concern about how to resolve it, both solutions look good for me.","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"910984487c66a4535659bc57cb6bc266cd91352e","unresolved":false,"context_lines":[{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_854ed4bf","line":4480,"in_reply_to":"5aa745d1_91761e3d","updated":"2015-04-16 09:59:59.000000000","message":"I think if at all, a try-catch block is needed, that should be in the get_online_cpus() method itself since this is also getting called in _get_host_numa_topology method which doesn\u0027t have enclosing try-catch.","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"1ba4b6b8a7c52fbb47e52c0bd21ba23b94d1414d","unresolved":false,"context_lines":[{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_ed77bad2","line":4480,"in_reply_to":"5aa745d1_cd651ee2","updated":"2015-04-16 12:13:05.000000000","message":"If we are looking at the same code, then this method doesn\u0027t raise a exception explicitly, it\u0027s Libvirt that may raise an exception. Did you mean the same?","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"7a90726cd7f14f0072c43ab165edded70d2784f8","unresolved":false,"context_lines":[{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_a34e63bf","line":4480,"in_reply_to":"5aa745d1_ed77bad2","updated":"2015-04-16 12:55:19.000000000","message":"I have addressed this though now in PS 10","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"1a98358340cc0f0778ed42c2b2d63a37b1709c38","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"},{"line_number":4484,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_110c4e93","line":4481,"updated":"2015-04-16 08:20:22.000000000","message":"Since at least it returns an empty set, here the condition should be:\n\n  if available_ids \u003c\u003d online_pcpus:\n    ...\n\nSo your condition verify if the sum of ids in \u0027available_ids\u0027 is less or equal of the sum of ids in \u0027online_pcpus\u0027\n\nSo something like:\n  available_ids  \u003d [1, 5] \n  online_pcpus \u003d [2, 4]\n\nWill work but should fails since id 5 is out of the range. Also can i recommend you to follow the current code and raise an exception instead of make a return.","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"910984487c66a4535659bc57cb6bc266cd91352e","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"},{"line_number":4484,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_a55b1054","line":4481,"in_reply_to":"5aa745d1_110c4e93","updated":"2015-04-16 09:59:59.000000000","message":"I am not doing a sum of ids?? Can you explain your point a bit better?\nThe condition you have mentioned above will fail using the previous logic itself.\n\navailable_ids \u003c\u003d online_cpus will return false and then it would go to the old behavior and it will throw an exception there.","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"1ba4b6b8a7c52fbb47e52c0bd21ba23b94d1414d","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"},{"line_number":4484,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_cdd87ef2","line":4481,"in_reply_to":"5aa745d1_37ddf542","updated":"2015-04-16 12:13:05.000000000","message":"These are not lists. These are sets. The operator mentioned would do a subset check on the two lists. Hence this comment is not valid IMHO.","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"1a245be1b45f80330994426a70b6921daec9067b","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"},{"line_number":4484,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_37ddf542","line":4481,"in_reply_to":"5aa745d1_a55b1054","updated":"2015-04-16 11:57:07.000000000","message":"You are using operator with lists of integers here: available_ids \u003c\u003d online_pcpus. python will do the sum of each and then compare.\n\nYou change how the algorithm has been defined, i mean it\u0027s better to keep the same logic (easier to read)","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"eac56810e5d9e01ab0e0a5174a8767482f143121","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4481,"context_line":"        if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4482,"context_line":"            self._vcpu_total \u003d len(available_ids)"},{"line_number":4483,"context_line":"            return self._vcpu_total"},{"line_number":4484,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":8,"id":"5aa745d1_ea779700","line":4481,"in_reply_to":"5aa745d1_cdd87ef2","updated":"2015-04-16 14:40:28.000000000","message":"right my mistake, let me comment about what i think is not good here in the last PS","commit_id":"ae150e2ad2d775d12ce44830b64cf32bed75b38f"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f3d03c96f64614d67a97ac91dd0b52da8ebf94de","unresolved":false,"context_lines":[{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        try:"},{"line_number":4481,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4482,"context_line":"            if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"},{"line_number":4485,"context_line":"        except libvirt.libvirtError as ex:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aa745d1_ea06771f","line":4482,"updated":"2015-04-16 14:44:22.000000000","message":"So it does not make a lot of sense to have successfully get the list of online cpus and just skip if the range is not valid.\n\nIf you are able to get that list and we are out of the range so we should raise an exception else return vcpu_total. Then if we are not able to get that list (i.e bad version of libvirt) so we continue with the old behavior.","commit_id":"21492e1babd7ca49f841bfee1d90ef5f7ccb52f5"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"75284b594f6a26b91039b6781ccbe8d8e902fb55","unresolved":false,"context_lines":[{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        try:"},{"line_number":4481,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4482,"context_line":"            if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"},{"line_number":4485,"context_line":"        except libvirt.libvirtError as ex:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aa745d1_f0fc2a40","line":4482,"in_reply_to":"5aa745d1_ea06771f","updated":"2015-04-16 15:16:28.000000000","message":"That makes sense. Added :)","commit_id":"21492e1babd7ca49f841bfee1d90ef5f7ccb52f5"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f3d03c96f64614d67a97ac91dd0b52da8ebf94de","unresolved":false,"context_lines":[{"line_number":4484,"context_line":"                return self._vcpu_total"},{"line_number":4485,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4486,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4487,"context_line":"            LOG.warn(_LE(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4488,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"},{"line_number":4489,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4490,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aa745d1_1e41ee0b","line":4487,"updated":"2015-04-16 14:44:22.000000000","message":"s/_LE/_LW","commit_id":"21492e1babd7ca49f841bfee1d90ef5f7ccb52f5"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"75284b594f6a26b91039b6781ccbe8d8e902fb55","unresolved":false,"context_lines":[{"line_number":4484,"context_line":"                return self._vcpu_total"},{"line_number":4485,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4486,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4487,"context_line":"            LOG.warn(_LE(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4488,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"},{"line_number":4489,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4490,"context_line":"        if sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5aa745d1_b09582e5","line":4487,"in_reply_to":"5aa745d1_1e41ee0b","updated":"2015-04-16 15:16:28.000000000","message":"Done.","commit_id":"21492e1babd7ca49f841bfee1d90ef5f7ccb52f5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b06e559a5f7315fff246a9f63bcb7c964d3c8c25","unresolved":false,"context_lines":[{"line_number":4486,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4487,"context_line":"                                          \"One or more of the specified \""},{"line_number":4488,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4489,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4490,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4491,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4492,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"}],"source_content_type":"text/x-python","patch_set":12,"id":"5aa745d1_307852c4","line":4489,"updated":"2015-04-16 15:20:21.000000000","message":"well, I would have preferred to see the except condition closer to where it could be raised, ie. just after self._host.get_online_cpus()","commit_id":"0d06e888b95471eb6e1537ecf9efd30c13af3e47"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"25ca6c9342d59d5f80467ede91c2ba937c23b7de","unresolved":false,"context_lines":[{"line_number":4486,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4487,"context_line":"                                          \"One or more of the specified \""},{"line_number":4488,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4489,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4490,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4491,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4492,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"}],"source_content_type":"text/x-python","patch_set":12,"id":"5aa745d1_6f402ba6","line":4489,"in_reply_to":"5aa745d1_307852c4","updated":"2015-04-16 19:14:38.000000000","message":"Umm, the reason why I had a bitter try except block is because, then the semantics becomes easier to deal with. The whole block would actually not get executed if there\u0027s a libvirt error. But if we do raise the libvirt error, the original behavior could be preserved. Thoughts?","commit_id":"0d06e888b95471eb6e1537ecf9efd30c13af3e47"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70a5cafc998d6b1e42d472bc20475c45827bcb43","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        try:"},{"line_number":4481,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4482,"context_line":"            if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":13,"id":"5aa745d1_9214c0f5","line":4481,"updated":"2015-04-16 19:38:11.000000000","message":"IMHO, you should only have this line inside the block that catches the libvirterror exception. The rest of the code here shouldn\u0027t raise that, and thus should be outside.","commit_id":"571330a1cb0eaa084053623499dcb656907746a4"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"08747369a3348af1a60f4a7a787f31f81de8f74f","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        try:"},{"line_number":4481,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4482,"context_line":"            if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":13,"id":"5aa745d1_b2dd7c44","line":4481,"in_reply_to":"5aa745d1_9214c0f5","updated":"2015-04-16 19:39:36.000000000","message":"As I said in PS12, please do what Dan is saying. Sorry if you misunderstood me","commit_id":"571330a1cb0eaa084053623499dcb656907746a4"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"e86a40075ef1b0756a0cce7fd573fc87077d246f","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        try:"},{"line_number":4481,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4482,"context_line":"            if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":13,"id":"3ab451fb_dc304da1","line":4481,"in_reply_to":"5aa745d1_9214c0f5","updated":"2015-04-16 19:55:05.000000000","message":"Done","commit_id":"571330a1cb0eaa084053623499dcb656907746a4"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"e86a40075ef1b0756a0cce7fd573fc87077d246f","unresolved":false,"context_lines":[{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        try:"},{"line_number":4481,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4482,"context_line":"            if online_pcpus and available_ids \u003c\u003d online_pcpus:"},{"line_number":4483,"context_line":"                self._vcpu_total \u003d len(available_ids)"},{"line_number":4484,"context_line":"                return self._vcpu_total"}],"source_content_type":"text/x-python","patch_set":13,"id":"3ab451fb_bc25815d","line":4481,"in_reply_to":"5aa745d1_b2dd7c44","updated":"2015-04-16 19:55:05.000000000","message":"Done.","commit_id":"571330a1cb0eaa084053623499dcb656907746a4"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"89ef506475c3dd418a0c60bc23f428ef06f91853","unresolved":false,"context_lines":[{"line_number":4475,"context_line":"            return self._vcpu_total"},{"line_number":4476,"context_line":""},{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ab451fb_4ed35539","line":4478,"updated":"2015-04-17 07:26:53.000000000","message":"The algorithm is good I guess we can make it better but it is just a nit:\n\n        if online_pcpus:\n            if available_ids \u003e online_pcpus:\n               raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \"\n                                          \"One or more of the specified \"\n                                         \"cpuset is not online.\"))\n         elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:\n             raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \"\n                                       \"out of hypervisor cpu range.\"))\n         self._vcpu_total \u003d len(available_ids)\n         return self._vcpu_total","commit_id":"4804b6d711534a4d1c497790f9e7373669a921fc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a039c6cd5151a5b34e89f54fd55b3b949169ebe","unresolved":false,"context_lines":[{"line_number":4475,"context_line":"            return self._vcpu_total"},{"line_number":4476,"context_line":""},{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ab451fb_ee957640","line":4478,"in_reply_to":"3ab451fb_4ed35539","updated":"2015-04-17 12:49:43.000000000","message":"+1 with you, I don\u0027t like the online_pcpus \u003d None\n\nbut I think it\u0027s -1 for me.","commit_id":"4804b6d711534a4d1c497790f9e7373669a921fc"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"0191f5755b0e28feeb13378b93a0ae01228b1659","unresolved":false,"context_lines":[{"line_number":4475,"context_line":"            return self._vcpu_total"},{"line_number":4476,"context_line":""},{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fabf19d9_00d3ea39","line":4478,"in_reply_to":"3ab451fb_4ed35539","updated":"2015-04-18 11:12:49.000000000","message":"Agreed - have made this change.","commit_id":"4804b6d711534a4d1c497790f9e7373669a921fc"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"0191f5755b0e28feeb13378b93a0ae01228b1659","unresolved":false,"context_lines":[{"line_number":4475,"context_line":"            return self._vcpu_total"},{"line_number":4476,"context_line":""},{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fabf19d9_83b0248d","line":4478,"in_reply_to":"3ab451fb_ee957640","updated":"2015-04-18 11:12:49.000000000","message":"Done!","commit_id":"4804b6d711534a4d1c497790f9e7373669a921fc"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"6e44aa68a95df58dc8dc83c2ed08e2c306caf2d9","unresolved":false,"context_lines":[{"line_number":4475,"context_line":"            return self._vcpu_total"},{"line_number":4476,"context_line":""},{"line_number":4477,"context_line":"        available_ids \u003d hardware.get_vcpu_pin_set()"},{"line_number":4478,"context_line":"        # We get the list of online CPUs on the host and see if the requested"},{"line_number":4479,"context_line":"        # set falls under these. If not, we retain the old behavior."},{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fabf19d9_ee2a277e","line":4478,"in_reply_to":"fabf19d9_00d3ea39","updated":"2015-04-18 11:53:05.000000000","message":"I think there\u0027s a bug in this approach.\nIf available_ids \u003d [4, 5] and online_pcups \u003d [7, 8]\navailable_ids \u003e online_pcpus would result as False.\nSo that would mean - we would not throw any exceptions in this case.\n\nThe latest patchset should address this issue with another UT.","commit_id":"4804b6d711534a4d1c497790f9e7373669a921fc"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6a039c6cd5151a5b34e89f54fd55b3b949169ebe","unresolved":false,"context_lines":[{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"},{"line_number":4482,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4483,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4484,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4485,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4486,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ab451fb_0e9e6256","line":4483,"updated":"2015-04-17 12:49:43.000000000","message":"a test is missing for this branch","commit_id":"4804b6d711534a4d1c497790f9e7373669a921fc"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"0191f5755b0e28feeb13378b93a0ae01228b1659","unresolved":false,"context_lines":[{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"},{"line_number":4482,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4483,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4484,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4485,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4486,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"}],"source_content_type":"text/x-python","patch_set":14,"id":"fabf19d9_a3ba086d","line":4483,"in_reply_to":"3ab451fb_0e9e6256","updated":"2015-04-18 11:12:49.000000000","message":"Added 2 TCs for this.","commit_id":"4804b6d711534a4d1c497790f9e7373669a921fc"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4f81ecfb54b4ccefb00d9b9b6788b1a50c9bd39e","unresolved":false,"context_lines":[{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"},{"line_number":4482,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4483,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4484,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4485,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4486,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"}],"source_content_type":"text/x-python","patch_set":16,"id":"dac4157f_e23b5c9b","line":4483,"updated":"2015-04-19 01:03:16.000000000","message":"not sure we should catch this exception also. for example https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4756","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"2430fa98a18b3d749aa7475c588312934d87319d","unresolved":false,"context_lines":[{"line_number":4480,"context_line":"        online_pcpus \u003d None"},{"line_number":4481,"context_line":"        try:"},{"line_number":4482,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4483,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":4484,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4485,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4486,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"}],"source_content_type":"text/x-python","patch_set":16,"id":"dac4157f_0d0561c1","line":4483,"in_reply_to":"dac4157f_e23b5c9b","updated":"2015-04-19 05:09:14.000000000","message":"Yes. That was my point to Sahid but we thought we should be catching this. You can check patch set 8 for this conversation. My initial thought was to implement the try-catch in the get_online_cpus method but i think implementing here makes sense.\nThis method can possibly raise that exception like Sahid points out and we have to deal with it. The numa topology method should have also had it i feel. Any other reason why it shouldn\u0027t be there?","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4f81ecfb54b4ccefb00d9b9b6788b1a50c9bd39e","unresolved":false,"context_lines":[{"line_number":4484,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4485,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4486,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"},{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""}],"source_content_type":"text/x-python","patch_set":16,"id":"dac4157f_02d09052","line":4487,"updated":"2015-04-19 01:03:16.000000000","message":"nit, we should use LOG.warn(_LW(....), {...}) instead of use \u0027%\u0027","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"2430fa98a18b3d749aa7475c588312934d87319d","unresolved":false,"context_lines":[{"line_number":4484,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":4485,"context_line":"            LOG.warn(_LW(\"Couldn\u0027t retrieve the online CPUs due to a Libvirt \""},{"line_number":4486,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\") %"},{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""}],"source_content_type":"text/x-python","patch_set":16,"id":"dac4157f_6df61dd9","line":4487,"in_reply_to":"dac4157f_02d09052","updated":"2015-04-19 05:09:14.000000000","message":"I will make this change irrespective but trying to understand, what difference does it make? I see that there are methods in this driver class which have written the logs like this.","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"4f81ecfb54b4ccefb00d9b9b6788b1a50c9bd39e","unresolved":false,"context_lines":[{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":4494,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4495,"context_line":"                                      \"out of hypervisor cpu range.\"))"}],"source_content_type":"text/x-python","patch_set":16,"id":"dac4157f_023770b1","line":4492,"updated":"2015-04-19 01:03:16.000000000","message":"I\u0027m not sure we should raise error at here.\n\nLike at here, we can filter out the off line cpu\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4758\n\nOperator can set a cpu pin set. Then later Operator can specific some cpu offline, that behavior is make sense.","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"2430fa98a18b3d749aa7475c588312934d87319d","unresolved":false,"context_lines":[{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":4494,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4495,"context_line":"                                      \"out of hypervisor cpu range.\"))"}],"source_content_type":"text/x-python","patch_set":16,"id":"dac4157f_ad2af52a","line":4492,"in_reply_to":"dac4157f_023770b1","updated":"2015-04-19 05:09:14.000000000","message":"The behavior is actually derived from what was implemented earlier. If you look at the code block just below this comment, it\u0027s suggesting that we are going to raise an exception if there are cpusets specified by the operator that exceeds the total pcpus on the machine (essentially the total number of online cpus). So if we throw an exception there - and later operator wants to make a few more CPUs online - I don\u0027t know if that would be supported with or without this patch. Thoughts?","commit_id":"02303b0235fe7d442bf7bd0dac70b30b1bd12ded"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e96e49744dfeaf8e005d4ceeab3ae3faa7bb70a6","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_2290d942","line":4490,"updated":"2015-04-21 18:55:56.000000000","message":"This error message doesn\u0027t give a lot of detail, so we should log the available_ids and online_pcpus before raising the exception so someone has more information to debug against, e.g. what if one of the pcpus was offline temporarily when we hit this, then went back on again (is that possible?).","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ffb16e6339fc3cf09948618a5fecc69b19b66363","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_dd122062","line":4490,"in_reply_to":"dac4157f_2290d942","updated":"2015-04-21 19:34:17.000000000","message":"I\u0027m not sure there\u0027s a whole lot of value here. I could request something really crazy and spew things into the logs otherwise, right?","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"34a9ae03bf0c3370d76eb81a340e3459044c4fce","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_fd1603f0","line":4490,"in_reply_to":"dac4157f_3d37dbe4","updated":"2015-04-21 19:51:00.000000000","message":"But the pin request comes from the potentially untrusted client, right? So if I try to pin to cpus 1024-2048, I can generate logs that way right? If we lot the sets (or the difference) then i can generate some large log lines by making the range large so the set ends up large I think.\n\nI *think* this gets run as part of the boot request, so maybe putting it into the exception text would help as that would end up in the instance fault dump, which is at least visible to the person that initiated the request.\n\nAlso, the existing error that was here (L4494) doesn\u0027t do a log message, and I think that\u0027s because you can trigger it as a user.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"78dc5754ccd2335ca6b7c412819c50e52f31ed05","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_bd62cbab","line":4490,"in_reply_to":"dac4157f_3d37dbe4","updated":"2015-04-21 20:44:08.000000000","message":"I agree with Dan, that has to be provided in the exception message, not in a log information. Also, as the request-id is not persisted, I don\u0027t see much benefits to log that as it would be difficult for the operator to find in the logs the related error message unless we\u0027re explicitely providing the instance uuid (which we don\u0027t have in that method, which would need to be exposed and blah blah blah...)\n\nNow the question is : does it need to be done in the same patch or should it be done in a separate change ? I think the latter is fine with me.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"9b608653ecea90cc5fb3190ae3ce31a90d18e7f4","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_541018a8","line":4490,"in_reply_to":"dac4157f_91ba506d","updated":"2015-04-22 08:46:26.000000000","message":"I have included this as a part of the exception being thrown:\n\nmsg \u003d (_(\"Invalid vcpu_pin_set config, one or more of the \"\n         \"specified cpuset is not online. Online \"\n         \"cpuset(s): %(online)s, requested cpuset(s): %(req)s\"),\n         {\u0027online\u0027: list(online_pcpus), \u0027req\u0027: list(available_ids)})\n\nraise exception.Invalid(msg)\n\nDoes this sound good?","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"afa67099cf2ce297ac3a75bae8b9cd5e7fa86531","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_91ba506d","line":4490,"in_reply_to":"dac4157f_bd62cbab","updated":"2015-04-21 21:52:22.000000000","message":"Again, I don\u0027t see where you guys are seeing the user part of this, as I\u0027ve pointed out this is hit on init of the host when the compute manager comes up so this will explode and take down the compute service.  As an operator I\u0027d want details in the logs as to what is being specified and what libvirt is saying is online on the host so I can reconcile those w/o having to dive into the code to figure it out.\n\nI don\u0027t care if we log those values here or put them into the Invalid error message (already noted that), but it should get back up somewhere.\n\nI don\u0027t care about the request id and there isn\u0027t an instance uuid but that doesn\u0027t matter.  I could have logstash setup on my compute logs and get this big fat error when my compute service dies and query on that error in the logs to see what happened.\n\nI don\u0027t see why we\u0027d leave that to another patch, this is very very simple what I\u0027m asking for.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"12dda92026b20dafc3bfdad0837801b61d4110eb","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_3d37dbe4","line":4490,"in_reply_to":"dac4157f_dd122062","updated":"2015-04-21 19:37:08.000000000","message":"Let\u0027s assume you\u0027re not crazy and you put what should normally be a valid config value.  For some reason one of the pinned pcpu\u0027s is offline.  When this fails, the compute service dies.\n\nIf I\u0027m an operator, I wouldn\u0027t want to have to dig into the code to figure out where this is failing and see what the variables are, or dig into what libvirt is telling us from self._host.get_online_cpus().\n\nI\u0027d rather have something in the logs that says \u0027you are pinning to x but only y is online\u0027.  I\u0027m looking for a helpful message to an operator on what they do next, i.e. change your vcpu_pin_set value to match what\u0027s available (online) or get the pcpu\u0027s that you want to use online.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"68222cbeead39f87b1a75b8155b1874042d9e5a3","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_6397f248","line":4490,"in_reply_to":"dac4157f_fd1603f0","updated":"2015-04-21 20:39:14.000000000","message":"Even if this could be called as part of booting an instance, the client doesn\u0027t actually specify which CPUs to pin.  That\u0027s handled by nova.  The client just specifies a flavor which asks for dedicated CPUs.  The actual allocation of CPUs is not up to the client.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14b18d6b1d43061693df7dd5c75fdb085635c7f5","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4491,"context_line":"                                          \"One or more of the specified \""},{"line_number":4492,"context_line":"                                          \"cpuset is not online.\"))"},{"line_number":4493,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_fde1639b","line":4490,"in_reply_to":"dac4157f_fd1603f0","updated":"2015-04-21 20:04:07.000000000","message":"This is called from get_available_resources so it\u0027s on init_host and the periodic task that gets resources from the host from the resource tracker, so this can actually kill the service on startup which was why I was thinking more explicit logging of what is requested vs what\u0027s available right before we kill the service is best for debugging.\n\nThe other existing error message could have used work too (or logging total_pcpus before raising, IMO).","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"660705018139b10207ccabfc0d07e06340ca144d","unresolved":false,"context_lines":[{"line_number":4878,"context_line":""},{"line_number":4879,"context_line":"    def get_available_resource(self, nodename):"},{"line_number":4880,"context_line":"        \"\"\"Retrieve resource information."},{"line_number":4881,"context_line":""},{"line_number":4882,"context_line":"        This method is called when nova-compute launches, and"},{"line_number":4883,"context_line":"        as part of a periodic task that records the results in the DB."},{"line_number":4884,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_3d5b5b56","line":4881,"updated":"2015-04-21 20:06:41.000000000","message":"hence kaboom if this fails with an Invalid error.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"660705018139b10207ccabfc0d07e06340ca144d","unresolved":false,"context_lines":[{"line_number":4898,"context_line":"        # the RPC version as JSON. Can be changed when RPC broadcast is removed"},{"line_number":4899,"context_line":"        data[\"supported_instances\"] \u003d jsonutils.dumps("},{"line_number":4900,"context_line":"            self._get_instance_capabilities())"},{"line_number":4901,"context_line":""},{"line_number":4902,"context_line":"        data[\"vcpus\"] \u003d self._get_vcpu_total()"},{"line_number":4903,"context_line":"        data[\"memory_mb\"] \u003d self._get_memory_mb_total()"},{"line_number":4904,"context_line":"        data[\"local_gb\"] \u003d disk_info_dict[\u0027total\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"dac4157f_7d48b387","line":4901,"updated":"2015-04-21 20:06:41.000000000","message":"Called on init and periodic tasks here.","commit_id":"af0cf424a87d136407d45e1c55e157423d8c9367"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9dfedeb98159eb9704236a7102af1e069bb39866","unresolved":false,"context_lines":[{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                msg \u003d (_(\"Invalid vcpu_pin_set config, One or more of the \""},{"line_number":4491,"context_line":"                         \"specified cpuset is not online. Online cpuset(s): \""},{"line_number":4492,"context_line":"                         \"%(online)s, requested cpuset(s): %(req)s\"),"},{"line_number":4493,"context_line":"                       {\u0027online\u0027: list(online_pcpus),"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_af2dcda9","line":4490,"updated":"2015-04-22 14:23:03.000000000","message":"Either:\n\ns/One/one/\n\nor:\n\ns/config, One/config. One/","commit_id":"0fae2430dea33b8d151ef409a4190cc35114fc37"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9dfedeb98159eb9704236a7102af1e069bb39866","unresolved":false,"context_lines":[{"line_number":4490,"context_line":"                msg \u003d (_(\"Invalid vcpu_pin_set config, One or more of the \""},{"line_number":4491,"context_line":"                         \"specified cpuset is not online. Online cpuset(s): \""},{"line_number":4492,"context_line":"                         \"%(online)s, requested cpuset(s): %(req)s\"),"},{"line_number":4493,"context_line":"                       {\u0027online\u0027: list(online_pcpus),"},{"line_number":4494,"context_line":"                        \u0027req\u0027: list(available_ids)})"},{"line_number":4495,"context_line":"                raise exception.Invalid(msg)"},{"line_number":4496,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"}],"source_content_type":"text/x-python","patch_set":18,"id":"dac4157f_b21bf672","line":4493,"updated":"2015-04-22 14:23:03.000000000","message":"Are these sorted?  Might be easier to read in the error if the lists are sorted.","commit_id":"0fae2430dea33b8d151ef409a4190cc35114fc37"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"bce4dacd438e97491db3cb2a0902168bf0c00da6","unresolved":false,"context_lines":[{"line_number":4486,"context_line":"                         \"error: %(error)s with error code: %(error_code)s\"),"},{"line_number":4487,"context_line":"                     {\u0027error\u0027: ex, \u0027error_code\u0027: error_code})"},{"line_number":4488,"context_line":"        if online_pcpus:"},{"line_number":4489,"context_line":"            if not (available_ids \u003c\u003d online_pcpus):"},{"line_number":4490,"context_line":"                msg \u003d (_(\"Invalid vcpu_pin_set config, one or more of the \""},{"line_number":4491,"context_line":"                         \"specified cpuset is not online. Online cpuset(s): \""},{"line_number":4492,"context_line":"                         \"%(online)s, requested cpuset(s): %(req)s\"),"}],"source_content_type":"text/x-python","patch_set":21,"id":"bab921fb_5409882b","line":4489,"updated":"2015-04-23 10:55:22.000000000","message":"Nit: it takes you a while to realise these are sets, so don\u0027t need sorting, but its hardly worth a comment, just a note for other readers.","commit_id":"bead2f51f8ad651cc46f8436cb1b0c3f8a7c93a9"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"21ca54534879e9e309eff7f6ad696d15a09dabb9","unresolved":false,"context_lines":[{"line_number":4492,"context_line":"                         \"%(online)s, requested cpuset(s): %(req)s\"),"},{"line_number":4493,"context_line":"                       {\u0027online\u0027: sorted(online_pcpus),"},{"line_number":4494,"context_line":"                        \u0027req\u0027: sorted(available_ids)})"},{"line_number":4495,"context_line":"                raise exception.Invalid(msg)"},{"line_number":4496,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":4497,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4498,"context_line":"                                      \"out of hypervisor cpu range.\"))"}],"source_content_type":"text/x-python","patch_set":21,"id":"bab921fb_762c93ab","line":4495,"updated":"2015-04-23 10:38:43.000000000","message":"Paste my comment in previous patchsets....Hope to get other people\u0027s suggestion.\n\nI\u0027m not sure we should raise error at here.\nLike at here, we can filter out the off line cpu https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4758\n\nOperator can set a cpu pin set. Then later Operator can specific some cpu offline on live, that behavior is make sense.\n\nThis is different with the check at the line 4496. line 4496 is ensure the range of cpu id. It more like valid value checks.","commit_id":"bead2f51f8ad651cc46f8436cb1b0c3f8a7c93a9"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"54d47151536d01056769d1a0a4001ad92286ca92","unresolved":false,"context_lines":[{"line_number":4492,"context_line":"                         \"%(online)s, requested cpuset(s): %(req)s\"),"},{"line_number":4493,"context_line":"                       {\u0027online\u0027: sorted(online_pcpus),"},{"line_number":4494,"context_line":"                        \u0027req\u0027: sorted(available_ids)})"},{"line_number":4495,"context_line":"                raise exception.Invalid(msg)"},{"line_number":4496,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":4497,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4498,"context_line":"                                      \"out of hypervisor cpu range.\"))"}],"source_content_type":"text/x-python","patch_set":21,"id":"bab921fb_f4865cb8","line":4495,"in_reply_to":"bab921fb_3473b4b3","updated":"2015-04-23 11:03:19.000000000","message":"Alex Xu - while I agree that the operator may specify some cpusets that might not be online - and later expect them to be online - in which case, the below check too doesn\u0027t suffice that need. Let\u0027s say the operator specifies 1-4 as the cpuset to use. (and my code above is not present). Let\u0027s say there are only 2 active cpus, so total_pcpus will be 2. In such a case, you wouldn\u0027t allow the compute to start as per the old behavior. Hence, it needs a complete re-look at how we would want to support this scenario that goes beyond this patch IMHO.","commit_id":"bead2f51f8ad651cc46f8436cb1b0c3f8a7c93a9"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"bce4dacd438e97491db3cb2a0902168bf0c00da6","unresolved":false,"context_lines":[{"line_number":4492,"context_line":"                         \"%(online)s, requested cpuset(s): %(req)s\"),"},{"line_number":4493,"context_line":"                       {\u0027online\u0027: sorted(online_pcpus),"},{"line_number":4494,"context_line":"                        \u0027req\u0027: sorted(available_ids)})"},{"line_number":4495,"context_line":"                raise exception.Invalid(msg)"},{"line_number":4496,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":4497,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":4498,"context_line":"                                      \"out of hypervisor cpu range.\"))"}],"source_content_type":"text/x-python","patch_set":21,"id":"bab921fb_3473b4b3","line":4495,"in_reply_to":"bab921fb_762c93ab","updated":"2015-04-23 10:55:22.000000000","message":"We can do that in a follow on patch.","commit_id":"bead2f51f8ad651cc46f8436cb1b0c3f8a7c93a9"}]}
