)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0802844b2eb0c40cf356deb97368c173ff8733c7","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2019-03-27 09:27:59 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP - nova diagnostics command is not working with all interfaces"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This fixes issue with nova diagnostics command not"},{"line_number":10,"context_line":"working on instances with SR-IOV interfaces."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I8ef852d449e9e637d45e4ac92ffc5d1abd8d31c5"},{"line_number":13,"context_line":"Closes-Bug: #1821798"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_868552af","line":10,"range":{"start_line":8,"start_character":0,"end_line":10,"end_character":44},"updated":"2019-03-27 14:19:16.000000000","message":"i have manually validated this in the past\n\non an instance using macvtap sriov the nic performance counters are present.\n+----------------+--------------------------------------------------------------------------+\n| Property       | Value                                                                    |\n+----------------+--------------------------------------------------------------------------+\n| config_drive   | False                                                                    |\n| cpu_details    | [{\"utilisation\": null, \"id\": 0, \"time\": 3490000000}]                     |\n| disk_details   | [{\"read_requests\": 880, \"errors_count\": -1, \"read_bytes\": 20265984,      |\n|                | \"write_requests\": 73, \"write_bytes\": 260096}]                            |\n| driver         | libvirt                                                                  |\n| hypervisor     | kvm                                                                      |\n| hypervisor_os  | linux                                                                    |\n| memory_details | {\"used\": 0, \"maximum\": 0}                                                |\n| nic_details    | [{\"rx_packets\": 59, \"rx_drop\": 0, \"tx_octets\": 5155, \"tx_errors\": 0,     |\n|                | \"mac_address\": \"fa:16:3e:2c:5f:ef\", \"rx_octets\": 13205, \"rx_rate\": null, |\n|                | \"rx_errors\": 0, \"tx_drop\": 0, \"tx_packets\": 89, \"tx_rate\": null}]        |\n| num_cpus       | 1                                                                        |\n| num_disks      | 1                                                                        |\n| num_nics       | 1                                                                        |\n| state          | running                                                                  |\n| uptime         | 98                                                                       |\n+----------------+--------------------------------------------------------------------------+\n\nand on a instnace spawned with a direct mode sriov port the\nport is omited from the respocne as the hypervior/host does not have access to statistics\nfor the interface\n\n+----------------+---------------------------------------------------------------------+\n| Property       | Value                                                               |\n+----------------+---------------------------------------------------------------------+\n| config_drive   | False                                                               |\n| cpu_details    | [{\"utilisation\": null, \"id\": 0, \"time\": 3190000000}]                |\n| disk_details   | [{\"read_requests\": 816, \"errors_count\": -1, \"read_bytes\": 19089408, |\n|                | \"write_requests\": 36, \"write_bytes\": 142336}]                       |\n| driver         | libvirt                                                             |\n| hypervisor     | kvm                                                                 |\n| hypervisor_os  | linux                                                               |\n| memory_details | {\"used\": 0, \"maximum\": 0}                                           |\n| nic_details    | []                                                                  |\n| num_cpus       | 1                                                                   |\n| num_disks      | 1                                                                   |\n| num_nics       | 0                                                                   |\n| state          | running                                                             |\n| uptime         | 27                                                                  |\n+----------------+---------------------------------------------------------------------+\n\nwhen direct mode sriov is used the virtual fucntion (VF) is allocated directly to the guest and is bound to the vfio-pci\ndirver in the host kernel. when that is done the netdev,the ethX interface that is associated with VF,  is destroyed\nas the vf is nolonger attach to the host networkign stack. the result of this is, when the nova diagnostics\ncommand is executed and the nova driver asks libvirt for the nic statics ther is no netdev for libvirt to retive them from\non the host.\n\n\nas such the issue reported in the bug appears to be hardware /driver specific.\n\ni would guess that if the vf had a netdev before it is allocated it is added to the xml by libvirt after we define the domain and if the vf driver is backlisted on the host such that the vfio-pci driver is use used on the host for the vf it is not filled in.\n\ncan you confirm that before we proceed with this change as currently we are fixing a bug without understanding the root casue as we know this does not always fail.","commit_id":"30065560a12d0cfb34536ac993b353cfa90fb3e2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"83513aaeffc8cfa0bf236d44b02cb5c3bec8ae9e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Francois Palin \u003cfpalin@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-04-08 08:47:40 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP - nova diagnostics command is not working with all interfaces"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This fixes issue with nova diagnostics command not"},{"line_number":10,"context_line":"working on instances with SR-IOV interfaces."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5fc1f717_a1666a50","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":5},"updated":"2019-04-08 13:30:46.000000000","message":"when you remove this can you also add a release not with a fixes section","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"111479e6ed80298d5bab9f027dda2afce263ff91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5fc1f717_6fdcb2e0","line":14,"updated":"2019-04-10 13:56:55.000000000","message":"This should probably be more self-contained. Referencing the bug is nice for more details, but the commit message should be able to stand on its own. So for example:\n\nget_instance_diagnostics expected all interfaces to have a \u003ctarget\u003e element with a \"dev\" attribute in the instance XML. This is not the case for VFIO interfaces (\u003cinterface type\u003d\"hostdev\"\u003e). This caused an IndexError when looping over the interfaces. This patches fixes this by etc, etc.","commit_id":"2d7c789314cfff4eda789cbb033d8f2f0b704534"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"66cb18d0c3ae3925124f14a58736b90e6923e8e7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Francois Palin \u003cfpalin@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-04-12 16:54:01 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"nova diagnostics command is not working with all interfaces"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"get_instance_diagnostics expected all interfaces"},{"line_number":10,"context_line":"to have a \u003ctarget\u003e element with a \"dev\" attribute in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"3fce034c_e2d94483","line":7,"updated":"2019-04-13 00:03:22.000000000","message":"This subject line isn\u0027t great because it vaguely describes the problem, and not this patch. If you respin, please consider improving it :)","commit_id":"fedd02b455be8816e4de800750e348606060193e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"10c6ac786a38ff5b0494a4c05b2d09644514b6c9","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"get_instance_diagnostics expected all interfaces"},{"line_number":10,"context_line":"to have a \u003ctarget\u003e element with a \"dev\" attribute in"},{"line_number":11,"context_line":"the instance XML. This is not the case for VFIO"},{"line_number":12,"context_line":"interfaces (\u003cinterface type\u003d\"hostdev\"\u003e)."},{"line_number":13,"context_line":"This caused an IndexError when looping over"},{"line_number":14,"context_line":"the interfaces."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3fce034c_01a79551","line":11,"range":{"start_line":11,"start_character":18,"end_line":11,"end_character":47},"updated":"2019-04-15 16:36:10.000000000","message":"nit: this is not always the case for VFIO...\n\nit is somethimes added by libvirt but no need to change\nthis ists minor.\n\nbut the fact it si sometime added by libvir ti shwy i could not reproduce it locally as it was added in my test setup.","commit_id":"ab7c968b6f66404c032f62a952e353f94d3be165"}],"nova/tests/functional/api/client.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e77ddb080769f77860d9ba70c070848fdc3a9190","unresolved":false,"context_lines":[{"line_number":528,"context_line":"        return self.api_put(\u0027os-services/%s\u0027 % service_id, req).body[\u0027service\u0027]"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":"    def get_server_diagnostics(self, server_id):"},{"line_number":531,"context_line":"        return self.api_get(\u0027/servers/%s/diagnostics\u0027 % server_id).body"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffb9cba7_8b1018aa","line":531,"updated":"2019-05-01 16:34:19.000000000","message":"nit: you could replace [1] with a call to this helper now.\n\n[1] https://opendev.org/openstack/nova/src/commit/1f74441680b4376cd401ecc0b0449b464cf7a5fb/nova/tests/functional/api_sample_tests/test_server_diagnostics.py#L24","commit_id":"ab7c968b6f66404c032f62a952e353f94d3be165"}],"nova/tests/functional/libvirt/test_pci_sriov_servers.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"83513aaeffc8cfa0bf236d44b02cb5c3bec8ae9e","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        \u0027tenant_id\u0027: nova_fixtures.NeutronFixture.tenant_id,"},{"line_number":233,"context_line":"        \u0027shared\u0027: False,"},{"line_number":234,"context_line":"        \u0027provider:physical_network\u0027: \u0027physnet2\u0027,"},{"line_number":235,"context_line":"        \"provider:network_type\": \"none\","},{"line_number":236,"context_line":"    }"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    subnet_1 \u003d nova_fixtures.NeutronFixture.subnet_1.copy()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_41e226d6","line":235,"range":{"start_line":235,"start_character":8,"end_line":235,"end_character":40},"updated":"2019-04-08 13:30:46.000000000","message":"this will never be none\nif you want something different then flat\nyou can set this to vlan\nin which case you need to set the vlan id\ne.g.\n\u0027provider:segmentation_id\u0027: 42,","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":30009,"name":"François Palin","email":"fpalin@redhat.com","username":"FrancoisPalin"},"change_message_id":"a5cadb6d2baf462b9c17ae84223855a791eb1491","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        \u0027tenant_id\u0027: nova_fixtures.NeutronFixture.tenant_id,"},{"line_number":233,"context_line":"        \u0027shared\u0027: False,"},{"line_number":234,"context_line":"        \u0027provider:physical_network\u0027: \u0027physnet2\u0027,"},{"line_number":235,"context_line":"        \"provider:network_type\": \"none\","},{"line_number":236,"context_line":"    }"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    subnet_1 \u003d nova_fixtures.NeutronFixture.subnet_1.copy()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_d1d7ad22","line":235,"range":{"start_line":235,"start_character":8,"end_line":235,"end_character":40},"in_reply_to":"5fc1f717_41e226d6","updated":"2019-04-09 13:25:28.000000000","message":"Ok, will do","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"83513aaeffc8cfa0bf236d44b02cb5c3bec8ae9e","unresolved":false,"context_lines":[{"line_number":287,"context_line":"                \u0027subnet_id\u0027: subnet_2[\u0027id\u0027]"},{"line_number":288,"context_line":"            }"},{"line_number":289,"context_line":"        ],"},{"line_number":290,"context_line":"        \u0027binding:vif_type\u0027: \u0027ovs\u0027,"},{"line_number":291,"context_line":"        \u0027binding:vnic_type\u0027: \u0027direct\u0027,"},{"line_number":292,"context_line":"        \u0027binding:profile\u0027: {\u0027pci_vendor_info\u0027: \u00271377:0047\u0027,"},{"line_number":293,"context_line":"                            \u0027pci_slot\u0027: \u00270000:81:00.1\u0027,"},{"line_number":294,"context_line":"                            \u0027physical_network\u0027: \u0027physnet2\u0027}"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    }"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_6135e244","line":294,"range":{"start_line":290,"start_character":8,"end_line":294,"end_character":59},"updated":"2019-04-08 13:30:46.000000000","message":"so you are emulating hardware offloaded ovs.\nthis is technically different then standard sriov.\n\nnormally for sriov the vif_type would be \n\u0027hw_veb\u0027 for vnic_type direct.\n\nby the way you can import these constants form \nhttps://github.com/openstack/nova/blob/master/nova/network/model.py#L35","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":30009,"name":"François Palin","email":"fpalin@redhat.com","username":"FrancoisPalin"},"change_message_id":"a5cadb6d2baf462b9c17ae84223855a791eb1491","unresolved":false,"context_lines":[{"line_number":287,"context_line":"                \u0027subnet_id\u0027: subnet_2[\u0027id\u0027]"},{"line_number":288,"context_line":"            }"},{"line_number":289,"context_line":"        ],"},{"line_number":290,"context_line":"        \u0027binding:vif_type\u0027: \u0027ovs\u0027,"},{"line_number":291,"context_line":"        \u0027binding:vnic_type\u0027: \u0027direct\u0027,"},{"line_number":292,"context_line":"        \u0027binding:profile\u0027: {\u0027pci_vendor_info\u0027: \u00271377:0047\u0027,"},{"line_number":293,"context_line":"                            \u0027pci_slot\u0027: \u00270000:81:00.1\u0027,"},{"line_number":294,"context_line":"                            \u0027physical_network\u0027: \u0027physnet2\u0027}"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"    }"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_511e5d0c","line":294,"range":{"start_line":290,"start_character":8,"end_line":294,"end_character":59},"in_reply_to":"5fc1f717_6135e244","updated":"2019-04-09 13:25:28.000000000","message":"Ok, will do that, makes more sense","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c10adb89cfe93773b5a182c20a06b86ac6ed5f5f","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # this copy is here as nova sometimes modifies the returned port"},{"line_number":322,"context_line":"        # locally and we want to avoid that nova modifies the fixture internals"},{"line_number":323,"context_line":"        return {\u0027port\u0027: copy.deepcopy(port)}"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"class GetServerDiagnosticsServerWithVfTestV21(_PCIServersTestBase):"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_e16552b0","line":324,"updated":"2019-04-08 13:25:06.000000000","message":"This looks like it was taken almost wholesale from the NUMAAffinityNeutronFixture fixture in nova/tests/functional/libvirt/test_numa_servers.py. Any reason you couldn\u0027t move that to a common location and reuse it here?","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":30009,"name":"François Palin","email":"fpalin@redhat.com","username":"FrancoisPalin"},"change_message_id":"a5cadb6d2baf462b9c17ae84223855a791eb1491","unresolved":false,"context_lines":[{"line_number":321,"context_line":"        # this copy is here as nova sometimes modifies the returned port"},{"line_number":322,"context_line":"        # locally and we want to avoid that nova modifies the fixture internals"},{"line_number":323,"context_line":"        return {\u0027port\u0027: copy.deepcopy(port)}"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"class GetServerDiagnosticsServerWithVfTestV21(_PCIServersTestBase):"},{"line_number":327,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_11b8d50e","line":324,"in_reply_to":"5fc1f717_e16552b0","updated":"2019-04-09 13:25:28.000000000","message":"I will move the common code to base.py","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c10adb89cfe93773b5a182c20a06b86ac6ed5f5f","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        self.neutron \u003d self.useFixture(PciSriovNeutronFixture(self))"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.LibvirtDriver._create_image\u0027)"},{"line_number":359,"context_line":"    def _create_server_with_VF(self, img_mock):"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        host_info \u003d fakelibvirt.NUMAHostInfo(cpu_nodes\u003d2, cpu_sockets\u003d1,"},{"line_number":362,"context_line":"                                             cpu_cores\u003d2, cpu_threads\u003d2,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_c11616f3","line":359,"range":{"start_line":359,"start_character":0,"end_line":359,"end_character":47},"updated":"2019-04-08 13:25:06.000000000","message":"Any reason this has to be separate?","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":30009,"name":"François Palin","email":"fpalin@redhat.com","username":"FrancoisPalin"},"change_message_id":"a5cadb6d2baf462b9c17ae84223855a791eb1491","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        self.neutron \u003d self.useFixture(PciSriovNeutronFixture(self))"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.LibvirtDriver._create_image\u0027)"},{"line_number":359,"context_line":"    def _create_server_with_VF(self, img_mock):"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        host_info \u003d fakelibvirt.NUMAHostInfo(cpu_nodes\u003d2, cpu_sockets\u003d1,"},{"line_number":362,"context_line":"                                             cpu_cores\u003d2, cpu_threads\u003d2,"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_31f871c2","line":359,"range":{"start_line":359,"start_character":0,"end_line":359,"end_character":47},"in_reply_to":"5fc1f717_c11616f3","updated":"2019-04-09 13:25:28.000000000","message":"I will put this code inside the test method.\n(I guess I had this method initially in class PCIServersTest, and I just copied it a is to this new class)","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"111479e6ed80298d5bab9f027dda2afce263ff91","unresolved":false,"context_lines":[{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self.assertIsNotNone(diagnostics[\u0027nic_details\u0027][0][\u0027tx_packets\u0027])"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        self.assertIsNone(diagnostics[\u0027nic_details\u0027][1][\u0027tx_packets\u0027])"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"class PCIServersTest(_PCIServersTestBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_826a15b0","line":277,"updated":"2019-04-10 13:56:55.000000000","message":"This is probably enough to make sure we\u0027re not breaking the API and still including the empty fields form the target-dev-less interface.","commit_id":"2d7c789314cfff4eda789cbb033d8f2f0b704534"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e77ddb080769f77860d9ba70c070848fdc3a9190","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        self.api.microversion \u003d self.microversion"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        # The ultimate base class _IntegratedTestBase uses NeutronFixture but"},{"line_number":231,"context_line":"        # we need a bit more intelligent neutron for these tests. Applying the"},{"line_number":232,"context_line":"        # new fixture here means that we re-stub what the previous neutron"},{"line_number":233,"context_line":"        # fixture already stubbed."},{"line_number":234,"context_line":"        self.neutron \u003d self.useFixture(base.LibvirtNeutronFixture(self))"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def test_get_server_diagnostics_server_with_VF(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ffb9cba7_2b6ecc2e","line":233,"range":{"start_line":231,"start_character":66,"end_line":233,"end_character":34},"updated":"2019-05-01 16:34:19.000000000","message":"ack","commit_id":"ab7c968b6f66404c032f62a952e353f94d3be165"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0802844b2eb0c40cf356deb97368c173ff8733c7","unresolved":false,"context_lines":[{"line_number":9142,"context_line":"                # is not supported by the underlying hypervisor being"},{"line_number":9143,"context_line":"                # used by libvirt"},{"line_number":9144,"context_line":"                stats \u003d domain.interfaceStats(interface)"},{"line_number":9145,"context_line":""},{"line_number":9146,"context_line":"                for dev_interface in xml_doc.findall(\u0027./devices/interface\u0027):"},{"line_number":9147,"context_line":"                    target \u003d dev_interface.find(\u0027./target\u0027)"},{"line_number":9148,"context_line":"                    if target is None:"},{"line_number":9149,"context_line":"                        continue"},{"line_number":9150,"context_line":""},{"line_number":9151,"context_line":"                    if target.get(\u0027dev\u0027) \u003d\u003d interface:"},{"line_number":9152,"context_line":"                        mac_address \u003d dev_interface.find(\u0027mac\u0027).get(\u0027address\u0027)"},{"line_number":9153,"context_line":"                        diags.add_nic(mac_address\u003dmac_address,"},{"line_number":9154,"context_line":"                                      rx_octets\u003dstats[0],"},{"line_number":9155,"context_line":"                                      rx_errors\u003dstats[2],"},{"line_number":9156,"context_line":"                                      rx_drop\u003dstats[3],"},{"line_number":9157,"context_line":"                                      rx_packets\u003dstats[1],"},{"line_number":9158,"context_line":"                                      tx_octets\u003dstats[4],"},{"line_number":9159,"context_line":"                                      tx_errors\u003dstats[6],"},{"line_number":9160,"context_line":"                                      tx_drop\u003dstats[7],"},{"line_number":9161,"context_line":"                                      tx_packets\u003dstats[5])"},{"line_number":9162,"context_line":"            except libvirt.libvirtError:"},{"line_number":9163,"context_line":"                pass"},{"line_number":9164,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_e9f1876c","line":9161,"range":{"start_line":9145,"start_character":0,"end_line":9161,"end_character":58},"updated":"2019-03-27 14:19:16.000000000","message":"this seam very inefficient.\nwe are already looping over each of the interfaces on line 9139 and now for each interface we retrieve all the xml element again and loop over them.\n\nhttps://github.com/openstack/nova/blob/771e102654b54d257f437f16be2aeeef51fa0783/nova/virt/libvirt/driver.py#L9009-L9024\n\ninterface containts the same content as dev_interface.\n\n\ninstead you can just do \n\ntarget \u003d interface.find(\u0027./target\u0027)\nif target is none:\n  continue\nstats \u003d domain.interfaceStats(interface)\ndiags.add_nic(rx_octets\u003dstats[0],\n              rx_errors\u003dstats[2],\n              rx_drop\u003dstats[3],\n              rx_packets\u003dstats[1],\n              tx_octets\u003dstats[4],\n              tx_errors\u003dstats[6],\n              tx_drop\u003dstats[7],\n              tx_packets\u003dstats[5])","commit_id":"30065560a12d0cfb34536ac993b353cfa90fb3e2"},{"author":{"_account_id":30009,"name":"François Palin","email":"fpalin@redhat.com","username":"FrancoisPalin"},"change_message_id":"949ed875d12879689ebd142cfa22cbcfbe5f80f7","unresolved":false,"context_lines":[{"line_number":9142,"context_line":"                # is not supported by the underlying hypervisor being"},{"line_number":9143,"context_line":"                # used by libvirt"},{"line_number":9144,"context_line":"                stats \u003d domain.interfaceStats(interface)"},{"line_number":9145,"context_line":""},{"line_number":9146,"context_line":"                for dev_interface in xml_doc.findall(\u0027./devices/interface\u0027):"},{"line_number":9147,"context_line":"                    target \u003d dev_interface.find(\u0027./target\u0027)"},{"line_number":9148,"context_line":"                    if target is None:"},{"line_number":9149,"context_line":"                        continue"},{"line_number":9150,"context_line":""},{"line_number":9151,"context_line":"                    if target.get(\u0027dev\u0027) \u003d\u003d interface:"},{"line_number":9152,"context_line":"                        mac_address \u003d dev_interface.find(\u0027mac\u0027).get(\u0027address\u0027)"},{"line_number":9153,"context_line":"                        diags.add_nic(mac_address\u003dmac_address,"},{"line_number":9154,"context_line":"                                      rx_octets\u003dstats[0],"},{"line_number":9155,"context_line":"                                      rx_errors\u003dstats[2],"},{"line_number":9156,"context_line":"                                      rx_drop\u003dstats[3],"},{"line_number":9157,"context_line":"                                      rx_packets\u003dstats[1],"},{"line_number":9158,"context_line":"                                      tx_octets\u003dstats[4],"},{"line_number":9159,"context_line":"                                      tx_errors\u003dstats[6],"},{"line_number":9160,"context_line":"                                      tx_drop\u003dstats[7],"},{"line_number":9161,"context_line":"                                      tx_packets\u003dstats[5])"},{"line_number":9162,"context_line":"            except libvirt.libvirtError:"},{"line_number":9163,"context_line":"                pass"},{"line_number":9164,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_4f2c56ab","line":9161,"range":{"start_line":9145,"start_character":0,"end_line":9161,"end_character":58},"in_reply_to":"5fc1f717_e9f1876c","updated":"2019-03-27 20:21:02.000000000","message":"Method _get_io_devices() only appends to its returned dict the actual value for target dev.\nExample returned dict:  {\u0027volumes\u0027: [\u0027hda\u0027, \u0027vda\u0027, \u0027vdb\u0027, \u0027vdc\u0027], \u0027ifaces\u0027: [\u0027tap274487d1-60\u0027]}\n\nSo in this context, in this loop (https://github.com/openstack/nova/blob/771e102654b54d257f437f16be2aeeef51fa0783/nova/virt/libvirt/driver.py#L9139):\n        for interface in dom_io[\"ifaces\"]:\ninterface will be \u0027tap274487d1-60\u0027, and not one of the various xml interfaces with associated sub-elements.\n\nAnd this is the reason why the proposed code change reads the domain xml, finds the matching mac address for the given target dev, and adds the mac address directly in the add_nic call, instead of doing it in a separate loop with unreliable indexing. \n\nI agree that the code could be made more compact and treat all cases (interfaces with/without target dev) in a single for loop.","commit_id":"30065560a12d0cfb34536ac993b353cfa90fb3e2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0802844b2eb0c40cf356deb97368c173ff8733c7","unresolved":false,"context_lines":[{"line_number":9163,"context_line":"                pass"},{"line_number":9164,"context_line":""},{"line_number":9165,"context_line":"        # add nic for interfaces that don\u0027t have target dev"},{"line_number":9166,"context_line":"        for dev_interface in xml_doc.findall(\u0027./devices/interface\u0027):"},{"line_number":9167,"context_line":"            target \u003d dev_interface.find(\u0027./target\u0027)"},{"line_number":9168,"context_line":"            if target is None:"},{"line_number":9169,"context_line":"                mac_address \u003d dev_interface.find(\u0027mac\u0027).get(\u0027address\u0027)"},{"line_number":9170,"context_line":"                diags.add_nic(mac_address\u003dmac_address)"},{"line_number":9171,"context_line":""},{"line_number":9172,"context_line":"        return diags"},{"line_number":9173,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_e9bf475e","line":9170,"range":{"start_line":9166,"start_character":7,"end_line":9170,"end_character":54},"updated":"2019-03-27 14:19:16.000000000","message":"this change should not be needed.","commit_id":"30065560a12d0cfb34536ac993b353cfa90fb3e2"},{"author":{"_account_id":30009,"name":"François Palin","email":"fpalin@redhat.com","username":"FrancoisPalin"},"change_message_id":"949ed875d12879689ebd142cfa22cbcfbe5f80f7","unresolved":false,"context_lines":[{"line_number":9163,"context_line":"                pass"},{"line_number":9164,"context_line":""},{"line_number":9165,"context_line":"        # add nic for interfaces that don\u0027t have target dev"},{"line_number":9166,"context_line":"        for dev_interface in xml_doc.findall(\u0027./devices/interface\u0027):"},{"line_number":9167,"context_line":"            target \u003d dev_interface.find(\u0027./target\u0027)"},{"line_number":9168,"context_line":"            if target is None:"},{"line_number":9169,"context_line":"                mac_address \u003d dev_interface.find(\u0027mac\u0027).get(\u0027address\u0027)"},{"line_number":9170,"context_line":"                diags.add_nic(mac_address\u003dmac_address)"},{"line_number":9171,"context_line":""},{"line_number":9172,"context_line":"        return diags"},{"line_number":9173,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_cf11265f","line":9170,"range":{"start_line":9166,"start_character":7,"end_line":9170,"end_character":54},"in_reply_to":"5fc1f717_e9bf475e","updated":"2019-03-27 20:21:02.000000000","message":"I agree that the code could be made more compact and treat all cases (interfaces with/without target dev) in a single for loop.","commit_id":"30065560a12d0cfb34536ac993b353cfa90fb3e2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"964356d849df0ae5be37b14f5f57051ef8c84589","unresolved":false,"context_lines":[{"line_number":9146,"context_line":"                diags.add_nic(mac_address\u003dmac_address)"},{"line_number":9147,"context_line":"                continue"},{"line_number":9148,"context_line":""},{"line_number":9149,"context_line":"            # add nic with stats"},{"line_number":9150,"context_line":"            # note that iface is actually the interface target dev value"},{"line_number":9151,"context_line":"            for iface in dom_io[\"ifaces\"]:"},{"line_number":9152,"context_line":"                try:"},{"line_number":9153,"context_line":"                    if target.get(\u0027dev\u0027) \u003d\u003d iface:"},{"line_number":9154,"context_line":"                        # interfaceStats might launch an exception if the"},{"line_number":9155,"context_line":"                        # method is not supported by the underlying hypervisor"},{"line_number":9156,"context_line":"                        # being used by libvirt"},{"line_number":9157,"context_line":"                        stats \u003d domain.interfaceStats(iface)"},{"line_number":9158,"context_line":""},{"line_number":9159,"context_line":"                        diags.add_nic(mac_address\u003dmac_address,"},{"line_number":9160,"context_line":"                                      rx_octets\u003dstats[0],"},{"line_number":9161,"context_line":"                                      rx_errors\u003dstats[2],"},{"line_number":9162,"context_line":"                                      rx_drop\u003dstats[3],"},{"line_number":9163,"context_line":"                                      rx_packets\u003dstats[1],"},{"line_number":9164,"context_line":"                                      tx_octets\u003dstats[4],"},{"line_number":9165,"context_line":"                                      tx_errors\u003dstats[6],"},{"line_number":9166,"context_line":"                                      tx_drop\u003dstats[7],"},{"line_number":9167,"context_line":"                                      tx_packets\u003dstats[5])"},{"line_number":9168,"context_line":""},{"line_number":9169,"context_line":"                except libvirt.libvirtError:"},{"line_number":9170,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_6e1ee70c","line":9167,"range":{"start_line":9149,"start_character":11,"end_line":9167,"end_character":58},"updated":"2019-03-28 12:10:12.000000000","message":"this is again quadratic you want to have a break\nout of the innerl loop here at a minium\n\nideally you would loop over the interces in a domain once and only once.\n\nwhy not just do\n\ndev \u003d target.get(\u0027dev\u0027)\nif dev:\n    stats \u003d domain.interfaceStats(dev)\n    diags.add_nic(mac_address\u003dmac_address,\n                  rx_octets\u003dstats[0],\n                  rx_errors\u003dstats[2],\n                  rx_drop\u003dstats[3],\n                  rx_packets\u003dstats[1],\n                  tx_octets\u003dstats[4],\n                  tx_errors\u003dstats[6],\n                  tx_drop\u003dstats[7],\n                  tx_packets\u003dstats[5])","commit_id":"32efb549109ad7fc7c4f816abe09c44619064408"},{"author":{"_account_id":30009,"name":"François Palin","email":"fpalin@redhat.com","username":"FrancoisPalin"},"change_message_id":"22ffaf3350f606a1253ffe2e101c664e66ccc857","unresolved":false,"context_lines":[{"line_number":9146,"context_line":"                diags.add_nic(mac_address\u003dmac_address)"},{"line_number":9147,"context_line":"                continue"},{"line_number":9148,"context_line":""},{"line_number":9149,"context_line":"            # add nic with stats"},{"line_number":9150,"context_line":"            # note that iface is actually the interface target dev value"},{"line_number":9151,"context_line":"            for iface in dom_io[\"ifaces\"]:"},{"line_number":9152,"context_line":"                try:"},{"line_number":9153,"context_line":"                    if target.get(\u0027dev\u0027) \u003d\u003d iface:"},{"line_number":9154,"context_line":"                        # interfaceStats might launch an exception if the"},{"line_number":9155,"context_line":"                        # method is not supported by the underlying hypervisor"},{"line_number":9156,"context_line":"                        # being used by libvirt"},{"line_number":9157,"context_line":"                        stats \u003d domain.interfaceStats(iface)"},{"line_number":9158,"context_line":""},{"line_number":9159,"context_line":"                        diags.add_nic(mac_address\u003dmac_address,"},{"line_number":9160,"context_line":"                                      rx_octets\u003dstats[0],"},{"line_number":9161,"context_line":"                                      rx_errors\u003dstats[2],"},{"line_number":9162,"context_line":"                                      rx_drop\u003dstats[3],"},{"line_number":9163,"context_line":"                                      rx_packets\u003dstats[1],"},{"line_number":9164,"context_line":"                                      tx_octets\u003dstats[4],"},{"line_number":9165,"context_line":"                                      tx_errors\u003dstats[6],"},{"line_number":9166,"context_line":"                                      tx_drop\u003dstats[7],"},{"line_number":9167,"context_line":"                                      tx_packets\u003dstats[5])"},{"line_number":9168,"context_line":""},{"line_number":9169,"context_line":"                except libvirt.libvirtError:"},{"line_number":9170,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_25b31d9a","line":9167,"range":{"start_line":9149,"start_character":11,"end_line":9167,"end_character":58},"in_reply_to":"5fc1f717_6e1ee70c","updated":"2019-03-28 14:00:47.000000000","message":"ok, so you are suggesting to just ignore dom_io[\"ifaces\"] and go\nstraight for retrieving stats with dev value. Yes, this makes sense.","commit_id":"32efb549109ad7fc7c4f816abe09c44619064408"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"83513aaeffc8cfa0bf236d44b02cb5c3bec8ae9e","unresolved":false,"context_lines":[{"line_number":9137,"context_line":"                               errors_count\u003dstats[4])"},{"line_number":9138,"context_line":"            except libvirt.libvirtError:"},{"line_number":9139,"context_line":"                pass"},{"line_number":9140,"context_line":""},{"line_number":9141,"context_line":"        for interface in xml_doc.findall(\u0027./devices/interface\u0027):"},{"line_number":9142,"context_line":"            mac_address \u003d interface.find(\u0027mac\u0027).get(\u0027address\u0027)"},{"line_number":9143,"context_line":"            target \u003d interface.find(\u0027./target\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_01a2de9a","line":9140,"updated":"2019-04-08 13:30:46.000000000","message":"yep this looks better to me thanks for reworking.","commit_id":"43ebac0bbf58fe5fe6bf9e6c2ededc396b8b651d"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"111479e6ed80298d5bab9f027dda2afce263ff91","unresolved":false,"context_lines":[{"line_number":9083,"context_line":"            pass"},{"line_number":9084,"context_line":"        # get io status"},{"line_number":9085,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":9086,"context_line":""},{"line_number":9087,"context_line":"        dom_io \u003d LibvirtDriver._get_io_devices(xml)"},{"line_number":9088,"context_line":"        for guest_disk in dom_io[\"volumes\"]:"},{"line_number":9089,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_0fde4e10","line":9086,"updated":"2019-04-10 13:56:55.000000000","message":"Random whitespace?","commit_id":"2d7c789314cfff4eda789cbb033d8f2f0b704534"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"111479e6ed80298d5bab9f027dda2afce263ff91","unresolved":false,"context_lines":[{"line_number":9186,"context_line":""},{"line_number":9187,"context_line":"            # add nic that has no target (therefore no stats)"},{"line_number":9188,"context_line":"            if target is None:"},{"line_number":9189,"context_line":"                diags.add_nic(mac_address\u003dmac_address)"},{"line_number":9190,"context_line":"                continue"},{"line_number":9191,"context_line":""},{"line_number":9192,"context_line":"            # add nic with stats"}],"source_content_type":"text/x-python","patch_set":4,"id":"5fc1f717_0f1bae80","line":9189,"updated":"2019-04-10 13:56:55.000000000","message":"This will show up in the API as the nic_details field with everything null except the MAC address, which I think is fine and doesn\u0027t need a new microversion. I\u0027ll let Alex confirm.","commit_id":"2d7c789314cfff4eda789cbb033d8f2f0b704534"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"8fc1729041a64b5aa92901072e848d9850904ca8","unresolved":false,"context_lines":[{"line_number":9186,"context_line":""},{"line_number":9187,"context_line":"            # add nic that has no target (therefore no stats)"},{"line_number":9188,"context_line":"            if target is None:"},{"line_number":9189,"context_line":"                diags.add_nic(mac_address\u003dmac_address)"},{"line_number":9190,"context_line":"                continue"},{"line_number":9191,"context_line":""},{"line_number":9192,"context_line":"            # add nic with stats"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fce034c_e68742b6","line":9189,"in_reply_to":"5fc1f717_0f1bae80","updated":"2019-04-11 02:41:23.000000000","message":"This change does not seems to break API contract, so i agree with Artom that it does not need new microversion.\n\nWhen server diagnostics was made standard format in microversion 2.48, it was decided that all the fields have to be present 1. either empty list (for list type fields like nic_details) 2. of not empty list then, all sub-fields has to be present with their respective values or null[1].\n\n1st point is done by API controller method side [2] and 2nd by add_nic() method[3]\n\nFor testing side, Tempest schema verify those behaviours [4] and so does tempest-full-py pass for this change.\n\n[1] https://specs.openstack.org/openstack/nova-specs/specs/pike/implemented/restore-vm-diagnostics.html#proposed-change \n\n[2] https://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/api/openstack/compute/views/server_diagnostics.py#L46\n\n[3] https://github.com/openstack/nova/blob/d42a007425d9adb691134137e1e0b7dda356df62/nova/objects/diagnostics.py#L144\n\n[4] https://github.com/openstack/tempest/blob/3c5b61396dca81ef1cd81e8d1e93cf12bea6b81f/tempest/lib/api_schema/response/compute/v2_48/servers.py#L83","commit_id":"2d7c789314cfff4eda789cbb033d8f2f0b704534"}]}