| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-28 | |||
| 16:25:05 | mriedem | if you have an old dest compute, the migrate_data won't have the numa stuff from the claim right? | |
| 16:25:24 | mriedem | so the source (if new) can't rely on it | |
| 16:25:35 | artom | mriedem, other way around, new dest, old source | |
| 16:25:44 | artom | So source won't send the updated XML, but dest will have claimed for it | |
| 16:25:55 | artom | So we need to do conditional claiming based on source compute version | |
| 16:26:15 | mriedem | i think that's what the file-backed memory live migration does... | |
| 16:26:16 | bauzas | you could make the migrate_data parameter a sentinel | |
| 16:26:22 | mriedem | it checks the compute service version for the source from the dest | |
| 16:26:47 | artom | Alright, lemme update the spec | |
| 16:27:02 | bauzas | artom: just change this | |
| 16:27:06 | bauzas | other things were nits | |
| 16:27:16 | bauzas | just for documenting the spec | |
| 16:27:18 | artom | Alright if I keep it kinda high-level? So "conditional claim", but without specifying what that condition will look like (flag, version checking, etc)? | |
| 16:27:20 | bauzas | later when we review | |
| 16:27:28 | dansmith | artom: you can't check the version | |
| 16:27:42 | mriedem | artom: this is the code i'm thinking of https://github.com/openstack/nova/blob/62245235bc15da6abcdfd3df1c24bd856d69fbb4/nova/virt/libvirt/driver.py#L6636 | |
| 16:27:51 | artom | dansmith, I guess not on the compute, eh? | |
| 16:27:59 | artom | Only conductor, so it has to be flag | |
| 16:28:07 | bauzas | mriedem: we don't want to fail | |
| 16:28:16 | dansmith | artom: I'm not sure what you mean | |
| 16:28:19 | bauzas | mriedem: we want to blindly accept the migration and not claim | |
| 16:28:23 | mriedem | bauzas: you don't want to claim on the dest if the source isn't going to use it | |
| 16:28:24 | mriedem | right? | |
| 16:28:38 | bauzas | that's right | |
| 16:28:39 | artom | dansmith, can the dest compute check the source compute version? | |
| 16:28:47 | artom | And do the new thing only if the source compute is hew? | |
| 16:28:48 | artom | *new | |
| 16:28:49 | dansmith | artom: don't do it that way | |
| 16:28:50 | mriedem | my point is, ^ is how the dest checks the source compute version today - that pattern could be re-used, not the specific failure thing | |
| 16:29:04 | dansmith | artom: because version pinning might mean you have a new compute, but it didn't send something new | |
| 16:29:23 | artom | dansmith, oh right | |
| 16:29:46 | artom | Wait up though | |
| 16:29:57 | bauzas | artom: https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L93 | |
| 16:30:11 | bauzas | artom: that's one example of a flag (the _sentinel value) | |
| 16:30:23 | dansmith | the file-backed stuff chooses not to migrate at all if the target isn't going to support it I think, but in your case you have to allow it, you just need to do the old thing | |
| 16:30:40 | mriedem | if the source isnt going to suppor it | |
| 16:30:43 | mriedem | *support | |
| 16:30:49 | bauzas | shit, I need to bail out | |
| 16:30:58 | bauzas | I just dropped my poop and then I leave | |
| 16:30:59 | bauzas | excellent | |
| 16:31:08 | dansmith | bauzas: um... | |
| 16:31:10 | artom | You panda | |
| 16:31:17 | artom | Eats poops and leaves | |
| 16:31:21 | artom | No wait, it's "shoots" | |
| 16:31:21 | mriedem | dansmith: i think the point is, from the dest, if the source is old, we don't claim on the dest and we don't put the numa things in migrate_data, | |
| 16:31:22 | artom | Dammit >< | |
| 16:31:26 | bauzas | context is https://www.meetup.com/fr-FR/Groupe-dutilisateurs-Python-Grenoble/events/256520367/ | |
| 16:31:38 | mriedem | because and old source isn't going to use those anyway, and also wouldn't know to rollback the claim or whatever | |
| 16:31:43 | bauzas | for once we have a meetup that talks OpenStack here... | |
| 16:32:16 | dansmith | mriedem: it really needs to work both ways, whether the source is old or new | |
| 16:32:25 | mriedem | if the dest is old, | |
| 16:32:29 | mriedem | and the source is new, | |
| 16:32:35 | mriedem | migrate_data won't have the new numa field in it, | |
| 16:32:42 | artom | If the source is new it's fine, dest will just ignore the new field | |
| 16:32:43 | mriedem | and the new source node just won't do any of the new stuff | |
| 16:32:59 | artom | (Which reminds me, it means we need to do claims on the dest) | |
| 16:33:02 | artom | (And cleanup) | |
| 16:33:14 | mriedem | reminds you? the spec already says the claims happen on the dest | |
| 16:33:17 | mriedem | they have to happen on the dest | |
| 16:33:26 | dansmith | the spec should probably have the old/new src/dst truth table in it if it doesn't already | |
| 16:33:32 | artom | I mean, in the code | |
| 16:33:32 | mriedem | ++ on that | |
| 16:33:44 | artom | Any of the new stuff we do needs to be in code that runs on the dest | |
| 16:33:44 | mriedem | artom: i'm not sure what you're saying | |
| 16:33:50 | mriedem | yes the claim needs to happen on the dest | |
| 16:33:52 | mriedem | like the spec says :) | |
| 16:34:03 | artom | Does it? I thought I left it at "implementation detail" | |
| 16:34:07 | mriedem | oy | |
| 16:34:15 | mriedem | "Any of the new stuff we do needs to be in code that runs on the dest" is also not accurate | |
| 16:34:26 | jaypipes | Sundar: ty sir! | |
| 16:34:29 | artom | Well, yeah | |
| 16:34:30 | mriedem | obviously there will be new code on the source to generate the xml, using data from the dest, to send the xml to the dest | |
| 16:34:42 | artom | Creating claims and cleaning them | |
| 16:34:45 | artom | Has to be done on the dest | |
| 16:34:48 | bauzas | I'm fine with leaving details for the implementation | |
| 16:35:02 | bauzas | I just want to make sure we all agree on the behaviour for upgrades | |
| 16:35:03 | mriedem | create yes, i'm not entirely sure about clean (drop_move_claim), but sure | |
| 16:35:36 | bauzas | which is, whatever we write, in case a live migration happens from a Rocky node to a Stein node, we will just blindly accept the migration and not do the claim | |
| 16:35:40 | mriedem | as noted in my review comments, rollback doesn't always cleanup on the dest | |
| 16:35:43 | mriedem | today anyway | |
| 16:35:44 | bauzas | artom: ^ | |
| 16:35:55 | dansmith | bauzas: I thought you dropped poop and left? | |
| 16:36:07 | dansmith | I must have misunderstood :) | |
| 16:36:09 | artom | OK, I think I need to go back to the spec after a bit of thinking | |
| 16:36:19 | artom | dansmith, he's relishing his poop | |
| 16:36:22 | bauzas | dansmith: hah, I just feel I need to discuss a bit more | |
| 16:36:38 | mriedem | artom: probably easiest to just start with a mixed compute upgrade table or something, | |
| 16:37:00 | mriedem | with (1) old source, old dest - what happens? (2) old source, new dest, what happens, (3) new source, old dest, (4) new source, new dest | |
| 16:37:09 | dansmith | a truth table, like I said | |
| 16:37:10 | bauzas | +1 | |
| 16:37:19 | artom | Yeah, sounds like a good idea | |
| 16:37:20 | bauzas | the rest can be left for implementation | |
| 16:37:46 | bauzas | anyway, I'm just flushing my stuff now | |
| 16:37:53 | bauzas | see ya | |
| 16:38:19 | mriedem | if we do that, i'm not entirely sure how much we need https://review.openstack.org/#/c/611088/ now | |
| 16:39:24 | artom | mriedem, you're assuming the NUMA live migration code lands in Stein :) | |
| 16:39:35 | artom | I know I'm an all-star, but common | |
| 16:41:32 | artom | Pheeding first though, I'm phamished | |
| 16:41:52 | cdent | does first come before or after phirst? | |
| 16:42:10 | artom | Aww man, I missed the "phirst" opportunity :( | |