Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-09-13
08:14:10 opendevreview Sylvain Bauza proposed openstack/nova master: Add Dalmatian prelude section https://review.opendev.org/c/openstack/nova/+/928995
08:26:38 bauzas gibi: I have full attention to the regression bug now
08:26:54 bauzas so, that sounds the conditional clause isn't enough
08:27:18 bauzas something in the cleanup really deletes the instance path
08:29:58 gibi I just published a set of observations in https://review.opendev.org/c/openstack/nova/+/928970
08:32:12 gibi If I have to guess the complication is around: > NUMA pinned VMs and an NFS mount for the VMs, but we also use cinder boot volumes
08:33:07 gibi I guess we won't be able to avoid setting up a multinode devstack..
08:36:28 bauzas I tried yesterday to write a functional test but that was hard
08:37:00 bauzas I would have to create a specific tempfs for the instance paths in order to correctly reproduce it
08:37:19 bauzas but I failed finding where we mock the paths
08:37:31 bauzas ++
08:37:43 bauzas (oops, wrong window)
08:40:15 gibi I will try to set up multinode devstack with NFS but I cannot promise it will work today
08:41:18 bauzas I have two nodes, I can also try to modify them
08:41:42 bauzas either way, we'll probably need to come to a decision today or monday
08:41:58 bauzas we can't delay RC1 more
08:42:24 bauzas but I'd like some core quorum
08:43:18 gibi an alternative is to revert the power mgmt live migration fix. Declare that that power mgmt does not work with live migration and fix the whole thing later
08:44:07 bauzas that's what I had in mind
08:44:26 bauzas or a third alternative is ship power management but let the call be optional
08:44:46 bauzas if you don't setup power management, you shouldn't be impacted
08:44:49 gibi we could hack a fix together where shared storage live migration works if power mgmt is not used, and power mgmt live migration works if no shared storage is used (basically the original proposal from the reported) but that is uggly
08:45:14 bauzas that's the concern I have with the current master, which inconditionnally says you're power manageable
08:45:14 gibi and power mgmt with shared storage will still not work
08:45:44 bauzas I wish we had a flag on the migrate_data object that would signal that info
08:45:53 bauzas like we did for mdevs or vpmems
08:46:27 gibi that would not remove the problem that shared storage live migration does not work if you have power mgmt, mdevs, or vpmems.
08:46:37 gibi as those would trigger calling clenaup
08:46:38 bauzas yup, but that's something we can doc
08:47:04 bauzas here the problem is that shared storage is broken *anyway*
08:47:20 bauzas and you can't opt out from it
08:48:28 gibi yepp it seem shared storage live migration only works we we don't call cleanup
08:48:48 gibi but I'm not 100% sure whw
08:48:50 gibi why
08:49:27 gibi do you have any comment on my las comment https://review.opendev.org/c/openstack/nova/+/928970/comment/c28b9ea3_30ed15a5/
08:49:58 gibi especially around deleting the instance dir when the VM is volume backed?
08:51:50 bauzas even if the instance is volume backed, we still have an instance dir, right?
08:51:55 bauzas we just don't have disks
08:53:42 bauzas I could imagine that they have NFS on /var/lib/instances
08:53:57 bauzas so the BFV instances are present on that shared path
08:54:22 bauzas in that case,
08:54:25 bauzas cleanup_instance_dir = migrate_data.is_shared_block_storage
08:54:36 bauzas it will cleanup the dir, right ?
08:55:01 bauzas oh wait no
08:55:17 bauzas that's not shared storage, that's shared RBD image backend
08:55:44 gibi the instance dir for a volume booted instance (without config drive) only holds the console.log fiel nothing else
08:56:22 bauzas I don't disagree, but that's still an instance dir
08:56:23 gibi is_shared_block_storage will be true if the instance is booted from volume
08:56:44 gibi (and in other cases too)
08:56:54 bauzas okay, so that's why this is deleting the instance dir
08:56:59 gibi Is it a problem if we delete hte instance dir
08:57:03 gibi ?
08:57:08 bauzas good question
08:57:26 bauzas as you said, the instance dir is quite empty
08:57:38 bauzas where is stored the domain definition ?
09:01:51 gibi that is nova does not store the xml
09:02:21 gibi * nova does not store the xml
09:02:30 gibi I'm not sure libvirt stores it by default
09:03:21 gibi it is under /run/libvirt/qemu/instance-00000002.xml
09:03:31 gibi in devstack
09:04:36 bauzas okay
09:05:44 bauzas your question is actually good : does it harm if we delete an instance dir from a BFV instance that's shared ?
09:06:09 bauzas https://github.com/openstack/nova/blob/16d815990b53b9afa35fc4da38609f87820fa690/nova/virt/libvirt/driver.py#L1708-L1711
09:06:24 bauzas I enjoy mdbooth's comment : "I'm pretty sure this is wrong"
09:07:24 bauzas from what I read from both comments is that this is EXPECTED that we delete the instance dir if the instance is BFV
09:07:44 bauzas not sure we do support shared storage with BFV
09:07:50 bauzas see ?
09:12:34 gibi I see
09:13:14 gibi I'm not sure wht mdbooth's comment really mean. I.e. does the comment really implies that the logic is wrong and we should change it?
09:16:19 gibi we need to figure out if live migration on shared storage with a volume backed VM actually fails if the instance dir is deleted
09:16:34 gibi we never seen the real failure from the reporter
09:16:45 gibi they just stated that the dir is deleted
09:17:06 gibi the only thing they said is
09:17:11 gibi "making the VM unusable for future operations."
09:17:36 gibi I need to drop for an hour but will continue from here
09:17:48 bauzas 🤷‍
09:18:38 bauzas sure, let's continue to discuss this later once more people are here
09:19:21 gibi one thing we can suggest is to only delete the instance dir if the instance is volume backed but the instance path is not shared
09:19:45 gibi I can amend the my suggested patch with that logic
09:19:49 gibi and they can test
09:31:17 bauzas yeah, seems a good point
09:31:41 bauzas but this would revert the previous logic :)
10:01:44 elinux any one available here ?
10:08:30 elinux Anyone online
10:08:36 supamatt o/
10:41:58 frickler stephenfin: sean-k-mooney: https://review.opendev.org/c/openstack/python-openstackclient/+/929236 should fix the second part of the evacuate command, not sure about the microversion thing yet
10:45:38 sean-k-mooney frickler so i think this is an sdk regression?
10:46:53 sean-k-mooney i guess it was intorduced with https://github.com/openstack/python-openstackclient/commit/e6dc0f39c0891ea551867b77663a463b5f76987c perhaps
10:48:03 sean-k-mooney frickler: the actul password handeling code has not changed in over a year
10:48:57 sean-k-mooney but the args are now being passed to an sdk client instead of nova client
10:50:00 opendevreview Matthew Heler proposed openstack/nova master: Fix regression with live migration on shared storage https://review.opendev.org/c/openstack/nova/+/928970
10:50:37 sean-k-mooney frickler: the microversion we used is https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id62
10:50:47 sean-k-mooney which removed foraced evacuates
10:51:44 supamatt bauzas the latest patch I posted for 928970 works with shared storage now
10:52:53 sean-k-mooney frickler: the password is adminPass
10:53:00 sean-k-mooney in the rest api
10:54:31 opendevreview Matthew Heler proposed openstack/nova master: Fix regression with live migration on shared storage https://review.opendev.org/c/openstack/nova/+/928970
10:59:02 gibi supamatt: nice, I think that was what I imagined above by "one thing we can suggest is to only delete the instance dir if the instance is volume backed but the instance path is not shared"
10:59:57 gibi supamatt: so the patch you posted proves my theory.
11:00:38 gibi lets wait for bauzas as well but I think we can make your patch mergeable by adding some unti tests around it and a release notes

Earlier   Later