Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-01-09
13:59:17 stephenfin It does, yeah :)
13:59:25 gibi kashyap: I'm not even sure I understand the problem statement in that patch
13:59:29 stephenfin I wonder if this is a pip bug?
14:00:37 gibi it feels like a tox shortcoming as tox calls pip install
14:00:46 gibi to install the deps of the package
14:02:40 gibi basically the install_package_deps step in tox ignores the deps
14:03:20 gibi so even if we put the constraint in the deps it has no effect
14:03:51 gibi install_package_deps does use the install_commmand hence our solution in placement to put the constraint there.
14:07:15 kashyap gibi: In this case, Intel "IceLake CPU" is not correctly recognized due to missing flag, "mpx". (And another problem is Nova's now-broken suboptimal CPU comparison code)
14:07:47 kashyap gibi: The quickest (and still valid) solution to this (and similar) problems is to _remove_ the CPU comparison that Nova does at all. As libvirt will do the Right Thing.
14:08:27 kashyap gibi: ... which can be done by reviving this short older patch (which you +1ed in the past). See the commit: https://review.opendev.org/c/openstack/nova/+/772917/
14:09:56 gibi but that was abandoned in favor of https://review.opendev.org/q/topic:bp%252Fcpu-selection-with-hypervisor-consideration where https://review.opendev.org/c/openstack/nova/+/762330 needs substantial work
14:10:16 kashyap gibi: Yeah, indeed! That said: libvirt developers themselves now tell me that we (Nova) doesn't need to do that check anymore
14:10:45 kashyap gibi: Read this comment from Jiri here:
14:10:50 kashyap https://bugzilla.redhat.com/show_bug.cgi?id=2138381#c7
14:11:02 kashyap Especially the 2nd paragraph
14:11:25 kashyap gibi: That substantial work is more fragile and I don't have cycles to baby-sit it. The best course is to remove the check w/ the shorter patch, which is still correct
14:11:40 kashyap As it provides most benefit with the shortest patch, IMHO.
14:11:55 gibi OK. do you suggest to revive https://review.opendev.org/c/openstack/nova/+/772917/ ?
14:12:27 gibi will that solve the issue behind https://review.opendev.org/c/openstack/nova/+/869536 too?
14:12:44 kashyap Yes, definitely. Based on that commit message rationale _and_ the advice of CPU modelling maintainer from libvirt
14:13:09 kashyap gibi: Yes
14:13:20 kashyap I'll comment there
14:14:56 kashyap Before removing that patch, we also have to deprecate (and remove later) this workaround: CONF.workarounds.skip_cpu_compare_on_dest
14:16:44 gibi I'm OK with this approach
14:17:00 gibi I don't believe we have the bandwidth to land https://review.opendev.org/q/topic:bp%252Fcpu-selection-with-hypervisor-consideration
14:17:02 kashyap gibi: Uh, I made a messy mistake in thinking: please ignore the above. Here's my correction:
14:17:39 kashyap gibi: We should actually get rid of _this_ compare_cpu() check in _ceck_cpu_compatibility() method -
14:17:42 kashyap https://github.com/openstack/nova/blob/8a476061c5e034016668cd9e5a20c4430ef6b68d/nova/virt/libvirt/driver.py#L991
14:19:08 kashyap gibi: But the reason is the same. (The only correction is a different
14:19:13 kashyap ... method in Nova.
14:20:30 gibi ack
14:20:56 gibi feel free to ping me if I need to re-review the https://review.opendev.org/c/openstack/nova/+/772917/
14:22:10 kashyap Nod, noted. Not that one, I'm sure we need to get rid of the check in _check_cpu_compatibility(). Thank you!
14:24:53 sean-k-mooney kashyap: in general im not sure i agree that we should relay on libvirt for the cpu compat check
14:25:06 kashyap sean-k-mooney: Why? What reason do we have?
14:25:24 sean-k-mooney i know libvirt will do it properly but i thing this is somethign that nova shoudl do not the hyperviros in general
14:25:33 kashyap FWIW, I think this is the right approach after thinking about it for a long time, and relying on the advice of SMEs on this topic.
14:25:41 kashyap Doing it ourselves is costly and error-prone at this point.
14:26:07 sean-k-mooney it may be the right approch form a libvirt point of view
14:26:11 kashyap When the hypervisor (in this case libvirt + QEMU) is _already_ doing it, let's please rely on it
14:26:23 sean-k-mooney but nova should have validateed the destination for compatibality before we invokve libvirt
14:26:26 kashyap sean-k-mooney: No, their advice is for management tools in general.
14:26:43 sean-k-mooney in fact form an nova point of veiw we shoudl have valdiated it entirly as part fo schudling
14:26:46 kashyap sean-k-mooney: That destination check is there (but we have added a workaround to skip it - as that's not required too)
14:26:58 sean-k-mooney form a nova point of view pre-livemigrate is already quite late
14:27:00 kashyap Yeah, "ideally..." scenarios are hard at this point :-)
14:28:10 kashyap sean-k-mooney: That's the one on destination skip: https://code.engineering.redhat.com/gerrit/c/nova/+/405286
14:28:27 kashyap (Same reasoning in my commit applies for src check)
14:28:36 sean-k-mooney so form my perspective not validating all requirement in pre-livemigrateis incorrect but i understand why you want o delegate this to libvirt
14:29:03 kashyap Thank you. I'm getting tired of some these reports and playing whack-a-mole :(
14:30:09 sean-k-mooney so i think goign forward we may need to reqest a new feature in libvirt or desgin a new feature in nova
14:30:42 kashyap sean-k-mooney: What would the new RFE for libvirt be?
14:30:44 sean-k-mooney i consider it a bug to not validate all requiremnt like cpu compatiabliy so eventully i woudl like a more relyable way to do that validation
14:31:11 sean-k-mooney kashyap: im not quite sure
14:31:41 kashyap libvirt precisely did have several RFEs and it took a few years to work out all these issues and come to this point of "doing the right thing" on src + dest
14:31:47 sean-k-mooney in general i would like a more declaritive way to understand if live migratoin is possibel
14:32:06 sean-k-mooney so that we can model this in placment in some way but i dont know what that woudl look like
14:32:57 sean-k-mooney the imperitive check we have right now by invoking cpu_compare or the new apis is not really compatible with nova current schduling model
14:33:11 sean-k-mooney that is why they happen late after schuding as pre miggrate checks
14:33:26 kashyap (Right. That's future goodness if we have cycles. :))
14:34:14 kashyap gibi: sean-k-mooney: Unrelated - do you know what's off in my F35 env. for flake8 to fail this way? - https://paste.opendev.org/show/bLBgrdYhl1i7hqAPCvqP/
14:35:01 sean-k-mooney proably the import_lib version
14:35:10 sean-k-mooney althogh i think we dropped support for python 3.7
14:35:30 sean-k-mooney we used to have a workaround for older python versions
14:35:39 kashyap Right; I just learn that flake8 on master min requires 3.8
14:35:53 sean-k-mooney not quite master required 3.8
14:36:22 sean-k-mooney but importlib_metadata gain suppport for some feautre sin 3.8
14:36:38 sean-k-mooney before that we had a workaorund when we had 3.6 suoprt still
14:37:39 sean-k-mooney anyway if you can use 3.8 that woudl be better
14:39:16 kashyap Yeah, trying :)
14:39:50 sean-k-mooney you could change [testenv:pep8] to [testenv:pep8{,-py38,-py39,-py310}]
14:40:04 sean-k-mooney pep8 is runnign with your systems default python
14:40:29 sean-k-mooney so your other option is to update the alternitives to make python3.8 the default python
14:40:47 sean-k-mooney actully you can create a python venv with 3.8 and run tox form that too if needed
14:41:52 kashyap It's a new venv; but the pre-commit hook seems to use system python
14:45:24 sean-k-mooney what im suggestign is you should do python3.8 -m venv .venv
14:45:32 sean-k-mooney and install precommit and tox in that
14:45:49 sean-k-mooney and use those to mange your tox envs and commit ectra
14:46:02 sean-k-mooney if you are not able to update your default system python to 3.8+
14:46:30 kashyap Python 3.10.8
14:46:30 kashyap $> /usr/bin/python3 --version
14:46:48 kashyap Default system is already well above 3.8+. I just don't know how pre-commit is getting 3.7
14:46:59 sean-k-mooney od
14:47:08 sean-k-mooney i would unistall and reinstal it
14:47:25 kashyap Yep, tryin
15:16:15 kashyap This fixed it for me:
15:16:17 kashyap $> pre-commit install --allow-missing-config
15:16:21 kashyap $> rm -rf /home/kashyapc/.cache/pre-commit/
15:18:36 opendevreview Balazs Gibizer proposed openstack/nova master: Remove basepython def from tox.ini https://review.opendev.org/c/openstack/nova/+/869545
15:19:53 gibi bauzas: ^^ needed to unblock the nova gate
15:20:02 gibi bauzas: and https://review.opendev.org/c/openstack/placement/+/868418 needed to unblock the placement gate
15:21:26 opendevreview Balazs Gibizer proposed openstack/placement master: Make tox.ini tox 4.0.0 compatible https://review.opendev.org/c/openstack/placement/+/868418
15:37:20 darkhorse artom: I tried to boot from image that is related to the shelved instance but failed. I don't see an image created when I shelve an instance. I tried openstack images list and also checked in the glace > images table in mariadb but nothing is created when I shelve an instance.
15:46:16 artom darkhorse, has to be shelved_offloaded
15:46:33 artom That's either a manual step after the instance is shelved, or done automatically by the cloud depending on config
15:46:51 darkhorse artom: yes its shelved_offloaded.
15:47:08 artom Err, there should be an image...
15:47:42 artom Unless it's boot from volume? I'm not sure about that case
15:48:06 darkhorse no its not boot from volume

Earlier   Later