Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-01-18
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
12:27:06 sean-k-mooney i dont think it is synconos jsut look that way with light load
12:27:19 sean-k-mooney gibi: you could simulate that with cgroups/taskset
12:27:33 lyarwood gibi: so the old code is also a mess
12:27:33 gibi sean-k-mooney: above kashyap and lyarwood said it looks synchronous
12:27:36 sean-k-mooney you could limit the execution time of libvirtd
12:27:51 gibi lyarwood: fair the old code is a mess and can be factored to be cleaner
12:28:01 sean-k-mooney gibi: i see havent looked at ti too closely
12:28:12 lyarwood gibi: and about things being sync or async, I was talking about the failure event
12:28:28 gibi lyarwood: ohh, I missed that then
12:28:37 sean-k-mooney so the even is asyc right

Earlier   Later