)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4794d1e2e562c1a9601a9903ef059403471eb259","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: Remove unnecessary try-catch around \u0027getCPUMap\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This was added in the original review [1] but seems to be totally"},{"line_number":10,"context_line":"unnecessary: the only reason this can fail is if libvirt version is less"},{"line_number":11,"context_line":"than 1.2.5 [2] or if the ACL checks fail [3]. The former isn\u0027t an issue"},{"line_number":12,"context_line":"since our minimum is greater than this and the latter isn\u0027t an issue"},{"line_number":13,"context_line":"since that feature is off by default and there\u0027s no reason it would be"},{"line_number":14,"context_line":"turned on for a nova host, since nova is a trusted management"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7faddb67_f86060a7","line":11,"range":{"start_line":10,"start_character":13,"end_line":11,"end_character":44},"updated":"2019-08-20 21:04:22.000000000","message":"Sorry, I\u0027m having trouble with this assertion (mainly because you pointed to source code). Can really none of this other stuff [1] result in an exception?\n\n[1] https://github.com/libvirt/libvirt/blob/76be4f5ddac608873378e5bc43eb12731f7ddcf2/src/util/virhostcpu.c","commit_id":"11751e94f1ae53ad1fcafc02cf864694adb842a0"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"026eb951d10fbd83799829263b59494fb843dafa","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: Remove unnecessary try-catch around \u0027getCPUMap\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This was added in the original review [1] but seems to be totally"},{"line_number":10,"context_line":"unnecessary: the only reason this can fail is if libvirt version is less"},{"line_number":11,"context_line":"than 1.2.5 [2] or if the ACL checks fail [3]. The former isn\u0027t an issue"},{"line_number":12,"context_line":"since our minimum is greater than this and the latter isn\u0027t an issue"},{"line_number":13,"context_line":"since that feature is off by default and there\u0027s no reason it would be"},{"line_number":14,"context_line":"turned on for a nova host, since nova is a trusted management"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7faddb67_4a950131","line":11,"range":{"start_line":10,"start_character":13,"end_line":11,"end_character":44},"in_reply_to":"7faddb67_d8f31672","updated":"2019-08-21 21:01:36.000000000","message":"\u003e Also worth noting that we have already have other callers to this\n \u003e function that aren\u0027t wrapped and those haven\u0027t been causing an\n \u003e issue, so there\u0027s that\n\nAh, yeah, looked around at those. If nothing else, that seems like a reasonable justification.","commit_id":"11751e94f1ae53ad1fcafc02cf864694adb842a0"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"57f82ab0fb05fb5fca4ec894982b1c0c1beadd66","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: Remove unnecessary try-catch around \u0027getCPUMap\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This was added in the original review [1] but seems to be totally"},{"line_number":10,"context_line":"unnecessary: the only reason this can fail is if libvirt version is less"},{"line_number":11,"context_line":"than 1.2.5 [2] or if the ACL checks fail [3]. The former isn\u0027t an issue"},{"line_number":12,"context_line":"since our minimum is greater than this and the latter isn\u0027t an issue"},{"line_number":13,"context_line":"since that feature is off by default and there\u0027s no reason it would be"},{"line_number":14,"context_line":"turned on for a nova host, since nova is a trusted management"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7faddb67_d8f31672","line":11,"range":{"start_line":10,"start_character":13,"end_line":11,"end_character":44},"in_reply_to":"7faddb67_f86060a7","updated":"2019-08-21 14:05:36.000000000","message":"I mean, things like high energy cosmic rays and libvirt bugs mean stuff can always break. However, the function this is calling [1] is rather simple and doesn\u0027t contain any error cases that I can see. The errors I highlighted occurred at other layers and, as noted, don\u0027t affect us. I double checked this all with Dan Berrange and he agreed.\n\nAlso worth noting that we have already have other callers to this function that aren\u0027t wrapped and those haven\u0027t been causing an issue, so there\u0027s that\n\n[1] https://github.com/libvirt/libvirt/blob/76be4f5ddac608873378e5bc43eb12731f7ddcf2/src/util/virhostcpu.c#L1076-L1105","commit_id":"11751e94f1ae53ad1fcafc02cf864694adb842a0"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8043f88077509185f46204b127ba24bf36fb0612","unresolved":false,"context_lines":[{"line_number":4508,"context_line":"        elif ((emulator_threads_policy \u003d\u003d"},{"line_number":4509,"context_line":"              fields.CPUEmulatorThreadsPolicy.SHARE) and"},{"line_number":4510,"context_line":"              shared_ids):"},{"line_number":4511,"context_line":"            online_pcpus \u003d self._host.get_online_cpus()"},{"line_number":4512,"context_line":"            cpuset \u003d shared_ids \u0026 online_pcpus"},{"line_number":4513,"context_line":"            if not cpuset:"},{"line_number":4514,"context_line":"                msg \u003d (_(\"Invalid cpu_shared_set config, one or more of the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_cddfbad4","line":4511,"updated":"2019-07-22 12:07:42.000000000","message":"No try/catch here...","commit_id":"63fb72805ca4dcd57328b8afbdb1dd4a8a981689"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8043f88077509185f46204b127ba24bf36fb0612","unresolved":false,"context_lines":[{"line_number":6539,"context_line":""},{"line_number":6540,"context_line":"        cells \u003d []"},{"line_number":6541,"context_line":"        allowed_cpus \u003d hardware.get_vcpu_pin_set()"},{"line_number":6542,"context_line":"        online_cpus \u003d self._host.get_online_cpus()"},{"line_number":6543,"context_line":"        if allowed_cpus:"},{"line_number":6544,"context_line":"            allowed_cpus \u0026\u003d online_cpus"},{"line_number":6545,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_ade27e9f","line":6542,"updated":"2019-07-22 12:07:42.000000000","message":"Nor here.","commit_id":"63fb72805ca4dcd57328b8afbdb1dd4a8a981689"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"4794d1e2e562c1a9601a9903ef059403471eb259","unresolved":false,"context_lines":[{"line_number":5864,"context_line":"                       {\u0027online\u0027: sorted(online_pcpus),"},{"line_number":5865,"context_line":"                        \u0027req\u0027: sorted(available_ids)})"},{"line_number":5866,"context_line":"                raise exception.Invalid(msg)"},{"line_number":5867,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":5868,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":5869,"context_line":"                                      \"out of hypervisor cpu range.\"))"},{"line_number":5870,"context_line":"        return len(available_ids)"},{"line_number":5871,"context_line":""},{"line_number":5872,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_d859e4fc","side":"PARENT","line":5869,"range":{"start_line":5867,"start_character":8,"end_line":5869,"end_character":70},"updated":"2019-08-20 21:04:22.000000000","message":"what\u0027s the justification for getting rid of this check?","commit_id":"56f193468ae2913ae0cbe74a5cf5ad0e37600d40"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"57f82ab0fb05fb5fca4ec894982b1c0c1beadd66","unresolved":false,"context_lines":[{"line_number":5864,"context_line":"                       {\u0027online\u0027: sorted(online_pcpus),"},{"line_number":5865,"context_line":"                        \u0027req\u0027: sorted(available_ids)})"},{"line_number":5866,"context_line":"                raise exception.Invalid(msg)"},{"line_number":5867,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":5868,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":5869,"context_line":"                                      \"out of hypervisor cpu range.\"))"},{"line_number":5870,"context_line":"        return len(available_ids)"},{"line_number":5871,"context_line":""},{"line_number":5872,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_e2a89f78","side":"PARENT","line":5869,"range":{"start_line":5867,"start_character":8,"end_line":5869,"end_character":70},"in_reply_to":"7faddb67_d859e4fc","updated":"2019-08-21 14:05:36.000000000","message":"\u0027online_pcpus\u0027 will *always* be present now so this would never trigger going forward. Just as importantly, \u0027online_pcpus\u0027 will always be \u003c\u003d \u0027total_pcpus\u0027 so the previous check is doing the exact same thing.","commit_id":"56f193468ae2913ae0cbe74a5cf5ad0e37600d40"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"026eb951d10fbd83799829263b59494fb843dafa","unresolved":false,"context_lines":[{"line_number":5864,"context_line":"                       {\u0027online\u0027: sorted(online_pcpus),"},{"line_number":5865,"context_line":"                        \u0027req\u0027: sorted(available_ids)})"},{"line_number":5866,"context_line":"                raise exception.Invalid(msg)"},{"line_number":5867,"context_line":"        elif sorted(available_ids)[-1] \u003e\u003d total_pcpus:"},{"line_number":5868,"context_line":"            raise exception.Invalid(_(\"Invalid vcpu_pin_set config, \""},{"line_number":5869,"context_line":"                                      \"out of hypervisor cpu range.\"))"},{"line_number":5870,"context_line":"        return len(available_ids)"},{"line_number":5871,"context_line":""},{"line_number":5872,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":8,"id":"7faddb67_ca73314d","side":"PARENT","line":5869,"range":{"start_line":5867,"start_character":8,"end_line":5869,"end_character":70},"in_reply_to":"7faddb67_e2a89f78","updated":"2019-08-21 21:01:36.000000000","message":"Thanks, another look with fresh eyes and I actually understand a bit more what\u0027s going on here.\n\nTIL that .issubset() can be spelled \u003c\u003d","commit_id":"56f193468ae2913ae0cbe74a5cf5ad0e37600d40"}]}