)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f5b832930337fbe343a0860481f5eead577e79be","unresolved":false,"context_lines":[{"line_number":7,"context_line":"[WIP] Support SRIOV interface attach and detach"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * make the bp approved for V"},{"line_number":11,"context_line":" * test coverage"},{"line_number":12,"context_line":" * doc"},{"line_number":13,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf51134e_f852df34","line":10,"range":{"start_line":10,"start_character":0,"end_line":10,"end_character":29},"updated":"2020-07-22 16:03:24.000000000","message":"Done","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8e7c2090e8e32dca3c3a6d60b0936796333b0bb","unresolved":false,"context_lines":[{"line_number":10,"context_line":" * make the bp approved for V"},{"line_number":11,"context_line":" * test coverage"},{"line_number":12,"context_line":" * doc"},{"line_number":13,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"},{"line_number":14,"context_line":"     qemu 2.11 does not fully work, but libvirt 6.0.0 and qemu 4.2"},{"line_number":15,"context_line":"     works."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Part of blueprint sriov-interface-attach-detach"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf51134e_59ea4673","line":14,"range":{"start_line":13,"start_character":5,"end_line":14,"end_character":34},"updated":"2020-07-16 18:43:19.000000000","message":"strange i would have expected this to work with any suported libvirt/qemu that meets our minium versions.","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"28a0bbf94b5b97d6ec8aeef6633ea5fd422d7f0d","unresolved":false,"context_lines":[{"line_number":10,"context_line":" * make the bp approved for V"},{"line_number":11,"context_line":" * test coverage"},{"line_number":12,"context_line":" * doc"},{"line_number":13,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"},{"line_number":14,"context_line":"     qemu 2.11 does not fully work, but libvirt 6.0.0 and qemu 4.2"},{"line_number":15,"context_line":"     works."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Part of blueprint sriov-interface-attach-detach"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf51134e_aee7c8ac","line":14,"range":{"start_line":13,"start_character":5,"end_line":14,"end_character":34},"in_reply_to":"bf51134e_59ea4673","updated":"2020-07-17 15:57:33.000000000","message":"Me too. I got a statement from danpb[1] that it is a bug so I will not sweat on it much but just note it in the documentation that there can be bugs. \n\nAlso our downstream team hit by another bug in recent qemu that leads to segfault during detach but it is already fixed. [2]\n\n[1] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2020-07-14.log.html#t2020-07-14T09:40:51\n[2] https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1867519","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"df00be6c2324b24bd2a975050c0be356aa40415f","unresolved":false,"context_lines":[{"line_number":10,"context_line":" * make the bp approved for V"},{"line_number":11,"context_line":" * test coverage"},{"line_number":12,"context_line":" * doc"},{"line_number":13,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"},{"line_number":14,"context_line":"     qemu 2.11 does not fully work, but libvirt 6.0.0 and qemu 4.2"},{"line_number":15,"context_line":"     works."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Part of blueprint sriov-interface-attach-detach"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf51134e_04a7b3a9","line":14,"range":{"start_line":13,"start_character":5,"end_line":14,"end_character":34},"in_reply_to":"bf51134e_aee7c8ac","updated":"2020-07-17 17:19:15.000000000","message":"oh yes the qemu crash\n\nya that is a think that we have fixed doenwtream and backported to older qemu versions\nthat makes sense i forgot about that.","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3806a880a32ac9b805434939abd83e4e7124b822","unresolved":false,"context_lines":[{"line_number":7,"context_line":"[WIP] Support SRIOV interface attach and detach"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_b94e94be","line":10,"updated":"2020-07-23 12:43:11.000000000","message":"I could do something similar with a simple vnic_type\u003ddirect port. If I fast enough with an attach - detach sequence then the state of the PCI device goes from available -\u003e claimed but does not go to allocated . If I hit detach when the PCI device still just in claimed then the PCI device is not freed. Instead after a periodic run it goes to allocated regardless of the successful detach. If it was the last PCI device available on the host then the subsequent attach will fail as there is no free device for it. \n\nI will look into the code to find the reason...","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ea1a4e2223736993da898589947eb21556018b47","unresolved":false,"context_lines":[{"line_number":7,"context_line":"[WIP] Support SRIOV interface attach and detach"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_8db0aa8d","line":10,"in_reply_to":"bf51134e_b94e94be","updated":"2020-07-23 16:38:17.000000000","message":"I think I\u0027m solved this by not just claiming the pci device but also allocating it during the interface attach call. I cannot reproduce the problem any more. @Sean: could you please check if you can still see the problem with the latest PS?","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"01f96c6d1a5fe32f367ff4ef46e0724425d44c8d","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_a6be88fd","line":11,"updated":"2020-07-22 16:04:10.000000000","message":"So far I could not reproduce this issue with vnic_type\u003ddirect ports http://paste.openstack.org/show/796216/","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"abc59971027c085f699a0e6f4e2fdb9e81dbaf53","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_5a70f408","line":11,"in_reply_to":"bf51134e_2907c41f","updated":"2020-07-24 13:38:47.000000000","message":"The driver fails to find the interface in the domain to be detached based on the vif. The domina has a target_dev filled but the config generated from the vif does not.\n\nhttps://github.com/openstack/nova/blob/4a925cf01ac6ca313ff10c3075a86d65095de299/nova/virt/libvirt/guest.py#L246\n\nhttp://paste.openstack.org/show/796287/\n\n(I\u0027m writing this monologue as I will be off for a week and probably will forget most of what I found this week)","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e61628f4c6bc3cfd02473d895fb95d0692a2cbc9","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_7a7fb8aa","line":11,"in_reply_to":"bf51134e_5a70f408","updated":"2020-07-24 13:51:47.000000000","message":"ether target_dev is populated depends on your libvirt version and host os i think. we have had issue with it in the past.\nits not really reliable we have had issue with it for the nova diagnostics command.\n\nreally we shoudl avoid using the mac and use the pci adress in all likely hood althoguht hat would only apply to sriov interfacs.","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"939e477a72dae230b8dc5ab44ff4237d70ac907e","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_ab3625b3","line":11,"in_reply_to":"bf51134e_7a7fb8aa","updated":"2020-07-24 14:48:47.000000000","message":"\u003e ether target_dev is populated depends on your libvirt version and\n \u003e host os i think. we have had issue with it in the past.\n \u003e its not really reliable we have had issue with it for the nova\n \u003e diagnostics command.\n \u003e \n \u003e really we shoudl avoid using the mac and use the pci adress in all\n \u003e likely hood althoguht hat would only apply to sriov interfacs.\n\nSo we should have two branches: \n* for sriov devs (direct, macvtap, direct-physical) try to find the device in the domain based on PCI address\n* for non sriov devs (vnic_type\u003dnormal) use the current code that tries to find the device in the domain based on MAC address","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e48fb3a4f3085917652a30ebb3ce074c6132e193","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_eb52dd11","line":11,"in_reply_to":"bf51134e_7a7fb8aa","updated":"2020-07-24 14:45:58.000000000","message":"PS5 solves the macvtap detach case but as Sean foresee it does not work for the direct-physical case as no \u003cinterface\u003e is in the domain for direct-physical. \n\n(Pdb) l\n223  \t\n224  \t        if cfg:\n225  \t            interfaces \u003d self.get_all_devices(\n226  \t                vconfig.LibvirtConfigGuestInterface)\n227  \t            import pdb; pdb.set_trace()\n228  -\u003e\t            for interface in interfaces:\n229  \t                # NOTE(leehom) LibvirtConfigGuestInterface get from domain and\n230  \t                # LibvirtConfigGuestInterface generated by\n231  \t                # nova.virt.libvirt.vif.get_config must be identical.\n232  \t                # NOTE(arches) Skip checking target_dev for vhostuser\n233  \t                # vif type; target_dev is not a valid value for vhostuser.\n(Pdb) p interfaces\n[]\n(Pdb)","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c10347f8ceced2b4373a05b702cc79f9ad9e0b24","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_e9de6c3c","line":11,"in_reply_to":"bf51134e_8d6b8a1d","updated":"2020-07-24 10:09:25.000000000","message":"I think this is a rabbit hole. Even if the MAC is left on the VF after the neutron port in unbound, that VF is in DOWN state. Whenever that VF is pulled UP, the client that does the UPing should make sure the the VF is configured properly. OpenStack does that during bind so I think we are safe.","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ea1a4e2223736993da898589947eb21556018b47","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_8d6b8a1d","line":11,"in_reply_to":"bf51134e_994910b9","updated":"2020-07-23 16:38:17.000000000","message":"With vnic_type\u003dmacvtap I think I see what you saw.\n1) booted a VM with a macvtap interface. it set the MAC of the neutron port to the interface\n2) I deleted the VM and the interface on the host still has the MAC of the neutron port\n\nWith this procedure I can create a situation when a host has more VF interfaces with the same MAC, even.\n\nHowever this is not caused by the current patch. As I can create this issue with simply booting and deleting VMs. So something with the macvtap handling might be incorrect in the baseline. I probably need help with this from you, Sean.","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3806a880a32ac9b805434939abd83e4e7124b822","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_994910b9","line":11,"in_reply_to":"bf51134e_a6be88fd","updated":"2020-07-23 12:43:11.000000000","message":"Here I\u0027m planning to blacklisting the VF driver on my host as it might be that when the VF is attached back to the host the driver on the host does the cleanup of the MAC in my case.","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"168f511d656d69e84737aa3d6e8a889a6908d3d4","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_9ac39c38","line":11,"in_reply_to":"bf51134e_ab3625b3","updated":"2020-08-03 16:26:19.000000000","message":"matching based only on MAC address is not enough the current complex condition is added to fix a bug where two interfaces had the same MAC https://review.opendev.org/#/c/372243/25\n\nI go into the direction where both LibvirtConfigGuestHostdevPCI and LibvirtConfigGuestInterface knows how to match vif generated config with libvirt returned config.\n\nby this I was able to resolve the issue with direct-physical detach","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"92a7919d249ed79f36854707cff73575f8a0680e","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_2907c41f","line":11,"in_reply_to":"bf51134e_e9366c51","updated":"2020-07-24 10:57:43.000000000","message":"Moreover the detach interface for a macvtap port does not remove the device definition of the interface from the domain xml either. This explains why libvirt does not remove the tap from the host.\n\nThere is a WARNING during detach of a macvtap port:\n\nJul 24 10:55:17 master0 nova-compute[38941]: WARNING nova.virt.libvirt.driver [None req-ed883189-378f-4712-af4b-e43cfa1fd31b admin admin] [instance: 9fe5ecca-9213-424c-8fe3-c64e4ae62cdd] Detaching interface fa:16:3e:67:31:da failed because the device is no longer found on the guest.\n\nOriginating from [1]. So I assume there is a bug in the libvirt driver regarding macvtap detach.\n\n[1] https://github.com/openstack/nova/blob/4a925cf01ac6ca313ff10c3075a86d65095de299/nova/virt/libvirt/driver.py#L2201-L2213","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6987b16d12ae49e3601376357002b39c6604e409","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"TODO:"},{"line_number":10,"context_line":" * macvtap attach - detach - attach failute"},{"line_number":11,"context_line":" * after detach the VF should loose the neutron MAC"},{"line_number":12,"context_line":" * test coverage"},{"line_number":13,"context_line":" * doc"},{"line_number":14,"context_line":"   * Specify which libvirt / qemu version is needed as libvirt 4.0.0 and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"bf51134e_e9366c51","line":11,"in_reply_to":"bf51134e_e9de6c3c","updated":"2020-07-24 10:29:58.000000000","message":"Bah, there are other macvtap related issue (maybe this one what you, Sean, reported originally). Detach interface does not remove the macvtap interface, just server delete removes the mavctap interface from the host.\n\nhttp://paste.openstack.org/show/796280/","commit_id":"ba94f7ab82bedae172dfb197a059a404cf0bce6c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":13,"context_line":"For detach:"},{"line_number":14,"context_line":"* Frees PciDevice and deletes the InstancePciRequests"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"On the libvirt driver side the following small fixes was necessar:"},{"line_number":17,"context_line":"* Fixes PCI address generation to avoid double 0x prefixes in LibvirtConfigGuestHostdevPCI"},{"line_number":18,"context_line":"* Adds support for comparing LibvirtConfigGuestHostdevPCI objects"},{"line_number":19,"context_line":"* Extends the comparison of LibvirtConfigGuestInterface to support"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"9f560f44_1cb94778","line":16,"range":{"start_line":16,"start_character":53,"end_line":16,"end_character":65},"updated":"2020-09-07 16:30:28.000000000","message":"were necessary","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":20,"context_line":"  macvtap interfaces where target_dev is only known by libvirt but not"},{"line_number":21,"context_line":"  nova"},{"line_number":22,"context_line":"* generalize guest.get_interface_by_cfg() to work with both"},{"line_number":23,"context_line":"  LibvirtConfigGuest[Inteface|HostdevPCI] objects"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Implements: blueprint sriov-interface-attach-detach"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"9f560f44_7cb1638f","line":23,"range":{"start_line":23,"start_character":21,"end_line":23,"end_character":29},"updated":"2020-09-07 16:30:28.000000000","message":"Interface","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"doc/source/admin/pci-passthrough.rst":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"   **Limitations**"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"   * Attaching SR-IOV ports to existing servers was not supported until the"},{"line_number":33,"context_line":"     22.0.0 Victoria release. Due to various bugs in libvirt and qemu we"},{"line_number":34,"context_line":"     recommend to use at least libvirt version 6.0.0 and at least qemu version"},{"line_number":35,"context_line":"     4.2."},{"line_number":36,"context_line":"   * Cold migration (resize) of servers with SR-IOV devices attached was not"},{"line_number":37,"context_line":"     supported until the 14.0.0 Newton release, see"},{"line_number":38,"context_line":"     `bug 1512800 \u003chttps://bugs.launchpad.net/nova/+bug/1512880\u003e`_ for details."}],"source_content_type":"text/x-rst","patch_set":17,"id":"9f560f44_9f9a6e6c","line":35,"range":{"start_line":32,"start_character":1,"end_line":35,"end_character":9},"updated":"2020-09-07 11:18:22.000000000","message":"note for others this is primary due to a qemu crash on interface detach that is fixed in later releases.\nat least downstream this is backported so on rhel/centos older release might be ok but this is why this is a recommendation rather then a requirement.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"   **Limitations**"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"   * Attaching SR-IOV ports to existing servers was not supported until the"},{"line_number":33,"context_line":"     22.0.0 Victoria release. Due to various bugs in libvirt and qemu we"},{"line_number":34,"context_line":"     recommend to use at least libvirt version 6.0.0 and at least qemu version"},{"line_number":35,"context_line":"     4.2."},{"line_number":36,"context_line":"   * Cold migration (resize) of servers with SR-IOV devices attached was not"},{"line_number":37,"context_line":"     supported until the 14.0.0 Newton release, see"},{"line_number":38,"context_line":"     `bug 1512800 \u003chttps://bugs.launchpad.net/nova/+bug/1512880\u003e`_ for details."}],"source_content_type":"text/x-rst","patch_set":17,"id":"9f560f44_6ff4e35b","line":35,"range":{"start_line":32,"start_character":1,"end_line":35,"end_character":9},"in_reply_to":"9f560f44_9f9a6e6c","updated":"2020-09-09 15:30:59.000000000","message":"Ack, thanks","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"nova/api/openstack/compute/attach_interfaces.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                exception.SecurityGroupCannotBeApplied,"},{"line_number":176,"context_line":"                exception.NetworkInterfaceTaggedAttachNotSupported,"},{"line_number":177,"context_line":"                exception.NetworksWithQoSPolicyNotSupported,"},{"line_number":178,"context_line":"                exception.InterfaceAttachPciClaimFailed) as e:"},{"line_number":179,"context_line":"            raise exc.HTTPBadRequest(explanation\u003de.format_message())"},{"line_number":180,"context_line":"        except (exception.InstanceIsLocked,"},{"line_number":181,"context_line":"                exception.FixedIpAlreadyInUse,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_4a310ded","line":178,"updated":"2020-09-09 15:30:59.000000000","message":"note to self : we don\u0027t modify an API behaviour, we just make sure we provide a good HTTP4Ox return for a new verification","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"nova/compute/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8e7c2090e8e32dca3c3a6d60b0936796333b0bb","unresolved":false,"context_lines":[{"line_number":7399,"context_line":""},{"line_number":7400,"context_line":"    def _deallocate_port_for_instance("},{"line_number":7401,"context_line":"            self, context, instance, port_id, raise_on_failure\u003dFalse,"},{"line_number":7402,"context_line":"            pci_devices\u003dNone):"},{"line_number":7403,"context_line":"        try:"},{"line_number":7404,"context_line":"            result \u003d self.network_api.deallocate_port_for_instance("},{"line_number":7405,"context_line":"                context, instance, port_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_799c6a0d","line":7402,"range":{"start_line":7402,"start_character":12,"end_line":7402,"end_character":23},"updated":"2020-07-16 18:43:19.000000000","message":"will this ever be plural?\npci_device?","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f5b832930337fbe343a0860481f5eead577e79be","unresolved":false,"context_lines":[{"line_number":7399,"context_line":""},{"line_number":7400,"context_line":"    def _deallocate_port_for_instance("},{"line_number":7401,"context_line":"            self, context, instance, port_id, raise_on_failure\u003dFalse,"},{"line_number":7402,"context_line":"            pci_devices\u003dNone):"},{"line_number":7403,"context_line":"        try:"},{"line_number":7404,"context_line":"            result \u003d self.network_api.deallocate_port_for_instance("},{"line_number":7405,"context_line":"                context, instance, port_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_3885775a","line":7402,"range":{"start_line":7402,"start_character":12,"end_line":7402,"end_character":23},"in_reply_to":"bf51134e_799c6a0d","updated":"2020-07-22 16:03:24.000000000","message":"You are right. In the context of _deallocate_port_for_instance() the devices is singular. I was driven by the other signatures where we always handle a InstancePCIRequests which is more or less a list. \n\nI will make it singular.\n\nDone.","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d429eaf286ff2a4193d185e82719c44a8e06639b","unresolved":false,"context_lines":[{"line_number":7413,"context_line":"                            instance\u003dinstance)"},{"line_number":7414,"context_line":"        else:"},{"line_number":7415,"context_line":"            if pci_devices:"},{"line_number":7416,"context_line":"                self.rt.unclaim_pci_devices(context, pci_devices, instance)"},{"line_number":7417,"context_line":""},{"line_number":7418,"context_line":"                # remove pci device from instance"},{"line_number":7419,"context_line":"                instance.pci_devices.objects \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_f44cffc8","line":7416,"range":{"start_line":7416,"start_character":24,"end_line":7416,"end_character":43},"updated":"2020-07-16 19:20:46.000000000","message":"i mentioned this on irc but for other reviews\ni have confirm that while we allow sriov device to be detached today we do not free the pci device claim when we do that.\n\nthe sirov live migration feature is able to fix the issue but shelve and cold migration will not and result in addtion devices being claimed on the destination host that are then unused.\n\nso likely we should add a patch before this to block detach if its an sriov interface(we determin this via the vnic_type)\nlike we currently block attach.\nwe might want to make that configurable like we did for numa live migration and then backport that to older releases.\n\nthen this patch can remove the block and enable full support for attach and detach.\n\ni have confrim that when we delete the interface we  also free the unused claims so on older release it will eventurally fix iteslef but with out sriov live migratoin added in train there is no way to fix the issue via the api unless you delete the instance.","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f5b832930337fbe343a0860481f5eead577e79be","unresolved":false,"context_lines":[{"line_number":7413,"context_line":"                            instance\u003dinstance)"},{"line_number":7414,"context_line":"        else:"},{"line_number":7415,"context_line":"            if pci_devices:"},{"line_number":7416,"context_line":"                self.rt.unclaim_pci_devices(context, pci_devices, instance)"},{"line_number":7417,"context_line":""},{"line_number":7418,"context_line":"                # remove pci device from instance"},{"line_number":7419,"context_line":"                instance.pci_devices.objects \u003d ["}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_f8a5bfa0","line":7416,"range":{"start_line":7416,"start_character":24,"end_line":7416,"end_character":43},"in_reply_to":"bf51134e_f44cffc8","updated":"2020-07-22 16:03:24.000000000","message":"Thanks. I assume you will prepare the patch that block the detach until this. Let me know if you need my help with that.","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8e7c2090e8e32dca3c3a6d60b0936796333b0bb","unresolved":false,"context_lines":[{"line_number":7478,"context_line":""},{"line_number":7479,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7480,"context_line":""},{"line_number":7481,"context_line":"        pci_numa_affinity_policy \u003d hardware.get_pci_numa_policy_constraint("},{"line_number":7482,"context_line":"            instance.flavor, instance.image_meta)"},{"line_number":7483,"context_line":"        pci_reqs \u003d objects.InstancePCIRequests("},{"line_number":7484,"context_line":"            requests\u003d[], instance_uuid\u003dinstance.uuid)"},{"line_number":7485,"context_line":"        self.network_api.create_resource_requests("}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_d97816c2","line":7482,"range":{"start_line":7481,"start_character":1,"end_line":7482,"end_character":49},"updated":"2020-07-16 18:43:19.000000000","message":"yep this is needed for the affinity policy feature and is easy to miss :) +1","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b8e7c2090e8e32dca3c3a6d60b0936796333b0bb","unresolved":false,"context_lines":[{"line_number":7547,"context_line":"            phase\u003dfields.NotificationPhase.START)"},{"line_number":7548,"context_line":""},{"line_number":7549,"context_line":"        bind_host_id \u003d self.driver.network_binding_host_id(context, instance)"},{"line_number":7550,"context_line":""},{"line_number":7551,"context_line":"        requested_networks \u003d objects.NetworkRequestList("},{"line_number":7552,"context_line":"            objects\u003d["},{"line_number":7553,"context_line":"                objects.NetworkRequest("},{"line_number":7554,"context_line":"                    network_id\u003dnetwork_id,"},{"line_number":7555,"context_line":"                    port_id\u003dport_id,"},{"line_number":7556,"context_line":"                    address\u003drequested_ip,"},{"line_number":7557,"context_line":"                    tag\u003dtag,"},{"line_number":7558,"context_line":"                )"},{"line_number":7559,"context_line":"            ]"},{"line_number":7560,"context_line":"        )"},{"line_number":7561,"context_line":""},{"line_number":7562,"context_line":"        pci_devices \u003d self._allocate_pci_device_for_interface_attach("},{"line_number":7563,"context_line":"            context, instance, requested_networks)"},{"line_number":7564,"context_line":""},{"line_number":7565,"context_line":"        network_info \u003d self.network_api.allocate_for_instance("},{"line_number":7566,"context_line":"            context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_746d8fca","line":7563,"range":{"start_line":7550,"start_character":0,"end_line":7563,"end_character":50},"updated":"2020-07-16 18:43:19.000000000","message":"so in the case of sriov interfaces you need to pre create them in neutron first and then attach them via the port id.\nwe also dont support attaching multipel interfaces in one call.\n\nso i dont think this is the correct interface.\n\ninstead of creat a list of requested networks we shoudl be retriving the neutron port.\nwe can then check the vnic_type and see if its a sriov type\nby checking if its in nova.network.model.VIF_TYPES_SRIOV.\n\nif it is then we need to call _allocate_pci_device_for_interface_attach\npassing in the instance and the port","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f5b832930337fbe343a0860481f5eead577e79be","unresolved":false,"context_lines":[{"line_number":7547,"context_line":"            phase\u003dfields.NotificationPhase.START)"},{"line_number":7548,"context_line":""},{"line_number":7549,"context_line":"        bind_host_id \u003d self.driver.network_binding_host_id(context, instance)"},{"line_number":7550,"context_line":""},{"line_number":7551,"context_line":"        requested_networks \u003d objects.NetworkRequestList("},{"line_number":7552,"context_line":"            objects\u003d["},{"line_number":7553,"context_line":"                objects.NetworkRequest("},{"line_number":7554,"context_line":"                    network_id\u003dnetwork_id,"},{"line_number":7555,"context_line":"                    port_id\u003dport_id,"},{"line_number":7556,"context_line":"                    address\u003drequested_ip,"},{"line_number":7557,"context_line":"                    tag\u003dtag,"},{"line_number":7558,"context_line":"                )"},{"line_number":7559,"context_line":"            ]"},{"line_number":7560,"context_line":"        )"},{"line_number":7561,"context_line":""},{"line_number":7562,"context_line":"        pci_devices \u003d self._allocate_pci_device_for_interface_attach("},{"line_number":7563,"context_line":"            context, instance, requested_networks)"},{"line_number":7564,"context_line":""},{"line_number":7565,"context_line":"        network_info \u003d self.network_api.allocate_for_instance("},{"line_number":7566,"context_line":"            context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_3815779c","line":7563,"range":{"start_line":7550,"start_character":0,"end_line":7563,"end_character":50},"in_reply_to":"bf51134e_746d8fca","updated":"2020-07-22 16:03:24.000000000","message":"\u003e so in the case of sriov interfaces you need to pre create them in\n \u003e neutron first and then attach them via the port id.\n\nYes, for the SRIOV case we always have the port_id filled in the interface_attach() call. However we still support the non SRIOV case when the network_id or the requested_ip parameter of the interface_attach call describes the interface to be attached.\n\n \u003e we also dont support attaching multipel interfaces in one call.\n \u003e \n\nTrue. I\u0027ve made the return value singular: pci_device. \n\n \u003e so i dont think this is the correct interface.\n \u003e \n \u003e instead of creat a list of requested networks we shoudl be\n \u003e retriving the neutron port.\n \u003e we can then check the vnic_type and see if its a sriov type\n \u003e by checking if its in nova.network.model.VIF_TYPES_SRIOV.\n \u003e \n \u003e if it is then we need to call _allocate_pci_device_for_interface_attach\n \u003e passing in the instance and the port\n\nThe nova.network.neutron.API.create_resource_requests() called by _allocate_pci_device_for_interface_attach() actually does what you described. Sure it does a bit more as it can handle networks not just ports as it is used during boot. I would like to re-use as much as possible from create_resource_requests() as 1) it apparently does the right thing 2) it is used and tested during boot so we know it works 3) if something new needs to be supported regarding neutron ports then it will anyhow need to be added both to the boot and to the interface attach code path, so I like if such addition can be done in a single place.\n\n\nAlso please note that the now unused self.network_api.allocate_port_for_instance() also created the NetworkRequestsList and called allocate_for_instance() in the baseline. What I did is that I removed the allocate_port_for_instance() helper and I create the NetworkRequestsList directly here, so it can also be reused for the create_resource_requests() call as well.","commit_id":"49075263c8014cbe5628667a0aa8f1a7ada1d779"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":7418,"context_line":"                # remove pci device from instance"},{"line_number":7419,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7420,"context_line":""},{"line_number":7421,"context_line":"                # remove pci request from instance"},{"line_number":7422,"context_line":"                instance.pci_requests.requests \u003d ["},{"line_number":7423,"context_line":"                    pci_req for pci_req in instance.pci_requests.requests"},{"line_number":7424,"context_line":"                    if pci_req.request_id !\u003d pci_device.request_id]"},{"line_number":7425,"context_line":""},{"line_number":7426,"context_line":"            if port_allocation:"},{"line_number":7427,"context_line":"                # Deallocate the resources in placement that were used by the"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_ed1bff7d","line":7424,"range":{"start_line":7421,"start_character":1,"end_line":7424,"end_character":67},"updated":"2020-08-04 00:08:37.000000000","message":"nit: it makes me a little sad that we are removing a single item form a list by making a copy of all the list element minus that element.\n\nthese likely will be short lists and really this is just copying pointers to the element not the elements so i guess its not terrible but ya\n\ni kind of want this to be \n\nfor pci_req in instance.pci_requests.requests:\n    if pci_req.request_id \u003d\u003d pci_device.request_id:\n        instance.pci_requests.requests.remove(pci_req)\n        break\n\nthat is more efficent an roughtly as readable\nthe only thing that make me nervous is it is not correct if you forget the breaks as we are technically modifyin the collection we are iterating over and it can be simple to forget the break.\n\nso the list comprehension is safer even if it is much less efficient.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1438bdccd5cb19de607f0a27e391c89914e2ef8c","unresolved":false,"context_lines":[{"line_number":7418,"context_line":"                # remove pci device from instance"},{"line_number":7419,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7420,"context_line":""},{"line_number":7421,"context_line":"                # remove pci request from instance"},{"line_number":7422,"context_line":"                instance.pci_requests.requests \u003d ["},{"line_number":7423,"context_line":"                    pci_req for pci_req in instance.pci_requests.requests"},{"line_number":7424,"context_line":"                    if pci_req.request_id !\u003d pci_device.request_id]"},{"line_number":7425,"context_line":""},{"line_number":7426,"context_line":"            if port_allocation:"},{"line_number":7427,"context_line":"                # Deallocate the resources in placement that were used by the"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_09fba778","line":7424,"range":{"start_line":7421,"start_character":1,"end_line":7424,"end_character":67},"in_reply_to":"9f560f44_ed1bff7d","updated":"2020-08-05 16:07:10.000000000","message":"I could use \n\n  instance.pci_requests.requests \u003d filter(\n      lambda pci_req:\n          pci_req.request_id !\u003d pci_device.request_id,\n      instance.pci_requests.requests,\n  )\n\nfor the same effect but I think list comprehension is more pythonic than filter with a lambda.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":7467,"context_line":"            the interface"},{"line_number":7468,"context_line":"        \"\"\""},{"line_number":7469,"context_line":""},{"line_number":7470,"context_line":"        if len(requested_networks.objects) !\u003d 1:"},{"line_number":7471,"context_line":"            raise ValueError("},{"line_number":7472,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7473,"context_line":""},{"line_number":7474,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7475,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_2855e5fa","line":7472,"range":{"start_line":7470,"start_character":7,"end_line":7472,"end_character":74},"updated":"2020-08-04 00:08:37.000000000","message":"nit: you could just put an assert there\n\nassert  len(requested_networks.objects) \u003d\u003d 1\n\nits certainly a precondition violation but im not sure its a value error. base on https://docs.python.org/3/library/exceptions.html#ValueError\n\n\"Raised when an operation or function receives an argument that has the right type but an inappropriate value, and the situation is not described by a more precise exception such as IndexError.\"\n\ni guess it would be i just generally prefer assert over\n\nif (condition):\n    raise SomeError()\n\nis more or less what assert does \nhttps://docs.python.org/3/reference/simple_stmts.html#the-assert-statement\n\ni guess the real question is do we want this to be a runtime check always or just when debugging?\n\nby the way you dont have ot cahge this or the similar stament on line 7488\n\nim more asking this as a style question as i woudld kind of like us to use assert more but that said this is what we do more or less today so if we want to keep the current style im ok with that its just more typing :)","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"937159b61e2460c07f56f617eee0b45a0899af1f","unresolved":false,"context_lines":[{"line_number":7467,"context_line":"            the interface"},{"line_number":7468,"context_line":"        \"\"\""},{"line_number":7469,"context_line":""},{"line_number":7470,"context_line":"        if len(requested_networks.objects) !\u003d 1:"},{"line_number":7471,"context_line":"            raise ValueError("},{"line_number":7472,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7473,"context_line":""},{"line_number":7474,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7475,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_46900044","line":7472,"range":{"start_line":7470,"start_character":7,"end_line":7472,"end_character":74},"in_reply_to":"9f560f44_2855e5fa","updated":"2020-08-04 16:20:52.000000000","message":"asserts shouldn\u0027t be used in production code since they get stripped out if you run Python with optimizations enabled [1]\n\n[1] https://stackoverflow.com/a/1693127/613428","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fc0ab4b825e87f0f13fa30a8f46f1e8c92f14294","unresolved":false,"context_lines":[{"line_number":7467,"context_line":"            the interface"},{"line_number":7468,"context_line":"        \"\"\""},{"line_number":7469,"context_line":""},{"line_number":7470,"context_line":"        if len(requested_networks.objects) !\u003d 1:"},{"line_number":7471,"context_line":"            raise ValueError("},{"line_number":7472,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7473,"context_line":""},{"line_number":7474,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7475,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_46c80021","line":7472,"range":{"start_line":7470,"start_character":7,"end_line":7472,"end_character":74},"in_reply_to":"9f560f44_46900044","updated":"2020-08-04 16:26:46.000000000","message":"i know that is why i was asking if we want this to be a runtime check or just an assertion.\n\nwe should never be passing more then one item in the list anyway\n\nif this was called with user generated input then we would need the if\n\nsince this is only ever called internally then we should never violate the precondition that the list only has one item. if this ever fires it means we have a bug so im wondering if this should just be a debug assert or if it should be a runtime check.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1438bdccd5cb19de607f0a27e391c89914e2ef8c","unresolved":false,"context_lines":[{"line_number":7467,"context_line":"            the interface"},{"line_number":7468,"context_line":"        \"\"\""},{"line_number":7469,"context_line":""},{"line_number":7470,"context_line":"        if len(requested_networks.objects) !\u003d 1:"},{"line_number":7471,"context_line":"            raise ValueError("},{"line_number":7472,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7473,"context_line":""},{"line_number":7474,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7475,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_8932170f","line":7472,"range":{"start_line":7470,"start_character":7,"end_line":7472,"end_character":74},"in_reply_to":"9f560f44_46c80021","updated":"2020-08-05 16:07:10.000000000","message":"For me it boils down to test coverage. By using asserts we basically assume that a future change violating our current assumption about the length of this array will be covered with test cases. So that the an assert here can catch a violation. And honestly I don\u0027t believe in such nice word any more where changes are covered with test cases.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":7485,"context_line":"            # The requested port does not require a PCI device so nothing to do"},{"line_number":7486,"context_line":"            return"},{"line_number":7487,"context_line":""},{"line_number":7488,"context_line":"        if len(pci_reqs.requests) \u003e 1:"},{"line_number":7489,"context_line":"            raise ValueError("},{"line_number":7490,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7491,"context_line":""},{"line_number":7492,"context_line":"        pci_req \u003d pci_reqs.requests[0]"},{"line_number":7493,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_e8eacd0c","line":7490,"range":{"start_line":7488,"start_character":8,"end_line":7490,"end_character":74},"updated":"2020-08-04 00:08:37.000000000","message":"just a side note i wonder if this will ever need to change if/wehn we have to support cyborg smart nics. e.g. would we have a case where we have 2 pci device for example.\n\nalthough i think not given we wouldnot be tracking cyborg managed pci device in the pci tracker anyway. they would not be included in the whitelist so ya i think we can require this allways be 1.\n\nthis is more a note to my self as im walk through this by the way.\n\nyou could also write this as\n\nif len(pci_reqs.requests) !\u003d 1:\n    ...\n\nsince we know it snot empty on line 7484 above.\nor \n\nassert len(pci_reqs.requests) \u003d\u003d 1","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":7489,"context_line":"            raise ValueError("},{"line_number":7490,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7491,"context_line":""},{"line_number":7492,"context_line":"        pci_req \u003d pci_reqs.requests[0]"},{"line_number":7493,"context_line":""},{"line_number":7494,"context_line":"        devices \u003d self.rt.claim_pci_devices(context, pci_reqs)"},{"line_number":7495,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_28f16502","line":7492,"range":{"start_line":7492,"start_character":0,"end_line":7492,"end_character":38},"updated":"2020-08-04 00:08:37.000000000","message":"yep so since its always lenght 1 at this point we can safely just reference the first element of the list.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":7562,"context_line":"        network_info \u003d self.network_api.allocate_for_instance("},{"line_number":7563,"context_line":"            context,"},{"line_number":7564,"context_line":"            instance,"},{"line_number":7565,"context_line":"            vpn\u003dFalse,"},{"line_number":7566,"context_line":"            requested_networks\u003drequested_networks,"},{"line_number":7567,"context_line":"            bind_host_id\u003dbind_host_id,"},{"line_number":7568,"context_line":"            attach\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_88ba3107","line":7565,"range":{"start_line":7565,"start_character":10,"end_line":7565,"end_character":22},"updated":"2020-08-04 00:08:37.000000000","message":"whats vpn?\n\n\nlater\n-----\n\nits a bool ignored by the neutron drive meaning this was likely an old nova networks parameter\n\nhttps://github.com/openstack/nova/blob/d4c857dfcb1ccfa5410de55671e69c722bbc990e/nova/network/neutron.py#L1017\n\ncan we remove it form that function instead?\n\nits also a postional arg not a keyword arg. so this should acutlly just be False or None. not vpn\u003dFase\n\nthis is what the -1 is for by the way the previous comment in this file are more style question the issues with the code.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1438bdccd5cb19de607f0a27e391c89914e2ef8c","unresolved":false,"context_lines":[{"line_number":7562,"context_line":"        network_info \u003d self.network_api.allocate_for_instance("},{"line_number":7563,"context_line":"            context,"},{"line_number":7564,"context_line":"            instance,"},{"line_number":7565,"context_line":"            vpn\u003dFalse,"},{"line_number":7566,"context_line":"            requested_networks\u003drequested_networks,"},{"line_number":7567,"context_line":"            bind_host_id\u003dbind_host_id,"},{"line_number":7568,"context_line":"            attach\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_4433c6f6","line":7565,"range":{"start_line":7565,"start_character":10,"end_line":7565,"end_character":22},"in_reply_to":"9f560f44_88ba3107","updated":"2020-08-05 16:07:10.000000000","message":"Sure, I can remove vpn param in a separate patch. Done in https://review.opendev.org/#/c/744933/","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":7608,"context_line":"            # tracks this device as allocated. This way we can avoid a possible"},{"line_number":7609,"context_line":"            # race condition when a detach arrives for a device that is only"},{"line_number":7610,"context_line":"            # in claimed state."},{"line_number":7611,"context_line":"            self.rt.allocate_pci_devices_for_instance(context, instance)"},{"line_number":7612,"context_line":""},{"line_number":7613,"context_line":"        instance.save()"},{"line_number":7614,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_88fd11b3","line":7611,"range":{"start_line":7611,"start_character":12,"end_line":7611,"end_character":72},"updated":"2020-08-04 00:08:37.000000000","message":"did you confim this wont works with multiple devices and does not have issue if the vm already has some devices allocated.\n\nit might make sense to only allocate the device you attached rater then all the vms devices. this might just work fine too but its going to update more db row unless we filter to only the ones that need to be updated.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":7440,"context_line":"                                    {\u0027port_id\u0027: port_id, \u0027error\u0027: ex},"},{"line_number":7441,"context_line":"                                    instance\u003dinstance)"},{"line_number":7442,"context_line":""},{"line_number":7443,"context_line":"    def _allocate_pci_device_for_interface_attach("},{"line_number":7444,"context_line":"        self,"},{"line_number":7445,"context_line":"        context: nova.context.RequestContext,"},{"line_number":7446,"context_line":"        instance: \u0027objects.Instance\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_97ad099d","line":7443,"range":{"start_line":7443,"start_character":8,"end_line":7443,"end_character":49},"updated":"2020-08-05 21:18:00.000000000","message":"this is badly named since it clims the pci device it does not set it to the allcoated state.\n\n it should be _cliam_pci_device_for_interface_attach(\n\nit calls \n  devices \u003d self.rt.claim_pci_devices(context, pci_reqs)\non line 7491\n\nclaimed deivces are reserved for future use by the instance\nallocated devices are currnelty beign used by the instance when it is finished building or attahcing.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"67a393d0c64e7e5de23ffa805fc955435f405243","unresolved":false,"context_lines":[{"line_number":7440,"context_line":"                                    {\u0027port_id\u0027: port_id, \u0027error\u0027: ex},"},{"line_number":7441,"context_line":"                                    instance\u003dinstance)"},{"line_number":7442,"context_line":""},{"line_number":7443,"context_line":"    def _allocate_pci_device_for_interface_attach("},{"line_number":7444,"context_line":"        self,"},{"line_number":7445,"context_line":"        context: nova.context.RequestContext,"},{"line_number":7446,"context_line":"        instance: \u0027objects.Instance\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d26ab4b0","line":7443,"range":{"start_line":7443,"start_character":8,"end_line":7443,"end_character":49},"in_reply_to":"9f560f44_97ad099d","updated":"2020-08-27 15:00:15.000000000","message":"You are right. I\u0027ve renamed it.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":7486,"context_line":"            raise ValueError("},{"line_number":7487,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7488,"context_line":""},{"line_number":7489,"context_line":"        pci_req \u003d pci_reqs.requests[0]"},{"line_number":7490,"context_line":""},{"line_number":7491,"context_line":"        devices \u003d self.rt.claim_pci_devices(context, pci_reqs)"},{"line_number":7492,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_9737c94d","line":7489,"range":{"start_line":7489,"start_character":7,"end_line":7489,"end_character":38},"updated":"2020-08-05 21:18:00.000000000","message":"this is not used until after the call to self.rt.claim_pci_devices(context, pci_reqs)\nit would be clearer if you swapped the lines","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"67a393d0c64e7e5de23ffa805fc955435f405243","unresolved":false,"context_lines":[{"line_number":7486,"context_line":"            raise ValueError("},{"line_number":7487,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7488,"context_line":""},{"line_number":7489,"context_line":"        pci_req \u003d pci_reqs.requests[0]"},{"line_number":7490,"context_line":""},{"line_number":7491,"context_line":"        devices \u003d self.rt.claim_pci_devices(context, pci_reqs)"},{"line_number":7492,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_52d34460","line":7489,"range":{"start_line":7489,"start_character":7,"end_line":7489,"end_character":38},"in_reply_to":"9f560f44_9737c94d","updated":"2020-08-27 15:00:15.000000000","message":"I would like to keep it right after the guard conditions above that ensures there are exactly one request in the list.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":7488,"context_line":""},{"line_number":7489,"context_line":"        pci_req \u003d pci_reqs.requests[0]"},{"line_number":7490,"context_line":""},{"line_number":7491,"context_line":"        devices \u003d self.rt.claim_pci_devices(context, pci_reqs)"},{"line_number":7492,"context_line":""},{"line_number":7493,"context_line":"        if not devices:"},{"line_number":7494,"context_line":"            LOG.info(\u0027Failed to allocated PCI devices during interface attach \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_f76d6532","line":7491,"range":{"start_line":7491,"start_character":18,"end_line":7491,"end_character":62},"updated":"2020-08-05 21:18:00.000000000","message":"this is not correct.\nit does not pass the instance numa topology to the pci resouce tracker so it wont take into account wehre the vm is pinned.\n\nits passing none\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1718-L1719\n\nsriov live migration was merged a few months before numa live migration and this was never updated.\n\nso this is a latent bug.\nit should have been updated as part of the numa live migration feature.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"67a393d0c64e7e5de23ffa805fc955435f405243","unresolved":false,"context_lines":[{"line_number":7488,"context_line":""},{"line_number":7489,"context_line":"        pci_req \u003d pci_reqs.requests[0]"},{"line_number":7490,"context_line":""},{"line_number":7491,"context_line":"        devices \u003d self.rt.claim_pci_devices(context, pci_reqs)"},{"line_number":7492,"context_line":""},{"line_number":7493,"context_line":"        if not devices:"},{"line_number":7494,"context_line":"            LOG.info(\u0027Failed to allocated PCI devices during interface attach \u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_32e170d1","line":7491,"range":{"start_line":7491,"start_character":18,"end_line":7491,"end_character":62},"in_reply_to":"9f560f44_f76d6532","updated":"2020-08-27 15:00:15.000000000","message":"for interface attach it seems to be easy to fix as the instance numa topology does not change. \n\nfor the live migration case I\u0027m provided a separate patch https://review.opendev.org/#/c/748453","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":7595,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":7596,"context_line":""},{"line_number":7597,"context_line":"        if pci_device:"},{"line_number":7598,"context_line":"            # NOTE(gibi): The _allocate_pci_device_for_interface_attach() call"},{"line_number":7599,"context_line":"            # found a pci device but it only marked the device as claimed. The"},{"line_number":7600,"context_line":"            # periodic update_available_resource would move the device to"},{"line_number":7601,"context_line":"            # allocated state. But as driver.attach_interface() has been"},{"line_number":7602,"context_line":"            # succeeded we now know that the interface is also allocated"},{"line_number":7603,"context_line":"            # (used by) to the instance. So make sure the pci tracker also"},{"line_number":7604,"context_line":"            # tracks this device as allocated. This way we can avoid a possible"},{"line_number":7605,"context_line":"            # race condition when a detach arrives for a device that is only"},{"line_number":7606,"context_line":"            # in claimed state."},{"line_number":7607,"context_line":"            self.rt.allocate_pci_devices_for_instance(context, instance)"},{"line_number":7608,"context_line":""},{"line_number":7609,"context_line":"        instance.save()"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_b7964d3d","line":7606,"range":{"start_line":7598,"start_character":11,"end_line":7606,"end_character":31},"updated":"2020-08-05 21:18:00.000000000","message":"so in my testing this is remaining in the claimed sate for quite some time presumably until the periodic task is run.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"67a393d0c64e7e5de23ffa805fc955435f405243","unresolved":false,"context_lines":[{"line_number":7595,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":7596,"context_line":""},{"line_number":7597,"context_line":"        if pci_device:"},{"line_number":7598,"context_line":"            # NOTE(gibi): The _allocate_pci_device_for_interface_attach() call"},{"line_number":7599,"context_line":"            # found a pci device but it only marked the device as claimed. The"},{"line_number":7600,"context_line":"            # periodic update_available_resource would move the device to"},{"line_number":7601,"context_line":"            # allocated state. But as driver.attach_interface() has been"},{"line_number":7602,"context_line":"            # succeeded we now know that the interface is also allocated"},{"line_number":7603,"context_line":"            # (used by) to the instance. So make sure the pci tracker also"},{"line_number":7604,"context_line":"            # tracks this device as allocated. This way we can avoid a possible"},{"line_number":7605,"context_line":"            # race condition when a detach arrives for a device that is only"},{"line_number":7606,"context_line":"            # in claimed state."},{"line_number":7607,"context_line":"            self.rt.allocate_pci_devices_for_instance(context, instance)"},{"line_number":7608,"context_line":""},{"line_number":7609,"context_line":"        instance.save()"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_fce34a32","line":7606,"range":{"start_line":7598,"start_character":11,"end_line":7606,"end_character":31},"in_reply_to":"9f560f44_b7964d3d","updated":"2020-08-27 15:00:15.000000000","message":"Interesting. I added some logs [1] to the claim and allocate codepath to see what is happening in your env. \n\n[1]https://review.opendev.org/#/c/748454/","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":7604,"context_line":"            # tracks this device as allocated. This way we can avoid a possible"},{"line_number":7605,"context_line":"            # race condition when a detach arrives for a device that is only"},{"line_number":7606,"context_line":"            # in claimed state."},{"line_number":7607,"context_line":"            self.rt.allocate_pci_devices_for_instance(context, instance)"},{"line_number":7608,"context_line":""},{"line_number":7609,"context_line":"        instance.save()"},{"line_number":7610,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_1702990b","line":7607,"range":{"start_line":7607,"start_character":11,"end_line":7607,"end_character":72},"updated":"2020-08-05 21:18:00.000000000","message":"im not sure why this appears to not work.\n\nbut i find it odd that for claim we directly assgin\n\nhttps://github.com/openstack/nova/blob/6557d672b05cbcbc9b8313016dc6eba4e9cc85b3/nova/pci/manager.py#L276\n\nand for allocate we append\n\nhttps://github.com/openstack/nova/blob/6557d672b05cbcbc9b8313016dc6eba4e9cc85b3/nova/pci/manager.py#L267\n\nso i think this is also a bug\nif i claim to interfaces before the periodic run i think the second claim might clobber the first.\n\ntrying to attach a macvtap and then remove it and attach again i managed to make the second attach fail likely becaues the macvtap was not clean up proplery.\n\nin that case the pci device remaied claimed event though it was not attached to the vm.\n\nso i think we have some bugs in the error path.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"67a393d0c64e7e5de23ffa805fc955435f405243","unresolved":false,"context_lines":[{"line_number":7604,"context_line":"            # tracks this device as allocated. This way we can avoid a possible"},{"line_number":7605,"context_line":"            # race condition when a detach arrives for a device that is only"},{"line_number":7606,"context_line":"            # in claimed state."},{"line_number":7607,"context_line":"            self.rt.allocate_pci_devices_for_instance(context, instance)"},{"line_number":7608,"context_line":""},{"line_number":7609,"context_line":"        instance.save()"},{"line_number":7610,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_260935dc","line":7607,"range":{"start_line":7607,"start_character":11,"end_line":7607,"end_character":72},"in_reply_to":"9f560f44_1702990b","updated":"2020-08-27 15:00:15.000000000","message":"Appending for allocations make sense as the instance might already have some devs allocated. \n\nAssigning in the claim might be problematic in your case when the claimed -\u003e allocated transition only happens in the periodic task. If the \u0027allocate_pci_devices_for_instance()\u0027 puts the dev to allocated then the only way to have two claim to race if two attach_interface is racing which is being fixed now by aarents in https://review.opendev.org/#/c/747957/ with proper locking.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"08c831c16f8f97e6752c4007da90c4216ec1c527","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"9f560f44_7492a1fe","line":10592,"updated":"2020-08-28 14:09:01.000000000","message":"pep8: N343: Doubled word \u0027the\u0027 typo found","commit_id":"9f6436ea5ed6d59c5ee1025fe426756ea554876f"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"f853b5effc629b9bf9cc75a10697b5a379baf837","unresolved":false,"context_lines":[{"line_number":7481,"context_line":""},{"line_number":7482,"context_line":"        if not pci_reqs.requests:"},{"line_number":7483,"context_line":"            # The requested port does not require a PCI device so nothing to do"},{"line_number":7484,"context_line":"            return"},{"line_number":7485,"context_line":""},{"line_number":7486,"context_line":"        if len(pci_reqs.requests) \u003e 1:"},{"line_number":7487,"context_line":"            raise ValueError("}],"source_content_type":"text/x-python","patch_set":11,"id":"9f560f44_3202f40c","line":7484,"updated":"2020-08-28 20:30:49.000000000","message":"pep8: error: Return value expected","commit_id":"c1885849c3574c77418c8d1ac008f07bd1701611"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a452122839f94ecab44b547ca3343291215e9f01","unresolved":false,"context_lines":[{"line_number":7481,"context_line":""},{"line_number":7482,"context_line":"        if not pci_reqs.requests:"},{"line_number":7483,"context_line":"            # The requested port does not require a PCI device so nothing to do"},{"line_number":7484,"context_line":"            return"},{"line_number":7485,"context_line":""},{"line_number":7486,"context_line":"        if len(pci_reqs.requests) \u003e 1:"},{"line_number":7487,"context_line":"            raise ValueError("}],"source_content_type":"text/x-python","patch_set":11,"id":"9f560f44_c2574126","line":7484,"in_reply_to":"9f560f44_3202f40c","updated":"2020-09-01 15:24:51.000000000","message":"Done","commit_id":"c1885849c3574c77418c8d1ac008f07bd1701611"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7463,"context_line":"                            instance\u003dinstance)"},{"line_number":7464,"context_line":"        else:"},{"line_number":7465,"context_line":"            if pci_device:"},{"line_number":7466,"context_line":"                self.rt.unclaim_pci_devices(context, pci_device, instance)"},{"line_number":7467,"context_line":""},{"line_number":7468,"context_line":"                # remove pci device from instance"},{"line_number":7469,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7470,"context_line":""},{"line_number":7471,"context_line":"                # remove pci request from instance"},{"line_number":7472,"context_line":"                instance.pci_requests.requests \u003d ["}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_1ed3d602","line":7469,"range":{"start_line":7466,"start_character":0,"end_line":7469,"end_character":63},"updated":"2020-09-09 15:30:59.000000000","message":"This is good to unclaim first the resources as we could hold for the lock so then it\u0027s reasonable to remove the object afterwards.\n\nThis being said, I\u0027m afraid of us releasing resources usage before and then failing to remove the object, but I assume this being a very low possibility ? (ie. can we get an exception from the object remove call ? nope, I don\u0027t think so)","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"954bc3ded4538024d122d9b31f2dfb455ec35bcf","unresolved":false,"context_lines":[{"line_number":7463,"context_line":"                            instance\u003dinstance)"},{"line_number":7464,"context_line":"        else:"},{"line_number":7465,"context_line":"            if pci_device:"},{"line_number":7466,"context_line":"                self.rt.unclaim_pci_devices(context, pci_device, instance)"},{"line_number":7467,"context_line":""},{"line_number":7468,"context_line":"                # remove pci device from instance"},{"line_number":7469,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7470,"context_line":""},{"line_number":7471,"context_line":"                # remove pci request from instance"},{"line_number":7472,"context_line":"                instance.pci_requests.requests \u003d ["}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_418e13d1","line":7469,"range":{"start_line":7466,"start_character":0,"end_line":7469,"end_character":63},"in_reply_to":"9f560f44_1ed3d602","updated":"2020-09-09 16:14:57.000000000","message":"Yes there is an attach/detach lock. I don\u0027t think either remove or unclaim_pci_devices could reasonably fail for other than implementation error. \n\nIf I add a blanket\n\n  except Exception:\n\nblock, then what would be a reasonable behavior? If unclaim fails, what should we do other than failing the detach? \n\nTo avoid failing at remove after unclaim succeeded I can change the remove call to:\n\nif pci_device in instance.pci_devices.objects:\n    instance.pci_devices.objects.remove(pci_device)\n\n\nWhat do is sounds?","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ce162e5b3ae151adeab84a65f1d11534572e1c13","unresolved":false,"context_lines":[{"line_number":7463,"context_line":"                            instance\u003dinstance)"},{"line_number":7464,"context_line":"        else:"},{"line_number":7465,"context_line":"            if pci_device:"},{"line_number":7466,"context_line":"                self.rt.unclaim_pci_devices(context, pci_device, instance)"},{"line_number":7467,"context_line":""},{"line_number":7468,"context_line":"                # remove pci device from instance"},{"line_number":7469,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7470,"context_line":""},{"line_number":7471,"context_line":"                # remove pci request from instance"},{"line_number":7472,"context_line":"                instance.pci_requests.requests \u003d ["}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_7405d379","line":7469,"range":{"start_line":7466,"start_character":0,"end_line":7469,"end_character":63},"in_reply_to":"9f560f44_418e13d1","updated":"2020-09-09 16:40:51.000000000","message":"yeah this conditional looks okay to me.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85c4377f15d52e2d4d25fa762b6c87f643afdf8b","unresolved":false,"context_lines":[{"line_number":7463,"context_line":"                            instance\u003dinstance)"},{"line_number":7464,"context_line":"        else:"},{"line_number":7465,"context_line":"            if pci_device:"},{"line_number":7466,"context_line":"                self.rt.unclaim_pci_devices(context, pci_device, instance)"},{"line_number":7467,"context_line":""},{"line_number":7468,"context_line":"                # remove pci device from instance"},{"line_number":7469,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7470,"context_line":""},{"line_number":7471,"context_line":"                # remove pci request from instance"},{"line_number":7472,"context_line":"                instance.pci_requests.requests \u003d ["}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_5b0b8e7b","line":7469,"range":{"start_line":7466,"start_character":0,"end_line":7469,"end_character":63},"in_reply_to":"9f560f44_7405d379","updated":"2020-09-10 09:08:35.000000000","message":"done in the fup in the instance object","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":7467,"context_line":""},{"line_number":7468,"context_line":"                # remove pci device from instance"},{"line_number":7469,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7470,"context_line":""},{"line_number":7471,"context_line":"                # remove pci request from instance"},{"line_number":7472,"context_line":"                instance.pci_requests.requests \u003d ["},{"line_number":7473,"context_line":"                    pci_req for pci_req in instance.pci_requests.requests"},{"line_number":7474,"context_line":"                    if pci_req.request_id !\u003d pci_device.request_id]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_df8ea625","line":7471,"range":{"start_line":7470,"start_character":0,"end_line":7471,"end_character":50},"updated":"2020-09-07 11:18:22.000000000","message":"nit i would proably remove this and the newline above and jsut group both the object and request removal togther to signal that they both mus be done.\n\nas a follow up it would also be nice to have a function on instance object that woudl do this when called with a pci_device.\n\ne.g. \ndef remove_pci_dev_request(self, pci_device):\n   self.pci_devices.objects.remove(pci_device)\n   self.pci_requests.requests \u003d [\n                    pci_req for pci_req in self.pci_requests.requests\n                    if pci_req.request_id !\u003d pci_device.request_id]\n\ninstance.remove_pci_dev_request(pci_device)","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7467,"context_line":""},{"line_number":7468,"context_line":"                # remove pci device from instance"},{"line_number":7469,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7470,"context_line":""},{"line_number":7471,"context_line":"                # remove pci request from instance"},{"line_number":7472,"context_line":"                instance.pci_requests.requests \u003d ["},{"line_number":7473,"context_line":"                    pci_req for pci_req in instance.pci_requests.requests"},{"line_number":7474,"context_line":"                    if pci_req.request_id !\u003d pci_device.request_id]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_5e0a6e89","line":7471,"range":{"start_line":7470,"start_character":0,"end_line":7471,"end_character":50},"in_reply_to":"9f560f44_99156940","updated":"2020-09-09 15:30:59.000000000","message":"+1 on both sean\u0027s comments and glad to see it addressed in a FUP","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":7467,"context_line":""},{"line_number":7468,"context_line":"                # remove pci device from instance"},{"line_number":7469,"context_line":"                instance.pci_devices.objects.remove(pci_device)"},{"line_number":7470,"context_line":""},{"line_number":7471,"context_line":"                # remove pci request from instance"},{"line_number":7472,"context_line":"                instance.pci_requests.requests \u003d ["},{"line_number":7473,"context_line":"                    pci_req for pci_req in instance.pci_requests.requests"},{"line_number":7474,"context_line":"                    if pci_req.request_id !\u003d pci_device.request_id]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_99156940","line":7471,"range":{"start_line":7470,"start_character":0,"end_line":7471,"end_character":50},"in_reply_to":"9f560f44_df8ea625","updated":"2020-09-07 14:33:54.000000000","message":"Done in a follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7498,"context_line":"        context: nova.context.RequestContext,"},{"line_number":7499,"context_line":"        instance: \u0027objects.Instance\u0027,"},{"line_number":7500,"context_line":"        requested_networks: \u0027objects.NetworkRequestsList\u0027"},{"line_number":7501,"context_line":"    ) -\u003e ty.Optional[\u0027objects.PciDevice\u0027]:"},{"line_number":7502,"context_line":"        \"\"\"Claim PCI device if the requested interface needs it"},{"line_number":7503,"context_line":""},{"line_number":7504,"context_line":"        If a PCI device is claimed then the requested_networks is updated"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_9e008668","line":7501,"updated":"2020-09-09 15:30:59.000000000","message":"nit: gosh I don\u0027t like the indent above, but meh.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7517,"context_line":"            the interface or None if no device is requested"},{"line_number":7518,"context_line":"        \"\"\""},{"line_number":7519,"context_line":""},{"line_number":7520,"context_line":"        if len(requested_networks.objects) !\u003d 1:"},{"line_number":7521,"context_line":"            raise ValueError("},{"line_number":7522,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7523,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_9e352648","line":7520,"range":{"start_line":7520,"start_character":15,"end_line":7520,"end_character":41},"updated":"2020-09-09 15:30:59.000000000","message":"I\u0027m almost 100% sure that you could just have verified the number of networks by calling len() directly but I could be wrong and I don\u0027t want a respin for such nit either way.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85c4377f15d52e2d4d25fa762b6c87f643afdf8b","unresolved":false,"context_lines":[{"line_number":7517,"context_line":"            the interface or None if no device is requested"},{"line_number":7518,"context_line":"        \"\"\""},{"line_number":7519,"context_line":""},{"line_number":7520,"context_line":"        if len(requested_networks.objects) !\u003d 1:"},{"line_number":7521,"context_line":"            raise ValueError("},{"line_number":7522,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7523,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_db1e7eb9","line":7520,"range":{"start_line":7520,"start_character":15,"end_line":7520,"end_character":41},"in_reply_to":"9f560f44_4118d374","updated":"2020-09-10 09:08:35.000000000","message":"Done","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"954bc3ded4538024d122d9b31f2dfb455ec35bcf","unresolved":false,"context_lines":[{"line_number":7517,"context_line":"            the interface or None if no device is requested"},{"line_number":7518,"context_line":"        \"\"\""},{"line_number":7519,"context_line":""},{"line_number":7520,"context_line":"        if len(requested_networks.objects) !\u003d 1:"},{"line_number":7521,"context_line":"            raise ValueError("},{"line_number":7522,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7523,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_4118d374","line":7520,"range":{"start_line":7520,"start_character":15,"end_line":7520,"end_character":41},"in_reply_to":"9f560f44_9e352648","updated":"2020-09-09 16:14:57.000000000","message":"Will do in the follow up patch","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7521,"context_line":"            raise ValueError("},{"line_number":7522,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7523,"context_line":""},{"line_number":7524,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7525,"context_line":""},{"line_number":7526,"context_line":"        pci_numa_affinity_policy \u003d hardware.get_pci_numa_policy_constraint("},{"line_number":7527,"context_line":"            instance.flavor, instance.image_meta)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_fe3d422c","line":7524,"range":{"start_line":7524,"start_character":28,"end_line":7524,"end_character":57},"updated":"2020-09-09 15:30:59.000000000","message":"ditto with the above comment","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85c4377f15d52e2d4d25fa762b6c87f643afdf8b","unresolved":false,"context_lines":[{"line_number":7521,"context_line":"            raise ValueError("},{"line_number":7522,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7523,"context_line":""},{"line_number":7524,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7525,"context_line":""},{"line_number":7526,"context_line":"        pci_numa_affinity_policy \u003d hardware.get_pci_numa_policy_constraint("},{"line_number":7527,"context_line":"            instance.flavor, instance.image_meta)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_5b80aebf","line":7524,"range":{"start_line":7524,"start_character":28,"end_line":7524,"end_character":57},"in_reply_to":"9f560f44_013b7bd0","updated":"2020-09-10 09:08:35.000000000","message":"Done","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"954bc3ded4538024d122d9b31f2dfb455ec35bcf","unresolved":false,"context_lines":[{"line_number":7521,"context_line":"            raise ValueError("},{"line_number":7522,"context_line":"                \"Only support one interface per interface attach request\")"},{"line_number":7523,"context_line":""},{"line_number":7524,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7525,"context_line":""},{"line_number":7526,"context_line":"        pci_numa_affinity_policy \u003d hardware.get_pci_numa_policy_constraint("},{"line_number":7527,"context_line":"            instance.flavor, instance.image_meta)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_013b7bd0","line":7524,"range":{"start_line":7524,"start_character":28,"end_line":7524,"end_character":57},"in_reply_to":"9f560f44_fe3d422c","updated":"2020-09-09 16:14:57.000000000","message":"ditto","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":7523,"context_line":""},{"line_number":7524,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7525,"context_line":""},{"line_number":7526,"context_line":"        pci_numa_affinity_policy \u003d hardware.get_pci_numa_policy_constraint("},{"line_number":7527,"context_line":"            instance.flavor, instance.image_meta)"},{"line_number":7528,"context_line":"        pci_reqs \u003d objects.InstancePCIRequests("},{"line_number":7529,"context_line":"            requests\u003d[], instance_uuid\u003dinstance.uuid)"},{"line_number":7530,"context_line":"        self.network_api.create_resource_requests("}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_dfdce619","line":7527,"range":{"start_line":7526,"start_character":8,"end_line":7527,"end_character":49},"updated":"2020-09-07 11:18:22.000000000","message":"nit: this also feels like it could be a property on the instance object but that could be done in a followup or at a later date.\n\nim not sure how many places we look it up but instance.pci_numa_affinity_policy would be nice here.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":7523,"context_line":""},{"line_number":7524,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7525,"context_line":""},{"line_number":7526,"context_line":"        pci_numa_affinity_policy \u003d hardware.get_pci_numa_policy_constraint("},{"line_number":7527,"context_line":"            instance.flavor, instance.image_meta)"},{"line_number":7528,"context_line":"        pci_reqs \u003d objects.InstancePCIRequests("},{"line_number":7529,"context_line":"            requests\u003d[], instance_uuid\u003dinstance.uuid)"},{"line_number":7530,"context_line":"        self.network_api.create_resource_requests("}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_f93625b4","line":7527,"range":{"start_line":7526,"start_character":8,"end_line":7527,"end_character":49},"in_reply_to":"9f560f44_dfdce619","updated":"2020-09-07 14:33:54.000000000","message":"This operates on flavor and image_meta and while here we are using the instance.flavor and instance.image_meta the original place it is used [1] does not have an instance object. So the suggested refactoring does not really make sense from [1] perspective.\n\n[1] nova.compute.api.API._validate_and_build_base_options","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"69b2b438ddc16dc9613bfb02ccb60b644bae9d35","unresolved":false,"context_lines":[{"line_number":7523,"context_line":""},{"line_number":7524,"context_line":"        requested_network \u003d requested_networks.objects[0]"},{"line_number":7525,"context_line":""},{"line_number":7526,"context_line":"        pci_numa_affinity_policy \u003d hardware.get_pci_numa_policy_constraint("},{"line_number":7527,"context_line":"            instance.flavor, instance.image_meta)"},{"line_number":7528,"context_line":"        pci_reqs \u003d objects.InstancePCIRequests("},{"line_number":7529,"context_line":"            requests\u003d[], instance_uuid\u003dinstance.uuid)"},{"line_number":7530,"context_line":"        self.network_api.create_resource_requests("}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_e182b0c6","line":7527,"range":{"start_line":7526,"start_character":8,"end_line":7527,"end_character":49},"in_reply_to":"9f560f44_f93625b4","updated":"2020-09-10 13:34:49.000000000","message":"ah ok that makes sense.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":7555,"context_line":"        # Update the requested network with the pci request id"},{"line_number":7556,"context_line":"        requested_network.pci_request_id \u003d pci_req.request_id"},{"line_number":7557,"context_line":""},{"line_number":7558,"context_line":"        instance.pci_requests.requests.append(pci_req)"},{"line_number":7559,"context_line":"        instance.pci_devices.objects.append(device)"},{"line_number":7560,"context_line":""},{"line_number":7561,"context_line":"        return device"},{"line_number":7562,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_9fd6eef1","line":7559,"range":{"start_line":7558,"start_character":0,"end_line":7559,"end_character":51},"updated":"2020-09-07 11:18:22.000000000","message":"nit: if you add remove_pci_request then you could also move\nthis to an add_pci_request function on the instance.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":7555,"context_line":"        # Update the requested network with the pci request id"},{"line_number":7556,"context_line":"        requested_network.pci_request_id \u003d pci_req.request_id"},{"line_number":7557,"context_line":""},{"line_number":7558,"context_line":"        instance.pci_requests.requests.append(pci_req)"},{"line_number":7559,"context_line":"        instance.pci_devices.objects.append(device)"},{"line_number":7560,"context_line":""},{"line_number":7561,"context_line":"        return device"},{"line_number":7562,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_f9758599","line":7559,"range":{"start_line":7558,"start_character":0,"end_line":7559,"end_character":51},"in_reply_to":"9f560f44_9fd6eef1","updated":"2020-09-07 14:33:54.000000000","message":"Done in a follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7555,"context_line":"        # Update the requested network with the pci request id"},{"line_number":7556,"context_line":"        requested_network.pci_request_id \u003d pci_req.request_id"},{"line_number":7557,"context_line":""},{"line_number":7558,"context_line":"        instance.pci_requests.requests.append(pci_req)"},{"line_number":7559,"context_line":"        instance.pci_devices.objects.append(device)"},{"line_number":7560,"context_line":""},{"line_number":7561,"context_line":"        return device"},{"line_number":7562,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_1edd96dc","line":7559,"range":{"start_line":7558,"start_character":0,"end_line":7559,"end_character":51},"in_reply_to":"9f560f44_f9758599","updated":"2020-09-09 15:30:59.000000000","message":"note to self : This is just adding a new element to a list, no exception is expected here. Not a problem like L7469","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7621,"context_line":"        )"},{"line_number":7622,"context_line":""},{"line_number":7623,"context_line":"        pci_device \u003d self._claim_pci_device_for_interface_attach("},{"line_number":7624,"context_line":"            context, instance, requested_networks)"},{"line_number":7625,"context_line":""},{"line_number":7626,"context_line":"        network_info \u003d self.network_api.allocate_for_instance("},{"line_number":7627,"context_line":"            context,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_9e5c067e","line":7624,"updated":"2020-09-09 15:30:59.000000000","message":"mmm, so we can possibly raise a ValueError here but I don\u0027t see this exception as possibly expected L7565. Does this harm ?","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85c4377f15d52e2d4d25fa762b6c87f643afdf8b","unresolved":false,"context_lines":[{"line_number":7621,"context_line":"        )"},{"line_number":7622,"context_line":""},{"line_number":7623,"context_line":"        pci_device \u003d self._claim_pci_device_for_interface_attach("},{"line_number":7624,"context_line":"            context, instance, requested_networks)"},{"line_number":7625,"context_line":""},{"line_number":7626,"context_line":"        network_info \u003d self.network_api.allocate_for_instance("},{"line_number":7627,"context_line":"            context,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_4efd8e6c","line":7624,"in_reply_to":"9f560f44_21795fc4","updated":"2020-09-10 09:08:35.000000000","message":"Done in the followup","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"954bc3ded4538024d122d9b31f2dfb455ec35bcf","unresolved":false,"context_lines":[{"line_number":7621,"context_line":"        )"},{"line_number":7622,"context_line":""},{"line_number":7623,"context_line":"        pci_device \u003d self._claim_pci_device_for_interface_attach("},{"line_number":7624,"context_line":"            context, instance, requested_networks)"},{"line_number":7625,"context_line":""},{"line_number":7626,"context_line":"        network_info \u003d self.network_api.allocate_for_instance("},{"line_number":7627,"context_line":"            context,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_21795fc4","line":7624,"in_reply_to":"9f560f44_9e5c067e","updated":"2020-09-09 16:14:57.000000000","message":"Hm, I will change both ValueError in _claim_pci_device_for_interface_attach to InterfaceAttachFailed exception to unify the error handling.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":7670,"context_line":"            # (used by) to the instance. So make sure the pci tracker also"},{"line_number":7671,"context_line":"            # tracks this device as allocated. This way we can avoid a possible"},{"line_number":7672,"context_line":"            # race condition when a detach arrives for a device that is only"},{"line_number":7673,"context_line":"            # in claimed state."},{"line_number":7674,"context_line":"            self.rt.allocate_pci_devices_for_instance(context, instance)"},{"line_number":7675,"context_line":""},{"line_number":7676,"context_line":"        instance.save()"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_be9aaa0f","line":7673,"updated":"2020-09-09 15:30:59.000000000","message":"++","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":7709,"context_line":"            raise exception.PortNotFound(_(\"Port %s is not \""},{"line_number":7710,"context_line":"                                           \"attached\") % port_id)"},{"line_number":7711,"context_line":""},{"line_number":7712,"context_line":"        pci_device \u003d None"},{"line_number":7713,"context_line":"        pci_req \u003d pci_req_module.get_instance_pci_request_from_vif("},{"line_number":7714,"context_line":"            context, instance, condemned)"},{"line_number":7715,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_7fcb1552","line":7712,"range":{"start_line":7712,"start_character":0,"end_line":7712,"end_character":25},"updated":"2020-09-07 16:30:28.000000000","message":"drop this","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71e27f796980c37e13bcd41c0dbfd3df4213dd64","unresolved":false,"context_lines":[{"line_number":7709,"context_line":"            raise exception.PortNotFound(_(\"Port %s is not \""},{"line_number":7710,"context_line":"                                           \"attached\") % port_id)"},{"line_number":7711,"context_line":""},{"line_number":7712,"context_line":"        pci_device \u003d None"},{"line_number":7713,"context_line":"        pci_req \u003d pci_req_module.get_instance_pci_request_from_vif("},{"line_number":7714,"context_line":"            context, instance, condemned)"},{"line_number":7715,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_62fc1972","line":7712,"range":{"start_line":7712,"start_character":0,"end_line":7712,"end_character":25},"in_reply_to":"9f560f44_7fcb1552","updated":"2020-09-08 07:47:39.000000000","message":"Done in the follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":7716,"context_line":"        pci_device \u003d None"},{"line_number":7717,"context_line":"        if pci_req:"},{"line_number":7718,"context_line":"            pci_devices \u003d [pci_device"},{"line_number":7719,"context_line":"                            for pci_device in instance.pci_devices.objects"},{"line_number":7720,"context_line":"                            if pci_device.request_id \u003d\u003d pci_req.request_id]"},{"line_number":7721,"context_line":""},{"line_number":7722,"context_line":"            if not pci_devices:"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_dfbce1bc","line":7719,"range":{"start_line":7719,"start_character":27,"end_line":7719,"end_character":28},"updated":"2020-09-07 16:30:28.000000000","message":"nit","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71e27f796980c37e13bcd41c0dbfd3df4213dd64","unresolved":false,"context_lines":[{"line_number":7716,"context_line":"        pci_device \u003d None"},{"line_number":7717,"context_line":"        if pci_req:"},{"line_number":7718,"context_line":"            pci_devices \u003d [pci_device"},{"line_number":7719,"context_line":"                            for pci_device in instance.pci_devices.objects"},{"line_number":7720,"context_line":"                            if pci_device.request_id \u003d\u003d pci_req.request_id]"},{"line_number":7721,"context_line":""},{"line_number":7722,"context_line":"            if not pci_devices:"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_4201156a","line":7719,"range":{"start_line":7719,"start_character":27,"end_line":7719,"end_character":28},"in_reply_to":"9f560f44_dfbce1bc","updated":"2020-09-08 07:47:39.000000000","message":"Done in the follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":1868,"context_line":""},{"line_number":1869,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair\u003dTrue)"},{"line_number":1870,"context_line":"    def claim_pci_devices("},{"line_number":1871,"context_line":"        self, context, pci_requests, instance_numa_topology\u003dNone):"},{"line_number":1872,"context_line":"        \"\"\"Claim instance PCI resources"},{"line_number":1873,"context_line":""},{"line_number":1874,"context_line":"        :param context: security context"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_dfe181d2","line":1871,"range":{"start_line":1871,"start_character":64,"end_line":1871,"end_character":66},"updated":"2020-09-07 16:30:28.000000000","message":"nit:\n\n      self, context, pci_requests, instance_numa_topology\u003dNone,\n  ):","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71e27f796980c37e13bcd41c0dbfd3df4213dd64","unresolved":false,"context_lines":[{"line_number":1868,"context_line":""},{"line_number":1869,"context_line":"    @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair\u003dTrue)"},{"line_number":1870,"context_line":"    def claim_pci_devices("},{"line_number":1871,"context_line":"        self, context, pci_requests, instance_numa_topology\u003dNone):"},{"line_number":1872,"context_line":"        \"\"\"Claim instance PCI resources"},{"line_number":1873,"context_line":""},{"line_number":1874,"context_line":"        :param context: security context"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_e2ef89ae","line":1871,"range":{"start_line":1871,"start_character":64,"end_line":1871,"end_character":66},"in_reply_to":"9f560f44_dfe181d2","updated":"2020-09-08 07:47:39.000000000","message":"Done in the follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"nova/exception.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"fabe16f519252b6a4159d761da8349488a3f9c67","unresolved":false,"context_lines":[{"line_number":1353,"context_line":"                \"for project \u0027%(project_id)s\u0027.\")"},{"line_number":1354,"context_line":""},{"line_number":1355,"context_line":""},{"line_number":1356,"context_line":"class InterfaceAttachPciClaimFailed(Invalid):"},{"line_number":1357,"context_line":"    msg_fmt \u003d _(\"Failed to claim PCI device for %(instance_uuid)s during \""},{"line_number":1358,"context_line":"                \"interface attach\")"},{"line_number":1359,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_beb3ca99","line":1356,"range":{"start_line":1356,"start_character":36,"end_line":1356,"end_character":43},"updated":"2020-09-09 15:30:59.000000000","message":"cool, we expect Invalid exception so this is the right baseclass to inherit from.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"nova/tests/functional/libvirt/test_pci_sriov_servers.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5aff0e692081e02ec2ed9302020c2de5eee0e78c","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _test_detach_attach(self, first_port_id, second_port_id):"},{"line_number":249,"context_line":"        # This test takes two ports that requires PCI claim."},{"line_number":250,"context_line":"        # Starts a compute with on PF and once connected VF."},{"line_number":251,"context_line":"        # Then starts a VM with the first port. Then detach it, then"},{"line_number":252,"context_line":"        # re-attach it. These expected to be successful. Then try to attach the"},{"line_number":253,"context_line":"        # second port and asserts that it fails due no free PCI device left on"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_39071420","line":250,"range":{"start_line":250,"start_character":42,"end_line":250,"end_character":46},"updated":"2020-09-04 14:09:31.000000000","message":"one","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5aff0e692081e02ec2ed9302020c2de5eee0e78c","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _test_detach_attach(self, first_port_id, second_port_id):"},{"line_number":249,"context_line":"        # This test takes two ports that requires PCI claim."},{"line_number":250,"context_line":"        # Starts a compute with on PF and once connected VF."},{"line_number":251,"context_line":"        # Then starts a VM with the first port. Then detach it, then"},{"line_number":252,"context_line":"        # re-attach it. These expected to be successful. Then try to attach the"},{"line_number":253,"context_line":"        # second port and asserts that it fails due no free PCI device left on"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_f9159c4a","line":250,"range":{"start_line":250,"start_character":32,"end_line":250,"end_character":34},"updated":"2020-09-04 14:09:31.000000000","message":"one","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _test_detach_attach(self, first_port_id, second_port_id):"},{"line_number":249,"context_line":"        # This test takes two ports that requires PCI claim."},{"line_number":250,"context_line":"        # Starts a compute with on PF and once connected VF."},{"line_number":251,"context_line":"        # Then starts a VM with the first port. Then detach it, then"},{"line_number":252,"context_line":"        # re-attach it. These expected to be successful. Then try to attach the"},{"line_number":253,"context_line":"        # second port and asserts that it fails due no free PCI device left on"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_4c6848b9","line":250,"range":{"start_line":250,"start_character":42,"end_line":250,"end_character":46},"in_reply_to":"9f560f44_39071420","updated":"2020-09-07 14:33:54.000000000","message":"Done","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _test_detach_attach(self, first_port_id, second_port_id):"},{"line_number":249,"context_line":"        # This test takes two ports that requires PCI claim."},{"line_number":250,"context_line":"        # Starts a compute with on PF and once connected VF."},{"line_number":251,"context_line":"        # Then starts a VM with the first port. Then detach it, then"},{"line_number":252,"context_line":"        # re-attach it. These expected to be successful. Then try to attach the"},{"line_number":253,"context_line":"        # second port and asserts that it fails due no free PCI device left on"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_2c6354df","line":250,"range":{"start_line":250,"start_character":32,"end_line":250,"end_character":34},"in_reply_to":"9f560f44_f9159c4a","updated":"2020-09-07 14:33:54.000000000","message":"Done","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5aff0e692081e02ec2ed9302020c2de5eee0e78c","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        # Starts a compute with on PF and once connected VF."},{"line_number":251,"context_line":"        # Then starts a VM with the first port. Then detach it, then"},{"line_number":252,"context_line":"        # re-attach it. These expected to be successful. Then try to attach the"},{"line_number":253,"context_line":"        # second port and asserts that it fails due no free PCI device left on"},{"line_number":254,"context_line":"        # the host."},{"line_number":255,"context_line":"        host_info \u003d fakelibvirt.HostInfo(cpu_nodes\u003d2, cpu_sockets\u003d1,"},{"line_number":256,"context_line":"                                         cpu_cores\u003d2, cpu_threads\u003d2,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_ac000402","line":253,"range":{"start_line":253,"start_character":48,"end_line":253,"end_character":55},"updated":"2020-09-04 14:09:31.000000000","message":"as no","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":250,"context_line":"        # Starts a compute with on PF and once connected VF."},{"line_number":251,"context_line":"        # Then starts a VM with the first port. Then detach it, then"},{"line_number":252,"context_line":"        # re-attach it. These expected to be successful. Then try to attach the"},{"line_number":253,"context_line":"        # second port and asserts that it fails due no free PCI device left on"},{"line_number":254,"context_line":"        # the host."},{"line_number":255,"context_line":"        host_info \u003d fakelibvirt.HostInfo(cpu_nodes\u003d2, cpu_sockets\u003d1,"},{"line_number":256,"context_line":"                                         cpu_cores\u003d2, cpu_threads\u003d2,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_6c5d4c14","line":253,"range":{"start_line":253,"start_character":48,"end_line":253,"end_character":55},"in_reply_to":"9f560f44_ac000402","updated":"2020-09-07 14:33:54.000000000","message":"Done","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5aff0e692081e02ec2ed9302020c2de5eee0e78c","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        self.assertEqual(\u0027test_compute0\u0027, updated_port[\u0027binding:host_id\u0027])"},{"line_number":284,"context_line":"        self.assertIn(first_port_id, self._get_attached_port_ids(server[\u0027id\u0027]))"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        # Try to attach a second VF but no free PCI device left"},{"line_number":287,"context_line":"        ex \u003d self.assertRaises("},{"line_number":288,"context_line":"            client.OpenStackApiException, self._attach_port, server[\u0027id\u0027],"},{"line_number":289,"context_line":"            second_port_id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_6c16ec4b","line":286,"range":{"start_line":286,"start_character":24,"end_line":286,"end_character":35},"updated":"2020-09-04 14:09:31.000000000","message":"the second port","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        self.assertEqual(\u0027test_compute0\u0027, updated_port[\u0027binding:host_id\u0027])"},{"line_number":284,"context_line":"        self.assertIn(first_port_id, self._get_attached_port_ids(server[\u0027id\u0027]))"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        # Try to attach a second VF but no free PCI device left"},{"line_number":287,"context_line":"        ex \u003d self.assertRaises("},{"line_number":288,"context_line":"            client.OpenStackApiException, self._attach_port, server[\u0027id\u0027],"},{"line_number":289,"context_line":"            second_port_id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f560f44_0ca49007","line":286,"range":{"start_line":286,"start_character":24,"end_line":286,"end_character":35},"in_reply_to":"9f560f44_6c16ec4b","updated":"2020-09-07 14:33:54.000000000","message":"Done","commit_id":"c219e5a55591153e33d319914cff347c1aef9c92"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":199,"context_line":"    PCI_PASSTHROUGH_WHITELIST \u003d [jsonutils.dumps(x) for x in ("},{"line_number":200,"context_line":"        {"},{"line_number":201,"context_line":"            \u0027vendor_id\u0027: fakelibvirt.PCI_VEND_ID,"},{"line_number":202,"context_line":"            \u0027product_id\u0027: fakelibvirt.PF_PROD_ID,"},{"line_number":203,"context_line":"            \"physical_network\": \"physnet2\","},{"line_number":204,"context_line":"        },"},{"line_number":205,"context_line":"        {"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_3f3b6238","line":202,"range":{"start_line":202,"start_character":0,"end_line":202,"end_character":49},"updated":"2020-09-07 11:18:22.000000000","message":"this is the pf","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        },"},{"line_number":205,"context_line":"        {"},{"line_number":206,"context_line":"            \u0027vendor_id\u0027: fakelibvirt.PCI_VEND_ID,"},{"line_number":207,"context_line":"            \u0027product_id\u0027: fakelibvirt.VF_PROD_ID,"},{"line_number":208,"context_line":"            \"physical_network\": \"physnet2\","},{"line_number":209,"context_line":"        },"},{"line_number":210,"context_line":"    )]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_1f0ade77","line":207,"range":{"start_line":207,"start_character":4,"end_line":207,"end_character":49},"updated":"2020-09-07 11:18:22.000000000","message":"and this is the vfs","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":240,"context_line":"    def _attach_port(self, instance_uuid, port_id):"},{"line_number":241,"context_line":"        self.api.attach_interface("},{"line_number":242,"context_line":"            instance_uuid,"},{"line_number":243,"context_line":"            {\u0027interfaceAttachment\u0027: {"},{"line_number":244,"context_line":"                \u0027port_id\u0027: port_id}})"},{"line_number":245,"context_line":"        fake_notifier.wait_for_versioned_notifications("},{"line_number":246,"context_line":"            \u0027instance.interface_attach.end\u0027)"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_9fd38901","line":244,"range":{"start_line":243,"start_character":0,"end_line":244,"end_character":37},"updated":"2020-09-07 16:30:28.000000000","message":"nit: one line","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"85c4377f15d52e2d4d25fa762b6c87f643afdf8b","unresolved":false,"context_lines":[{"line_number":240,"context_line":"    def _attach_port(self, instance_uuid, port_id):"},{"line_number":241,"context_line":"        self.api.attach_interface("},{"line_number":242,"context_line":"            instance_uuid,"},{"line_number":243,"context_line":"            {\u0027interfaceAttachment\u0027: {"},{"line_number":244,"context_line":"                \u0027port_id\u0027: port_id}})"},{"line_number":245,"context_line":"        fake_notifier.wait_for_versioned_notifications("},{"line_number":246,"context_line":"            \u0027instance.interface_attach.end\u0027)"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_8e3fa623","line":244,"range":{"start_line":243,"start_character":0,"end_line":244,"end_character":37},"in_reply_to":"9f560f44_9fd38901","updated":"2020-09-10 09:08:35.000000000","message":"Done in the follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        fake_notifier.wait_for_versioned_notifications("},{"line_number":246,"context_line":"            \u0027instance.interface_attach.end\u0027)"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _test_detach_attach(self, first_port_id, second_port_id):"},{"line_number":249,"context_line":"        # This test takes two ports that requires PCI claim."},{"line_number":250,"context_line":"        # Starts a compute with one PF and one connected VF."},{"line_number":251,"context_line":"        # Then starts a VM with the first port. Then detach it, then"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_1f7f7edf","line":248,"range":{"start_line":248,"start_character":8,"end_line":248,"end_character":27},"updated":"2020-09-07 11:18:22.000000000","message":"+1","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ce162e5b3ae151adeab84a65f1d11534572e1c13","unresolved":false,"context_lines":[{"line_number":286,"context_line":"        # Try to attach the second port but no free PCI device left"},{"line_number":287,"context_line":"        ex \u003d self.assertRaises("},{"line_number":288,"context_line":"            client.OpenStackApiException, self._attach_port, server[\u0027id\u0027],"},{"line_number":289,"context_line":"            second_port_id)"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"        self.assertEqual(400, ex.response.status_code)"},{"line_number":292,"context_line":"        self.assertIn(\u0027Failed to claim PCI device\u0027, str(ex))"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_34035b12","line":289,"updated":"2020-09-09 16:40:51.000000000","message":"okay, the claiming failure is covered here, we\u0027re good.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        attached_ports \u003d self._get_attached_port_ids(server[\u0027id\u0027])"},{"line_number":294,"context_line":"        self.assertIn(first_port_id, attached_ports)"},{"line_number":295,"context_line":"        self.assertNotIn(second_port_id, attached_ports)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def test_detach_attach_direct(self):"},{"line_number":298,"context_line":"        self._test_detach_attach("},{"line_number":299,"context_line":"            self.neutron.sriov_port[\u0027id\u0027], self.neutron.sriov_port2[\u0027id\u0027])"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def test_detach_macvtap(self):"},{"line_number":302,"context_line":"        self._test_detach_attach("},{"line_number":303,"context_line":"            self.neutron.macvtap_port[\u0027id\u0027],"},{"line_number":304,"context_line":"            self.neutron.macvtap_port2[\u0027id\u0027])"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def test_detach_direct_physical(self):"},{"line_number":307,"context_line":"        self._test_detach_attach("},{"line_number":308,"context_line":"            self.neutron.sriov_pf_port[\u0027id\u0027],"},{"line_number":309,"context_line":"            self.neutron.sriov_pf_port2[\u0027id\u0027])"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"class GetServerDiagnosticsServerWithVfTestV21(_PCIServersTestBase):"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_5f6c5635","line":309,"range":{"start_line":296,"start_character":0,"end_line":309,"end_character":46},"updated":"2020-09-07 11:18:22.000000000","message":"cool so you are testing all 3 primary vnic types\n+1\n\nthere is one other you could check which is vnic_type\u003dvirtio-forwarder but it will be similar to macvtap and only used with vrouter so i think its fine to skip that since it need an out of tree network backend to use it.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"14585ee8b657903d7d23d54ed034e23a493e48cd","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        attached_ports \u003d self._get_attached_port_ids(server[\u0027id\u0027])"},{"line_number":294,"context_line":"        self.assertIn(first_port_id, attached_ports)"},{"line_number":295,"context_line":"        self.assertNotIn(second_port_id, attached_ports)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def test_detach_attach_direct(self):"},{"line_number":298,"context_line":"        self._test_detach_attach("},{"line_number":299,"context_line":"            self.neutron.sriov_port[\u0027id\u0027], self.neutron.sriov_port2[\u0027id\u0027])"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def test_detach_macvtap(self):"},{"line_number":302,"context_line":"        self._test_detach_attach("},{"line_number":303,"context_line":"            self.neutron.macvtap_port[\u0027id\u0027],"},{"line_number":304,"context_line":"            self.neutron.macvtap_port2[\u0027id\u0027])"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"    def test_detach_direct_physical(self):"},{"line_number":307,"context_line":"        self._test_detach_attach("},{"line_number":308,"context_line":"            self.neutron.sriov_pf_port[\u0027id\u0027],"},{"line_number":309,"context_line":"            self.neutron.sriov_pf_port2[\u0027id\u0027])"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":""},{"line_number":312,"context_line":"class GetServerDiagnosticsServerWithVfTestV21(_PCIServersTestBase):"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_f92d8546","line":309,"range":{"start_line":296,"start_character":0,"end_line":309,"end_character":46},"in_reply_to":"9f560f44_5f6c5635","updated":"2020-09-07 14:33:54.000000000","message":"I never tried the virtio-forwarder in devstack (I guess I could not set it up) as far as I understand the basic logic is the same as macvtap so I skip this for now.","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":10293,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10294,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027),"},{"line_number":10295,"context_line":"            mock.patch.object(self.compute,"},{"line_number":10296,"context_line":"            \u0027_claim_pci_device_for_interface_attach\u0027,"},{"line_number":10297,"context_line":"            return_value\u003dNone)"},{"line_number":10298,"context_line":"        ) as (cap, mock_lock, mock_claim_pci):"},{"line_number":10299,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_df2fc1f3","line":10296,"updated":"2020-09-07 16:30:28.000000000","message":"nit: missing indentation","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71e27f796980c37e13bcd41c0dbfd3df4213dd64","unresolved":false,"context_lines":[{"line_number":10293,"context_line":"                             supports_attach_interface\u003dTrue),"},{"line_number":10294,"context_line":"            mock.patch(\u0027oslo_concurrency.lockutils.lock\u0027),"},{"line_number":10295,"context_line":"            mock.patch.object(self.compute,"},{"line_number":10296,"context_line":"            \u0027_claim_pci_device_for_interface_attach\u0027,"},{"line_number":10297,"context_line":"            return_value\u003dNone)"},{"line_number":10298,"context_line":"        ) as (cap, mock_lock, mock_claim_pci):"},{"line_number":10299,"context_line":"            vif \u003d self.compute.attach_interface(self.context,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_e244a9ba","line":10296,"in_reply_to":"9f560f44_df2fc1f3","updated":"2020-09-08 07:47:39.000000000","message":"done in the follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":10348,"context_line":"                              \u0027allocate_pci_devices_for_instance\u0027),"},{"line_number":10349,"context_line":"            mock.patch.object(instance, \u0027save\u0027)"},{"line_number":10350,"context_line":"        ) as ("},{"line_number":10351,"context_line":"                mock_capabilities, mock_create_resource_req, mock_claim_pci,"},{"line_number":10352,"context_line":"                mock_allocate_pci, mock_save):"},{"line_number":10353,"context_line":""},{"line_number":10354,"context_line":"            pci_req \u003d objects.InstancePCIRequest(request_id\u003duuids.pci_req)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_1f365952","line":10351,"range":{"start_line":10351,"start_character":12,"end_line":10351,"end_character":16},"updated":"2020-09-07 16:30:28.000000000","message":"nit","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71e27f796980c37e13bcd41c0dbfd3df4213dd64","unresolved":false,"context_lines":[{"line_number":10348,"context_line":"                              \u0027allocate_pci_devices_for_instance\u0027),"},{"line_number":10349,"context_line":"            mock.patch.object(instance, \u0027save\u0027)"},{"line_number":10350,"context_line":"        ) as ("},{"line_number":10351,"context_line":"                mock_capabilities, mock_create_resource_req, mock_claim_pci,"},{"line_number":10352,"context_line":"                mock_allocate_pci, mock_save):"},{"line_number":10353,"context_line":""},{"line_number":10354,"context_line":"            pci_req \u003d objects.InstancePCIRequest(request_id\u003duuids.pci_req)"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_c241a5a8","line":10351,"range":{"start_line":10351,"start_character":12,"end_line":10351,"end_character":16},"in_reply_to":"9f560f44_1f365952","updated":"2020-09-08 07:47:39.000000000","message":"done in the follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"31779cf11439a5af9b0841b975b224932895ac2e","unresolved":false,"context_lines":[{"line_number":10480,"context_line":"                              \u0027_claim_pci_device_for_interface_attach\u0027,"},{"line_number":10481,"context_line":"                              return_value\u003dNone),"},{"line_number":10482,"context_line":"        ) as ("},{"line_number":10483,"context_line":"                mock_notify, mock_attach, mock_allocate, mock_deallocate,"},{"line_number":10484,"context_line":"                mock_dict, mock_claim_pci):"},{"line_number":10485,"context_line":""},{"line_number":10486,"context_line":"            mock_allocate.return_value \u003d nwinfo"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_3f587d88","line":10483,"range":{"start_line":10483,"start_character":12,"end_line":10483,"end_character":16},"updated":"2020-09-07 16:30:28.000000000","message":"nit","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71e27f796980c37e13bcd41c0dbfd3df4213dd64","unresolved":false,"context_lines":[{"line_number":10480,"context_line":"                              \u0027_claim_pci_device_for_interface_attach\u0027,"},{"line_number":10481,"context_line":"                              return_value\u003dNone),"},{"line_number":10482,"context_line":"        ) as ("},{"line_number":10483,"context_line":"                mock_notify, mock_attach, mock_allocate, mock_deallocate,"},{"line_number":10484,"context_line":"                mock_dict, mock_claim_pci):"},{"line_number":10485,"context_line":""},{"line_number":10486,"context_line":"            mock_allocate.return_value \u003d nwinfo"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_2214e19f","line":10483,"range":{"start_line":10483,"start_character":12,"end_line":10483,"end_character":16},"in_reply_to":"9f560f44_3f587d88","updated":"2020-09-08 07:47:39.000000000","message":"Done in a follow up","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":2103,"context_line":"        super(LibvirtConfigGuestHostdevPCI, self).\\"},{"line_number":2104,"context_line":"                __init__(mode\u003d\u0027subsystem\u0027, type\u003d\u0027pci\u0027,"},{"line_number":2105,"context_line":"                         **kwargs)"},{"line_number":2106,"context_line":"        self.domain \u003d None"},{"line_number":2107,"context_line":"        self.bus \u003d None"},{"line_number":2108,"context_line":"        self.slot \u003d None"},{"line_number":2109,"context_line":"        self.function \u003d None"},{"line_number":2110,"context_line":""},{"line_number":2111,"context_line":"    def __eq__(self, other):"},{"line_number":2112,"context_line":"        if isinstance(other, LibvirtConfigGuestHostdevPCI):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_68d23d02","line":2109,"range":{"start_line":2106,"start_character":7,"end_line":2109,"end_character":28},"updated":"2020-08-04 00:08:37.000000000","message":"it might be nice to add a comment with the bit widths of each field. unfuturnetly it does not look like the typing module support fixed width integers so unfortunetlly we cant enforce that with mypy or other typechecking.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1438bdccd5cb19de607f0a27e391c89914e2ef8c","unresolved":false,"context_lines":[{"line_number":2103,"context_line":"        super(LibvirtConfigGuestHostdevPCI, self).\\"},{"line_number":2104,"context_line":"                __init__(mode\u003d\u0027subsystem\u0027, type\u003d\u0027pci\u0027,"},{"line_number":2105,"context_line":"                         **kwargs)"},{"line_number":2106,"context_line":"        self.domain \u003d None"},{"line_number":2107,"context_line":"        self.bus \u003d None"},{"line_number":2108,"context_line":"        self.slot \u003d None"},{"line_number":2109,"context_line":"        self.function \u003d None"},{"line_number":2110,"context_line":""},{"line_number":2111,"context_line":"    def __eq__(self, other):"},{"line_number":2112,"context_line":"        if isinstance(other, LibvirtConfigGuestHostdevPCI):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_29166bf2","line":2109,"range":{"start_line":2106,"start_character":7,"end_line":2109,"end_character":28},"in_reply_to":"9f560f44_68d23d02","updated":"2020-08-05 16:07:10.000000000","message":"These are stored as strings so typing module cannot help anyway. I\u0027ve added some comment base on your comment.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1743a76cdfe4db94ac8efc7f07d016a2b199b227","unresolved":false,"context_lines":[{"line_number":2113,"context_line":"            # NOTE(gibi): nova generates hexa string without 0x prefix but"},{"line_number":2114,"context_line":"            # libvirt uses that prefix when returns the config so we need to"},{"line_number":2115,"context_line":"            # normalize the strings before comparison"},{"line_number":2116,"context_line":"            return (int(self.domain, 16) \u003d\u003d int(other.domain, 16) and"},{"line_number":2117,"context_line":"                    int(self.bus, 16) \u003d\u003d int(other.bus, 16) and"},{"line_number":2118,"context_line":"                    int(self.slot, 16) \u003d\u003d int(other.slot, 16) and"},{"line_number":2119,"context_line":"                    int(self.function, 16) \u003d\u003d int(other.function, 16))"},{"line_number":2120,"context_line":"        else:"},{"line_number":2121,"context_line":"            return False"},{"line_number":2122,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_687fdd14","line":2119,"range":{"start_line":2116,"start_character":10,"end_line":2119,"end_character":70},"updated":"2020-08-04 00:08:37.000000000","message":"technically the function is in octal so this shoudl be 3\nthe slot and bugs are also not technically in hex.\n\nthe pci address is a 32 bit int encoded as follows\n\ndomain (16 bits), bus (8 bits), device (5 bits) and function (3 bits).\n\n\nso technically you coudld decode it more correctly as follows\n\n return (int(self.domain, 16) \u003d\u003d int(other.domain, 16) and\n                    int(self.bus, 8) \u003d\u003d int(other.bus, 8) and\n                    int(self.slot, 5) \u003d\u003d int(other.slot, 5) and\n                    int(self.function, 3) \u003d\u003d int(other.function, 3))\n\nwhat you have will technically work but it will decode invalid pci addresses too.\n\nwe dont really do the righ think in the format_dom below either but it technialy will work if we dont exceed the value that fit in those bit limits.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"1438bdccd5cb19de607f0a27e391c89914e2ef8c","unresolved":false,"context_lines":[{"line_number":2113,"context_line":"            # NOTE(gibi): nova generates hexa string without 0x prefix but"},{"line_number":2114,"context_line":"            # libvirt uses that prefix when returns the config so we need to"},{"line_number":2115,"context_line":"            # normalize the strings before comparison"},{"line_number":2116,"context_line":"            return (int(self.domain, 16) \u003d\u003d int(other.domain, 16) and"},{"line_number":2117,"context_line":"                    int(self.bus, 16) \u003d\u003d int(other.bus, 16) and"},{"line_number":2118,"context_line":"                    int(self.slot, 16) \u003d\u003d int(other.slot, 16) and"},{"line_number":2119,"context_line":"                    int(self.function, 16) \u003d\u003d int(other.function, 16))"},{"line_number":2120,"context_line":"        else:"},{"line_number":2121,"context_line":"            return False"},{"line_number":2122,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_9e1b6b7f","line":2119,"range":{"start_line":2116,"start_character":10,"end_line":2119,"end_character":70},"in_reply_to":"9f560f44_687fdd14","updated":"2020-08-05 16:07:10.000000000","message":"Libvirt returns the address pieces as hexa strings:\n\n\u003caddress domain\u003d\u00270x0000\u0027 bus\u003d\u00270x81\u0027 slot\u003d\u00270x00\u0027 function\u003d\u00270x1\u0027/\u003e\n\nAnd int(\u00270x1\u0027, 3) is an error as 0x is a prefix for a hexa number.\n\n\u003e\u003e\u003e int(\u00270x1\u0027, 3)\nTraceback (most recent call last):\n  File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\nValueError: invalid literal for int() with base 3: \u00270x1\u0027\n\nSo I do have to parse the strings as hexas.\n\nAlternatively I could do string manipulation. Something like:\n\n    @staticmethod\n    def _equal_without_hexa_prefix(a, b):\n        return a.split(\u0027x\u0027)[-1] \u003d\u003d b.split(\u0027x\u0027)[-1]\n\nBut don\u0027t really like this alternative.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":2113,"context_line":"            # NOTE(gibi): nova generates hexa string without 0x prefix but"},{"line_number":2114,"context_line":"            # libvirt uses that prefix when returns the config so we need to"},{"line_number":2115,"context_line":"            # normalize the strings before comparison"},{"line_number":2116,"context_line":"            return (int(self.domain, 16) \u003d\u003d int(other.domain, 16) and"},{"line_number":2117,"context_line":"                    int(self.bus, 16) \u003d\u003d int(other.bus, 16) and"},{"line_number":2118,"context_line":"                    int(self.slot, 16) \u003d\u003d int(other.slot, 16) and"},{"line_number":2119,"context_line":"                    int(self.function, 16) \u003d\u003d int(other.function, 16))"},{"line_number":2120,"context_line":"        else:"},{"line_number":2121,"context_line":"            return False"},{"line_number":2122,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_97806948","line":2119,"range":{"start_line":2116,"start_character":10,"end_line":2119,"end_character":70},"in_reply_to":"9f560f44_9e1b6b7f","updated":"2020-08-05 21:18:00.000000000","message":"its fine parsing in as hex sting will work.","commit_id":"e1566c1d3d1876328c800e1501b60158911bf6be"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":2107,"context_line":"        # These are returned from libvirt as hexadecimal strings with 0x prefix"},{"line_number":2108,"context_line":"        # even if they have a different meaningful range: domain 16 bit,"},{"line_number":2109,"context_line":"        # bus 8 bit, slot 5 bit, and function 3 bit"},{"line_number":2110,"context_line":"        # In the other hand nova generates these values without the 0x prefix"},{"line_number":2111,"context_line":"        self.domain \u003d None"},{"line_number":2112,"context_line":"        self.bus \u003d None"},{"line_number":2113,"context_line":"        self.slot \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_97f509f0","line":2110,"range":{"start_line":2110,"start_character":10,"end_line":2110,"end_character":12},"updated":"2020-08-05 21:18:00.000000000","message":"nit: on","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"67a393d0c64e7e5de23ffa805fc955435f405243","unresolved":false,"context_lines":[{"line_number":2107,"context_line":"        # These are returned from libvirt as hexadecimal strings with 0x prefix"},{"line_number":2108,"context_line":"        # even if they have a different meaningful range: domain 16 bit,"},{"line_number":2109,"context_line":"        # bus 8 bit, slot 5 bit, and function 3 bit"},{"line_number":2110,"context_line":"        # In the other hand nova generates these values without the 0x prefix"},{"line_number":2111,"context_line":"        self.domain \u003d None"},{"line_number":2112,"context_line":"        self.bus \u003d None"},{"line_number":2113,"context_line":"        self.slot \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_322cd097","line":2110,"range":{"start_line":2110,"start_character":10,"end_line":2110,"end_character":12},"in_reply_to":"9f560f44_97f509f0","updated":"2020-08-27 15:00:15.000000000","message":"Done","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"185e9a4d8bcc5034c47f47431a761dc676b89eb3","unresolved":false,"context_lines":[{"line_number":2116,"context_line":"    def __eq__(self, other):"},{"line_number":2117,"context_line":"        if not isinstance(other, LibvirtConfigGuestHostdevPCI):"},{"line_number":2118,"context_line":"            return False"},{"line_number":2119,"context_line":"            "},{"line_number":2120,"context_line":"        # NOTE(gibi): nova generates hexa string without 0x prefix but"},{"line_number":2121,"context_line":"        # libvirt uses that prefix when returns the config so we need to"},{"line_number":2122,"context_line":"        # normalize the strings before comparison"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_d795c188","line":2119,"range":{"start_line":2119,"start_character":0,"end_line":2119,"end_character":12},"updated":"2020-08-05 21:18:00.000000000","message":":O shock and horror.\ni use tab too :)","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"bea5ce0878da0b71ddda691e9499f358c5e616ad","unresolved":false,"context_lines":[{"line_number":2116,"context_line":"    def __eq__(self, other):"},{"line_number":2117,"context_line":"        if not isinstance(other, LibvirtConfigGuestHostdevPCI):"},{"line_number":2118,"context_line":"            return False"},{"line_number":2119,"context_line":"            "},{"line_number":2120,"context_line":"        # NOTE(gibi): nova generates hexa string without 0x prefix but"},{"line_number":2121,"context_line":"        # libvirt uses that prefix when returns the config so we need to"},{"line_number":2122,"context_line":"        # normalize the strings before comparison"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_57341193","line":2119,"updated":"2020-08-05 20:37:35.000000000","message":"pep8: W293 blank line contains whitespace","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"67a393d0c64e7e5de23ffa805fc955435f405243","unresolved":false,"context_lines":[{"line_number":2116,"context_line":"    def __eq__(self, other):"},{"line_number":2117,"context_line":"        if not isinstance(other, LibvirtConfigGuestHostdevPCI):"},{"line_number":2118,"context_line":"            return False"},{"line_number":2119,"context_line":"            "},{"line_number":2120,"context_line":"        # NOTE(gibi): nova generates hexa string without 0x prefix but"},{"line_number":2121,"context_line":"        # libvirt uses that prefix when returns the config so we need to"},{"line_number":2122,"context_line":"        # normalize the strings before comparison"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_921f9c59","line":2119,"range":{"start_line":2119,"start_character":0,"end_line":2119,"end_character":12},"in_reply_to":"9f560f44_d795c188","updated":"2020-08-27 15:00:15.000000000","message":"Done. I blame pycharm.","commit_id":"10191794dd3c2e788b755dd475e78623ed2ec83b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7d9f7af62573330cef222edef7f3823ffd788532","unresolved":false,"context_lines":[{"line_number":1692,"context_line":""},{"line_number":1693,"context_line":"        # NOTE(arches) Skip checking target_dev for vhostuser"},{"line_number":1694,"context_line":"        # vif type; target_dev is not a valid value for vhostuser."},{"line_number":1695,"context_line":"        # NOTE(gibi): For macvtap cases the domain has a target_dev"},{"line_number":1696,"context_line":"        # generated by libvirt. It is not set by the vif driver code"},{"line_number":1697,"context_line":"        # so it is not in config returned by the vif driver so we"},{"line_number":1698,"context_line":"        # should not match on that."},{"line_number":1699,"context_line":"        return ("},{"line_number":1700,"context_line":"            self.mac_addr \u003d\u003d other.mac_addr and"},{"line_number":1701,"context_line":"            self.net_type \u003d\u003d other.net_type and"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_5fda96e8","line":1698,"range":{"start_line":1695,"start_character":7,"end_line":1698,"end_character":35},"updated":"2020-09-07 11:18:22.000000000","message":"+1","commit_id":"a1169be58e15a1cfa7865667e2ca229cf22243b2"}]}