Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-07-28
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 artom So we have to admit that *some* of the existing XML is authoritative in a live migraiton
16:45:52 sean-k-mooney regenrating all the xml is out of the question for a live migration
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
16:47:48 sean-k-mooney yes but i dont think we should do that
16:48:17 sean-k-mooney this is not something that should be change in a bug
16:48:38 sean-k-mooney its a deeper desigin choice that need to be consierded carfully
16:48:55 stephenfin I'm not proposing that. Just saying it's something we should aspire to
16:49:19 artom stephenfin, so we're back to the whole "Is XML canonical" debate?
16:49:32 stephenfin and that this is a tiny step in that direction that we'd do well to take, IMO of course
16:49:36 sean-k-mooney artom: well its not and i dont think it should be
16:50:02 stephenfin artom: yes, but we've just made a brief stop on our way somewhere else
16:50:03 artom stephenfin, so I think that's part of the issue - we first need to decide whether that's a direction we'd *all* like to take :)
16:50:07 sean-k-mooney stephenfin: technically its a step a way form it since you are not modifying it and regenrating it
16:50:23 stephenfin sean-k-mooney: how so?
16:50:42 sean-k-mooney making the xml canonical mean not regenerating it ever
16:50:48 artom OK, the new angle has clearly not worked :)
16:50:54 sean-k-mooney so that modifcatin done out of band are preserved
16:51:06 artom I'll register my disagreement on the patch, then leave it up to the powers that be, and go feed myself and the kiddos
16:51:29 sean-k-mooney so regenreating the entire xml form info passed back is the opisce of makeing the xml cannonical
16:51:36 stephenfin sean-k-mooney: oh, gotcha. Yeah, I'm on the "nova is authoritative" side of the fence
16:51:43 sean-k-mooney same
16:52:01 sean-k-mooney which is why i would be totally happy to always regenerate the xml form nova datastuctures
16:52:25 sean-k-mooney kind of like the migrate data but im asserting you already have the info you need with out having to pass it
16:52:33 sean-k-mooney not that the approch is wrong
16:52:35 stephenfin it really sounds like you're advocating for the same position as me
16:52:38 stephenfin ah
16:52:50 sean-k-mooney jsut that you should not have two places to get the info
16:53:03 sean-k-mooney the falvor/image and the migrate data object
16:53:10 stephenfin so I'm for it, artom is against it, and sean-k-mooney is kind of for it but not the way I've done it
16:53:23 stephenfin lovely :)
16:53:44 sean-k-mooney well 3 enginerrs your normally expect at least 4 oppinions
16:53:47 artom stephenfin, if we end up doing a push towards "Nova is authoritative, XML can suck it" I wont' stand in anyone's way
16:53:49 sean-k-mooney 3 is light :P
16:54:06 artom stephenfin, I just think in the current context it's premature
16:54:12 sean-k-mooney artom: that is the status quoe today
16:54:27 sean-k-mooney so the null hypotous is that nova is authritavive and xml is not
16:54:33 artom sean-k-mooney, well, except when it isn't? Like live-migration? :)
16:54:39 artom In practice, I mean
16:54:40 stephenfin Okay, I figured I had the information to hand when generating the migration data object, so it made sense to slot it in there, even if I could technically generate it on the other end
16:54:44 sean-k-mooney nova is still athuritive
16:55:02 sean-k-mooney if you ever modify the xml then the vm is not supproted anymore
16:55:06 artom sean-k-mooney, I guess? Is it really though, if it's using the existing XML to pull info?
16:55:39 sean-k-mooney nova created the exsting xml
16:55:50 artom Heh, true
16:55:53 sean-k-mooney and nothing outside of nova is allowed to modify it
16:55:54 artom Turtles all the way down!
16:55:55 sean-k-mooney well bar libvirt
16:55:57 stephenfin artom: As before. Maybe. Maybe not. It seemed fairly trivial, consistent and would always be correct, even it was slightly overengineered
16:56:23 artom stephenfin, all true. My beef is about the last part.
16:56:32 sean-k-mooney ok so https://review.opendev.org/#/c/743568/1 is artoms backportable change
16:56:33 stephenfin Yup, got that now
16:56:48 sean-k-mooney and https://review.opendev.org/#/c/743588/1 is your cleanup
16:56:51 stephenfin sean-k-mooney: nah, that's me too
16:57:04 stephenfin Add a TODO, resolve TODO in follow-up
16:57:14 sean-k-mooney ok well the first is intended to be backported and then a master only followup
16:57:24 stephenfin yup
16:57:35 stephenfin that's the overengineered idea, anyway
16:57:38 stephenfin :)
16:58:24 sean-k-mooney how did we end up with 2 elements
16:58:35 stephenfin read the bug report
16:58:39 stephenfin libvirt does it
16:59:12 sean-k-mooney really because it looks like we shoudl be updating the xml to only have one right
16:59:17 sean-k-mooney with the orginal code
16:59:35 stephenfin nope, we just update the first one we find
16:59:40 stephenfin not expecting there to be more
16:59:48 sean-k-mooney oh libvirt splits them
17:00:14 sean-k-mooney so <vcpusched vcpus="2-3" scheduler="fifo" priority="1"/>
17:00:23 sean-k-mooney becomes <vcpusched vcpus="2" scheduler="fifo" priority="1"/>
17:00:26 sean-k-mooney <vcpusched vcpus="3" scheduler="fifo" priority="1"/>
17:00:30 sean-k-mooney then we update the first one
17:00:59 stephenfin yup
17:01:43 sean-k-mooney then why can we not delete the vcpushed element and just not add the new one
17:01:57 stephenfin that's precisely what I'm doing
17:02:03 sean-k-mooney oh that is what your doing
17:02:10 sean-k-mooney in https://review.opendev.org/#/c/743568/1/nova/virt/libvirt/migration.py
17:02:17 stephenfin yuuup
17:02:46 stephenfin loads of context in the bug report and commit message itself, and it's pretty trivial to reproduce
17:02:52 stephenfin and with that, I'm out
17:02:54 stephenfin o/
17:03:13 sean-k-mooney so i would just read the policy form the element before we remove it
17:03:31 sean-k-mooney here https://review.opendev.org/#/c/743568/1/nova/virt/libvirt/migration.py@130
17:03:50 sean-k-mooney we can even assert that its the same for all element if we want
17:03:52 stephenfin <stephenfin> we should be trying to avoid introspecting that XML to try guess what was done previously
17:04:23 sean-k-mooney well yes which is why i said we shoulc caluated it form the flaovr/image
17:04:48 sean-k-mooney but if you dont want to do that and dont want to hardcode tehn introscpect is the only other option
17:05:19 sean-k-mooney wait
17:05:46 sean-k-mooney the check is "if 'sched_vcpus' and 'sched_priority' in info:"

Earlier   Later