| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-07-28 | |||
| 16:30:52 | sean-k-mooney | stephenfin: right but we dont plan on allowing user to request that | |
| 16:31:23 | artom | stephenfin, so if it's set in the flavor, do we need this change? | |
| 16:31:34 | artom | As you can see, I'm having trouble being convinced :) | |
| 16:31:46 | sean-k-mooney | if it was in the flavor or image no | |
| 16:31:57 | sean-k-mooney | we woudl only need it if it was set by a host level config option | |
| 16:32:17 | stephenfin | artom: I'm confused. What would change? | |
| 16:32:17 | artom | sean-k-mooney, yep, and I think you and I would be against such a thing | |
| 16:32:28 | artom | Precisely because it would require stephenfin's patch :P | |
| 16:32:52 | artom | stephenfin, I'm saying - if the scheduler is set via flavor extra spec, we don't need to update it as part of the live migration XML update | |
| 16:33:02 | sean-k-mooney | stephenfin: i guess the issue is right now i dont think your change adds anything useful | |
| 16:33:15 | artom | Because the destination host should be able to accept the existing scheduler | |
| 16:33:27 | artom | (Presumably because we've scheduled to it because of a trait or something) | |
| 16:33:35 | sean-k-mooney | we have no plans to expose sched_scheduler to the user or operator as something that can be set in anyway | |
| 16:33:52 | stephenfin | artom: oh, perhaps the difference here is that I'm saying we shouldn't be _updating_ the XML elements | |
| 16:34:09 | stephenfin | we should be regenerating them based on authoritative sources that tell us what they should be set to | |
| 16:34:16 | stephenfin | I figured the value of doing this is to provide such an authoritative source for what the scheduler policy should be | |
| 16:34:40 | sean-k-mooney | stephenfin: this value will always be fifo if realtime is enabled | |
| 16:34:45 | sean-k-mooney | otherwise it will be unset | |
| 16:34:57 | sean-k-mooney | so we shoudl not need any addtion info in the migrate_data | |
| 16:34:59 | artom | And if we ever want to support something other than fifo, it won't change during a live migration | |
| 16:35:01 | stephenfin | right, at the moment that's the case | |
| 16:35:11 | artom | Only during a rebuild with a different image | |
| 16:35:24 | stephenfin | artom: okay, but if it doesn't change how do I get that information | |
| 16:35:40 | artom | stephenfin, you don't need to? Keep the existing one | |
| 16:35:49 | stephenfin | I'm not keeping the original elements though | |
| 16:35:53 | artom | Oh, *facepalm* | |
| 16:35:56 | artom | I get it now | |
| 16:35:58 | sean-k-mooney | stephenfin: if hw:cpu_realtime=true or hw_cpu_realtime=true its fifo] | |
| 16:36:01 | stephenfin | I'm throwing them away and recalculating them | |
| 16:36:01 | artom | You're clobbering first | |
| 16:36:05 | stephenfin | yessss | |
| 16:36:15 | artom | So... don't clobber them :) | |
| 16:36:23 | artom | Or stash the stuff we know won't chang | |
| 16:36:23 | stephenfin | and parsing the existing XML feels gross | |
| 16:36:25 | artom | *change | |
| 16:36:32 | stephenfin | that's what I thought I was doing | |
| 16:36:40 | stephenfin | stashing it in the migrate_data object | |
| 16:36:54 | sean-k-mooney | stephenfin: you dont need to stash this | |
| 16:36:54 | openstackgerrit | Takashi Natsume proposed openstack/python-novaclient master: Add a cleanup for a server in a functional test https://review.opendev.org/743589 | |
| 16:36:56 | stephenfin | i.e. providing an authoritative source | |
| 16:37:07 | artom | Isn't that overkill? | |
| 16:37:14 | sean-k-mooney | you can compute it form the flavor and image | |
| 16:37:27 | stephenfin | sean-k-mooney: Yes, that's exactly what I do | |
| 16:37:30 | stephenfin | on the source side | |
| 16:37:35 | stephenfin | then I stash it in the migrate data | |
| 16:37:37 | stephenfin | :) | |
| 16:37:43 | sean-k-mooney | by they would you do that | |
| 16:37:54 | sean-k-mooney | you can comptue it form the falvor and image anyhwere its needed | |
| 16:38:02 | artom | You're proposing we add a a field to an object that does, like, 42 transits over the wire, instead of parsing some XML in a single line? | |
| 16:38:40 | stephenfin | yes | |
| 16:38:46 | artom | OK :) | |
| 16:38:49 | artom | I disagree :) | |
| 16:38:53 | sean-k-mooney | me too | |
| 16:39:00 | stephenfin | I'm saying our flavor and image metadata code is authoritative and the only thing we can trust | |
| 16:39:02 | sean-k-mooney | also you dont need to parse | |
| 16:39:12 | stephenfin | as evidenced by the fact this bug exists | |
| 16:39:19 | sean-k-mooney | what bug | |
| 16:39:34 | stephenfin | libvirt can do anything to our XML once we pass it off | |
| 16:39:54 | artom | stephenfin, true, but we can still have some expectations | |
| 16:40:10 | sean-k-mooney | oh we are generating <vcpusched vcpus="2-3" scheduler="fifo" priority="1"/> | |
| 16:40:12 | sean-k-mooney | <vcpusched vcpus="3" scheduler="fifo" priority="1"/> | |
| 16:40:17 | stephenfin | we should be trying to avoid introspecting that XML to try guess what was done previously | |
| 16:40:21 | stephenfin | sean-k-mooney: yup | |
| 16:40:25 | sean-k-mooney | ya you dont need to introspect the xml | |
| 16:40:30 | sean-k-mooney | or pass any data | |
| 16:40:40 | sean-k-mooney | you jst need to check the flavor and image extraspecs | |
| 16:40:41 | stephenfin | then I need to hardcode the policy | |
| 16:40:48 | sean-k-mooney | yes | |
| 16:40:57 | sean-k-mooney | its hardcoded today based on if you enable realtime | |
| 16:41:10 | artom | stephenfin, I mean, you're already "parsing" the XML when you're removing the existing vcpusched elements | |
| 16:41:26 | artom | Use that to stash the scheduler | |
| 16:41:40 | stephenfin | sean-k-mooney: right, in a function that's miles away | |
| 16:41:42 | artom | sean-k-mooney, I don't think we have the flavor/image in libvirt/migration.py, there would be plumbing required | |
| 16:41:53 | sean-k-mooney | we do | |
| 16:42:01 | sean-k-mooney | if we have the instance we have the flavor and image | |
| 16:42:22 | stephenfin | artom: I'm not parsing it though. I'm finding it and totally replacing it with my own freshly baked XML | |
| 16:42:23 | sean-k-mooney | stephenfin: if you plan to backprot this bug fix | |
| 16:42:27 | sean-k-mooney | you can change the object | |
| 16:42:30 | artom | sean-k-mooney, I don't think we have the instance either | |
| 16:42:34 | sean-k-mooney | i think we woudl want to backport it to train | |
| 16:42:45 | stephenfin | sean-k-mooney: that's why it's a separate change :) | |
| 16:42:46 | sean-k-mooney | so form my point of vew we cant add the new fiedl | |
| 16:43:06 | artom | stephenfin, agree to disagree I guess? We're obviously doing a poor job of convincing the other :P | |
| 16:43:21 | stephenfin | I've hardcoded it for expediency, but I think it's a bad idea to be doing that | |
| 16:43:22 | artom | Whoever the cores are that review that can make the call | |
| 16:43:58 | sean-k-mooney | stephenfin: why im asserting we actvly never want to make the schduler policy configurable | |
| 16:43:59 | stephenfin | artom: it would appear so. Guess I best go request cookies be sent to gibi in advance of his return ;) | |
| 16:44:30 | sean-k-mooney | its not a case of we dont expose this to day and might want to in the futre it we dont support it today and dont want to in the future | |
| 16:45:11 | artom | stephenfin, lemme try another angle. If we push your logic, we end up regenerating the *whole* XML upon live migration, including potentially shuffling PCI addresses | |
| 16:45:32 | sean-k-mooney | well we cant do that | |
| 16:45:40 | artom | sean-k-mooney, my point exactly :) | |
| 16:45:52 | sean-k-mooney | regenrating all the xml is out of the question for a live migration | |
| 16:45:52 | artom | So we have to admit that *some* of the existing XML is authoritative in a live migraiton | |
| 16:46:04 | artom | So why not the realtim scheduler? | |
| 16:46:07 | sean-k-mooney | regenreat the cputune element is a differnt matter | |
| 16:46:50 | artom | (Btw, Real Tim is my MC name) | |
| 16:47:11 | stephenfin | We can't do it yet | |
| 16:47:20 | stephenfin | If we properly tracked everything, we could totally do that | |
| 16:47:34 | stephenfin | which would require adding more stuff to migrate data | |
| 16:47:37 | stephenfin | like I'm doing here | |