Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-01-18
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 gibi sean-k-mooney: above kashyap and lyarwood said it looks synchronous
12:27:33 lyarwood gibi: so the old code is also a mess
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
12:28:38 lyarwood gibi: I think the success event can be async
12:28:39 gibi lyarwood: so the happy case still can be async?
12:28:45 sean-k-mooney but on a fast host it will be almost instant
12:28:49 lyarwood right
12:28:50 gibi cool, then the event waiting code is better
12:29:14 lyarwood right that allows us to wait longer
12:29:31 lyarwood as we should get an event back when the device does eventually detach
12:30:40 gibi I will do more test to see why we don't get any event back from libvirt at the first detach attempt
12:30:45 gibi in the above scenario
12:30:56 gibi as that missing event is bothering me
12:31:00 sean-k-mooney gibi: you could try doing "sudo cpulimit -l 1 -e libvirtd" or "sudo cpulimit -l 1 -p <libvirtd pid>"
12:31:09 gibi sean-k-mooney: thanks I will try
12:31:26 sean-k-mooney that should limit libvirt to 1% of 1 core
12:33:43 lyarwood gibi: we don't get the event as the device is still attached to the live config
12:34:06 lyarwood gibi: and failure is for sync failures where QEMU rejects the request to detach
12:34:07 gibi lyarwood: but we expect that libvirt will detach from the live config as we asked for it
12:34:17 gibi in the first request
12:34:19 lyarwood gibi: we expect that it should try
12:34:32 gibi and if fails then it should tell us it failed, isn't it?
12:35:04 kashyap gibi: lyarwood: So, Peter Krempa came back; let me post the chat summary snippet here, from #virt, OFTC:
12:35:05 lyarwood gibi: no, again the failure event is for when QEMU tells libvirt that it can't detach the device when the request is initially made
12:35:36 lyarwood gibi: if that request is accepted but the guestos doesn't allow the device to detach then we don't get anything back
12:35:49 lyarwood that's my understanding at least, lets see what kashyap has
12:36:50 kashyap lyarwood: Yeah; now I hear some additional subtleties; I'm going to post the full summary on the change; and we all can read and discuss here
12:48:20 kashyap gibi: lyarwood: Done; see here (I also like DanPB's advice on dealin w/ active vs. inactive config separately): https://review.opendev.org/c/openstack/nova/+/770246/7/nova/virt/libvirt/driver.py#2330
12:51:29 kashyap Let me know if that answers your question; if not, please note there, if anything doesn't add up :-)
12:51:34 gibi sorry, I'm on a call
12:51:53 kashyap No rush at all
13:00:34 gibi kashyap: thanks for the summary, I will work DanPB's suggestion into the patch as handling the live an the persistent config separately feels cleaner to me too
13:03:01 kashyap Np
13:22:21 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION and NEXT_MIN_{LIBVIRT,QEMU}_VERSION https://review.opendev.org/c/openstack/nova/+/754700
13:22:21 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Drop support for Xen https://review.opendev.org/c/openstack/nova/+/743231
13:43:36 openstackgerrit Brin Zhang proposed openstack/nova master: Nova supports password encrypted VNC https://review.opendev.org/c/openstack/nova/+/622336
14:24:40 stephenfin sean-k-mooney: Can you fix the failures on https://review.opendev.org/c/openstack/nova-specs/+/765901 real quick?
14:25:42 sean-k-mooney oh it failed yes ill do it now
14:26:04 sean-k-mooney sorry should have ran pep8
14:30:27 sean-k-mooney stephenfin: for the leading underscore
14:30:30 sean-k-mooney for [2]
14:30:37 sean-k-mooney shoudl it be [_2]
14:30:38 stephenfin *trailing
14:30:42 stephenfin [2]_
14:30:51 sean-k-mooney oh
14:31:07 sean-k-mooney and the first one does not need that?
14:31:20 stephenfin you already have it on the first one :)
14:31:41 sean-k-mooney oh for [1]_
14:31:49 sean-k-mooney i have another instance of [2] before that
14:31:56 sean-k-mooney cool ill fix them all
14:35:36 openstackgerrit sean mooney proposed openstack/nova-specs master: add port scoped numa affinity spec https://review.opendev.org/c/openstack/nova-specs/+/765901
14:36:19 sean-k-mooney stephenfin: i think ^ fixes everything
14:36:30 sean-k-mooney let me know if there is anything else and ill respin
14:36:39 stephenfin will do
14:44:36 stephenfin sean-k-mooney: I have my doubt about that policy amendment to the modernize os-hypervisors API spec
14:44:53 stephenfin "namely the idea that we shoudl allow users with the PROECT_ADMIN role to list all hyperviors there project is allowed to acess via the summary endpoint"
14:45:41 stephenfin there doesn't appear to be anything that restrict projects to specific hypervisor(s)
14:45:59 stephenfin at least there's no tenant or project attribute of the ComputeNode or Service objects
14:46:07 stephenfin did you have something specific in mind?
15:00:13 openstackgerrit Balazs Gibizer proposed openstack/nova master: Move query param generation to RequestGroup o.vo https://review.opendev.org/c/openstack/nova/+/756894
15:01:45 openstackgerrit Balazs Gibizer proposed openstack/nova master: Refactor _claim_pci_device_for_interface_attach to prepare for qos https://review.opendev.org/c/openstack/nova/+/756895
15:03:08 openstackgerrit Balazs Gibizer proposed openstack/nova master: Allow extending an existing instance allocation https://review.opendev.org/c/openstack/nova/+/757109
15:04:31 openstackgerrit Balazs Gibizer proposed openstack/nova master: Make remove allocation symmetric with add allocation https://review.opendev.org/c/openstack/nova/+/757110
15:06:22 openstackgerrit Balazs Gibizer proposed openstack/nova master: Factor out port resource deallocation https://review.opendev.org/c/openstack/nova/+/757111
15:08:09 openstackgerrit Balazs Gibizer proposed openstack/nova master: Refactor update_pci_request_spec_with_allocated_interface_name https://review.opendev.org/c/openstack/nova/+/757614
15:09:29 openstackgerrit Balazs Gibizer proposed openstack/nova master: Enable mypy on scheduler/utils.py https://review.opendev.org/c/openstack/nova/+/769942
15:10:51 openstackgerrit Balazs Gibizer proposed openstack/nova master: Refactor ResourceRequest constructor https://review.opendev.org/c/openstack/nova/+/769720

Earlier   Later