Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-12-19
14:45:47 mriedem done
14:46:28 sean-k-mooney mriedem: that occurred to me as well but i was not sure how involved that would be. e.g. updating the db from the deleted value. also not sure how safe it would be in some cases such as if the divece was chaged
14:46:40 sean-k-mooney ill link it in the bug for context too
14:46:54 mriedem there is also https://bugs.launchpad.net/nova/+bug/1633120
14:46:54 openstack Launchpad bug 1633120 in OpenStack Compute (nova) "Nova scheduler tries to assign an already-in-use SRIOV QAT VF to a new instance" [Undecided,Confirmed]
14:46:58 mriedem all the same issue,
14:47:07 mriedem change pci whitelist and restart nova compute blows away allocated pci devices
14:48:03 sean-k-mooney you know when we start tracking these device in placement we are not going to be able to just blow away the resouce provider anymore if there are allocation against it
14:48:20 mriedem that was my reply on mgariepy's bug,
14:48:48 mriedem long-term when pci device inventory and allocation is in placement, this shouldn't be a problem,
14:49:05 mriedem because even if nova-compute tried to remove device inventory, if it's in-use by a consumer (instance) the inventory delete request will fail with a 409
14:49:44 mriedem where is the code that removes / deletes devices on restart of nova-compute? shouldn't that be smarter and not delete those devices which are assigned to an instance?
14:50:17 mriedem as we can see in https://paste.ubuntu.com/p/Pn76QVmwqr/ pci device 8 is allocated to an instance but was deleted anyway
14:50:21 sean-k-mooney mriedem: well we will still need to resouce track to handel the pci device assingment aspect and we will need to be able to coralt it back to the rp
14:50:55 mriedem so in 4 years when pci devices are handled with placement, this will not suck as bad anymore yeah?
14:51:10 mriedem can't we just not delete allocated pci devices?
14:51:13 sean-k-mooney mriedem: actuly i think it will suck more
14:51:21 mriedem obviously that is some kind of referential constraint
14:52:12 sean-k-mooney we could delay deleting the entry until it is deallcoated
14:52:40 sean-k-mooney if the admin change the whitelist to prevent a device form being used we would still want to stop new instaces form using it
14:52:57 sean-k-mooney but it would be resoable to assume that an existing instance could continue to use it
14:53:35 mriedem i don't know how you delay that
14:54:02 mriedem without new logic / data modeling on the pci device record to say, "pending delete" or something once it's no longer allocated
14:54:09 mriedem but by then you'd have to see if it's back in the pci whitelist
14:54:29 sean-k-mooney this is where we figure out the available devices https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L119
14:54:35 mriedem i would think a simple solution is fail nova-compute restart if you tried scrubbing an entry from the whitelist for an allocated devie
14:54:46 mriedem jaypipes: have you ever heard of this craziness? ^
14:55:59 mriedem jaypipes: if you change the pci whitelist config to remove an already allocated device, and restart nova-compute, compute deletes the pci device record that is still allocated to an instance
14:56:14 mriedem if you then add it back to the whitelist and restart, nova-compute creates a new 'available' pci device record and the scheduler can try to use it
14:56:15 sean-k-mooney so we could just change the if form self.dev_filter.device_assignable(dev) to self.dev_filter.device_assignable(dev) or is_allocated(deve) where is allocated is a new fuction
14:56:22 mriedem which the hypervisor will reject
14:56:30 mriedem you can also lose the old device assigned to the old instance if you reboot the instance
14:57:26 mriedem sean-k-mooney: maybe? i don't know what devices_json is
14:57:32 mriedem is that from the db or the config?
14:58:41 mriedem god even finding the object code to see where pci device records are deleted is hard
14:59:04 mriedem oh it's in save() i should have known!
14:59:45 mriedem https://github.com/openstack/nova/blob/master/nova/objects/pci_device.py#L244
14:59:56 sean-k-mooney so this is suspicious https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L166-L181
15:01:08 mriedem idk, seems like a very easy solution to avoid screwing up the db state would just be raise an exception here https://github.com/openstack/nova/blob/master/nova/objects/pci_device.py#L244 if self.instance_uuid is not None
15:01:17 mriedem i.e. you can't remove/delete an allocated pci device
15:02:31 sean-k-mooney yes but it looks like https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L166-L181 was maybe working around that
15:04:02 mriedem that was added 6 years ago https://github.com/openstack/nova/commit/4855239497050c9ee03fed627c5f41d6b59eddc6
15:04:05 mriedem and clearly it sucks
15:04:26 gibi mriedem: FYI my wall of text as a review guide for the bandwidth series http://lists.openstack.org/pipermail/openstack-discuss/2018-December/001129.html
15:04:37 sean-k-mooney yes im reading the original commit curently
15:05:35 mriedem gibi: ack thanks
15:06:56 sean-k-mooney mriedem: if we simply dont do existed.status = 'removed' with "continue" in the excpet clause that may be enough.
15:07:24 sean-k-mooney mriedem: i lean more and more to its working how its was intended but what was intended is dumb and we shoudl fix it
15:07:45 mriedem mgariepy: did you see this warning in the nova-compute logs? https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L169
15:08:41 mriedem sean-k-mooney: yeah i don't understand that logic at all
15:08:55 mriedem i guess it's saying the hypervisor is no longer reporting that device so we need to forcefully remove it/
15:08:56 mriedem ?
15:09:26 sean-k-mooney mriedem: the orginal code comment was lost at some point from the set_hvdevs fucntion
15:09:30 sean-k-mooney "Devices should not be hot-plugged when assigned to a guest,
15:09:32 sean-k-mooney but possibly the hypervisor has no such guarantee. The best
15:09:34 sean-k-mooney we can do is to give a warning if a device is changed
15:09:36 sean-k-mooney or removed while assigned.
15:09:38 sean-k-mooney "
15:09:44 mriedem oh but the new_devs are filtered through the whitelist
15:09:46 mriedem in device_assignable
15:09:53 mriedem https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L120
15:10:08 mriedem so it's not that the hypervisor is no longer reporting the devices, it's that the whitelist changed
15:10:14 mriedem which is exactly the bug
15:10:15 sean-k-mooney yes
15:10:59 sean-k-mooney so if we change the filtering to allow allocated deivce in addtion to the whitelist it will be fine
15:11:49 sean-k-mooney when the device is deallocated form the guest it will be removed from the aviable device on the next periodic sync
15:11:59 sean-k-mooney as it si nolonger in the whitelist or allocated
15:12:53 stephenfin mriedem: Not to distract you now, but did you make a mistake on https://github.com/openstack/nova/commit/cdf8ba5acb ? You've said it fixes https://bugs.launchpad.net/nova/+bug/1784579 but that bug is for live migration, not compute service restart which is what your commit addresses
15:12:53 openstack Launchpad bug 1784579 in OpenStack Compute (nova) queens "unable to live migrate instance after update to queens" [Medium,Confirmed]
15:13:19 stephenfin mriedem: I ask because I found a similar bug which does deal with the compute service restart https://bugs.launchpad.net/nova/+bug/1738373
15:13:19 openstack Launchpad bug 1738373 in OpenStack Compute (nova) "nova-compute cannot restart if _init_host failed" [Undecided,In progress] - Assigned to Xiao Gong (gongxiao)
15:13:24 mgariepy mriedem, https://paste.ubuntu.com/p/GVJQqMSTrM/
15:13:37 mgariepy yes i did saw the warning.
15:14:49 mriedem mgariepy: yup and that matches the instance uuid in https://paste.ubuntu.com/p/Pn76QVmwqr/
15:14:53 mriedem for the deleted pci device
15:15:31 sean-k-mooney mriedem: mgariepy ill write this up in the bug and i might go fix it. that said im going on vaction today so i might not get to it untill january
15:15:46 sean-k-mooney if other want to work on it in the interim feel free.
15:16:46 mriedem sean-k-mooney: i just did https://bugs.launchpad.net/nova/+bug/1633120
15:16:46 openstack Launchpad bug 1633120 in OpenStack Compute (nova) "Nova scheduler tries to assign an already-in-use SRIOV QAT VF to a new instance" [Undecided,Confirmed]
15:17:29 sean-k-mooney mriedem: ok in that case ill close https://bugs.launchpad.net/nova/+bug/1809040 as a duplicate of https://bugs.launchpad.net/nova/+bug/1633120
15:17:29 openstack Launchpad bug 1633120 in OpenStack Compute (nova) "duplicate for #1809040 Nova scheduler tries to assign an already-in-use SRIOV QAT VF to a new instance" [Undecided,Confirmed]
15:17:57 mriedem sean-k-mooney: already did
15:18:07 mriedem stephenfin: yes bug 1784579 is about os-vif port binding failed errors right?
15:18:08 openstack bug 1784579 in OpenStack Compute (nova) queens "unable to live migrate instance after update to queens" [Medium,Confirmed] https://launchpad.net/bugs/1784579
15:18:20 jaypipes mriedem: apologies. internet down for last 15 minutes in Sarasota... last thing I got from you was "is that from the db or the config"
15:18:29 sean-k-mooney mriedem: in that case ill pay sean-k-mooney 1 million dollars :)
15:19:00 mriedem jaypipes: oh just this terrible code https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L177
15:19:14 stephenfin mriedem: Yup, but it's to do with live migration and the fix is only for the service startup code path
15:19:18 mriedem sean-k-mooney: if you want, throw up a patch that changes ^ to a continue, fix whatever test hits that and i'll +2
15:19:40 stephenfin At least, assuming I'm reading it right. I'll do some digging but just wanted to sanity check it before I dived down the rabbit hole :)
15:19:56 mriedem stephenfin: the live migratoin fails because of the port binding failures
15:20:06 jaypipes mriedem: I think you meant this code: https://github.com/openstack/nova/blob/a1dba961f0018a4995d208a290f4a859ce295840/nova/pci/manager.py#L1-L357
15:20:22 mriedem i see what you did there
15:20:28 jaypipes u like that?
15:21:03 sean-k-mooney jaypipes: your not wronge
15:21:15 mriedem stephenfin: comment
15:21:16 mriedem 2
15:21:17 mriedem "To summarize, it looks like the pre_live_migration method on the destination host fails to plug vifs and you end up with the "binding_failed" error, which is raised and makes the source live_migration method fail as expected. The failure is on the dest host. As a result, the info cache is updated with "binding_failed" which causes the source compute restart to fail here:"
15:21:23 jaypipes sean-k-mooney: but you are.
15:21:30 jaypipes wronge that is.

Earlier   Later