Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-12-19
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 openstack Launchpad bug 1784579 in OpenStack Compute (nova) queens "unable to live migrate instance after update to queens" [Medium,Confirmed]
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: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: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: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 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:16:46 mriedem sean-k-mooney: i just did 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: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: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.
15:21:36 jaypipes sean-k-mooney: :P
15:22:13 sean-k-mooney jaypipes: it is currently more functional then cyborge's pci passthough support at least ...
15:22:19 mriedem stephenfin: so no i didn't fix the original reason for the port binding failure in pre_live_migration, because that could have been for any number of reasons (neutron agent was down on the dest host?)
15:22:38 mriedem i fixed a symptom of that failure, which was nova-compute failed to restart after that failure
15:22:54 mriedem     to recover / fix the instance networking info"
15:22:54 mriedem as the commit message says, "Admittedly this isn't the smartest thing and doesn't attempt
15:22:59 stephenfin mriedem: I'm missing something. Why make changes to 'ComputeManager.init_host' (via '_init_instance') in that commit? The exception was being seen in the live migration flow
15:23:01 stephenfin ahhhhh
15:23:21 mriedem 1. live migratoin fails, port binding failed - that gets saved in the info cache
15:23:32 mriedem 2. restart source compute - that blows up because it wasn't handling binding_failed vif types in the os-vif conversion code
15:23:39 mriedem i handle #2
15:23:46 mriedem #1 is sort of out of my control

Earlier   Later