| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-27 | |||
| 16:08:15 | openstackgerrit | sean mooney proposed openstack/nova master: libvirt: use domain capabilities to get supported device models https://review.opendev.org/666915 | |
| 16:08:15 | openstackgerrit | sean mooney proposed openstack/nova master: Add transform_image_metadata request filter https://review.opendev.org/665775 | |
| 16:09:54 | mriedem | sean-k-mooney: we can override the virt_type in local.conf for the job to use kvm | |
| 16:10:04 | mriedem | since this is a hack test patch anyway | |
| 16:10:07 | sean-k-mooney | yes we can | |
| 16:11:24 | sean-k-mooney | artom: i think the nodeset you want is ubuntu-bionic-vexxhost but i need to double check | |
| 16:11:44 | artom | sean-k-mooney, yeah, I'm going in completely blind and relying on the gate to tell me what's wrong | |
| 16:11:50 | artom | So that's entirely possible | |
| 16:11:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: DNM: Run LM integration tests with NUMA flavor https://review.opendev.org/678887 | |
| 16:11:57 | sean-k-mooney | you use it like this | |
| 16:11:59 | sean-k-mooney | https://opendev.org/openstack/magnum/src/branch/master/.zuul.yaml#L152-L158 | |
| 16:12:24 | artom | mriedem, *tips fedora* your help is appreciated | |
| 16:13:33 | sean-k-mooney | do you want to test master our your change? | |
| 16:17:07 | openstackgerrit | sean mooney proposed openstack/nova master: Libvirt: add nfv job https://review.opendev.org/652197 | |
| 16:17:43 | sean-k-mooney | i think ^ will fix my nfv job | |
| 16:19:16 | sean-k-mooney | the multi node version of that enable live migration alther i might need to add the flag to allow that | |
| 16:25:02 | openstackgerrit | sean mooney proposed openstack/nova master: Libvirt: add nfv job https://review.opendev.org/652197 | |
| 16:36:10 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add support for translating CPU policy extra specs, image meta https://review.opendev.org/671801 | |
| 16:36:11 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Fold in argument to '_update_provider_tree_for_vgpu' https://review.opendev.org/676729 | |
| 16:36:11 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reshaper for PCPU https://review.opendev.org/674895 | |
| 16:36:12 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Start checking compute usage in functional tests https://review.opendev.org/678902 | |
| 16:37:03 | stephenfin | bauzas: fwiw, I think I've everything in https://review.opendev.org/674894 resolved if you wanted to take another look | |
| 16:37:19 | stephenfin | I, however, am out of here o/ | |
| 16:39:28 | openstackgerrit | sean mooney proposed openstack/nova master: DNM: Run LM integration tests with NUMA flavor https://review.opendev.org/678887 | |
| 16:39:34 | sean-k-mooney | artom: mriedem dansmith i think ^ will fix it | |
| 16:45:46 | efried | stephenfin: you still around? | |
| 16:46:04 | efried | I've been in meetings for 2h wanting to talk to you about this, but afraid I may have missed my window :( | |
| 16:50:14 | artom | sean-k-mooney, ack, much thanks! | |
| 16:50:49 | sean-k-mooney | we should have 3 jobs trying to test numa with the gate as we speak | |
| 16:51:04 | sean-k-mooney | the singel and multinode jobs i wrote and running and yours | |
| 16:51:30 | sean-k-mooney | i assume i should be able to go test the latest version of your code again manually? | |
| 16:56:15 | sean-k-mooney | oh i just rediscovered somthing quite useful | |
| 16:57:11 | sean-k-mooney | the experimenatl queue is way fater the check. which i kind of knew other pipleine are faster but i forgot | |
| 16:57:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Specify availability_zone to unshelve https://review.opendev.org/663851 | |
| 17:03:07 | mriedem | alex_xu: for your tomorrow, please take another pass on ^ | |
| 17:03:14 | mriedem | so we can get 2.77 in and move on | |
| 17:06:06 | dansmith | artom: it might be easier to discuss https://review.opendev.org/#/c/635669/39/nova/compute/resource_tracker.py here | |
| 17:06:36 | artom | dansmith, shoot :) | |
| 17:07:04 | dansmith | artom: my point is that you're assuming that if we're live migrating an instance, you can pass an empty PCIRequests object into the migration claim, because a live-migrating instance couldn't possibly have pci devices, right? | |
| 17:07:49 | artom | dansmith, no, because we don't want the claim to think it has any PCI devices | |
| 17:08:02 | dansmith | um | |
| 17:08:10 | dansmith | artom: why is that? | |
| 17:08:37 | artom | Because for a subset of PCI devices (SRIOV Neutron ports), that's handled buy Sean's code | |
| 17:09:00 | artom | So we don't want to refuse based something that should otherwise work | |
| 17:09:15 | artom | The missing piece here is that all other PCI devices *should* get refused | |
| 17:09:28 | dansmith | should get refused somewhere earlier in the stack right? | |
| 17:09:32 | artom | Yeah | |
| 17:09:49 | dansmith | that's the part I don't like | |
| 17:10:03 | artom | I agree - it's also current/latent | |
| 17:10:30 | artom | IIRC we just blow up if we try to migrate an instance with PCI alias-based passthrough | |
| 17:11:00 | sean-k-mooney | we check and fail the migration here https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L216-L221 | |
| 17:11:01 | dansmith | artom: it's not latent because it's the code you're adding | |
| 17:11:20 | sean-k-mooney | if the instance has a pcirequest that is not a neutron port we never call artoms code | |
| 17:11:51 | dansmith | sean-k-mooney: currently. | |
| 17:11:54 | artom | sean-k-mooney, aha, thanks for that - that confirms my idea that we should do claims *after* your MigrationPreCheck | |
| 17:12:10 | artom | So I'll have to change that | |
| 17:12:34 | sean-k-mooney | artom: https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L317-L326 | |
| 17:12:55 | sean-k-mooney | we do the pci check before calling check_can_live_migrate_destiation | |
| 17:13:11 | sean-k-mooney | so we should do the claims in check_can_live_migrate_destination | |
| 17:13:20 | artom | Ah, sorry, missed the wider context in your link. OK, then we're good. | |
| 17:13:22 | dansmith | artom: so my concern is that you say "we should never get here" and throw up an empty PCIRequests object, which I think goes into the migration context, and eventually gets applied to the instance | |
| 17:13:46 | artom | dansmith, ah, indeed | |
| 17:13:48 | sean-k-mooney | dansmith: we get there for cold migration | |
| 17:13:57 | dansmith | artom: so if the conductor stuff changes in the future and we *do* get here, I worry that we could dump the PCIRequests from the instance when we apply | |
| 17:14:23 | dansmith | sean-k-mooney: I know, but cold migration is still handled.. that's not my problem.. my problem is that live migration is *begging* to become a silent data corruption problem in the future I think | |
| 17:14:47 | artom | dansmith, so for live migration _test_pci in the claim would have to be skipped entirely | |
| 17:14:58 | sean-k-mooney | we put the check in specifcally to prevent issue related to pci devices | |
| 17:15:12 | sean-k-mooney | we would only remove it if we added support for it | |
| 17:15:20 | artom | ... while still keeping pci_requests? | |
| 17:15:23 | dansmith | artom: it looks to me like for cold, we get the requests and filter them based on their handled-by-neutron-ness or something | |
| 17:15:29 | sean-k-mooney | which i dont think we ever will as we cant safely hot unplug generic pci devices | |
| 17:16:00 | dansmith | artom: blindly without checking | |
| 17:16:13 | sean-k-mooney | ok so why don t we jsut do the same check again | |
| 17:16:23 | dansmith | you guys are saying it's fine to make dangerous assumptions because "nobody will ever ask for X" but that's *always* a bad assumption | |
| 17:16:47 | dansmith | sean-k-mooney: that's my point, assert that the thing is the way you think it is, instead of just assuming you can ignore all of it because something upstream of you did the check | |
| 17:16:51 | artom | dansmith, so if we keep pci_requests, but skip _test_pci... | |
| 17:16:57 | dansmith | because in the future when that check is gone, different, mixed-versions, etc... | |
| 17:17:25 | sean-k-mooney | dansmith: no im not im saying we specificaly thought about this edgecase in the spec and code and added a check to prevet the dangours edge case for the sriov mirgation | |
| 17:17:26 | dansmith | artom: would it make sense to just make _test_pci check the neutron-ness (or whatever) and not freak out over those things? | |
| 17:17:39 | sean-k-mooney | but im totally fine with adding another chacke for numa path | |
| 17:17:47 | sean-k-mooney | just copy past | |
| 17:17:49 | sean-k-mooney | for pci_request in self.instance.pci_requests.requests: | |
| 17:17:51 | sean-k-mooney | if pci_request.source != objects.InstancePCIRequest.NEUTRON_PORT: | |
| 17:17:53 | artom | dansmith, so move the check that sean-k-mooney linked into the claim? | |
| 17:17:53 | sean-k-mooney | # allow only VIF related PCI requests in live migration. | |
| 17:17:55 | sean-k-mooney | raise exception.MigrationPreCheckError( | |
| 17:17:57 | sean-k-mooney | reason= "non-VIF related PCI requests for instance " | |
| 17:17:59 | sean-k-mooney | "are not allowed for live migration.") | |
| 17:18:02 | sean-k-mooney | no | |
| 17:18:11 | sean-k-mooney | we dont want to fail late | |
| 17:18:41 | sean-k-mooney | bu just check again in the compute node | |
| 17:18:44 | dansmith | can we not do the same check in _test_pci, and ignore any that are vif-related, and fail if we find non-vif-related ones/ | |
| 17:19:01 | artom | dansmith, conversely, make apply_migration_context() smarter, and only set the new_ fields on the instance if they're set on the migration_context? | |
| 17:19:09 | dansmith | artom: no | |
| 17:19:29 | dansmith | artom: because that is deeply-buried silent ignoring of data you're trying to save, just because you only think that will happen from the migration path | |
| 17:19:56 | dansmith | this is all about making claims work for pci devices, so make it work, don't just "fix" the problem with lots of side effects everywhere else | |
| 17:20:22 | artom | dansmith, claims *already* work for PCI devices | |
| 17:20:32 | dansmith | sorry, making claims work during live migration | |
| 17:20:34 | dansmith | mind-o there | |
| 17:20:35 | artom | They're used when booting and cold migrating | |