| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-11-15 | |||
| 12:31:35 | sean-k-mooney | now you have shown that changing the behaivor for new isntnace should be safe | |
| 12:31:53 | sean-k-mooney | that is great it does not change the fact that we had previously agreed not too | |
| 12:31:55 | kashyap | Correct | |
| 12:31:59 | kashyap | Well | |
| 12:32:14 | kashyap | We should revisit previous "agreements" if they don't make sense in light of new info | |
| 12:32:16 | sean-k-mooney | if people are ok with makeing the change we can but we should at least disucss that | |
| 12:32:44 | kashyap | Yes, but I hope it won't become a big drag. | |
| 12:35:35 | gibi | kashyap, sean-k-mooney: OK, so the cirros -> virtio change is considered harmless during a hard reboot, then I'm OK with the direction of the patch | |
| 12:35:58 | kashyap | Cool; yes. And that is proved by the tests. Thanks. | |
| 12:44:22 | kashyap | sean-k-mooney: Wait: what is still not correct in the broader direction? On hard-reboot, we recreate the XML, and yes, it *will* change from CirrOS to virtio - the QEMU folks confirmed that harmless ABI change is okay | |
| 12:44:33 | kashyap | sean-k-mooney: So what is still "not correct in this patch"? | |
| 12:44:38 | kashyap | Please keep the explanation short. | |
| 12:50:30 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova-specs master: Integration With Off-path Network Backends https://review.opendev.org/c/openstack/nova-specs/+/787458 | |
| 12:54:00 | sean-k-mooney | kashyap: gibi has just now indicated he is ok with the change in direction. other have not bar perhaps stephen. the patch as written assumed that change in direction would happen without disucssing it | |
| 12:54:53 | kashyap | sean-k-mooney: Okay, for that part - I'll explicitly state it in the commit message; and as a comment somewhere. | |
| 12:54:55 | sean-k-mooney | the other comment i left tell you how to modify the patch to confrom to what we had previosuly agreed. e.g. record the current used video model | |
| 12:55:55 | kashyap | sean-k-mooney: What is the use-case for still recording the used video model? | |
| 12:56:25 | kashyap | When we know that changing it even for existing instances on hard-reboot is benign and harmless? | |
| 12:56:32 | sean-k-mooney | consitency with the change fo default of machine type | |
| 12:57:28 | kashyap | Sorry, that's not a valid use-case. (While I agree in general to be consistent where it makes sense) | |
| 12:57:33 | sean-k-mooney | in this case you are instead following what we did for the rng which is ok | |
| 12:58:03 | kashyap | We should not add needless extra code for "consistency" if it is not solving any problem or improving maintainability | |
| 12:58:19 | kashyap | Yep; like for RNG indeed | |
| 13:03:55 | dmitriis | sean-k-mooney, gibi: updated the spec https://review.opendev.org/c/openstack/nova-specs/+/787458. This comment may need further discussion https://review.opendev.org/c/openstack/nova-specs/+/787458/comment/4337b974_a214c6f9/ during the implementation (I added code references and the actual log to give an impression of what I am talking about). I | |
| 13:03:55 | dmitriis | agree about the need to extend has_bind_time_event (which I added to the spec) but, from what I can see, this won't help with the actual problem of the virt driver code trying to wait for the `network-vif-plugged` event that gets discarded by Nova in the absence of a waiter. | |
| 13:05:50 | sean-k-mooney | dmitriis: the wait in the virt driver shoudl be filtered by the bindtyime/plugtime event fucntions | |
| 13:09:22 | sean-k-mooney | dmitriis: if we know that a backend only sends bind time event reliably its oke to ignore events sent at other times | |
| 13:10:49 | dmitriis | sean-k-mooney: so you are proposing that _get_neutron_events https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/virt/libvirt/driver.py#L7191-L7198 calls get_bind_time_events https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/network/model.py#L567-L572 for filtering, right? | |
| 13:10:52 | sean-k-mooney | dmitriis: if you modify has_bind_time_event to account for the new vnic type | |
| 13:11:23 | sean-k-mooney | https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/virt/libvirt/driver.py#L7221-L7222 shoudl return an empty [] | |
| 13:11:26 | opendevreview | Kashyap Chamarthy proposed openstack/nova master: libvirt: Introduce config class for QEMU's "tb-cache" https://review.opendev.org/c/openstack/nova/+/816823 | |
| 13:12:03 | opendevreview | Takashi Kajinami proposed openstack/nova master: Deprecate [wsgi] secure_proxy_ssl_header https://review.opendev.org/c/openstack/nova/+/817927 | |
| 13:12:17 | sean-k-mooney | dmitriis: yes more or less | |
| 13:12:31 | opendevreview | Takashi Kajinami proposed openstack/nova master: Deprecate [wsgi] secure_proxy_ssl_header https://review.opendev.org/c/openstack/nova/+/817927 | |
| 13:12:33 | sean-k-mooney | i belive that shoudl eb using the plugtime event funciton | |
| 13:12:51 | sean-k-mooney | dmitriis: since we have already passed binding the port | |
| 13:13:02 | sean-k-mooney | dmitriis: currently it is https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/virt/libvirt/driver.py#L7191-L7198 | |
| 13:15:01 | sean-k-mooney | dmitriis: effectvly it s implementaiton shoudl be like this https://github.com/openstack/nova/blob/0e0196d979cf1b8e63b9656358116a36f1f09ede/nova/network/model.py#L581-L586 | |
| 13:15:09 | sean-k-mooney | i suspect that is a latent bug actully | |
| 13:16:06 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove use of 'bind' arguments https://review.opendev.org/c/openstack/nova/+/817747 | |
| 13:16:06 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove unnecessary warning filters https://review.opendev.org/c/openstack/nova/+/817748 | |
| 13:17:08 | sean-k-mooney | actully dmitriis :) this might be the cause of our revert resize issue | |
| 13:17:09 | dmitriis | sean-k-mooney: the VIF object itself has has_bind_time_event actually https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/network/model.py#L488-L497 . So I can extend it to accept `None` for the migration object and add handling for VNIC_TYPE since that's available in the VIF itself. | |
| 13:17:35 | dmitriis | sean-k-mooney: oh well, good thing we looked at this code then :^) | |
| 13:18:58 | sean-k-mooney | yes i would extend the vif class | |
| 13:20:32 | sean-k-mooney | artom has a repoducer patch up for the resize bug i think | |
| 13:20:53 | sean-k-mooney | so we might fix that issue for you in a seperate patch so we can backport it | |
| 13:22:30 | sean-k-mooney | this is where we repoduced our downstream bug https://review.opendev.org/c/openstack/nova/+/817303 | |
| 13:23:14 | sean-k-mooney | the revert resize test fail in a very specific situation whic i think is basially the same thing you are seeing | |
| 13:23:16 | sean-k-mooney | https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_f60/817303/2/check/nova-ovs-hybrid-plug/f60d54c/testr_results.html | |
| 13:24:51 | dmitriis | sean-k-mooney: ok, I'll fix it in my WIP spec implementation proposal https://review.opendev.org/c/openstack/nova/+/812111 and try it out on the actual test environment. We can have a separate patch to fix the issue and that will leave my patch with an extension to has_bind_time_event to handle VNIC_TYPE_SMARTNIC. | |
| 13:25:15 | sean-k-mooney | yep that makes sense to me | |
| 13:29:52 | sean-k-mooney | hum actully for the revert case we pass in the filtered events list | |
| 13:29:54 | sean-k-mooney | https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/virt/libvirt/driver.py#L11204-L11210 | |
| 13:30:07 | opendevreview | Kashyap Chamarthy proposed openstack/nova master: Deprecate support for floppy drives https://review.opendev.org/c/openstack/nova/+/802022 | |
| 13:30:08 | sean-k-mooney | dmitriis: but this is still broken for your usecase | |
| 13:30:13 | sean-k-mooney | so we should still fix it | |
| 13:31:10 | dmitriis | sean-k-mooney: yes, it looks like the "active" state check might be there for that case but it doesn't help in my case | |
| 13:31:47 | sean-k-mooney | the active state chagne on the neutron port? | |
| 13:32:39 | dmitriis | sean-k-mooney: yes, on this line https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/virt/libvirt/driver.py#L7198 | |
| 13:32:56 | dmitriis | this state gets updated via the info cache from what I understand | |
| 13:33:15 | sean-k-mooney | not quite | |
| 13:33:35 | sean-k-mooney | it will but that is not related | |
| 13:33:40 | sean-k-mooney | this is the admin state | |
| 13:33:56 | dmitriis | I see | |
| 13:34:03 | sean-k-mooney | we skip port that have been marked as down | |
| 13:34:52 | dmitriis | sean-k-mooney: is it OK if I extend has_bind_time_event to accept `None` for the `migration` argument? https://github.com/openstack/nova/blob/0e0196d979cf1b8e63b9656358116a36f1f09ede/nova/network/model.py#L488-L497 | |
| 13:35:37 | dmitriis | I don't have it in _get_neutron_events https://github.com/openstack/nova/blob/e28afc564700a1a35e3bf0269687d5734251b88a/nova/virt/libvirt/driver.py#L7191-L7198 and I'd rather not try to instantiate one there | |
| 13:35:40 | sean-k-mooney | i think so i have been trying to figure out what that should look like and or if you should jsut replace migration with same_host=True/False | |
| 13:36:20 | kashyap | sean-k-mooney: Hmm, this page should also list "bochs" as an option, right? - https://docs.openstack.org/glance/xena/admin/useful-image-properties.html | |
| 13:36:37 | kashyap | I think I forgot to updte the docs for that :-( I'm going to do that now | |
| 13:36:49 | sean-k-mooney | yes eventually and you should update the glance metadefs if you have not already done so | |
| 13:36:51 | kashyap | For hw_video_model, I mean | |
| 13:37:24 | sean-k-mooney | https://github.com/openstack/glance/blob/master/etc/metadefs/compute-libvirt-image.json#L71-L84 | |
| 13:37:44 | sean-k-mooney | that used to generate the horizon and heat dropdown menues | |
| 13:38:16 | sean-k-mooney | you should be able to do both in the same patch | |
| 13:39:22 | kashyap | Right; I updated this file: etc/metadefs/compute-libvirt-image.json and also this file: doc/source/admin/useful-image-properties.rst | |
| 13:39:44 | dmitriis | sean-k-mooney: yes, making `has_bind_time_event` accept a boolean seems cleaner than passing None. There aren't any other migration attributes used so far in it. | |
| 13:40:43 | kashyap | sean-k-mooney: I wonder if I should add a belated release note to the Glance docs? | |
| 13:41:31 | dmitriis | sean-k-mooney: I've put generic wording into the spec for now https://review.opendev.org/c/openstack/nova-specs/+/787458/12/specs/yoga/approved/integration-with-off-path-network-backends.rst#343 I think it encompasses whatever we decide implementation-wise. | |
| 13:41:43 | sean-k-mooney | dmitriis: right it also an encapsulation thing e.g. pass the minium it need rather then large objects | |
| 13:41:55 | dmitriis | sean-k-mooney: yes, agreed | |
| 13:43:42 | opendevreview | Takashi Kajinami proposed openstack/nova master: Deprecate [wsgi] secure_proxy_ssl_header https://review.opendev.org/c/openstack/nova/+/817927 | |
| 13:45:27 | sean-k-mooney | dmitriis: something like this i think woudl make sense https://paste.opendev.org/show/811000/ | |
| 13:46:06 | sean-k-mooney | actuly result &= self['vnic_type'] != 'smartnic' shoudl be result |= self['vnic_type'] == 'smartnic' | |
| 13:46:30 | kashyap | Posted the docs: https://review.opendev.org/c/openstack/glance/+/817950 (Add "bochs" as a possible video model) | |
| 13:47:07 | sean-k-mooney | i.e. if the vnic_type is smartnic then we know the vif has bind time events | |
| 13:49:01 | dmitriis | sean-k-mooney: yeah, that's certainly better than my original workaround https://git.launchpad.net/~dmitriis/+git/nova/commit/?h=2021-09-10-off-path-net-backends&id=76bd42c59a33d3c9d410444ac32d3babde472ba4 | |
| 13:52:18 | kashyap | sean-k-mooney: Thank you for the quick review | |
| 15:37:56 | Zer0Byte | hey guys | |
| 15:39:04 | Zer0Byte | im having a issue with nova for one reason i can't live migrate vms from one node to a specific node im getting this error while im trying to move to these node Failed to retrieve allocations for consumer d9f8c2ed-74b0-4eb9-8b1d-30f397ec2102: Expected to find allocations for source node resource provider f7751b40-7f22-4b2f-8d40-b1479a369969. Retry the operation without forcing a destination host. | |
| 15:39:17 | Zer0Byte | checking openstack resource provider | |
| 15:39:31 | Zer0Byte | don't show the specifc node | |
| 15:39:49 | Zer0Byte | who should create on openstack resource provider these record? | |
| 15:43:52 | sean-k-mooney | Zer0Byte: nova | |
| 15:44:12 | sean-k-mooney | are you upgrading form a deployment without placement to one with placment | |
| 15:44:38 | sean-k-mooney | if so there are some specific steps you need to do to enusre the allcoation are created | |
| 15:45:10 | Zer0Byte | no was runinng perfect i notice some performance degradation to keystone because switft that make me belive that nova can't communicate with placement on these moment | |
| 15:46:22 | sean-k-mooney | hum you suspect that the keystone authention some how did not work when it was trying to retive the allcoations | |
| 15:46:36 | sean-k-mooney | have you check the instnace in quetion to see if it has allcoation manually | |