Earlier  
Posted Nick Remark
#openstack-nova - 2020-08-05
14:05:46 sean-k-mooney efried: i was considring writing an operator for deploying zuul at one point and i was looking at kudo amoung other things as a way to do it
14:06:48 sean-k-mooney well zuul, nodepoll , zookeyper, german and gerrit
14:07:08 sean-k-mooney but i just wrote hte manifest by had instead.
14:16:06 openstackgerrit sean mooney proposed openstack/os-vif master: support pyroute2 0.5.13 https://review.opendev.org/744809
14:22:45 openstackgerrit Merged openstack/nova master: Document nova in tree virt drivers https://review.opendev.org/740061
15:17:04 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP zuul: nova-multinode-evacuate https://review.opendev.org/744883
15:53:02 openstackgerrit Balazs Gibizer proposed openstack/nova master: Move equality check into LibvirtConfigGuestInterface https://review.opendev.org/744524
15:53:03 openstackgerrit Balazs Gibizer proposed openstack/nova master: Remove unused vpn param from allocate_for_instance https://review.opendev.org/744933
15:54:15 openstackgerrit Balazs Gibizer proposed openstack/nova master: Only unplug vif after the device is detached from libvirt https://review.opendev.org/744934
15:54:15 openstackgerrit Balazs Gibizer proposed openstack/nova master: [WIP] Support SRIOV interface attach and detach https://review.opendev.org/740995
15:57:53 openstackgerrit Balazs Gibizer proposed openstack/nova master: DNM: add logging for device matching https://review.opendev.org/744936
16:04:21 openstackgerrit Harshavardhan Metla proposed openstack/nova stable/rocky: [stable-only] Moved the quoted section https://review.opendev.org/744681
16:05:50 melwitt lyarwood: I just barely started looking at ceph job failures on stable/queens, saw this error message "libvirtError: internal error: unable to execute QEMU command 'device_add': Property 'virtio-blk-device.drive' can't find value 'drive-virtio-disk1'" https://zuul.opendev.org/t/openstack/build/fa85e5eeb1f84389b5dd259c8a829552/log/controller/logs/screen-n-cpu.txt#32824
16:06:03 melwitt does that ring any bells to you?
16:09:01 melwitt hm nvm maybe https://ask.openstack.org/en/question/95328/ceph-cinder-attach-volume-to-running-instance
16:21:01 openstackgerrit Harshavardhan Metla proposed openstack/nova stable/rocky: "[stable-only]" Removed the quoted section https://review.opendev.org/744681
16:26:46 stephenfin melwitt, bauzas: not sure if you'll know this off the top of your head, but should we be releasing resources before we confirm a resize?
16:27:49 stephenfin Based on https://review.opendev.org/#/c/641806/21, it seems we do (via the resource tracker's periodic task run)
16:28:01 melwitt stephenfin: releasing as in deleting allocations on the source before confirming? AFAIK no, supposed to hold allocations on the source and dest until confirm/revert
16:31:43 melwitt I think that patch's commit message is saying prior to that patch, we were reporting resources held on the source *after* a confirm resize (until the next periodic run) whereas after a confirm, the resources should be shown as released on the source. but only after the confirm, not before
16:32:33 stephenfin ah, that makes sense
16:32:41 stephenfin so now we're freeing those resources immediately
16:33:12 melwitt immediately upon a confirm, yeah
16:34:06 melwitt or rather reporting them immediately after a confirm. the claim has always been dropped at the time of confirm, but apparently the reporting was delayed potentially by one periodic task run
16:34:12 openstack Launchpad bug 1879878 in OpenStack Compute (nova) "VM become Error after confirming resize with Error info CPUUnpinningInvalid on source node " [Medium,Confirmed] - Assigned to Stephen Finucane (stephenfinucane)
16:34:12 stephenfin okay, cool. There's a bug in it that I'm working on (https://bugs.launchpad.net/nova/+bug/1879878, fwiw); tl;dr: if the periodic runs between the API request and the 'confirm_resize' call in the RT, the ComputeNode.numa_topology gets out of whack
16:34:43 melwitt ah, gotcha
16:36:21 melwitt oh interesting, so if the periodic task fires while confirm resize is still processing, it turns the instance to ERROR
16:37:00 openstackgerrit Balazs Gibizer proposed openstack/nova master: Move equality check into LibvirtConfigGuestInterface https://review.opendev.org/744524
16:37:11 stephenfin yeah, the '_update_available_resource' function of the RT seems to regenerate ComputeNode objects from scratch, including the embedded numa_topology
16:37:47 stephenfin and it doesn't seem to be accounting from the still-present allocations on the host once the instance has been confirmed
16:38:12 openstackgerrit Balazs Gibizer proposed openstack/nova master: Only unplug vif after the device is detached from libvirt https://review.opendev.org/744934
16:39:25 openstackgerrit Balazs Gibizer proposed openstack/nova master: [WIP] Support SRIOV interface attach and detach https://review.opendev.org/740995
16:39:26 openstackgerrit Balazs Gibizer proposed openstack/nova master: DNM: add logging for device matching https://review.opendev.org/744936
16:42:01 melwitt stephenfin: note that sean-k-mooney's comments are correct though, that if this is in a version older than claims in placement, races are expected (especially since we don't have numa in placement). in your repro, is that with cpu_dedicated_set and on the master branch?
16:42:38 gibi sean-k-mooney: added a patch top of the sriov_attach series with extra logging that can help you figuring out why the macvtap and direct-physical devices are not detached for you
16:42:42 gibi sean-k-mooney: https://review.opendev.org/744936
16:42:59 gibi also added a patch to move the vif unplug after libvirt detach
16:43:02 openstackgerrit Stephen Finucane proposed openstack/nova master: TODO https://review.opendev.org/744951
16:43:02 openstack bug 1879878 in OpenStack Compute (nova) "VM become Error after confirming resize with Error info CPUUnpinningInvalid on source node " [Medium,Confirmed] https://launchpad.net/bugs/1879878 - Assigned to Stephen Finucane (stephenfinucane)
16:43:02 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: Add reproducer for bug #1879878 https://review.opendev.org/744950
16:43:05 stephenfin melwitt: ^
16:43:15 stephenfin I can reproduce trivially with new-style configuration
16:43:35 melwitt while using cpu_dedicated_set? /me looks
16:43:38 stephenfin yup
16:43:52 melwitt ok I see
16:45:34 stephenfin I think I'm close to a fix at least. Worst case scenario, we put mriedem's stuff inside a conditional to only run if we still know that it exists
16:45:52 stephenfin i.e. if it's in the list of tracked migrations and instances
16:46:17 stephenfin though I haven't figured out what that will leak yet
16:46:22 stephenfin tbd
16:47:22 melwitt sorry, what's the proposed fix? being that it's intended that resources be held on source and dest until confirm or revert
16:48:13 stephenfin https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L579-L591
16:48:29 stephenfin we only do that if the instance appears in our list of tracked_migrations or tracked_instances
16:50:44 stephenfin afaict, '_update_usage_from_instances' wipes those and regenerates them. When we confirm the resize, the instance is marked as deleted, which means it's not picked up by '_update_usage_from_instances'
16:51:22 stephenfin so the race is between that (the instance getting marked as deleted and '_update_usage_from_instances' running) and the call to 'drop_move_claim'
16:51:30 stephenfin I _think_
16:58:56 sean-k-mooney gibi: thanks ill restart with that now and see how it works
17:00:22 melwitt stephenfin: this is interesting bc the commit message of mriedem's change says "This fixes the issue by also updating usage in drop_move_claim when the instance is not in tracked_migrations but is in tracked_instances." (which would do part of what you're suggesting) but it appears not to be what is in the code
17:00:29 gibi sean-k-mooney: thanks. I will end my day about now but I will be back tomorrow morning
17:00:34 sean-k-mooney stephenfin: sound plausible i had not got to the point of diging into how the race happens
17:00:54 sean-k-mooney gibi: no worries
17:03:52 melwitt stephenfin: oh, nvm, I'm misreading this... it's removing usage by removing the instance from tracked_instances and then updating usage
17:08:48 sean-k-mooney melwitt: stephenfin if this race is happening the way you say
17:09:00 sean-k-mooney would this also happen with the old config options?
17:09:07 stephenfin sean-k-mooney: I think so
17:09:18 sean-k-mooney ok its just more obvious now?
17:09:25 stephenfin I think the bug is with that patch from mriedem
17:09:28 sean-k-mooney or they were just unlucky
17:09:38 stephenfin I don't think it's anything to do with the PCPU work
17:09:43 sean-k-mooney ok
17:10:16 stephenfin tbc, it's still a working theory but it would be easy prove out by backporting that fix to e.g. stable/stein where none of that code is present
17:10:18 sean-k-mooney i think the upgrade procedure we proposed for our downstream customer still makes sense
17:10:33 sean-k-mooney but i think they will need the backport of your two fixes
17:10:36 sean-k-mooney the one for this race
17:10:49 sean-k-mooney and the one for the isolate on smt host with new config options
17:10:51 sean-k-mooney right?
17:11:56 stephenfin the isolate on SMT fix is mostly unrelated and should be backported regardless
17:12:01 stephenfin still unsure about this race
17:12:14 sean-k-mooney ok
17:51:10 openstackgerrit Stephen Finucane proposed openstack/nova master: Don't unset Instance.old_flavor, new_flavor until necessary https://review.opendev.org/744958
17:51:10 openstack bug 1879878 in OpenStack Compute (nova) "VM become Error after confirming resize with Error info CPUUnpinningInvalid on source node " [Medium,Confirmed] https://launchpad.net/bugs/1879878 - Assigned to Stephen Finucane (stephenfinucane)
17:51:10 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: Add reproducer for bug #1879878 https://review.opendev.org/744950
17:51:38 stephenfin melwitt: I think that's the fix. Could you sanity check at some point?
17:51:52 melwitt stephenfin: yeah I'll take a look
17:52:43 stephenfin dansmith: You should probably look too ^ It's a variant of a patch I have for vTPM and as I think I've mentioned before, your name is all over the code I'm touching
18:36:08 dansmith okay I'm kinda heads-down on something else right now, but melwitt I guess let me know if you're unsure
18:36:47 melwitt I haven't looked yet but I think I can already say I'm unsure :)
19:43:59 lyarwood stephenfin: https://review.opendev.org/#/c/699291/10 - finally got back to this btw if you can take a look
20:11:53 openstackgerrit Artom Lifshitz proposed openstack/nova master: Handle Neutron errors in _post_live_migration() https://review.opendev.org/729763
20:29:28 openstackgerrit Artom Lifshitz proposed openstack/nova master: WIP: Centralize wait_for_unversioned_notification https://review.opendev.org/744985
20:33:37 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP zuul: nova-evacuate https://review.opendev.org/744883
20:41:46 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP zuul: nova-evacuate https://review.opendev.org/744883
20:58:26 lyarwood melwitt: https://review.opendev.org/#/c/743319/ - would you mind +W'ing that again as CI is finally green again
20:59:38 melwitt lyarwood: yeah, I will
21:01:53 lyarwood thanks :)
21:03:08 melwitt np
21:08:12 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP zuul: nova-evacuate https://review.opendev.org/744883
21:17:34 openstackgerrit Sean McGinnis proposed openstack/nova master: Add lsscsi to bindep https://review.opendev.org/744992
21:19:06 smcginnis Needed for an os-brick change in the latest release ^
21:24:58 sean-k-mooney smcginnis: should that not be listed in os-bricks bindep

Earlier   Later