| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-09-30 | |||
| 17:21:06 | sean-k-mooney | ah | |
| 17:21:17 | sean-k-mooney | except (neutron_client_exc.IpAddressGenerationFailureClient, neutron_client_exc.ExternalIpAddressExhaustedClient | |
| 17:21:30 | lajoskatona | SDK has general httpException, that can be used for such | |
| 17:22:35 | opendevreview | Takashi Kajinami proposed openstack/nova master: AMD SEV: omit iommu='on' for virtio devices https://review.opendev.org/c/openstack/nova/+/909635 | |
| 17:22:38 | sean-k-mooney | so neutron_client_exc.OverQuotaClient apars ot be the oen that is missing | |
| 17:22:55 | sean-k-mooney | right we are nologner rasign FloatingIpLimitExceeded | |
| 17:23:35 | sean-k-mooney | so for partity we would need to disambiuage the sdk_exc.ConflictException | |
| 17:23:57 | sean-k-mooney | and look at the message to know if its a quota issue or if we are out of ips | |
| 17:25:26 | sean-k-mooney | i think in both case we would expect the allocation to fail and we would want to return a 409 | |
| 17:25:47 | sean-k-mooney | so merging those together while it makes debuging hard does not seam to alter the api behavior | |
| 17:26:48 | sean-k-mooney | lajoskatona: are you wonderign if nova shoudl do that disambiguation ro if it shoudl be in the sdk | |
| 17:27:08 | sean-k-mooney | or are you wonderign if its required to treat them differently vs merging them into a singel expction | |
| 17:27:14 | lajoskatona | sean-k-mooney: +1, I check it tomorrow to look inside the message | |
| 17:27:57 | lajoskatona | sean-k-mooney: My first idea was to merge them | |
| 17:28:19 | sean-k-mooney | i think as long as the end result at the api/externally does nto change i.e. 409 | |
| 17:28:28 | sean-k-mooney | adn we have somethign reasonable in the debug logs | |
| 17:28:36 | lajoskatona | sean-k-mooney: for tomorrow I will have more test results from zuul, so I will see if this aproach can work :-) | |
| 17:28:37 | sean-k-mooney | so we can tell which it actully was its proably oke to merge them | |
| 17:28:46 | sean-k-mooney | ack | |
| 17:29:22 | sean-k-mooney | so for th ecase like this you could consider doing the merge in a seperate refactor first | |
| 17:29:54 | sean-k-mooney | then the adopt sdk patch is just doing that | |
| 17:30:17 | sean-k-mooney | i have not reviewd this in detail but in most cases i tlooks like you have found 1:1 mappings | |
| 17:30:37 | sean-k-mooney | but sure let me know what you find after zuul reports | |
| 17:31:17 | sean-k-mooney | lajoskatona: as an aside i think https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.py#L729-L744 is the only useage of neutronclient in watcher | |
| 17:31:38 | sean-k-mooney | and i dont think we actully need that info really any more | |
| 17:31:47 | sean-k-mooney | its part of the create_instance helper https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.py#L641 | |
| 17:31:56 | sean-k-mooney | and that on my todolist to see fi we can entrily delete | |
| 17:32:40 | sean-k-mooney | lajoskatona: so if nothing else im goign to see if we can remove the neutronclient dep entirly one way or another in 2026.1 | |
| 17:34:04 | sean-k-mooney | watcher used ot have its onw impleation of evacuate/coldmigrate where it woudl snapshot a vm, delete it and create a new one | |
| 17:34:33 | sean-k-mooney | i belive this is mostly dead code now | |
| 17:41:15 | lajoskatona | sean-k-mooney: thanks I add watcher to my todolist, I have anyway some pending patches waiting for the release to not bother people (like in manila-ui) | |
| 18:29:07 | melwitt | dansmith, bauzas: just some fyi, I have been realizing some issues while working on tempest tests for this series. I have added a few comments about it on the spec re-proposal here https://review.opendev.org/c/openstack/nova-specs/+/961564/1/specs/2026.1/approved/vtpm-live-migration.rst | |
| 18:30:15 | melwitt | (about [libvirt]supported_tpm_secret_security valid values and overloading of the image_hw_tpm_secret_security instance sysmeta key) | |
| 18:30:22 | dansmith | melwitt: I mostly gave up reviewing the spec re-proposal because it isn't easy to see the changes. So I've just been using the commit message to remind myself of the changes while looking at the code | |
| 18:30:26 | dansmith | I figure it's better to spend my time on the code anyway | |
| 18:31:56 | melwitt | dansmith: that's a reasonable approach, that was my thinking in enumerating the changes in the commit message. just saying I have noticed some things while doing tempest stuff and thought that might be the best place to note my findings | |
| 18:32:12 | dansmith | ack | |
| 18:33:56 | melwitt | it's all about the "opt in via resize" part of it. I have been writing tempest tests that do resize-confirm, resize-revert, and now I'm doing a rebuild-with-image-property one to see what happens | |
| 18:34:24 | melwitt | I am unsure how comprehensive we will want to be with the ability for a user to change their TPM secret security policy | |
| 18:48:24 | melwitt | dansmith: do you think that we should support a user _changing_ their secret security policy via a resize? or do you think the resize should be limited to legacy instances that don't have a policy set? because the former becomes a problem with us overloading the sysmeta key for it | |
| 18:49:30 | melwitt | *the image property sysmeta key for it | |
| 18:56:13 | sean-k-mooney | melwitt: interesting i had assumed the later that you would be able change via resize | |
| 18:57:46 | melwitt | sean-k-mooney: you mean the former I think :) but yes I also went in with that assumption but I want to pause and make sure that's what we want, bc if we want that, I think we need to not use "image_hw_tpm_secret_security" as the key to stash in the instance sysmeta. bc it makes flavor/resize conflict errors | |
| 18:58:37 | melwitt | (I have been finding things while adding more func tests and tempest tests) | |
| 18:59:04 | melwitt | *flavor/image conflict errors | |
| 18:59:22 | sean-k-mooney | well the flavor image conflcit check shoudl be checkign the glance image keys | |
| 18:59:27 | sean-k-mooney | are we using the stashed copy | |
| 18:59:28 | melwitt | , when the user may not have ever used an image prop | |
| 18:59:44 | sean-k-mooney | i guess we might be to ensure that out of band glance changes dont creap in | |
| 19:00:05 | sean-k-mooney | so we do not need ot use the same key | |
| 19:00:12 | melwitt | we are yeah. for the check virt.hardware.get_tpm_secret_security_constraint() for example | |
| 19:00:23 | sean-k-mooney | that was suggested just to minimise the number of places we have to check | |
| 19:01:06 | melwitt | yeah. using the same key becomes a problem if the user never used an image with that key in it and we have set it internally for other reasons | |
| 19:01:12 | sean-k-mooney | are you thinking we woudl add a new instance system metadata key ? | |
| 19:01:31 | sean-k-mooney | or were you leanign to only allowing resize if its not a confilct | |
| 19:01:51 | sean-k-mooney | meaning we shoudl not set it on update but treat it not being set as the old user policy | |
| 19:01:56 | melwitt | if we want to be able to convert via resize, yes. like convert from host => deployment, etc. unless someone has a different idea | |
| 19:01:58 | sean-k-mooney | i.e. defaulting in code instead of in the db | |
| 19:02:45 | sean-k-mooney | if we dont use a new key then the simplest thing to do is treat key not in data as the USER policy | |
| 19:02:50 | melwitt | because if we set image_hw_tpm_secret_security NOT from an image, it will get locked into the instance as if it had the image property, and it will reject any flavor hw:tpm_secret_security that does not agree with it | |
| 19:03:25 | sean-k-mooney | then the key will only be set if it actully comes form the image | |
| 19:03:32 | melwitt | everything currently works fine for legacy => host and legacy => deployment | |
| 19:03:51 | melwitt | the problem comes when you have user => host, user => deployment, host => deployment, etc. new instances basically | |
| 19:04:07 | sean-k-mooney | ya | |
| 19:04:26 | melwitt | but yeah I see what you are saying | |
| 19:04:45 | melwitt | that would work also | |
| 19:05:10 | sean-k-mooney | basicly image_metadata.get("image_hw_tpm_secret_security", "user") in virt.hardware.get_tpm_secret_security_constraint() | |
| 19:05:48 | sean-k-mooney | well | |
| 19:05:54 | sean-k-mooney | hum | |
| 19:05:57 | sean-k-mooney | htat does not quite work | |
| 19:06:12 | sean-k-mooney | in the resize case we still need to know if its set or not set | |
| 19:06:21 | sean-k-mooney | but for most other case it wil work | |
| 19:08:06 | sean-k-mooney | i think you can still do it pretty cleanly | |
| 19:08:16 | sean-k-mooney | but i dont know are we giving up anything else by doing it that way | |
| 19:08:44 | melwitt | yeah. it's hard to know ahead of time | |
| 19:09:18 | melwitt | what I'm doing now is writing up some more func tests and tempest tests to sort of demonstrate where the series is at the moment with regard to resize and rebuild behaviors | |
| 19:10:10 | melwitt | I was thinking looking at those would make it easier to reason about how it's working at the moment and help us decide what to do next. at least it helps for me | |
| 19:30:34 | sean-k-mooney | ya that sound like a good approch | |
| 19:33:22 | sean-k-mooney | hum so this is the curretn defintion https://review.opendev.org/c/openstack/nova/+/942502/17/nova/virt/hardware.py | |
| 19:33:49 | sean-k-mooney | and like the ohter get_*_constraint | |
| 19:34:20 | sean-k-mooney | functions it only thake the flavor/image and does not know if it is a resize ectra | |
| 19:34:41 | sean-k-mooney | so if we leave it as it it will work the way we want | |
| 19:35:01 | sean-k-mooney | but it means we will need to do the defaulting diffently | |
| 19:35:15 | sean-k-mooney | which isnot great | |
| 19:35:51 | sean-k-mooney | melwitt: there is another option actully | |
| 19:36:08 | sean-k-mooney | melwitt: instead of defaulting the value to "user" for the exiting vms | |
| 19:36:18 | sean-k-mooney | we could defautl it to "legacy" | |
| 19:36:40 | sean-k-mooney | which for all intents woudl be the same as user excpt it allows you to cange to a diffent policy | |
| 19:37:05 | sean-k-mooney | in other words we would asdd a new value to the enum just for upgrades | |
| 19:37:32 | sean-k-mooney | im not sure if that is a good idea or not but its an option to consider i guess | |
| 19:45:07 | melwitt | sean-k-mooney: hm yeah I hadn't considered that | |
| 19:45:55 | sean-k-mooney | we did somethign similar for pci neuma affinity. we added a "legacy" value for the old default when we added new policies | |
| 19:46:04 | sean-k-mooney | we intened to eventually change the default but that never happened | |
| 19:46:31 | sean-k-mooney | https://docs.openstack.org/nova/latest/configuration/config.html#pci.alias | |
| 19:46:34 | melwitt | ah, good to know | |
| 19:46:41 | sean-k-mooney | numa_policy | |
| 19:46:43 | sean-k-mooney | Required NUMA affinity of device. Valid values are: legacy, preferred, required, and socket. | |
| 19:47:06 | sean-k-mooney | i belive we intened to make preferred the default but we never got around to it | |
| 19:47:59 | melwitt | haha | |
| 19:48:41 | sean-k-mooney | ok im wrapping for today, enjoy your evening | |