| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-12-06 | |||
| 11:01:54 | opendevreview | Stephen Finucane proposed openstack/nova master: setup: Remove pbr's wsgi_scripts https://review.opendev.org/c/openstack/nova/+/902688 | |
| 11:07:10 | opendevreview | Stephen Finucane proposed openstack/nova master: Add new nova.wsgi module https://review.opendev.org/c/openstack/nova/+/902687 | |
| 11:07:11 | opendevreview | Stephen Finucane proposed openstack/nova master: setup: Remove pbr's wsgi_scripts https://review.opendev.org/c/openstack/nova/+/902688 | |
| 11:12:54 | stephenfin | sean-k-mooney: jfyi https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/thread/5RQGZZMCKKHAVH4OLSJCLBQNIHYDPGZD/ | |
| 11:13:02 | stephenfin | I tested it locally. Works fine. | |
| 11:13:14 | opendevreview | Merged openstack/nova master: Use split kernel/initramfs Cirros UEC image by default in jobs https://review.opendev.org/c/openstack/nova/+/902217 | |
| 11:18:03 | sean-k-mooney[m] | stephenfin: related i was able to demonstrate that the current support is broken https://review.opendev.org/c/openstack/pbr/+/900728 | |
| 11:18:09 | sean-k-mooney[m] | in pbr | |
| 11:18:46 | sean-k-mooney[m] | and i think i know how to fix it but ya havent had time to try and implement it since | |
| 12:55:06 | bauzas | sean-k-mooney: thanks for your review of my mdev live-mig spec, will look at your points a bit later | |
| 12:55:48 | bauzas | sean-k-mooney: now, about my problem with the CONF object, I checked and basically the cleanups we have in the oslo.config fixture don't delete the dynamically created groups | |
| 12:56:04 | bauzas | I just checked, we pass the same object between tests | |
| 12:56:30 | sean-k-mooney[m] | ack so this is just a gap in the fixture | |
| 12:56:31 | bauzas | fixing it would require our nova fixture to add some cleanup that would delete the dynamically generated groups | |
| 12:56:43 | sean-k-mooney[m] | ya | |
| 12:56:50 | sean-k-mooney[m] | i think that is fine | |
| 12:57:02 | sean-k-mooney[m] | the fixture should be fully resetting the object | |
| 12:57:21 | bauzas | yeah, so after https://github.com/openstack/nova/blob/09b651540f120ad30fa7127e87b27a6363044dcf/nova/tests/fixtures/conf.py#L69 we would add a addCleanup(devices.unregister_dynamic_opts) | |
| 12:57:51 | sean-k-mooney[m] | am ya i guess that works that would do the cleanup at the end for the test | |
| 12:57:58 | bauzas | sean-k-mooney: I just checked and calling self.conf.reset is useless | |
| 12:58:11 | sean-k-mooney[m] | but we could also just reassing an empty config bject to the gloabl | |
| 12:58:25 | bauzas | that's even already done by the oslo.config fixture itself https://github.com/openstack/oslo.config/blob/master/oslo_config/fixture.py#L41 | |
| 12:58:37 | bauzas | so there is no need to duplicate the useless call in the nova fixture | |
| 12:58:40 | sean-k-mooney[m] | that not what i ment | |
| 12:58:55 | sean-k-mooney[m] | i ment actully concruting a new object and assigning to the singleton | |
| 12:58:58 | bauzas | sean-k-mooney: yeah I thought about recreating a whole object | |
| 12:59:08 | bauzas | but I don't know how to make it cleanly | |
| 12:59:20 | sean-k-mooney[m] | ack | |
| 12:59:32 | sean-k-mooney[m] | lets try the self.addCleanup approch | |
| 13:00:23 | bauzas | for the sake of the series, I will remove https://review.opendev.org/c/openstack/nova/+/902084/3/nova/tests/unit/virt/libvirt/test_driver.py#26492 from the patch | |
| 13:00:48 | bauzas | and I'll provide this test in a separate change that would also add the addcleanup fix for the fixture | |
| 13:02:43 | bauzas | I maybe have an idea for recreating a new conf object but I need to test this | |
| 13:03:08 | bauzas | (stocking the original object at the beginning of setUp and then assigning it to self.conf in addcleanup) | |
| 13:03:31 | bauzas | shall work, provided oslo.config allows copying a CONF object | |
| 13:05:41 | eandersson | Anyone that can take a look at this minor patch? It just fixes some missing unt test coverage that was being ignored by coverage due to the threading model https://review.opendev.org/c/openstack/nova/+/900116 | |
| 13:27:57 | opendevreview | Sylvain Bauza proposed openstack/nova master: vgpu: Allow device_addresses to not be set https://review.opendev.org/c/openstack/nova/+/902084 | |
| 13:27:57 | opendevreview | Sylvain Bauza proposed openstack/nova master: WIP: add coverage for registered dyn opts https://review.opendev.org/c/openstack/nova/+/902771 | |
| 13:47:27 | gibi | sean-k-mooney: I'm +2 on the PCI grouping spec. I think the direction there is good and we will anyhow need to touch the implementation to learn the gotchas of the current code | |
| 13:48:33 | gibi | sean-k-mooney: I also see we are on a similar page with the PXB spec about opt-in ness | |
| 14:17:20 | sean-k-mooney | gibi: ok am im more or less ok with merging the pci grouping spec. | |
| 14:17:48 | sean-k-mooney | i was getting a bit burnt out with the reviews this morning so i wanted to take another skim over it after a break | |
| 14:18:19 | sean-k-mooney | but assuumign i dont see anything on the second pass ill upgrade to +2w and we can dig into the detail mroe in the impelmenteion review and update the spec accordingly if required | |
| 14:23:54 | bauzas | I need to look at the specs | |
| 14:24:03 | bauzas | ETOOMANYTHINGSHERE | |
| 14:28:01 | bauzas | sean-k-mooney: do you have time to discuss about https://review.opendev.org/c/openstack/nova-specs/+/900636/comment/2948412c_75d00114/ ? | |
| 14:30:56 | sean-k-mooney | am i technially have back to back meeting for the next 4 hours but on irc yes | |
| 14:31:08 | sean-k-mooney | gmeet ectra no | |
| 14:31:47 | sean-k-mooney | basically i would like to keep the conductor check minimal and just to the main compatiblity check in in pre_live_migrate | |
| 14:33:06 | sean-k-mooney | its what we do for sriov live migration https://specs.openstack.org/openstack/nova-specs/specs/train/implemented/libvirt-neutron-sriov-livemigration.html#resource-claims | |
| 14:33:38 | sean-k-mooney | well not quite | |
| 14:33:48 | sean-k-mooney | we actully added an rpc in check_can_live_migrate_destination | |
| 14:33:58 | sean-k-mooney | to do the claim i guess | |
| 14:34:21 | sean-k-mooney | if you are going to add an rpc to do that i woudl be fine with doing it in check_can_live_migrate_destination | |
| 14:35:06 | sean-k-mooney | baiscally i want to minimise the amount of rpc callses we need and reuse the ones we already have currently | |
| 14:36:51 | bauzas | sean-k-mooney: tbc, I tried to do like for other features | |
| 14:36:57 | opendevreview | Merged openstack/nova-specs master: resubmit per-process-healthchecks spec for 2024.1 https://review.opendev.org/c/openstack/nova-specs/+/897225 | |
| 14:37:17 | bauzas | I thought it would be nice to check the types by the conductor | |
| 14:37:31 | bauzas | because operators couldn't use traits or something else | |
| 14:37:38 | bauzas | say custom rcs | |
| 14:37:53 | sean-k-mooney | well if they are using traits ro customer reousces | |
| 14:37:55 | bauzas | we could do this in pre-live-migrate for sure, after the conductor | |
| 14:37:59 | sean-k-mooney | then the type is checked by placment | |
| 14:38:09 | bauzas | sean-k-mooney: yeah indeed, but that's optional | |
| 14:38:19 | sean-k-mooney | right i think that shoudl be requried | |
| 14:38:29 | bauzas | I don't think so | |
| 14:38:33 | sean-k-mooney | i know | |
| 14:38:44 | bauzas | some operators just want to provide VGPUs | |
| 14:38:52 | sean-k-mooney | so i think the conductor check could be doen but im worried it will be somewhat expensive | |
| 14:38:53 | bauzas | given every new GPU has specific types | |
| 14:39:11 | bauzas | sean-k-mooney: shouldn't be that expensive as we already have the calls | |
| 14:39:33 | sean-k-mooney | so what i can do is try and reivew the code and that section again | |
| 14:39:49 | sean-k-mooney | when i got to your spec i was starting tot get review burn out | |
| 14:40:28 | bauzas | no worries | |
| 14:40:41 | bauzas | this is somehow an implementation question | |
| 14:40:59 | bauzas | I mean, testing the types is a design point | |
| 14:41:11 | bauzas | but where can be something related to the implementation | |
| 14:41:32 | bauzas | I can modify my spec accordingly | |
| 14:42:01 | sean-k-mooney | so you plan ot extend https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L359-L362 | |
| 14:42:43 | sean-k-mooney | if check_can_live_migrate_destination was passed the currnt mdev type and uuid touple | |
| 14:42:55 | sean-k-mooney | and did the claim and populated it in migrate_data | |
| 14:43:00 | sean-k-mooney | i would be happy with that | |
| 14:43:18 | sean-k-mooney | that would be like how sriov works | |
| 14:43:42 | sean-k-mooney | what i didnt really liek about your proposal is it was only checking the types | |
| 14:45:17 | sean-k-mooney | we currntly call into the driver here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L8537-L8539 | |
| 14:45:32 | opendevreview | Merged openstack/nova-specs master: Re-propose "Add maxphysaddr support for Libvirt" for 2024.1 Caracal https://review.opendev.org/c/openstack/nova-specs/+/895135 | |
| 14:46:19 | sean-k-mooney | so you could update the in memory dict and pass back the uuids there | |
| 14:47:00 | sean-k-mooney | for sriov we do the claim here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L8563-L8565 but that is not in the driver specific section since the pci_tracker is driver independent | |
| 14:47:07 | bauzas | yeah I saw your optimization point | |
| 14:47:26 | bauzas | and yeah I saw for SRIOV | |
| 14:47:29 | sean-k-mooney | its not just optimisztion | |
| 14:47:48 | sean-k-mooney | what im hoping to prevent is racing for the same mdev betwen boots and live migrations | |
| 14:48:10 | sean-k-mooney | so if we can atomically allocate the mdev that will be used in check_can_live_migrate_destination | |
| 14:48:40 | sean-k-mooney | then i think that helps prevent a subset of possibel issues | |
| 14:48:54 | sean-k-mooney | perhaps im wrong but that was part of the motivation | |
| 14:49:09 | bauzas | oh | |
| 14:49:12 | bauzas | I see | |
| 14:49:17 | sean-k-mooney | doiong only the tyep check here seams wasteful but if we also selefct the mdev here | |
| 14:49:20 | bauzas | I need to remember | |
| 14:49:20 | sean-k-mooney | then im all for it | |
| 14:49:30 | bauzas | because I cared about the races indeed | |