)]}'
{"nova/tests/unit/virt/libvirt/test_vif.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7bfea9593f14a6e1cf0809d5e6fa71af0706074b","unresolved":false,"context_lines":[{"line_number":712,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_DIRECT,"},{"line_number":713,"context_line":"            image_meta\u003d{\u0027hw_vif_type\u0027: \u0027virtio\u0027})"},{"line_number":714,"context_line":"        self.assertIsNone(conf.vhost_rx_queue_size)"},{"line_number":715,"context_line":"        self.assertIsNone(conf.vhost_tx_queue_size)"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":718,"context_line":"    def test_virtio_vhost_queue_sizes_vnic_type_direct_physical("}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_52756ff9","line":715,"updated":"2019-06-19 18:05:29.000000000","message":"If I revert your functional change to vif.py, this still passes. That means there\u0027s no assertion that it\u0027s actually doing the different thing.","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c1ee2f92ac0f1f584fce3e59b0d0addaadc39616","unresolved":false,"context_lines":[{"line_number":712,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_DIRECT,"},{"line_number":713,"context_line":"            image_meta\u003d{\u0027hw_vif_type\u0027: \u0027virtio\u0027})"},{"line_number":714,"context_line":"        self.assertIsNone(conf.vhost_rx_queue_size)"},{"line_number":715,"context_line":"        self.assertIsNone(conf.vhost_tx_queue_size)"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"},{"line_number":718,"context_line":"    def test_virtio_vhost_queue_sizes_vnic_type_direct_physical("}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_91b437d3","line":715,"in_reply_to":"9fb8cfa7_52756ff9","updated":"2019-06-20 10:38:25.000000000","message":"Done","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"968a0ce4bd7a12ae1bd4001889570300a576938d","unresolved":false,"context_lines":[{"line_number":712,"context_line":"        _, _, conf \u003d self._test_virtio_config_queue_sizes("},{"line_number":713,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_DIRECT,"},{"line_number":714,"context_line":"            image_meta\u003d{\u0027hw_vif_model\u0027: \u0027virtio\u0027})"},{"line_number":715,"context_line":"        self.assertIsNone(conf.vhost_rx_queue_size)"},{"line_number":716,"context_line":"        self.assertIsNone(conf.vhost_tx_queue_size)"},{"line_number":717,"context_line":""},{"line_number":718,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_a147a8bc","line":715,"updated":"2019-06-20 15:03:24.000000000","message":"This is what fails now when I revert the vif.py change. That\u0027s because, I think, with model set (incorrectly) to VIRTIO we go on to setting the queue size and thus they\u0027re not None. Since this test is specifically for direct, can you just validate that the model is, uh, whatever it should be and not virtio? I feel like that\u0027s a much more obvious failure for catching a regression. The model being not set to virtio is what the bug is about, AFAICT, and this is testing something that is indirectly set if the model is set.","commit_id":"d6d26710752e87e189bfd0448c528053c282b1a7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1b63d88670a3b90a7a7f3a1b9b660a061c93e404","unresolved":false,"context_lines":[{"line_number":712,"context_line":"        _, _, conf \u003d self._test_virtio_config_queue_sizes("},{"line_number":713,"context_line":"            vnic_type\u003dnetwork_model.VNIC_TYPE_DIRECT,"},{"line_number":714,"context_line":"            image_meta\u003d{\u0027hw_vif_model\u0027: \u0027virtio\u0027})"},{"line_number":715,"context_line":"        self.assertIsNone(conf.vhost_rx_queue_size)"},{"line_number":716,"context_line":"        self.assertIsNone(conf.vhost_tx_queue_size)"},{"line_number":717,"context_line":""},{"line_number":718,"context_line":"    @mock.patch.object(host.Host, \"has_min_version\", return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_6761c039","line":715,"in_reply_to":"9fb8cfa7_a147a8bc","updated":"2019-06-20 16:19:02.000000000","message":"Done. I\u0027ve actually chosen another test to modify that seemed to be a better fix (and fixed a bug in that test while I\u0027m at it)","commit_id":"d6d26710752e87e189bfd0448c528053c282b1a7"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f2a692612e34b1b3dc315e8cc82558f5d014d6e1","unresolved":false,"context_lines":[{"line_number":879,"context_line":"        d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":880,"context_line":"        hostimpl \u003d host.Host(\"qemu:///system\")"},{"line_number":881,"context_line":"        image_meta \u003d objects.ImageMeta.from_dict("},{"line_number":882,"context_line":"            {\u0027properties\u0027: {\u0027hw_vif_model\u0027: \u0027virtio\u0027}})"},{"line_number":883,"context_line":"        conf \u003d d.get_base_config(None, \u0027ca:fe:de:ad:be:ef\u0027, image_meta,"},{"line_number":884,"context_line":"                                 None, \u0027kvm\u0027, network_model.VNIC_TYPE_DIRECT,"},{"line_number":885,"context_line":"                                 hostimpl)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_47483c86","line":882,"updated":"2019-06-20 16:30:00.000000000","message":"The test passes without this change, but I guess this is just here to make sure that even if we ask for it explicitly, we don\u0027t get a virtio model in the config.","commit_id":"a235451578ad7af25268e82500dff5ba6a64ea4e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e4597ddf506f8f1d1ceb56088eba2a57d519cf42","unresolved":false,"context_lines":[{"line_number":879,"context_line":"        d \u003d vif.LibvirtGenericVIFDriver()"},{"line_number":880,"context_line":"        hostimpl \u003d host.Host(\"qemu:///system\")"},{"line_number":881,"context_line":"        image_meta \u003d objects.ImageMeta.from_dict("},{"line_number":882,"context_line":"            {\u0027properties\u0027: {\u0027hw_vif_model\u0027: \u0027virtio\u0027}})"},{"line_number":883,"context_line":"        conf \u003d d.get_base_config(None, \u0027ca:fe:de:ad:be:ef\u0027, image_meta,"},{"line_number":884,"context_line":"                                 None, \u0027kvm\u0027, network_model.VNIC_TYPE_DIRECT,"},{"line_number":885,"context_line":"                                 hostimpl)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_a706d81f","line":882,"in_reply_to":"9fb8cfa7_47483c86","updated":"2019-06-20 16:34:14.000000000","message":"Correct","commit_id":"a235451578ad7af25268e82500dff5ba6a64ea4e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f2a692612e34b1b3dc315e8cc82558f5d014d6e1","unresolved":false,"context_lines":[{"line_number":887,"context_line":"                                         None, None, None, None)"},{"line_number":888,"context_line":"        self.assertIsNone(conf.vhost_queues)"},{"line_number":889,"context_line":"        self.assertIsNone(conf.driver_name)"},{"line_number":890,"context_line":"        self.assertIsNone(conf.model)"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"    def _test_model_qemu(self, *vif_objs, **kw):"},{"line_number":893,"context_line":"        libvirt_version \u003d kw.get(\u0027libvirt_version\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_27b74889","line":890,"updated":"2019-06-20 16:30:00.000000000","message":"If I revert your vif change, I never get here because the mock call assertion fails because the model is in there. But, good to have it regardless I think.","commit_id":"a235451578ad7af25268e82500dff5ba6a64ea4e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e4597ddf506f8f1d1ceb56088eba2a57d519cf42","unresolved":false,"context_lines":[{"line_number":887,"context_line":"                                         None, None, None, None)"},{"line_number":888,"context_line":"        self.assertIsNone(conf.vhost_queues)"},{"line_number":889,"context_line":"        self.assertIsNone(conf.driver_name)"},{"line_number":890,"context_line":"        self.assertIsNone(conf.model)"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"    def _test_model_qemu(self, *vif_objs, **kw):"},{"line_number":893,"context_line":"        libvirt_version \u003d kw.get(\u0027libvirt_version\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_c7018c08","line":890,"in_reply_to":"9fb8cfa7_27b74889","updated":"2019-06-20 16:34:14.000000000","message":"I spotted that too (it\u0027s how I realized we were missing the \u0027wraps\u0027 parameter) but I figured explicit \u003e implicit","commit_id":"a235451578ad7af25268e82500dff5ba6a64ea4e"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7bfea9593f14a6e1cf0809d5e6fa71af0706074b","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        vhost_queues \u003d None"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_37c22deb","line":150,"range":{"start_line":150,"start_character":52,"end_line":150,"end_character":58},"updated":"2019-06-19 18:05:29.000000000","message":"defined","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c1ee2f92ac0f1f584fce3e59b0d0addaadc39616","unresolved":false,"context_lines":[{"line_number":147,"context_line":"        vhost_queues \u003d None"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_f1ab3372","line":150,"range":{"start_line":150,"start_character":52,"end_line":150,"end_character":58},"in_reply_to":"9fb8cfa7_37c22deb","updated":"2019-06-20 10:38:25.000000000","message":"Done","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7bfea9593f14a6e1cf0809d5e6fa71af0706074b","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_32679b28","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"updated":"2019-06-19 18:05:29.000000000","message":"This whole section is now pretty hard to reason about I think. It\u0027s a conditional block, and based on the pattern in this method, it looks like it only applies in a subset of cases. But in reality, it applies in almost all cases. Further, L160 is now really an \"elif\" of your new L155, in a very strange way, only because it used to be run after old L151.\n\nI think something like the following would be a lot easier to reason about:\n\n if vnic_type not in network_model....DIRECT:\n     if virt_type in (\u0027kvm\u0027, ...):\n         model \u003d network_model.VIF_MODEL_VIRTIO\n else:\n     if image_meta:\n         model \u003d osinfo.HardwareProperties(...)\n\nand you might be able to collapse those even further into a single if..elif.","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"819669ab149db266a2f69f4b5f8848d44b678869","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_42558f4f","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"in_reply_to":"9fb8cfa7_23766ada","updated":"2019-06-20 13:52:01.000000000","message":"\u003e if the vnic type was direct or direct-physical we would take the\n \u003e else block and if the image metatdata was set that value would be\n \u003e used which should never happen for sriov direct or direct-physical\n \u003e ports.\n \u003e \n \u003e your if branch is also wrong as it would never use the image\n \u003e metadata in that case to set the model.\n\nThat\u0027s why I said \"something like\" and wrote pseudocode. I obviously reversed the logic trying to decode the mess that is here.","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c1ee2f92ac0f1f584fce3e59b0d0addaadc39616","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_71b183e1","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"in_reply_to":"9fb8cfa7_23766ada","updated":"2019-06-20 10:38:25.000000000","message":"This whole function needs to be rewritten, as Sahid notes above. That\u0027s not backportable though so I\u0027m going to keep this as-is and then refactor in a follow-up","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9a5dec13aefa422f60feb63b57da47e49c5f500c","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_41a7cc0c","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"in_reply_to":"9fb8cfa7_32679b28","updated":"2019-06-20 14:43:00.000000000","message":"\u003e This whole section is now pretty hard to reason about I think. It\u0027s\n \u003e a conditional block, and based on the pattern in this method, it\n \u003e looks like it only applies in a subset of cases. But in reality, it\n \u003e applies in almost all cases. Further, L160 is now really an \"elif\"\n \u003e of your new L155, in a very strange way, only because it used to be\n \u003e run after old L151.\n\nI don\u0027t think that\u0027s true. It seems there\u0027s a good chance that \u0027osinfo.HardwareProperties(image_meta).network_model\u0027 could return nothing. It will attempt to retrieve a network model from the image metadata, falling back to the OSInfo default if none is provided:\n\nhttps://github.com/openstack/nova/blob/c18f7f47f6/nova/virt/osinfo.py#L128-L129\n\nHowever, there won\u0027t be a default to retrieve if the \u0027os_distro\u0027 image metadata field isn\u0027t set:\n\nhttps://github.com/openstack/nova/blob/c18f7f47f6/nova/virt/osinfo.py#L123-L124\nhttps://github.com/openstack/nova/blob/c18f7f47f6/nova/virt/osinfo.py#L89-L92\nhttps://github.com/openstack/nova/blob/c18f7f47f6/nova/virt/osinfo.py#L98-L105\n\nI can\u0027t see anything in the nova or glance documentation suggesting \u0027os_distro\u0027 *must* be set in image metadata which suggests the later check to see if model has been set yet on line 160 is valid.\n\nOr am I missing something?","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"27a99f65781cbf9e9b09b4b67896ba3260d85d7c","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_23766ada","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"in_reply_to":"9fb8cfa7_32679b28","updated":"2019-06-20 00:50:17.000000000","message":"if the vnic type was direct or direct-physical we would take the else block and if the image metatdata was set that value would be used which should never happen for sriov direct or direct-physical ports.\n\nyour if branch is also wrong as it would never use the image metadata in that case to set the model.","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"948664dd33c2b07eb232435c5c8d75efd321f5fd","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_61e8d015","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"in_reply_to":"9fb8cfa7_41a7cc0c","updated":"2019-06-20 14:44:29.000000000","message":"BTW, if \u0027os_distro\u0027 is required then (a) both Google and our docs are failing me, hard and (b) the \u0027nova/virt/osinfo.py\u0027 needs some good cleanup","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"968a0ce4bd7a12ae1bd4001889570300a576938d","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_8164e44a","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"in_reply_to":"9fb8cfa7_61e8d015","updated":"2019-06-20 15:03:24.000000000","message":"Okay, yeah, agree that that\u0027s the case. It\u0027s obscure and gross, but whatever. I still don\u0027t like what\u0027s added here as it feels negative instead of positive like the cases below it and thus is more confusing, but, alas.","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"819669ab149db266a2f69f4b5f8848d44b678869","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # Note(moshele): Skip passthough vnic_types as they don\u0027t support"},{"line_number":150,"context_line":"        # virtio model and we don\u0027t care about user-define VIF types, given"},{"line_number":151,"context_line":"        # that they\u0027re likely wrong"},{"line_number":152,"context_line":"        if vnic_type not in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":154,"context_line":"            # image then honour that model"},{"line_number":155,"context_line":"            if image_meta:"},{"line_number":156,"context_line":"                model \u003d osinfo.HardwareProperties(image_meta).network_model"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"            # Else if the virt type is KVM/QEMU/VZ(Parallels), then use virtio"},{"line_number":159,"context_line":"            # according to the global config parameter"},{"line_number":160,"context_line":"            if (model is None and"},{"line_number":161,"context_line":"                virt_type in (\u0027kvm\u0027, \u0027qemu\u0027, \u0027parallels\u0027) and"},{"line_number":162,"context_line":"                        CONF.libvirt.use_virtio_for_bridges):"},{"line_number":163,"context_line":"                model \u003d network_model.VIF_MODEL_VIRTIO"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # Workaround libvirt bug, where it mistakenly"},{"line_number":166,"context_line":"        # enables vhost mode, even for non-KVM guests"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_22df3bd7","line":163,"range":{"start_line":152,"start_character":0,"end_line":163,"end_character":54},"in_reply_to":"9fb8cfa7_71b183e1","updated":"2019-06-20 13:52:01.000000000","message":"\u003e This whole function needs to be rewritten, as Sahid notes above.\n \u003e That\u0027s not backportable though so I\u0027m going to keep this as-is and\n \u003e then refactor in a follow-up\n\nA full refactor is not appropriate, but this change makes this substantially worse for understandability, IMHO, and my strong preference would be to not backport something that makes it more confusing.","commit_id":"087ba649c6becd3d3232e7171d7bced5e4b9a352"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f2a692612e34b1b3dc315e8cc82558f5d014d6e1","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        if vnic_type in network_model.VNIC_TYPES_DIRECT_PASSTHROUGH:"},{"line_number":153,"context_line":"            designer.set_vif_guest_frontend_config("},{"line_number":154,"context_line":"                conf, mac, model, driver, vhost_queues, rx_queue_size)"},{"line_number":155,"context_line":"            return conf"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"        # If the user has specified a \u0027vif_model\u0027 against the"},{"line_number":158,"context_line":"        # image then honour that model"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_e787f0b5","line":155,"updated":"2019-06-20 16:30:00.000000000","message":"Thank $DEITY","commit_id":"a235451578ad7af25268e82500dff5ba6a64ea4e"}]}