| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-06 | |||
| 19:14:49 | artom | I mean, I'd have done it right away by doing 2 patches, but w/e works | |
| 19:16:00 | stephenfin | artom: Yup, though only because I have to for Train (it's not Python 3-only) | |
| 19:16:39 | stephenfin | If it was, I'd leave them since they're a noop until mypy is enabled for that file (via mypy-files.txt) | |
| 19:17:13 | artom | stephenfin, ack, I'll remove my -1 then. I still don't fully get the problem/fix though :P | |
| 19:17:18 | stephenfin | a second patch is also doable, but I find the type hints help understand what's getting passed around | |
| 19:17:22 | stephenfin | I'm replying with that now | |
| 19:17:35 | artom | Yeah, I can understand that (the typing hints helping) | |
| 19:19:21 | stephenfin | artom: comments left. Let me know if that makes sense | |
| 19:20:56 | artom | stephenfin, ah, I think I get it - we don't "delta" the usage, we rebuild it from scratch every time. So if a certain migration/instance doesn't get counted as "tracked", it'll effectively unpin its PCPUs | |
| 19:21:11 | stephenfin | Yup, exactly | |
| 19:21:48 | artom | OK, great, so that's 1 hour to understand the concept. | |
| 19:21:53 | artom | Now to review the actual code :P | |
| 19:22:02 | artom | Need to pick up food first though | |
| 19:22:30 | stephenfin | TBH, that whole periodic task feels racy and generally wrong to me, but it does state that it's intention is to blow everything away and start from scratch so... | |
| 19:22:55 | stephenfin | yet another thing to add my "tech debt I really need to get back to" list, I guess | |
| 19:23:09 | artom | It's definitely not the first race in there | |
| 19:23:49 | artom | See: https://review.opendev.org/#/c/684409/ | |
| 19:23:57 | stephenfin | For sure. I decided not to start listing the related bugs of the related bugs, because it started getting ridiculous | |
| 19:24:19 | stephenfin | concurrency is hard | |
| 19:24:30 | artom | Yeah, chill with that, you trying to one-up George RR Martin or something? | |
| 19:26:43 | stephenfin | Try to write an explanation for https://review.opendev.org/744021 (with other nova devs that don't have this specific libvirt CPU pinning knowledge in mind) and let me know how you get on :P | |
| 19:44:54 | sean-k-mooney | stephenfin: speaking about racy periodics | |
| 19:45:40 | sean-k-mooney | its possible that we have a race between them starting in general on inital boot and the compute agent looking up it compute node uuid or maybe even creating the compute node record | |
| 19:46:41 | sean-k-mooney | i say its posible becuase im expecting a bug to be filed for http://pastebin.test.redhat.com/891237 | |
| 19:47:23 | sean-k-mooney | and that is my working theory on how we could be trying to update a compute node recored with a compute_node_id of none | |
| 19:49:14 | sean-k-mooney | oh but that one does not have have startup set.... | |
| 19:49:17 | sean-k-mooney | https://github.com/openstack/nova/blob/9ecefeb836964c52a5a2969b15c82b11c51d32ab/nova/compute/manager.py#L9723 | |
| 19:49:56 | sean-k-mooney | and it gets the compute nodes at the start https://github.com/openstack/nova/blob/9ecefeb836964c52a5a2969b15c82b11c51d32ab/nova/compute/manager.py#L9697-L9700 | |
| 19:50:09 | sean-k-mooney | well that theroy did not last long | |
| 19:52:47 | sean-k-mooney | oh i did not read that write it was trying to insert a default consturced/empty pci deivcie object | |
| 19:54:26 | sean-k-mooney | tommorows problem | |
| #openstack-nova - 2020-08-07 | |||
| 02:04:38 | openstackgerrit | Brin Zhang proposed openstack/nova master: [Trivial] Remove wrong format_message() conversion https://review.opendev.org/744280 | |
| 04:07:02 | openstackgerrit | Tony Su proposed openstack/nova master: Provider Config File: Function to further validate and retrieve configs https://review.opendev.org/676029 | |
| 04:07:03 | openstackgerrit | Tony Su proposed openstack/nova master: Provider Config File: Functions to merge provider configs to provider tree https://review.opendev.org/676522 | |
| 04:07:03 | openstackgerrit | Tony Su proposed openstack/nova master: Provider Config File: Enable loading and merging of provider configs https://review.opendev.org/693460 | |
| 04:47:44 | openstackgerrit | Luyao Zhong proposed openstack/nova master: introduce a new driver interface 'cleanup_instance' https://review.opendev.org/714653 | |
| 04:47:44 | openstackgerrit | Luyao Zhong proposed openstack/nova master: track error migrations in resource tracker https://review.opendev.org/745281 | |
| 04:47:45 | openstackgerrit | Luyao Zhong proposed openstack/nova master: track orphans in resource tracker https://review.opendev.org/745282 | |
| 06:59:00 | openstackgerrit | Luyao Zhong proposed openstack/nova master: track error migrations in resource tracker https://review.opendev.org/745281 | |
| 06:59:01 | openstackgerrit | Luyao Zhong proposed openstack/nova master: track orphans in resource tracker https://review.opendev.org/745282 | |
| 07:11:40 | gibi | good morning | |
| 07:38:10 | tony_su | stephenfin: gibi: alex_xu: new provider-config-file patches uploaded change log 1) all comments except one in the last patch relected in code or answered 2) add docstring for each test case | |
| 07:39:05 | tony_su | unfortunately, so far I don't know how to use nova-manage command to help provider config. | |
| 07:42:02 | tony_su | stephenfin: does it mean we need to upgrade nova-manage command to offer some new option to validate the validity of provider.yaml file? | |
| 07:42:43 | tony_su | for this comment https://review.opendev.org/#/c/693460/35/doc/source/admin/managing-resource-providers.rst@205 | |
| 07:52:01 | stephenfin | tony_su: Yeah, exactly. I'm thinking we simply add a command (e.g. 'nova-manage placement validate-schema $SCHEMA') that attempts to load the provided schema and provides an error code and message if it's not valid | |
| 07:53:49 | tony_su | Must we do it this time or can we do it later? I am not fairly faimiliar with nova-manage command and I dont' know how long it can be done. | |
| 07:54:17 | tony_su | I am a little bit worried we will miss Victoria if do so. | |
| 08:40:52 | gibi | tony_su, stephenfin: I'm fine handling the nova-manage part as an addition on top of the series as I don't consider that as a blocker for the feature to merge. Let's focuse on merging what is ready (I know I should spend some time reviewing that series too). | |
| 09:07:53 | openstackgerrit | Stephen Finucane proposed openstack/os-vif master: mypy: Add type annotations https://review.opendev.org/745162 | |
| 09:07:54 | openstackgerrit | Stephen Finucane proposed openstack/os-vif master: mypy: Extend type annotations to 'vif_plug_noop' https://review.opendev.org/745163 | |
| 09:31:11 | tony_su | gibi: stephenfin has been doing great, detailed review for each patch. your review can still effectively speedup our progress... | |
| 09:36:55 | kashyap | lyarwood: Can you remind me, please: does Nova stash 'host-capabilities' somewhere? | |
| 09:37:14 | kashyap | It does, IIRC | |
| 09:38:07 | lyarwood | kashyap: hmm I know we grab them at startup | |
| 09:38:14 | kashyap | lyarwood: Damn | |
| 09:38:26 | kashyap | lyarwood: So the problem I'm debugging w/ a TripleO upgrade dev is this: | |
| 09:39:47 | lyarwood | kashyap: it doesn't look like we stash them anywhere | |
| 09:40:06 | lyarwood | kashyap: we appear to be grabbing a fresh copy from libvirt everytime we need them | |
| 09:40:35 | kashyap | lyarwood: Hmm. Got a code pointer? | |
| 09:42:20 | lyarwood | kashyap: https://paste.centos.org/view/86fa4860 | |
| 09:44:21 | kashyap | lyarwood: Excellent! That helped | |
| 09:44:31 | kashyap | lyarwood: Context: | |
| 09:45:04 | kashyap | lyarwood: During FFU, some hosts report the CPU feature 'arch-facilities' (to make matters worse, it's called differently on EL8: 'arch-capabilities') | |
| 09:45:42 | kashyap | lyarwood: So migration from EL7 to EL8 fails (on appropriate hardware) with: "libvirt.libvirtError: internal error: Unknown CPU feature arch-facilities" | |
| 09:45:55 | kashyap | lyarwood: To 'workaround' that, we need to disable the 'arch-facilities' on the source host | |
| 09:46:02 | lyarwood | kashyap: isn't that a libvirt bug? | |
| 09:46:15 | kashyap | s/disable/comment-out | |
| 09:46:22 | lyarwood | kashyap: ah wait, is it the way we generate the xml on the src? | |
| 09:46:29 | kashyap | lyarwood: No, it's not :-( I talked w/ Jiri Denemark today | |
| 09:46:51 | kashyap | In short, we (Nova) need to improve on how we're doing CPU checks. Some checks are redundant (as libvirt does them anyway) | |
| 09:46:56 | lyarwood | kashyap: we can handle that in https://github.com/openstack/nova/blob/9ecefeb836964c52a5a2969b15c82b11c51d32ab/nova/virt/libvirt/migration.py#L56-L70 | |
| 09:47:02 | kashyap | lyarwood: Yeah, indeed. The guest XML itself doesn't have 'arch-facilities' | |
| 09:50:32 | kashyap | lyarwood: /me clicks | |
| 09:52:15 | kashyap | lyarwood: So, let me explain the sequence of the current workaround: | |
| 09:59:05 | kashyap | 1. Double-check none of the EL7 instance XML contain 'arch-facilities' CPU feature. Normally, the guests do _not_ have them. | |
| 09:59:08 | kashyap | 2. Run `virsh capabilities | grep arch-facilities` — it will be present | |
| 09:59:11 | kashyap | 3. Comment out 'arch-facilties' in /usr/share/libvirt/cpu_share.xml | |
| 09:59:14 | kashyap | 4. Restart 'libvirtd' and 'nova-compute' service (both are needed) | |
| 09:59:16 | kashyap | 5. Then run `virsh capabilities`, again: it will now be absent (expected) | |
| 09:59:19 | kashyap | 6. Now migrate the guest from EL7 to EL8: it _must_ succeed. | |
| 09:59:29 | kashyap | (The above is when migrating only from source hardware that supports 'arch-facilities' CPU feature.) | |
| 10:01:57 | lyarwood | urgh we need to fix this in Nova tbh | |
| 10:02:14 | lyarwood | we can check on the dest if arch-capabilities is set | |
| 10:02:26 | lyarwood | if it is, use it in the migration xml we send there | |
| 10:03:05 | lyarwood | if it isn't but arch-facilities is set on the src, remove that from the migration xml | |
| 10:03:12 | lyarwood | does that sound right? | |
| 10:05:40 | kashyap | lyarwood: Reading the scroll | |
| 10:05:49 | lyarwood | brb coffee | |
| 10:06:15 | kashyap | lyarwood: Right, that does sound correct. I'm gonna file a bug for it | |
| 10:06:43 | kashyap | lyarwood: Oh, but note: 'arch-facilties' is marked as *non-migratable* | |
| 10:10:35 | lyarwood | /o\ | |
| 10:10:42 | lyarwood | so should we always remove it? | |
| 10:12:50 | kashyap | lyarwood: I'd change the plan to this: just drop 'arch-facilities' and let libvirt handle it — this works, Jiri from libvirt confirms | |
| 10:13:44 | lyarwood | kashyap: ack that's simple enough then | |
| 10:15:58 | lyarwood | kashyap: as you're out on PTO next week I'd be happy to handle that once you've written up the bug | |
| 10:22:17 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Follow up for Ibf8dca4bd57b3bddb39955b53cc03564506f5754 https://review.opendev.org/745095 | |
| 10:25:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Add helpers for suspend, resume and reboot of server https://review.opendev.org/741285 | |
| 10:25:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Pass context, instance to '_create_guest' https://review.opendev.org/741286 | |
| 10:25:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Reject non-spawn operations for vTPM https://review.opendev.org/741500 | |