| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-07-28 | |||
| 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 | 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 | |