Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-11-15
10:30:59 songwenping_ hava you met before and is there some solutions?
10:31:13 kashyap sean-k-mooney[m]: I have answered all the questions there. Please re-adjust your -1. I don't see any technical obstacles here
10:37:08 gibi kashyap: sure, I will re-read the patch
10:37:21 gibi kashyap: but you should get the bp approved https://blueprints.launchpad.net/nova/+spec/virtio-as-default-display-device
10:37:25 kashyap Thank you
10:37:27 kashyap Ohh, right
10:38:03 opendevreview Takashi Kajinami proposed openstack/nova master: Deprecate [wsgi] secure_proxy_ssl_header https://review.opendev.org/c/openstack/nova/+/817927
10:38:07 kashyap gibi: It's a tiny patch; also see my summary of testing from 04 Aug.
10:38:46 gibi bauzas: ^^
10:39:18 kashyap (I've got some real live migration tests done by Red Hat QE folks to have Windows + Linux guests - and it all works as expected.)
10:40:14 opendevreview Takashi Kajinami proposed openstack/nova master: Deprecate [wsgi] secure_proxy_ssl_header https://review.opendev.org/c/openstack/nova/+/817927
10:42:02 opendevreview Takashi Kajinami proposed openstack/nova master: Deprecate [wsgi] secure_proxy_ssl_header https://review.opendev.org/c/openstack/nova/+/817927
10:45:12 kashyap gibi: Unrelated: Oddly, this simple class addition is failing only in the CI, not locally here. You see what I'm missing here? - https://paste.opendev.org/show/810997/
10:46:28 bauzas songwenping_: sorry I was afk
10:46:57 bauzas songwenping_: honestly, I don't know, it looks like a nvidia driver issue
10:47:25 bauzas gibi: what do you want me to look ?
10:47:33 songwenping_ bauzas:ok, thanks.
10:58:40 gibi bauzas: kashyap: would like to get a specless bp approved
10:59:00 kashyap bauzas: This one - https://blueprints.launchpad.net/nova/+spec/virtio-as-default-display-device
10:59:03 bauzas gibi: ok, let's discuss this during tomorrow's meeting
10:59:10 bauzas kashyap: add it to the meeting agenda
10:59:34 bauzas and please be around tomorrow during the meeting :)
10:59:52 kashyap Yes, I'll be around
10:59:57 kashyap Yea; thx
11:02:03 gibi ack, I will be on the meeting
11:17:59 elodilles bauzas: (sorry for the late reply) thanks, i've created the python-novaclient release: https://review.opendev.org/c/openstack/releases/+/817930
11:18:29 elodilles and rebased the ussuri-em patch on top of this
11:43:03 opendevreview Takashi Kajinami proposed openstack/nova master: Deprecate [wsgi] secure_proxy_ssl_header https://review.opendev.org/c/openstack/nova/+/817927
12:22:32 sean-k-mooney gibi: so technially the guest abi is mostly maintained when goign form cirrus to virtio as virtio has a vga compatiable interface that is compatiable with cirrus. it would be a guest visabel change
12:22:51 sean-k-mooney it apparently is not a harmful guest visable change
12:25:03 sean-k-mooney if we are ok with the change in default affectign existing instnace the kasahyap patch is workable if not then i would like him to adress the comments i left in the patch
12:27:28 kashyap Yep, this is a benign case.
12:27:52 kashyap sean-k-mooney[m]: What else comments? I addressed what you (incorrectly) thought was a problem
12:27:58 kashyap I don't see any outstanding questions there.
12:28:07 kashyap Also, recall:
12:28:27 sean-k-mooney you ignored all the other comment i made and you have not
12:28:43 kashyap In the patch comments, I've also outlined the integration tests (Windows + Linux) done by Red Hat QE as well
12:28:50 sean-k-mooney what i sated as a probelm is that the xml would change for existign instace after a hard reboot
12:29:14 sean-k-mooney it still will for vms create before the patch
12:29:30 kashyap I need to evaluate the merits of the other comments and see if they make sense.
12:29:39 kashyap Some of them maybe non-problems
12:29:39 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/798680/3/nova/virt/libvirt/driver.py
12:31:12 sean-k-mooney kashyap: the disconenct is that when we discussed this with dan and other before we said you should follow the patteren we used for machine type and recorerd the current video model for all instnace and just change the behavior for new ones
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

Earlier   Later