| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-27 | |||
| 14:17:07 | artom | dansmith, a not very convincing thing, apparently :) | |
| 14:17:08 | dansmith | migratedata is created after we're done talking to conductor for the last time, AFAIK | |
| 14:17:20 | dansmith | so I'm not sure how conductor would ever set it for us | |
| 14:17:36 | artom | Yeah, it's created by the source compute, IIRC | |
| 14:17:42 | artom | The source driver, in fact | |
| 14:17:51 | dansmith | hence why this flag should be set by the computes | |
| 14:18:15 | dansmith | it also depends on the config of each compute, | |
| 14:18:21 | dansmith | which you can't inspect from conductor | |
| 14:18:35 | dansmith | if the two computes involved are pinned to 5.0, then this can't happen even if they both support it | |
| 14:18:38 | artom | https://github.com/openstack/nova/blob/18.0.0/nova/compute/manager.py#L6056-L6089 | |
| 14:19:21 | stephenfin | gibi: calling 'self._delete_server' at the end of the test did the trick. I don't know why. Missing mocks or something like that. Good enough for me though | |
| 14:19:36 | artom | I need to get one kid to daycare, after that it's hax time | |
| 14:20:51 | artom | mriedem, you OK with squashing, as per dansmith's -1 here: https://review.opendev.org/#/c/635229/48? | |
| 14:21:04 | sean-k-mooney | ok i was suggeting passing in the booll here https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L318-L322 to check_can_live_migrate_destination but im fine with what dan suggest too | |
| 14:23:50 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add support for translating CPU policy extra specs, image meta https://review.opendev.org/671801 | |
| 14:23:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Fold in argument to '_update_provider_tree_for_vgpu' https://review.opendev.org/676729 | |
| 14:23:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reshaper for PCPU https://review.opendev.org/674895 | |
| 14:23:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Simplify 'fakelibvirt.HostInfo' object https://review.opendev.org/678861 | |
| 14:23:56 | mriedem | artom: my head is buried in something else atm so you're going to have to wait | |
| 14:24:07 | artom | mriedem, understood | |
| 14:25:08 | stephenfin | dansmith: If you're bored, I went and wrote that online migration to allow us to drop the Liberty-era NUMA topology blobs https://review.opendev.org/#/c/537414 I don't want to brag, but I think it's perfect | |
| 14:27:24 | dansmith | stephenfin: you know that if you say that, I double-fist red pens right? :D | |
| 14:27:52 | stephenfin | dansmith: I was counting on it ;) | |
| 14:27:57 | dansmith | heh | |
| 14:29:51 | gibi | stephenfin: I'm happy that you solved it. I have no idea what was the problem without digging | |
| 14:30:03 | sean-k-mooney | stephenfin: pet peve but i find the use of ~ for negation in filters really hard to spot | |
| 14:30:29 | sean-k-mooney | it took me 3 times to notice it was not containts give how far apart they are | |
| 14:31:20 | dansmith | artom: did you see my question about the multinode test? | |
| 14:31:24 | dansmith | artom: earlier in here | |
| 14:33:10 | artom | dansmith, yeah... I'd have to think about it some more, but I don't think it's possible to test it in the current gate | |
| 14:33:21 | artom | Or if it is, it'd be a very small part | |
| 14:33:22 | dansmith | stephenfin: you just added an online migration to the patch with the removal as before right? | |
| 14:33:44 | dansmith | artom: if I had a guest that just requested one numa node, wouldn't that boot in our current gate workers? | |
| 14:33:45 | artom | I *think* I'd rather concentrate on getting whitebox under openstack-qa and getting multi-NUMA node flavors from Fort Nebula | |
| 14:33:57 | stephenfin | dansmith: Yeah. That's what I should do, right? | |
| 14:34:22 | artom | dansmith, it might, but then what? | |
| 14:34:24 | dansmith | stephenfin: it doesn't change the fact that you can't remove that stuff in the same release | |
| 14:34:46 | dansmith | artom: live migrate such an instance between workers and that will exercise all of this code, even if it's a boring example right? | |
| 14:35:03 | stephenfin | Oh, because it would have to be a blocker migration to do that? | |
| 14:35:04 | dansmith | stephenfin: online migrations run after you've already rolled out new code | |
| 14:35:37 | dansmith | stephenfin: you can't have a blocker and a removal in the same release either, but I was saying a blocker migration is likely too expensive here because of what it has to check, so we should do it with time and warnings | |
| 14:36:07 | dansmith | stephenfin: i.e. land an online migration now, and a nova-status check, wait a release or two, then remove the compat code after everyone has had a chance to patch up all their data | |
| 14:36:21 | dansmith | stephenfin: a blocker migration helps make sure they clean up before they move, but again, too expensive here I tink | |
| 14:36:21 | stephenfin | Damn. So how long does that have to hang around for once I've the migration in-place? | |
| 14:36:41 | stephenfin | mkay | |
| 14:36:43 | dansmith | stephenfin: well, at least a cycle, but with no blocker, it'd be nice to leave it longer | |
| 14:37:04 | dansmith | maintaining compat sucks | |
| 14:37:17 | stephenfin | I'll probably pull that out of the series too, if I can | |
| 14:37:19 | dansmith | it's a good reason to try to get stuff right the first time | |
| 14:37:23 | dansmith | yeah, good idea | |
| 14:37:27 | stephenfin | true dat | |
| 14:38:44 | artom | dansmith, it'll exercise the code, but assuming the workers have identical NUMA topologies, we'll have no way of knowing | |
| 14:38:53 | artom | Because in those cases, current live migration "works" | |
| 14:39:08 | stephenfin | sean-k-mooney: fyi, six doesn't have a wrapper for the collections -> collections.abc thing | |
| 14:39:14 | stephenfin | there's an open bug but no patch for it | |
| 14:39:15 | artom | Both hosts being identical means we don't need to update the XML or claim any "new" resources | |
| 14:39:35 | sean-k-mooney | stephenfin: right but it does for is this python 3 or not | |
| 14:40:00 | sean-k-mooney | stephenfin: but ya i need to stil fix that actully so ill respin shortly | |
| 14:40:03 | stephenfin | Oh, right, gotcha | |
| 14:40:16 | stephenfin | Could you spin out 'flatten_iterable' into a separate patch too, in that case? | |
| 14:40:22 | stephenfin | If you have uses for it elsewhere | |
| 14:40:45 | dansmith | artom: no way of knowing what? if we see the debugs that show that you're doing the stuff, we'll know it's at least running your code, and not breaking something simple | |
| 14:40:52 | sean-k-mooney | i had thought you were using sum in a few places to flatten lists which was the other usecase i wanted to adress with it | |
| 14:41:06 | dansmith | artom: even if it generates the same xml on the other side, it's still doing all that work, even if for just a boring topo | |
| 14:41:12 | stephenfin | Yeah, I still don't think it's necessary when the sum thing is a well known pattern, but if you're going that way then it's definitely a patch in its own right | |
| 14:41:15 | sean-k-mooney | but i cant find them anymore. did you get rid of them recently | |
| 14:41:19 | dansmith | artom: seems like a worthwhile thing to have when a reviewer feels uneasy | |
| 14:41:19 | sean-k-mooney | and yes ill break it out | |
| 14:41:30 | bauzas | stephenfin: are you about to respin ? | |
| 14:41:33 | stephenfin | oh, no other uses? | |
| 14:41:34 | stephenfin | Hmm | |
| 14:41:35 | bauzas | I was on https://review.opendev.org/#/c/674894/11//COMMIT_MSG | |
| 14:41:36 | dansmith | artom: and I would think it would be pretty much just tweaking the flavor we create for a regular live migration job | |
| 14:41:44 | stephenfin | do we _really_ need it then? | |
| 14:41:48 | sean-k-mooney | yes | |
| 14:42:00 | sean-k-mooney | the sum pattern is not a common idiom | |
| 14:42:14 | sean-k-mooney | you are the only person i know of that has suggested it | |
| 14:42:35 | sean-k-mooney | so i would like to have a fucntion that names that algoritium | |
| 14:42:56 | sean-k-mooney | so we can reuse it and not have to rely on trible knowladge | |
| 14:43:02 | stephenfin | could we use itertools.chain? | |
| 14:43:15 | stephenfin | that does this same thing, right? | |
| 14:43:39 | sean-k-mooney | its close but it does not do the right things for maps | |
| 14:43:46 | stephenfin | no? | |
| 14:43:49 | sean-k-mooney | it allos does not flatten a list | |
| 14:44:00 | sean-k-mooney | *list of list | |
| 14:44:06 | sean-k-mooney | i could use it internally | |
| 14:44:39 | stephenfin | you can use '*' for that | |
| 14:44:42 | stephenfin | I mean, you've one caller | |
| 14:44:49 | stephenfin | This is so overengineered :D | |
| 14:44:58 | bauzas | sean-k-mooney: I honestly feel flattening an iterable of iterables is just a common pattern that doesn't really need to have a general method | |
| 14:44:59 | sean-k-mooney | but based on your feedback to not use the functional part of the standard libary like reduce i wrote it as genertor | |
| 14:45:26 | stephenfin | Yeah, I prefer reduce to this /o\ | |
| 14:45:30 | bauzas | sean-k-mooney: it's more like, you have so many ways to do what you want, pick one wisely | |
| 14:45:41 | stephenfin | I mean, I'm not in love with it, but 2 lines >> ~50 lines | |
| 14:45:47 | sean-k-mooney | ok i am going to take a break form irc now before i say some i should not | |
| 14:46:01 | bauzas | sean-k-mooney: like, returning a generator if you pass a couple of lists isn't really helpful, righT? | |
| 14:56:07 | mriedem | i think at this point i might just take this over to move it along https://review.opendev.org/#/c/663851/42 | |
| 14:56:30 | mriedem | alex_xu: gmann: efried: ^ any problem with me doing that? | |
| 14:57:14 | efried | I haven't been following it | |
| 14:57:14 | mriedem | it's close, in a runway for a couple more days, and there are at least 3 other changes open for review that are conflicting for the same microversion | |