Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-03-19
20:33:26 artom dansmith, they're not mutually exclusive, they definitely both run, and the NUMA live migration code runs _after_ the hack, so it clobbers the entire dst_numa_info object that was used as the hiding place for the "secret" cpu shared set info
20:34:18 dansmith I don't understand how that can be
20:34:26 artom dansmith lulz another call ^_^ ?
20:34:31 sean-k-mooney its because of the 3 conditions in the if
20:34:34 dansmith we have to put the dst info into the blob so it can be used to generate the xml right?
20:34:36 sean-k-mooney if ('cpu_pins' in info and
20:34:38 sean-k-mooney 'cell_pins' in info and
20:34:40 sean-k-mooney 'emulator_pins' in info):
20:34:52 sean-k-mooney in the non numa case only one of those 3 in checks is true
20:35:00 sean-k-mooney so we never enter the loop
20:35:08 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/877773/9/nova/virt/libvirt/migration.py#186
20:35:28 sean-k-mooney in the numa case the numa live migration code clobers the object that rene modifed
20:36:05 sean-k-mooney so it just happens to work becasue we get lucky and the case where its not clobpered we dont enter the loop
20:36:29 dansmith and to be clear, that's always the case or because we're missing a test scenario where we could get unlucky?
20:36:31 sean-k-mooney cell_pins and emulator_pins are not defiend in the non numa case
20:36:36 artom The case where it's not clobbered is when a VM doesn't have a NUMA topoogy
20:36:41 dansmith I guess the point is. yeah that ^
20:36:53 sean-k-mooney thats alwasy the case
20:37:07 artom So by pure luck we've made them functionally mutually exclusive
20:37:14 sean-k-mooney when we dont have a numa guest cell_pins and emulator_pins are not set
20:38:09 sean-k-mooney and in the numa case the dst_numa_info is regenerated by the numa live migration code
20:39:12 sean-k-mooney so we never execute _update_numa_xml with cpu_pins that contains the stashed value
20:39:24 dansmith usually when I say "this will be a nightmare of complexity to debug in the future" I have to wait years to collect on that debt.. glad this time it was under 24 hours :D
20:39:58 sean-k-mooney me too because i proably would have been the one trying to fiugre this out in a few years :)
20:40:05 dansmith yep
20:41:06 artom In a hypothetical few years wherein we've chosen to merge the haxx ;)
20:41:30 dansmith s/we've/we'd/ yeah
20:41:31 sean-k-mooney Uggla: have you had time to sync with artom on the change of direction
20:45:15 sean-k-mooney i need to go eat so ill call it a day there
20:45:27 Uggla sean-k-mooney, more or less. But I guess Artom and I will sync tomorrow....
20:45:31 sean-k-mooney im glad we know why it was working im not happy with how it was working
20:51:48 spatel_ sean-k-mooney I believe I talked about this in past. Just re-checking again. Does snapshot support include of memory footprint?
#openstack-nova - 2024-03-20
02:44:17 opendevreview Merged openstack/osc-placement master: Update master for stable/2024.1 https://review.opendev.org/c/openstack/osc-placement/+/912623
02:45:37 opendevreview Merged openstack/os-vif master: Update master for stable/2024.1 https://review.opendev.org/c/openstack/os-vif/+/912620
02:48:32 opendevreview Merged openstack/python-novaclient master: Update master for stable/2024.1 https://review.opendev.org/c/openstack/python-novaclient/+/912627
03:58:06 opendevreview Merged openstack/placement master: Update master for stable/2024.1 https://review.opendev.org/c/openstack/placement/+/913350
04:00:00 opendevreview Merged openstack/nova master: Update master for stable/2024.1 https://review.opendev.org/c/openstack/nova/+/913633
09:55:31 opendevreview sean mooney proposed openstack/nova master: Test live migration between hosts with differnet cpu_shared_sets https://review.opendev.org/c/openstack/nova/+/913744
10:03:50 sean-k-mooney[m] spatel_: no snapshot is just of the root disk.
10:04:09 sean-k-mooney[m] not memory
13:21:47 sean-k-mooney dansmith: just an fyi i added you to this anyway https://review.opendev.org/c/openstack/nova/+/913744 but that is proply executing Uggla migration change and testing it in a way we i think can merge and keep with little to no downside
13:23:16 sean-k-mooney its proably a converstaion for the ptg but i think there are a limited subset of whitebox test we may want to conser adding to exsitng jobs. for example we have a non voting barbican-tempest-plugin-simple-crypto job today.
13:24:44 sean-k-mooney i would like to either make that voting or have a diffent voting barbican job to test the ephmeral encyption series when taht is complete but i would alos like to enable the whitebox vtpm test in that job too. we dont need to enabel all of whitebox but it hink we can enable select tests in specific jobs if we agree that is desireable
13:26:00 sean-k-mooney i would like to add a whitebox tempest test to validate Uggla change at some point (i.e. checking the xml is updated correctly) and enable that in the live migration job if peopel were ok with that.
13:27:07 sean-k-mooney given whitebox is an offical deliverable of the qa project i dont think that is out of place but if we were to take that step we would certenly need to ensure that whitebox does not break teh nova gate going forward.
13:27:47 sean-k-mooney to that end if have also been explofing using the rdo zuul instance to instead execute the whitebox tests as a third party ci for nova
13:28:15 sean-k-mooney i proably wont have time to create a poc of that between now and the ptg
13:29:16 sean-k-mooney but im going to explore that option regardless of of we do decied to integrate a subset of whitebox in the first party ci or not
13:30:21 dansmith I'm not sure what you're trying to say... this tests mix of shared/pinned that you said we don't want to support anyway, or some other scenario?
13:30:38 sean-k-mooney dansmith: another senairo
13:31:03 sean-k-mooney specificly i want a test for two host with diffen cpu_shared_set and numa or cpu pinning
13:31:36 sean-k-mooney i.e. vanaill vm where the compute node just happen to have diffent cpu_shared_sets perhaps because the host have diffent number of cpus
13:32:36 sean-k-mooney in fact just to make that extra clear im going to update the patch to remove cpu_dedicated_set
13:32:47 dansmith I thought we already had that because we flip the shared/dedicated set between the two?
13:34:13 sean-k-mooney in whitebox we partly do, we just dont have any test that asserts the xml was updated for non numa instnaces
13:34:24 opendevreview Merged openstack/nova master: Correctly reset instance task state in rebooting hard https://review.opendev.org/c/openstack/nova/+/867832
13:34:33 sean-k-mooney we do live migrations with non numa instance we jsut dont check the xml
13:34:39 dansmith ah
13:34:59 sean-k-mooney so i just want to add a dedicated test with a vanial flaovr that checks as a follow up
13:35:07 sean-k-mooney i did in manually in the modifed ci job
13:35:15 sean-k-mooney but we can automate that
13:38:10 dansmith we should do that in a test though and not with a different job right? or is that what you mean by "automate that"
13:38:12 dansmith ?
13:38:27 sean-k-mooney yes not a diffent job
13:38:42 sean-k-mooney just a tempest testcase
13:41:05 dansmith ack
13:54:24 opendevreview Merged openstack/os-vif master: Update python classifier in setup.cfg https://review.opendev.org/c/openstack/os-vif/+/904613
13:57:48 opendevreview Merged openstack/os-vif master: tox: Drop envdir https://review.opendev.org/c/openstack/os-vif/+/907604
14:09:30 opendevreview sean mooney proposed openstack/nova master: Test live migration between hosts with differnet cpu_shared_sets https://review.opendev.org/c/openstack/nova/+/913744
14:10:15 sean-k-mooney update ^ to make it clear this is testing non numa instances for the vanilla case where the cpu_share_set just happens to be different on each host
14:16:08 dansmith sean-k-mooney: this is modifying our live migration job, but we don't run whitebox there right? so you're thinking this will cause us to run over the code in question and would fail if not on top of the fix?
14:16:14 sean-k-mooney im tempeted to push a second patch to test numa live migration in one of the other jobs but i wont for now. that litrally just need us to modify the flavor to have hw:mem_page_size=small but im not sure if we currently rely on memory over allcation in theyse job. i belvie we dont given the default for initial_ram_allocation_ratio is 1.0 so it should just work perhaps it worth
14:16:16 sean-k-mooney a try as it would be nice to get that coverage effectivly for free
14:16:53 sean-k-mooney dansmith: it does make use run the code in questio. it will not fail unless the code breaks live migratoin
14:17:00 sean-k-mooney but it not asserting that the xml is update
14:17:11 sean-k-mooney which is why i did that by hand by inspecting the logs
14:17:23 dansmith oh right, because it won't fail it will just be wrong
14:17:27 dansmith right, okay sorry
14:17:33 sean-k-mooney it would have caught if the int("of random string") was actully exploding
14:17:37 dansmith the caffeine absorption is slow this morning
14:18:07 sean-k-mooney ya so if we dint have the same number of cpus in both hosts adn we were using one on the source that was not on the dest it would fail
14:18:11 sean-k-mooney but that is not the case here
14:18:23 sean-k-mooney it woudl just be runnign on the wrong core
14:18:46 sean-k-mooney it woudl get fixed on teh next hard reboot of the vm
14:21:52 sean-k-mooney what happens today fi the core does not exist is we get an error for the call to libvirt to start the migration and rollback gracefuly and mark the migration as failed
14:22:29 sean-k-mooney the same thing happens if you have the power managemnt feature enabeld and the core is offline
14:23:00 sean-k-mooney qemu check for that and reports an error to libvirt which we then get and we rollback in the same way as if the cpu were not compatiable
15:43:28 opendevreview Merged openstack/nova stable/2024.1: [stable-only] Update .gitreview for stable/2024.1 https://review.opendev.org/c/openstack/nova/+/913631
15:43:36 opendevreview Merged openstack/nova stable/2024.1: [stable-only] Update TOX_CONSTRAINTS_FILE for stable/2024.1 https://review.opendev.org/c/openstack/nova/+/913632
16:19:55 zigo Who's trying to use systemctl in unit tests?!?
16:19:55 zigo https://buildd.debian.org/status/fetch.php?pkg=nova&arch=all&ver=2%3A29.0.0~rc1-1&stamp=1710857860&file=log
16:20:30 zigo Not a good idea, IMO. Or at least, this has to be explicited somewhere.
16:20:58 clarkb the upstream unittest jobs should remove sudo powers before running the tests. Unprivileged systemctl isn't the worst thing?
16:21:27 zigo It's not that...
16:21:28 zigo FileNotFoundError: [Errno 2] No such file or directory: 'systemctl'
16:21:37 zigo There's no systemd in the Debian buildd chroots ...
16:21:48 zigo Plus WTF is it even trying to do?
16:23:33 clarkb its checking if the system is running systemd and assumes if systemd is present then so is systemdctl
16:23:44 zigo sysdout, sysderr = processutils.execute("systemctl",
16:23:44 zigo "is-system-running",

Earlier   Later