| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-02-07 | |||
| 12:08:38 | opendevreview | Takashi Kajinami proposed openstack/nova-specs master: libvirt: Statless firmware support https://review.opendev.org/c/openstack/nova-specs/+/908297 | |
| 12:24:26 | greatgatsby_ | Hello. Is it possible to modify OS-EXT-SVR-ATTR:host via the nova command or OSC if it's incorrect (after compute crash)? Trying to avoid modifying via the database. | |
| 12:36:21 | bauzas | whoami-rajat: frickler: yeah was at lunch, correct this is nova-lvm jobn | |
| 13:26:39 | opendevreview | Sylvain Bauza proposed openstack/nova master: Fix verifying all the alloc requests from a multi-create https://review.opendev.org/c/openstack/nova/+/846786 | |
| 13:54:46 | opendevreview | Takashi Kajinami proposed openstack/nova-specs master: libvirt: AMD SEV-ES support https://review.opendev.org/c/openstack/nova-specs/+/907702 | |
| 14:01:47 | opendevreview | Takashi Kajinami proposed openstack/nova-specs master: libvirt: Statless firmware support https://review.opendev.org/c/openstack/nova-specs/+/908297 | |
| 14:02:28 | opendevreview | Takashi Kajinami proposed openstack/nova-specs master: libvirt: AMD SEV-ES support https://review.opendev.org/c/openstack/nova-specs/+/907702 | |
| 14:03:45 | opendevreview | Takashi Kajinami proposed openstack/nova-specs master: libvirt: Statless firmware support https://review.opendev.org/c/openstack/nova-specs/+/908297 | |
| 14:44:30 | opendevreview | Takashi Kajinami proposed openstack/nova-specs master: libvirt: Stateless firmware support https://review.opendev.org/c/openstack/nova-specs/+/908297 | |
| 16:09:23 | bauzas | dansmith: saw your comment on https://review.opendev.org/c/openstack/nova/+/904209/14/nova/virt/libvirt/driver.py#9838 | |
| 16:09:43 | bauzas | dansmith: you're right, we could have problems, that's why I wrote this in the spec : | |
| 16:10:17 | bauzas | "will persist that list of target mediated devices in some internal dictionary field of the LibvirtDriver instance, keyed by the instance UUID. " https://specs.openstack.org/openstack/nova-specs/specs/2024.1/approved/libvirt-mdev-live-migrate.html#proposed-change | |
| 16:10:51 | bauzas | but yeah, in case the operator restart the compute service, then the dict could be wrong | |
| 16:11:03 | bauzas | or the other way if we have a rabbit issue | |
| 16:11:11 | bauzas | so I don't know what to say here | |
| 16:11:40 | bauzas | that's why we discussed this in the PTG, we said we will try to remember those by a dict | |
| 16:12:49 | bauzas | https://etherpad.opendev.org/p/nova-caracal-ptg#L585 | |
| 16:13:04 | bauzas | anyway, let's dicuss this after our meeting | |
| 16:19:26 | zigo | It felt super fast in the host, not so much in the VMs... | |
| 16:23:17 | dansmith | bauzas: yeah, and L576 says "Dan Smith not very convinced" :) | |
| 16:23:40 | dansmith | so I think this is well in bounds for being concerned now that we're looking at the implementation | |
| 16:24:06 | bauzas | dansmith: let's find an alternative after our meeting :) | |
| 16:24:14 | bauzas | let's try* to find (tbc) | |
| 16:24:15 | dansmith | I'm certainly not trying to cause you trouble, I'm just expressing concern | |
| 16:24:19 | dansmith | ack | |
| 17:09:08 | bauzas | dansmith: I'm swamped in another meeting but my point is that I wonder if we should persist the internal dict | |
| 17:09:42 | bauzas | in case of a rabbit issue, people just restart their computes | |
| 17:10:09 | bauzas | that said, if we have a current live-migration, it would be eventually stopped, right? | |
| 17:10:13 | bauzas | or not ? | |
| 17:10:13 | dansmith | right, but if they do, they leak resources in placement right? | |
| 17:10:59 | bauzas | because of the target allocations ? | |
| 17:11:28 | dansmith | yeah | |
| 17:40:40 | bauzas | dansmith: sorry was in a meeting | |
| 17:41:11 | bauzas | so, what I can do is to provide some logs, for sure | |
| 17:42:41 | bauzas | then, I'll try to test with my hardware environment what would arrive if the operator tries to restart a compute while a live-migration is currently running | |
| 17:43:37 | bauzas | for the allocations, it would be like any other feature | |
| 17:54:23 | dansmith | bauzas: what if we had the compute service look for and clean up any allocations against its RP for instances that don't exist on it? basically "things that were reserved here, but are gone now"? | |
| 17:54:27 | dansmith | maybe we already do that and I'm missing it? | |
| 17:54:48 | dansmith | but if so, then a restart would *actually* clean up everything because the in-memory bit would be dumped and we'd clean up the stale reservations | |
| 17:54:52 | bauzas | dansmith: there are two things | |
| 17:55:17 | bauzas | there are VGPU allocations and there are mdev "reservations" | |
| 17:56:02 | bauzas | the internal dict is just here for making sure we don't pass a mdev to a new instance if that mdev is currently used by a live-migrating instance | |
| 17:56:14 | bauzas | so we just 'reserve' it | |
| 17:57:00 | bauzas | that said, and that's something I can verify, if the tarrget domain is created just when we start the live-migration, then we don't need to 'reserve' the mdev | |
| 17:58:16 | bauzas | maybe the context you miss is that a compute only knows which mdevs are used by instances by looking at the guest XMLs | |
| 17:59:18 | bauzas | my concern I had is that if we don't have a domain for a target guest yet while live-migrating, then the compute would not know that the related mdevs are currently used for that one | |
| 17:59:29 | bauzas | hence the internal dict | |
| 18:09:22 | dansmith | bauzas: thanks, I understand the difference between the dict reservation and the allocations | |
| 18:09:36 | dansmith | however, the allocations are not cleaned up by a restart (right?) even though the dict reservations are | |
| 18:09:52 | dansmith | I don't really like the latter requiring a restart, but the former seems very unfortunate to me | |
| 18:10:12 | bauzas | that's the same problem with any live-migration allocation, right? | |
| 18:11:55 | bauzas | dansmith: one question I have and that you could maybe know, are we providing an error for running live-migrations if we restart a target ? | |
| 18:13:00 | dansmith | bauzas: It seems to me like this is the first place where we're doing that allocation in the pre check no? | |
| 18:13:16 | dansmith | we grab pci stuff, but I don't think we persist it until later, if I'm reading correctly | |
| 18:13:56 | bauzas | I don't know *when* we create a target allocation | |
| 18:14:00 | bauzas | I need to look at the code | |
| 18:16:28 | bauzas | https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L558 | |
| 18:16:39 | dansmith | okay I just found that | |
| 18:16:58 | dansmith | so the conductor nukes any allocations that we have against the proposed node if we abort, yeah? | |
| 18:17:11 | bauzas | after pre-livemigrate sure | |
| 18:17:26 | bauzas | but then I wonder if we delete the allocations when we monitor the running live-migration | |
| 18:17:33 | bauzas | looking at the compute now | |
| 18:17:57 | dansmith | yeah, that's only if we decide the destination isn't a good fit, but doesn't address the failure later | |
| 18:20:18 | bauzas | https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L558 | |
| 18:20:30 | bauzas | here we error a migration if we have any exception | |
| 18:22:19 | dansmith | right, but after we start the migration, nothing will clean up the destination node's allocations right? | |
| 18:22:33 | bauzas | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L9692 | |
| 18:22:37 | bauzas | nah, found it | |
| 18:22:53 | bauzas | we delete the allocation when calling _rollback_live_migration() | |
| 18:23:26 | bauzas | and then we call the drivers's methods for cleaning up the residues... which then unreserve the mdev from the dict :) | |
| 18:23:39 | bauzas | so we're good | |
| 18:24:12 | bauzas | but I agree on adding more logs | |
| 18:24:15 | dansmith | the dict is on the destination, right/ | |
| 18:24:21 | dansmith | so we don't clean that up | |
| 18:24:29 | bauzas | I didn't done that in the past with allocating mdevs and that was needed | |
| 18:24:46 | bauzas | dansmith: no, we both call source and dest virt drivers | |
| 18:24:58 | bauzas | source is here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L9708-L9709 | |
| 18:25:03 | dansmith | bauzas: not if rabbit is down | |
| 18:25:10 | bauzas | and dest is there https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L9736-L9738 | |
| 18:25:25 | bauzas | dansmith: hah, I get your point now | |
| 18:26:04 | dansmith | just saying it seems like we need more post-disaster recovery bits, be it on compute restart or some periodic | |
| 18:26:18 | bauzas | there could be a situation with rabbit down that would hit the live-migration but we wouldn't unreserve the mdev | |
| 18:26:21 | bauzas | gotcha | |
| 18:26:24 | dansmith | but, given that we do clean these allocations if rabbit is not down and we fail, then fair enough, | |
| 18:26:58 | dansmith | but I think the logging of the internal private data that won't be reset until restart is important, at least so it's discoverable why something is reserved but there's no record of it | |
| 18:27:17 | bauzas | yeah good point | |
| 18:28:06 | bauzas | my kids are starving, so I need to leave or they will run errand in the street like zombies | |
| 18:28:23 | bauzas | but I'll try to be a bit more resilient | |
| 18:28:34 | bauzas | at least the first thing is to log | |
| 18:28:46 | bauzas | then I wonder whether we need some periodic for the cleaning case | |
| 18:29:18 | bauzas | (but I just wonder which conditional to write in that periodic :) ) | |
| 18:29:51 | bauzas | that should be "give me all the migrations that were errored and make sure we don't have a reserved mdev for those" | |
| 18:40:13 | dansmith | something like that | |
| 18:40:43 | sean-k-mooney | melwitt: i think i need to restack but i got https://paste.opendev.org/show/bRUFWOFqOa8kSeXyH12I/ when i tried to delete a vm | |
| 18:42:26 | melwitt | sean-k-mooney: oh, ok that's bc I added a db migration (that I forgot to mention) to add a column (for backing file secret uuid). so you'd have to nova-manage db sync. it's complaining about the lack of the column | |
| 18:43:02 | sean-k-mooney | oh ok | |
| 18:43:06 | sean-k-mooney | i just did a git checkout | |
| 18:43:19 | sean-k-mooney | and restarted things so ya di cna do a db sync | |
| 18:43:35 | melwitt | yeah, I did the same thing. just forgot about the db sync, sorry 😑 | |
| 18:43:58 | sean-k-mooney | i was really confused becasue the way this fails in the openstack client is really terrible | |