Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-07-28
15:45:34 stephenfin Aha, gotcha
15:45:40 stephenfin Looking forward to it :)
15:45:54 openstackgerrit Stephen Finucane proposed openstack/nova master: Handle multiple 'vcpusched' elements during live migrate https://review.opendev.org/743568
15:46:35 stephenfin artom: When you've a chance, could you sanity check that for me?
15:47:00 stephenfin I'll poke bauzas or lyarwood to review later
15:47:40 artom stephenfin, for sure
15:48:06 stephenfin ta
15:48:55 artom stephenfin, also, aren't you glad for those LOG.debug calls with the full XML dump? :)
15:49:18 stephenfin for sure
15:49:21 stephenfin logging ftw
15:50:20 kashyap stephenfin: Here, right now I don't have a better phrasing; you might say it's not even required, let's see -- https://review.opendev.org/#/c/741473/1/nova/conf/libvirt.py@261
15:54:11 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Pass the actual target in volumes policy https://review.opendev.org/742779
15:57:15 artom stephenfin, seems sane, left a note
16:00:35 stephenfin artom: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L66-L67
16:00:43 stephenfin https://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L53-L59
16:00:55 stephenfin the name is poor, admittedly
16:14:52 artom stephenfin, doh :( A ctrl-] would not have been that hard, on my part
16:15:39 sean-k-mooney what does ctrl-] do
16:15:43 sean-k-mooney a find?
16:15:48 sean-k-mooney got do definition
16:15:53 artom Go to definition, in vim with ctags
16:15:59 sean-k-mooney ah ok
16:16:15 artom Admittedly, it sometimes messes up if there are multiple methods called the same thing
16:22:22 openstackgerrit Stephen Finucane proposed openstack/nova master: Track CPU scheduler policy during live migration https://review.opendev.org/743588
16:22:37 stephenfin artom: follow-up there, if you're feeling generous
16:25:19 artom stephenfin, so... does it really make sense to do this, given we only support 1 policy now?
16:27:16 stephenfin Maybe. Maybe not. Given how trivial it is to do, I'd rather knock it off now than have to worry about it when we really do need it
16:27:41 stephenfin at that point we'd be down to needing service version checks, or whatever the new hotness is by then
16:29:01 artom stephenfin, so... this would be for a case where the scheduler is set on a per-host basis?
16:29:18 artom And we want to support live migration between different hosts/schedulers?
16:29:32 artom Wouldn't we do it as an image property/flavor extra spec?
16:30:29 sean-k-mooney stephenfin: given we dont support other schduler polcies and dont plan to intoduce that in the near term do we really need this cange
16:30:30 stephenfin if we did have different scheduler policies they'd likely be done via an image prop/extra spec, yes
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 artom sean-k-mooney, yep, and I think you and I would be against such a thing
16:32:17 stephenfin artom: I'm confused. What would change?
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 artom You're clobbering first
16:36:01 stephenfin I'm throwing them away and recalculating them
16:36:05 stephenfin yessss
16:36:15 artom So... don't clobber them :)
16:36:23 stephenfin and parsing the existing XML feels gross
16:36:23 artom Or stash the stuff we know won't chang
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 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:54 sean-k-mooney stephenfin: you dont need to stash this
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

Earlier   Later