| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-21 | |||
| 11:33:49 | gibi | sean-k-mooney: currently the parent_ifname is part of the extra_info | |
| 11:33:51 | sean-k-mooney | in fact we could store the info in extra_info for backwards compatiablity without changing the object | |
| 11:33:56 | sean-k-mooney | ya | |
| 11:33:57 | gibi | yeah | |
| 11:34:14 | gibi | lets try that | |
| 11:34:24 | sean-k-mooney | so we can add the other field there and keep object compat and perhaps add properties for access | |
| 11:34:31 | gibi | yepp | |
| 11:34:50 | gibi | and that way I can still use that in my PF MAC address bugfix and keep it backportable | |
| 11:34:59 | sean-k-mooney | yes | |
| 11:35:37 | gibi | cool, thanks for the brainstroming | |
| 11:35:54 | gibi | :D | |
| 11:36:15 | gibi | yeah I feel the mental pain due to that json blobs in the db | |
| 11:36:26 | gibi | but the bugfix backporting pain is bigger | |
| 11:36:31 | gibi | so that won | |
| 11:40:22 | sean-k-mooney | honestly the only thing i woudl replace it with is a key value mapping table | |
| 11:40:26 | sean-k-mooney | like instance extra | |
| 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 | |