Earlier  
Posted Nick Remark
#openstack-nova - 2021-01-18
11:19:52 lyarwood gibi: oh it doesn't need to be 60 seconds between attempts
11:20:08 lyarwood gibi: our previous issues were caused with a 5 second retry or lower iirc
11:20:14 gibi lyarwood: it is 60sec now and still see one or two timeout in a tempest run
11:20:24 lyarwood gibi: not between requests
11:20:36 lyarwood gibi: it builds up, the first is 5 seconds
11:21:17 openstackgerrit sean mooney proposed openstack/nova-specs master: add port scoped numa affinity spec https://review.opendev.org/c/openstack/nova-specs/+/765901
11:21:30 kashyap lyarwood: gibi: So, from the libvirt ocmmit that introduced the said event, it says the following:
11:21:33 kashyap [quote]
11:21:35 kashyap Since we didn't opt to use one single event for device lifecycle for a
11:21:38 kashyap VM we are missing one last event if the device removal failed. This
11:21:40 kashyap event will be emitted once we asked to eject the device but for some
11:21:43 kashyap reason it is not possible.
11:21:45 kashyap [/quote]
11:22:38 lyarwood right, I've also looked at the code and it looks like it's just a direct synchronous failure with that call, IOW it doesn't come after a timeout.
11:22:46 kashyap Looking at the code, I don't see particularly any timeout: https://www.redhat.com/archives/libvir-list/2016-April/msg00196.html
11:22:50 kashyap Jinx
11:22:57 kashyap But let me confirm w/ Peter
11:23:26 gibi lyarwood: but then who the event base code can help compared to the current retry code. that retry also wait syncronosly for libvirt to return
11:23:48 gibi if the event also sync then waiting for the event is equivalent to wait for the detach call to return from libvirt
11:23:59 gibi or do I miss something?
11:26:26 gibi I can verify something from the logs. I can check if there is a case when the detach returns before and then the detach event arrives significantly later
11:26:38 gibi if there is such case then waiting for the event make sense
11:26:40 kashyap gibi: Can you give a link to your patch, again, please?
11:26:47 gibi https://review.opendev.org/c/openstack/nova/+/770246
11:26:52 kashyap Thx
11:33:46 kashyap gibi: lyarwood: So ... it indeed looks like a sync failure; danpb on #virt also says this: that event is issued when ACPI refuses the unplug explicitly.
11:34:00 kashyap gibi: lyarwood: But if the guest OS is just slow or not responding, we don't get that event.
11:34:22 gibi kashyap: so there is a case when we eventually need to time out an retry?
11:34:47 gibi the case when the guest OS did not respond
11:35:13 kashyap gibi: Yeah, looks liks so -- how else do we take into acount the the guest OS's slowness or unresponsiveness
11:35:42 gibi but we should not retry too soon to avoid interrupting an ongoing detach in libvirt as that caused our issue in detach in the first place
11:36:15 kashyap Yeah, exactly that too...
11:36:29 kashyap gibi: Also, wait:
11:36:56 kashyap gibi: DanPB reminds: "if you get that event I don't htink you want to retry - that's an unconditional failure IIUC"
11:38:11 gibi kashyap: so if we get the failure event then we fail, if we get timeout we retry. if we get a sync failure about device missing then we retry with persistent=False
11:39:47 Yumeng hi sean-k-mooney, gibi, bauzas. Thank you for the discussion on Jan 15 about vGPU spec. I agree sean's suggestion about host reboot case and  updated a new patch . Pls help to review at your convenience.Thanks! https://review.opendev.org/c/openstack/nova-specs/+/750116
11:40:04 lyarwood gibi: yeah sorry had to go afk
11:40:27 hemanth_n lyarwood: could you please review https://review.opendev.org/c/openstack/nova/+/761725 when you get some time
11:40:40 lyarwood hemanth_n: ack will do
11:40:47 kashyap gibi: How does the sync failure of the missing device manifest?
11:41:15 gibi kashyap: https://review.opendev.org/c/openstack/nova/+/770246/7/nova/virt/libvirt/driver.py#45
11:41:24 lyarwood kashyap: if the device has already been removed from the persistent config but remains in the live config
11:41:27 gibi we get a libvirtError with VIR_ERR_DEVICE_MISSING
11:41:50 gibi as far as I see there is no failed event in this case
11:42:18 lyarwood gibi: yeah I don't think libvirt gets as far as calling QEMU
11:42:29 lyarwood gibi: for QEMU to then emit the event back to libvirt etc
11:42:39 kashyap Thx, both
11:44:06 kashyap Ah, looks like VIR_ERR_DEVICE_MISSING was added in libvirt for Nova :)
11:44:13 gibi ooh :)
11:44:43 kashyap Otherwise, we were `grep`ing for error message; which is unreliable
11:45:04 kashyap Proper error code was due anyway; Nova just uncovered the bug
11:46:13 gibi ahh I know that. I thought there was some deeper reason. Yeas lyarwood just removed the grepping from nova as we have new enough libvirt to rely on the error code instead
11:50:46 gibi Logs look good. I see cases where the detach sequence is the following:
11:50:46 gibi * first attempt times out after 60sec waiting for the event from libvirt
11:50:46 gibi * second attempt fails synchronously with device not found
11:50:47 gibi * third attempt with persistent=False succeeds
11:51:11 gibi so I assume the first attempt only removed the device from the persistent config
11:51:15 gibi hence the second attempt fail
11:51:29 gibi then the third succeed as it does not try to remove it from the persistent config
11:53:08 lyarwood yeah the first should always remove it from the persistent config
11:53:19 lyarwood so you can call the second with persistent=False
11:53:23 lyarwood as we did previously
11:54:42 kashyap gibi: FWIW, I summarized the IRC chat here: https://review.opendev.org/c/openstack/nova/+/770246/7/nova/virt/libvirt/driver.py#2330
11:55:02 gibi lyarwood: still that first timeout seems wrong, why not we get an event
11:55:34 lyarwood gibi: I think it's a single event once detached from both the persistent and live config
11:55:53 gibi lyarwood: and we need to send two detach to have detach it from both config?
11:56:06 gibi if yes then it make sense
12:08:56 openstackgerrit Stephen Finucane proposed openstack/nova master: libvirt: Remove 'hypervisor_version' from 'libvirt_info' https://review.opendev.org/c/openstack/nova/+/744199
12:13:19 lyarwood gibi: one request can detach the device from both
12:13:40 lyarwood gibi: it just depends on the guestOS for the device to be detached from the live config
12:15:20 gibi lyarwood: so the first detach attempt triggers the detach from both the persistent and the live config. The detach call returns, but we timeout waiting for the event. So I have to assume that either a) we didn't wait long enough to get the event b) we _have to_ send two detach to get the device detached from the live config.
12:16:09 gibi or c) libvirt does not send the event in some case
12:16:22 gibi but that seems wrong
12:17:33 lyarwood right so for a and b we retry the detach
12:17:48 lyarwood we can even check if the device is in either config before doing that
12:18:02 lyarwood but it should always be gone from the persistent config at that point
12:18:21 lyarwood just to go back to the original blind retry issue
12:19:05 lyarwood the issue with that approach in CI was the time between actual detach calls to libvirt and then down into QEMU was tiny
12:19:35 lyarwood so much so that they would overlap and cause the undocumented QEMU behaviour
12:20:15 lyarwood with your approach I really think it's safe to wait for longer between attempts to detach now, so something like 20 to 30 seconds
12:20:41 lyarwood I know you're waiting for 60 at the moment but as I said before the previous behaviour was to start at 5 seconds and increase that with each iteration
12:22:21 sean-k-mooney Yumeng: +1 on the vgpu spec. https://review.opendev.org/c/openstack/nova-specs/+/750116
12:22:44 sean-k-mooney gibi: bauzas ^ if you have time to re review i think that is ready
12:22:48 gibi I still don't see the difference between the old and the new approach. In both code wait a predefined time before retry, and in both code this waiting is only happen if the first try fails, if the first try succeeds then no waiting happens. So I don't see why we are in the better situation with the new code
12:23:12 gibi sean-k-mooney: ack
12:23:30 sean-k-mooney gibi: didnt the old code retyr regardless of failure or success
12:23:45 sean-k-mooney i tought it just started teh time out and retired without waiting
12:23:49 gibi sean-k-mooney: the old case also checked the existence of the device in the live config before retry
12:23:50 sean-k-mooney for an event
12:24:13 sean-k-mooney gibi: right but that is not a vaild check
12:24:28 gibi sean-k-mooney: that is what we do both in the old and the new code
12:24:35 sean-k-mooney the issue is that with new qemu issuing a second detach cancles the operation
12:24:56 gibi sean-k-mooney: but it seems waiting for the event times out
12:24:57 sean-k-mooney so it may still be in the live domain but that does not mean you can send a second detach
12:25:09 lyarwood right, the main thing I wanted here was to allow n-cpu to wait longer between requests to libvirt to detach
12:25:45 gibi sean-k-mooney: it seems that the libvirt detach call returns at the same time when the detach event is sent, so the whole thing seems synchronous
12:25:50 lyarwood thus making it harder if not impossible for us to hit the weird undocumented QEMU behaviour
12:26:07 gibi sean-k-mooney: but then I don't know how we was able to step on the first detach handling with the retry
12:26:52 sean-k-mooney gibi: i think that depends on the load
12:26:53 gibi lyarwood: but then finding the right timeout is the goal both in the new and the old code. but for that we don't have to blow up the old code

Earlier   Later