| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-30 | |||
| 15:24:01 | dansmith | well, especially given the upgrade issues, it'd also be nice to run the grenade LM job against this, even though it probably wouldn't have covered the situation I found earlier | |
| 15:24:23 | dansmith | sean-k-mooney: if it runs some LMs then that's better than noting | |
| 15:24:43 | dansmith | I'm also going to be tempted to hack the job to pin the rpc version at 5.1 to check his work | |
| 15:24:52 | sean-k-mooney | dansmith: well i have my https://review.opendev.org/#/c/652197/ nfv job | |
| 15:24:55 | dansmith | because breaking LM across versions is icky | |
| 15:24:59 | sean-k-mooney | that i was going to rework | |
| 15:25:16 | sean-k-mooney | so i was going to stop using fedora and jsut use ubunut with nested vert | |
| 15:25:29 | sean-k-mooney | the multinode version runs live migratio tests | |
| 15:26:00 | dansmith | the closer it is to a standard job, the easier it will be to verify, but anything is better than nothing | |
| 15:26:10 | sean-k-mooney | sure | |
| 15:26:21 | sean-k-mooney | whcih is why im swapping ot ubuntu | |
| 15:26:39 | sean-k-mooney | but we also need to move the nova-migration job to non legacy at some point | |
| 15:27:17 | sean-k-mooney | anyway i was planning to finish up early today given how late i have been working lately | |
| 15:28:02 | dansmith | understandable, and holiday weekend here for lots of people | |
| 15:28:47 | sean-k-mooney | dansmith: donnyd provide a mulit numa flavor we can use on FN and we can also use vexhost. so ill create a job on monday | |
| 15:29:14 | sean-k-mooney | assuming ye dont get the other one working | |
| 15:29:15 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: DNM: Run LM integration tests with NUMA flavor https://review.opendev.org/678887 | |
| 15:34:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Rename Claims resources to compute_node https://review.opendev.org/679470 | |
| 15:37:10 | mriedem | artom: i thought this was funny https://review.opendev.org/#/c/635669/47/nova/compute/resource_tracker.py@399 | |
| 15:37:29 | mriedem | "because the fake objects in the tests aren't correct, we can't do anything about the actual code looking weird" | |
| 15:38:17 | artom | mriedem, heh - so, either we create a migration for the test that already has the accepted status set, which is close to real like but then... | |
| 15:38:20 | artom | Well | |
| 15:38:23 | artom | No, it'd work | |
| 15:38:30 | artom | Assert it's still accepted | |
| 15:38:34 | artom | Thanks! | |
| 15:38:40 | artom | *real life | |
| 15:38:41 | mriedem | yeah.... | |
| 15:38:47 | mriedem | i think you're overthinking all of this :) | |
| 15:39:26 | artom | Like a lot of things in life :( | |
| 15:59:27 | mriedem | efried: as for the ironic + sdk series, i'll wait for your ack on https://review.opendev.org/#/c/656027/ since you brought up CERN performance anxiety | |
| 16:12:06 | openstackgerrit | Merged openstack/nova master: Follow up for specifying az to unshelve https://review.opendev.org/679164 | |
| 16:13:44 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle VirtDriverNotReady in _cleanup_running_deleted_instances https://review.opendev.org/679473 | |
| 16:14:23 | artom | dansmith, sanity check with you before I go off haxing - if in https://review.opendev.org/#/c/634606/62/nova/compute/manager.py@6481 I check for the presense of the migration kwarg, which is new in RPC 5.2 and is not None from the conductor, is that a good way to make sure we're not pinned to < 5.2? | |
| 16:14:25 | dustinc | mriedem, efried: thanks for help and reviews, checking out comments now | |
| 16:14:30 | dustinc | (on ironic sdk series) | |
| 16:15:53 | mriedem | artom: wouldn't https://review.opendev.org/#/c/634606/62/nova/compute/manager.py@6511 already be False in that case? | |
| 16:16:01 | dansmith | artom: isn't that the destination not the source? | |
| 16:16:37 | artom | Doh, right | |
| 16:16:47 | dansmith | artom: I quickly looked to see if you had an easy indication on the source like that and didn't see one | |
| 16:17:05 | dansmith | but yeah, that's a little obscure, but as long as it's commented you should be able to use something like that | |
| 16:17:13 | dansmith | but, not sure there is one for the source :/ | |
| 16:18:01 | artom | Right, it's set by the libvirt driver during the source check: https://review.opendev.org/#/c/634606/62/nova/virt/libvirt/driver.py | |
| 16:18:23 | artom | That's the bit that has to be conditional on RPC 5.2 | |
| 16:19:48 | artom | dansmith, feels ugly, but add a new sentinel param to check_can_live_migrate_source when it's called from check_can_live_migrate_destination? | |
| 16:19:58 | artom | Or just have check_can_live_migrate_source manually check the RPC version | |
| 16:20:48 | dansmith | it's always called from there no? | |
| 16:21:24 | artom | You mean _source from _destination? Yeah, I was saying it from context | |
| 16:21:49 | artom | *for | |
| 16:22:05 | dansmith | it's always called there, so you mean when it's called from a new-enough node to pass that flag, yeah? | |
| 16:22:57 | artom | dansmith, yeah. If the node has the new code, it'll set the sentinel, and if the RPC is new enough, it'll actually reach the source | |
| 16:23:22 | dansmith | yeah, I don | |
| 16:23:27 | dansmith | I don't love it, but... | |
| 16:25:38 | artom | Or. | |
| 16:26:14 | artom | When the destination gets the migrate_data back from the source, pop the src_supports_numa_live_migration sentinel if the destination did not receive a migration from the conductor | |
| 16:28:28 | dansmith | the migration from conductor doesn't tell the whole story, so I don't think it's legit to use that as your sentinel | |
| 16:28:42 | dansmith | conductor's rpc version may be pinned differently | |
| 16:29:11 | artom | Differently.... from the computes? | |
| 16:29:24 | artom | I thought it was a deployment-wide thing | |
| 16:29:58 | dansmith | differently from anything, sure | |
| 16:30:07 | dansmith | config is per-node | |
| 16:30:36 | dansmith | in general you'd kinda assume they're the same, but especially when doing the slide-puzzle migration approch, | |
| 16:30:52 | dansmith | you might need to pin some computes differently to be able to communicate while things are old | |
| 16:31:13 | artom | Hrmm, so in addition to this whole thing, I need to check that the migration from the conductor is not None | |
| 16:31:34 | dansmith | regardless of the pinning of the computes you mean? | |
| 16:31:46 | artom | Yeah | |
| 16:31:49 | dansmith | if there's something you're doing that assumes conductor passed an argument from compute 5.2, then for sure, | |
| 16:32:03 | dansmith | because if it's pinned at 5.1 or "auto" and there are older computes present, then..yeah | |
| 16:32:08 | dansmith | also, | |
| 16:32:30 | dansmith | if everyone is set to "auto", then the version they think is legit right now depends on when they started, or were restarted during the upgrade | |
| 16:32:53 | dansmith | like, as soon as the last compute restarts, its auto pin is set to current, but everyone else hasn't gotten their service restart kick yet, etc | |
| 16:33:07 | dansmith | so assuming they're all atomically set identically is not legit, IMHO | |
| 16:33:56 | artom | So I think migration=None ends up creating a migration object in _move_claim, which is not what we want | |
| 16:34:07 | artom | OK, I think I got a better handle on this, thanks! | |
| 16:34:36 | dansmith | ack, maybe comment that on the patch in the proper place for tracking | |
| 16:36:38 | openstackgerrit | Adam Spiers proposed openstack/nova master: Enable booting of libvirt guests with AMD SEV memory encryption https://review.opendev.org/666616 | |
| 16:38:02 | aspiers | efried: need your input on this (sorry) https://review.opendev.org/#/c/666616/38/nova/conf/libvirt.py@845 | |
| 16:38:38 | efried | aspiers: more than earlier? | |
| 16:39:01 | aspiers | efried: uhh, different topic I think | |
| 16:39:07 | efried | okay, hit me | |
| 16:39:20 | aspiers | URL above ^^^ | |
| 16:39:52 | efried | oh, sorry, stand by | |
| 16:40:05 | aspiers | alex_xu and stephenfin want the default for num_memory_encrypted_guests changed from None to 0 | |
| 16:40:36 | aspiers | I'm not convinced it's the best idea, but I guess I'd be OK with it. However I wouldn't be OK with ditching None altogether. | |
| 16:43:16 | openstackgerrit | Adam Spiers proposed openstack/nova master: Indent fake libvirt host capabilities fixtures more nicely https://review.opendev.org/679339 | |
| 16:43:36 | sean-k-mooney | aspiers: you could replace non with -1 to signify unlimited | |
| 16:43:48 | aspiers | sean-k-mooney: that's already mentioned in my comment | |
| 16:44:01 | sean-k-mooney | i didnt click | |
| 16:44:03 | aspiers | efried, stephenfin: dedent removed ^^^ | |
| 16:44:23 | sean-k-mooney | i just assumed that that would be reason for not using none | |
| 16:44:39 | aspiers | No, None currently means "try to auto-detect, but if you can't, unlimited" | |
| 16:44:53 | sean-k-mooney | ah right yes | |
| 16:45:14 | aspiers | Another reasonable default would be "try to auto-detect, but if you can't, 0" | |
| 16:45:28 | aspiers | So that operators have to explicitly request unlimited | |
| 16:45:30 | sean-k-mooney | ya | |
| 16:45:51 | aspiers | I'd be OK with that too, although it's getting a bit complicated | |
| 16:46:05 | sean-k-mooney | well that would be cloaset to that you want | |
| 16:46:14 | aspiers | Not necessarily | |
| 16:46:18 | aspiers | Read the comment ;-) | |
| 16:47:11 | sean-k-mooney | im more or less done for the day. just waiting for food to be delivers so im trying to avoid starting any more reviews or work. | |
| 16:47:38 | sean-k-mooney | anyway i was fine with what you had orginally proposed | |