| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-06-02 | |||
| 17:42:42 | sean-k-mooney | assuming your tests are otherwise correct :) | |
| 17:45:52 | sean-k-mooney | ricolin: the patch is deffently not correct but ill comment inline | |
| 17:46:27 | sean-k-mooney | ricolin: libvirt is never going to report a iommu model of auto or none | |
| 17:46:52 | sean-k-mooney | so you need to actully see what is reported form the domain caps api | |
| 17:47:35 | sean-k-mooney | by doing virsh domcapabilities --machine q35 --arch x86_64 | |
| 17:50:11 | sean-k-mooney | ricolin: but looking at that this is not somethign that is reported in that api | |
| 17:50:33 | sean-k-mooney | so instead of looking at the domaincap api you need to report the traits based on the libvirt version number | |
| 18:22:09 | melwitt | artom, sean-k-mooney: dunno if yall have seen this related preserve_on_delete bug from a few years ago https://bugs.launchpad.net/nova/+bug/1834463 | |
| 18:43:33 | opendevreview | Merged openstack/nova stable/ussuri: [stable-only] Make sdk broken job non voting until it is fixed https://review.opendev.org/c/openstack/nova/+/844309 | |
| 18:52:32 | ricolin | sean-k-mooney: so I need to check libvirt version before I put iommu in devices for fakelibvirt, right? | |
| 19:23:54 | artom | melwitt, hrmm, good find | |
| 19:43:24 | melwitt | artom: I looked through the code and saw that _heal_instance_info_cache preserves the existing value of preserve_on_delete. tried it out on devstack (created server with nova creating port, changed the value of preserve_on_delete to true in the database, saw _heal_instance_info_cache run a number of times, then detached the port) and it did not delete the port | |
| 19:49:24 | melwitt | I'm realizing the scenario in the above bug is different. they're saying they removed an interface by a manual database update, then nova added it back without (obviously) the original value of preserve_on_delete. I guess they are saying if they detach the port and then reattach it, they don't get the same value of preserve_on_delete. a bit different issue | |
| 19:56:45 | melwitt | although, they should get the same value bc if they reattach the port, nova won't consider it to be created by nova and thus should set preserve_on_delete = True | |
| 20:00:50 | sean-k-mooney | melwitt: they were updating the db | |
| 20:01:03 | sean-k-mooney | so really all bets are off at that point | |
| 20:01:24 | melwitt | just tried reattach and it indeed has preserve_on_delete = true. that means the bug report is very specifically the case where the interface gets removed from the info cache not via the API and then _heal_instance_info_cache runs. I don't know how that could happen during normal operation (no manual db update) | |
| 20:01:43 | sean-k-mooney | so there case does not make sense | |
| 20:01:47 | sean-k-mooney | well | |
| 20:02:04 | melwitt | yeah, I assumed they did the manual update to simplify a real world case but without any more data, I don't know how that case can happen | |
| 20:02:07 | sean-k-mooney | the booted with nova creating a nic | |
| 20:02:15 | sean-k-mooney | they somehow detached it without it gettting deleted | |
| 20:02:18 | sean-k-mooney | and then reattached it | |
| 20:02:41 | sean-k-mooney | so with the undocumented behavior when it got detach it shoudl have gotten delted | |
| 20:02:46 | sean-k-mooney | so there is not port to reattach | |
| 20:02:53 | melwitt | no, in their report they say they called server create with port_id passed in | |
| 20:03:12 | melwitt | so that means it begins with preserve_on_delete = true | |
| 20:03:15 | sean-k-mooney | oh then it shoudl have preserve on delete ture | |
| 20:03:23 | melwitt | yeah | |
| 20:03:37 | melwitt | no idea how what they say can happen "in real life" | |
| 20:04:01 | sean-k-mooney | so lest see | |
| 20:04:13 | sean-k-mooney | they are simulated the network info cache getting currpted | |
| 20:04:28 | sean-k-mooney | and then wating fo the heal taks to fix the info cache | |
| 20:04:39 | sean-k-mooney | and then nova things its created by it | |
| 20:05:00 | sean-k-mooney | i guess i can see that happeing if we lost the info of how the port was requested | |
| 20:05:28 | sean-k-mooney | so that is implying we sotre that in the info cache only | |
| 20:05:42 | melwitt | yeah, nova just sets the flag to true if it created the port, at port creation time. after that it's cache only | |
| 20:06:07 | sean-k-mooney | well thats broken | |
| 20:06:11 | sean-k-mooney | i guess we do that for attach | |
| 20:06:13 | sean-k-mooney | too | |
| 20:06:26 | sean-k-mooney | e.g. if we do attach network instead of attch port | |
| 20:07:53 | sean-k-mooney | we porably need to change this to sotre this in either the virtual interfaces tabel or instance_system_metadata if we want to avoid a db migration | |
| 20:08:30 | sean-k-mooney | the initall boot requeest would be stored in the request spec but we dont update that on network attach at least i dobt we do | |
| 20:09:25 | melwitt | it would be nice to save it somewhere... other than instance_info_caches if that table is apparently fraught with problems | |
| 20:11:47 | sean-k-mooney | well its ment to be a cache | |
| 20:11:52 | melwitt | request_spec seems like a good place? | |
| 20:12:00 | sean-k-mooney | as in we shoudl be able to drop it if we needed too | |
| 20:12:07 | melwitt | fair | |
| 20:12:16 | sean-k-mooney | request_sepc is in the api db | |
| 20:12:26 | melwitt | oh right :/ | |
| 20:12:48 | sean-k-mooney | so we could update teh requested networks in the api but we would have to wait till after the virt driver finsihed attaching | |
| 20:12:55 | sean-k-mooney | is this a call or a cast | |
| 20:13:06 | sean-k-mooney | i guess its a call | |
| 20:13:12 | sean-k-mooney | since its a 200 respone | |
| 20:13:19 | sean-k-mooney | https://docs.openstack.org/api-ref/compute/?expanded=add-network-detail%2Ccreate-interface-detail#create-interface= | |
| 20:13:25 | melwitt | yeah it's a call | |
| 20:13:54 | sean-k-mooney | so we coudl update teh request_spec network_requests list if we really wanted too | |
| 20:14:16 | melwitt | but nova-compute couldn't get to it without an upcall right | |
| 20:14:16 | sean-k-mooney | we just need to make sure to only do it if the call succeds | |
| 20:14:49 | sean-k-mooney | not via nova-comptue in the api | |
| 20:15:01 | sean-k-mooney | when we wait for the call | |
| 20:15:21 | sean-k-mooney | i think there are better places to store it however | |
| 20:15:22 | melwitt | if nova-compute needs to rebuild the info cache from nothing, like the db row update example in the bug | |
| 20:16:05 | sean-k-mooney | ya it should be able too | |
| 20:16:30 | sean-k-mooney | we have had cases where we lost ports in the cache due to buggy neturon backend or neutron policy issues | |
| 20:16:38 | sean-k-mooney | e.g. where neutorn returned an empty port list | |
| 20:16:44 | melwitt | nova-compute can't read it from request_specs without it being an upcall. am I missing something? | |
| 20:17:01 | sean-k-mooney | the heal logic will recreate the info cache entries form the neutron data if that happens | |
| 20:17:12 | sean-k-mooney | melwitt: correct it cant | |
| 20:17:32 | melwitt | so storing it in request spec doesn't help afaict | |
| 20:17:41 | sean-k-mooney | not really no | |
| 20:17:43 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/models.py#L784= | |
| 20:18:08 | sean-k-mooney | the virtual interfaces tabel shoudl store it but it has no feield we can abuse to store it without a db change | |
| 20:18:11 | melwitt | just saying it sounded like a good place to store it initially but if nova-compute can't read it, it doesn't solve this issue | |
| 20:18:47 | sean-k-mooney | instance_system_metadata can store it since it just a set of key value pairs | |
| 20:19:01 | sean-k-mooney | and thats in the cell db | |
| 20:19:07 | sean-k-mooney | so that is proably where i woudl stash it | |
| 20:19:09 | melwitt | yeah, that would work | |
| 20:20:02 | sean-k-mooney | so we jsut have the key be the <neutron port uuid>_preserve_on_delete | |
| 20:20:16 | sean-k-mooney | or store the list as a single key | |
| 20:20:42 | sean-k-mooney | that is proably better since its indexed by the instance_id anyway | |
| 20:20:56 | sean-k-mooney | it denormaises the db technially | |
| 20:21:45 | sean-k-mooney | but a preserve_on_delete_list key that we lookup by "select preserve_on_delete from instance_system_metadata where instance_id = xyz" | |
| 20:22:00 | sean-k-mooney | is much simpler to lookup | |
| 20:22:09 | sean-k-mooney | but either would work | |
| 21:10:21 | opendevreview | Merged openstack/placement stable/ussuri: Use 'functional-without-sample-db-tests' tox env for placement nova job https://review.opendev.org/c/openstack/placement/+/840773 | |
| 22:40:37 | opendevreview | melanie witt proposed openstack/nova stable/train: DNM Testing for ceph setup gate fail https://review.opendev.org/c/openstack/nova/+/844530 | |
| #openstack-nova - 2022-06-03 | |||
| 07:05:28 | gibi | o/ | |
| 07:27:25 | bauzas | \o | |
| 08:02:14 | opendevreview | Balazs Gibizer proposed openstack/nova master: Unparent PciDeviceSpec from PciAddressSpec https://review.opendev.org/c/openstack/nova/+/844491 | |
| 08:33:40 | gibi | elodilles, melwitt: what is the stable view on squashes? https://review.opendev.org/c/openstack/nova/+/843680/6#message-f9be680631a00541158243c55609a5804cc7777d | |
| 08:39:43 | gibi | stephenfin: do you have some context on https://review.opendev.org/c/openstack/hacking/+/816676 ? the hacking repo is green now with flake 4 but in nova we have a bunch of new findings. Was there any previous trial moving nova to the newer hacking? | |
| 08:42:14 | songwenping | bauzas:we donnot delete vgpu mdev when delete vm? | |
| 08:42:51 | sean-k-mooney | **cough** black **cough** | |
| 08:43:10 | sean-k-mooney | songwenping: ya our down stream qe may have also found that yesterday | |
| 08:43:18 | sean-k-mooney | songwenping: we are still confirming | |
| 08:44:11 | sean-k-mooney | songwenping: its unclear if something has changed recently i.e. if this only happens if you have mig mode enabled | |
| 08:44:24 | sean-k-mooney | or if this was always an oversight | |
| 08:44:27 | songwenping | sean-k-mooney: from this line https://github.com/openstack/nova/blob/8260979b71b29ce2666d37b3adc7c256482aa16d/nova/virt/libvirt/driver.py#L6485 we can reuse the mdev | |
| 08:44:42 | sean-k-mooney | songwenping: we should not be reusing the mdev | |