| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-21 | |||
| 11:40:38 | sean-k-mooney | * instance_system_metadata | |
| 11:40:58 | gibi | if the OVO model is just a Dict then changing the SQL model does not help architecturally | |
| 11:41:18 | sean-k-mooney | but that would not buy use much since we dont really query on the sub fileds | |
| 11:41:25 | sean-k-mooney | ya it doesnt really | |
| 11:42:53 | sean-k-mooney | if you work on this ping me and ill happily review i dont know if dmitriis woudl have time to work on it as a followup/techdebt cleanup | |
| 11:43:32 | gibi | sean-k-mooney: thanks. I will work on it as I need it for the PF MAC bugfix anyhow | |
| 11:44:35 | sean-k-mooney | :) ping me if you want me to review that too. speaking of which i should go look at the rest of your placement series | |
| 11:45:03 | sean-k-mooney | gibi: we defintly want to backport the PF fix if it ends up being posible by the way | |
| 11:45:25 | gibi | sean-k-mooney: I think it will be possible | |
| 11:45:40 | sean-k-mooney | i kind of feel like the backporatble fix and final fix would be different however | |
| 11:45:49 | gibi | sean-k-mooney: yeah, if you are done with the placement series then I will switch to that to fix the small thing in a followup | |
| 11:46:25 | gibi | sean-k-mooney: let's see about the backportability of the PF MAC | |
| 11:46:26 | sean-k-mooney | as in final fix would invovle multipel port bdingin but backporable would just unbind the port for the vm update the mac and reattach it or something like that | |
| 11:46:59 | gibi | sean-k-mooney: nope, the plan is to add the PF MAC to the binding:profile and let neutron do the overwrite of port.mac_addres based on that | |
| 11:47:10 | sean-k-mooney | oh right yes | |
| 11:47:20 | sean-k-mooney | you said that last week | |
| 11:47:32 | gibi | the neutron side is basically ready in https://review.opendev.org/c/openstack/neutron/+/829247 | |
| 11:47:41 | gibi | (I need to fix comments there) | |
| 11:47:49 | sean-k-mooney | are neutron ok to backport that? | |
| 11:47:57 | sean-k-mooney | or will we have to workaround that in the nova backport | |
| 11:48:07 | gibi | I think they are OK to backport that | |
| 11:48:14 | gibi | no issue was raise so far on that | |
| 11:48:30 | sean-k-mooney | ack then that makes our life simpler | |
| 11:48:35 | gibi | the nova side needs a redo as per the pci_utils / pci_info refactor https://review.opendev.org/c/openstack/nova/+/829248 | |
| 11:48:38 | sean-k-mooney | i was assuming they would not backport that change | |
| 11:48:58 | gibi | sean-k-mooney: at least they haven't indicated that it is not backportable | |
| 11:49:04 | gibi | I will double check with them | |
| 11:49:41 | sean-k-mooney | ack it looks pretty small and i dont think it would break existing usage | |
| 11:49:46 | sean-k-mooney | so it shoudl be safe to backport | |
| 11:49:55 | gibi | yepp that is my view too | |
| 11:50:44 | sean-k-mooney | its kind of a feature however but i would be look very favorably towards framing it as a bugfix as you have done in the release note | |
| 11:50:55 | sean-k-mooney | ill finish reviewing that since i have it open now | |
| 11:51:37 | gibi | yeah it is a bug in nova, that needs some featury thing from neutron :) | |
| 11:51:46 | sean-k-mooney | gibi: there is no way to detect this form the api right | |
| 11:51:50 | sean-k-mooney | e.g. that this is supproted | |
| 11:52:07 | gibi | it cannot be detected | |
| 11:52:09 | sean-k-mooney | just thining that on the nova side we could eventually move to only setting the PF port via this eventully | |
| 11:52:10 | gibi | but it is safe from upgrade | |
| 11:52:35 | gibi | yeah I left a note in the nove code to only remove the current PF MAC settings logic after couple of releases | |
| 11:52:44 | sean-k-mooney | perfect | |
| 12:00:17 | gibi | sean-k-mooney: btw the arch OVO backport is also crazyness https://review.opendev.org/c/openstack/nova/+/829989 but I have a totally differnt proposal as a possible direction as a comment within that patch | |
| 12:00:59 | sean-k-mooney | what exactly is that doing? | |
| 12:01:25 | gibi | so the Arch enum got new values | |
| 12:01:34 | gibi | due to the emulation support feature | |
| 12:01:35 | sean-k-mooney | right which should be added at the end | |
| 12:01:52 | gibi | and the ComputeNode object has a list of HVSpec objects with arch values | |
| 12:01:54 | sean-k-mooney | and then you would just raise an error if you try to backport | |
| 12:02:04 | gibi | HVSpec cannot backport itself | |
| 12:02:12 | gibi | the ComputeNode would need to backport it | |
| 12:02:24 | gibi | by removing too new HVSpec instances (with new arch) | |
| 12:02:29 | gibi | but OVO does not cooperate | |
| 12:02:33 | gibi | I even filled https://bugs.launchpad.net/oslo.versionedobjects/+bug/1961482 | |
| 12:02:48 | gibi | so the current patch up above works but ugly as hell | |
| 12:02:51 | sean-k-mooney | am well the compute node would not have the new values if it was not updated | |
| 12:03:11 | sean-k-mooney | and an instance object with the new values shoudl not be sendable to the compute node | |
| 12:03:24 | gibi | there is code path in live migration where an old compute loads the new compute from the DB | |
| 12:03:49 | sean-k-mooney | oh | |
| 12:04:00 | sean-k-mooney | thats a nasty edgecase | |
| 12:04:05 | gibi | grenade caught it | |
| 12:04:39 | sean-k-mooney | right ok so the list object would have to strip the new version instead of raise | |
| 12:04:44 | gibi | yes | |
| 12:04:51 | gibi | but it is really hard to do that with OVO | |
| 12:05:02 | gibi | (see the patch with a ton of comments) | |
| 12:05:10 | sean-k-mooney | i dont think we have had to do that in the past | |
| 12:05:19 | sean-k-mooney | at least i cant think of a case where this has come up | |
| 12:05:30 | gibi | my alternative proposal is not to publish HVSpec objects with new arch values until a min service version check is passed | |
| 12:05:43 | sean-k-mooney | most of the enums we have extended in the past have been only in teh instance object | |
| 12:06:01 | sean-k-mooney | ok ya that would work | |
| 12:06:16 | sean-k-mooney | we often dont enable feature until the cloud is fully upgraded | |
| 12:06:52 | sean-k-mooney | this woudl cause all the compute nodes to update the comptue node table once they are all upgraded | |
| 12:06:54 | gibi | I pulled dansmith into the discussion maybe he sees an easy fix in the OVO support for this case but if not then I will suggest chateaulav to add a service version check instead | |
| 12:07:08 | sean-k-mooney | well maybe not | |
| 12:07:30 | sean-k-mooney | we are extendign the values but that does not mean the compute node would use one of the new values | |
| 12:07:45 | gibi | today it uses in the gate | |
| 12:07:57 | gibi | I haven't checked it what enables all the new arch | |
| 12:08:15 | sean-k-mooney | its likely just the presence fo the qemu binary | |
| 12:08:30 | sean-k-mooney | on ubuntu it installs all of them by default | |
| 12:08:57 | sean-k-mooney | how is this reported in the compute node db recored by the way | |
| 12:09:18 | sean-k-mooney | ah supported_hv_specs | |
| 12:09:30 | gibi | yes, that one | |
| 12:10:21 | sean-k-mooney | i guess we dont wnat to defien that as the host native arch | |
| 12:10:32 | sean-k-mooney | since that would not really help if we had native hosts using one of the new options | |
| 12:10:57 | gibi | it seems the new arch coming from the host cababilities via libvirt | |
| 12:11:05 | sean-k-mooney | yes | |
| 12:11:21 | sean-k-mooney | those are dynmic based on which qemu binaries libvirt finds | |
| 12:11:25 | gibi | ahh I see | |
| 12:11:33 | gibi | the you are right about the binaries | |
| 12:12:37 | sean-k-mooney | technically its dynmaic based on which libvirt virt plugin too e.g. if you install the lxc or openvz plugins they also extend it but in this case its the presence of the riscv emulators | |
| 12:13:15 | sean-k-mooney | ignoring this for a miniute how close do you think the emulation work is for this cycle | |
| 12:13:29 | sean-k-mooney | i think the previous impression i heard was likely not this cycle | |
| 12:13:58 | sean-k-mooney | i am debating if defering the ricsv support woudl help or not | |
| 12:14:18 | sean-k-mooney | that woudl allow use to punt this enum change to next cycle | |
| 12:16:35 | sean-k-mooney | so basically im suggesting that perhaps its better for chateaulav to focus on the arm and mips supprot this cycle and we can adress teh riscv supprot next cycel btu if we dont think the code is likely to land before thruday which i gusse is unlikely it proably is not worth spliting it out | |
| 12:25:00 | gibi | sean-k-mooney: good point. I think the series is in an OK shape overall | |
| 12:25:21 | gibi | so if we could remove the OVO backport pain by not adding risc then we can land it this week I believe | |
| 12:26:22 | gibi | bauzas, melwitt: there is a gate bug on nova that is a duplicate of a gate bug in neutron https://bugs.launchpad.net/nova/+bug/1959349 https://bugs.launchpad.net/neutron/+bug/1940425 but if I mark the newer bug as duplicate then we loose the tracking of it | |
| 12:26:51 | gibi | bauzas, melwitt: is it OK if I mark it duplicate and then add nova as affected to the eariler neutron bug? | |
| 12:27:06 | sean-k-mooney | gibi: riscv was not orgianlly on there list i suggeted it be added since i tought that would be one of the more useful targets | |
| 12:27:20 | gibi | sean-k-mooney: OK, then I think it make sense to split it out | |
| 12:27:21 | sean-k-mooney | gibi: so defering that i dont think woudl compropise there usecase | |