Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-03-10
21:49:39 sean-k-mooney in your tests
21:49:48 artom Nope :)
21:49:56 sean-k-mooney well you might be but its a noop for you
21:49:56 artom Not that it as intentional
21:50:00 artom Or that I was aware it existed
21:50:26 sean-k-mooney you are calling it
21:50:51 sean-k-mooney but i does nothing for STANDARD/type-PCI
21:51:56 artom del pool['socket']
21:51:56 artom if 'socket' in pool:
21:51:56 artom Yeah, if I do for pool in pools:
21:52:01 artom At the end of my new filter
21:52:05 artom It fixes your func tests
21:52:39 artom I guess you've made your point about being purely functional and not having side effects ;)
21:53:49 sean-k-mooney making it it a pure fucntion may or may not have caught it depending on if we wrote the test right
21:54:43 artom I think the "best" solution would be to not set pool['socket'] altogether, and come at this from another angle: figure out which NUMA nodes are allowed, and filter on those
21:54:53 artom No side effects
21:55:09 sean-k-mooney well you should not be setting pool sockets
21:55:25 sean-k-mooney and you dont need too if you did what was in my last comment
21:56:01 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/772779/17#message-744974dfe27df3782bd2e6c066aa1b241fc7136a
21:57:01 sean-k-mooney the body of your filter could be basically this right
21:57:03 sean-k-mooney vm_sockets = [mappings[cell.id] for cell in numa_cells]
21:57:05 sean-k-mooney pools_to_remove = []
21:57:07 sean-k-mooney for pool in pools:
21:57:09 sean-k-mooney pool_socket = mappings.get(pool['numa_node'])
21:57:11 sean-k-mooney if not (pci_socket and pool_socket in vm_sockets):
21:57:13 sean-k-mooney pools_to_remove += pool
21:57:15 sean-k-mooney return [pool for pool in pools if pool not in pools_to_remove]
21:57:49 artom There are some unbound variables in there, so I can't quite grok it
21:57:56 artom But I can come up with something equivalent
21:58:16 sean-k-mooney where mappings is basically {numa_node:[socket_id,socket_id]
21:58:32 sean-k-mooney artom: mappings is the only one right
21:58:42 artom pci_socket as well
21:59:03 sean-k-mooney oh that is pool_socket
21:59:15 sean-k-mooney i forcot to rename it
22:00:27 sean-k-mooney artom: although maybe that shoudl be
22:00:57 sean-k-mooney if not pool_socket or pool_socket in vm_sockets:
22:01:13 sean-k-mooney for socket policy you dont allow device with no numa node correct
22:01:34 sean-k-mooney so that should be an or not an and
22:06:28 artom sean-k-mooney, well, I did a thing, and it made all tests pass.
22:06:36 artom It's brute force and dumb, but "easy" to understand
22:06:38 artom Shall I push?
22:06:40 sean-k-mooney artom: so i can "fix" this in my patch by adding pool_keys.pop('socket', None) to _find_pool
22:07:01 artom It'll kick my last patch out of the gate, and we'll have to get +2s and +As again tomorrow
22:07:48 sean-k-mooney i dont know its technically broken but you have a followup or i can fix it in my patch
22:08:00 sean-k-mooney we proably shoudl kick it out of the gate as much as i hate that
22:08:10 artom Yeah, it's the correct thing to do
22:08:21 artom As it, it's broken with any parent-having device
22:08:35 sean-k-mooney ya
22:09:09 openstackgerrit Artom Lifshitz proposed openstack/nova master: pci: implement the 'socket' NUMA affinity policy https://review.opendev.org/c/openstack/nova/+/772779
22:09:10 openstackgerrit Artom Lifshitz proposed openstack/nova master: Support per port numa policies with SR-IOV https://review.opendev.org/c/openstack/nova/+/773792
22:09:17 sean-k-mooney so the minimal fix is just add one line pool_keys.pop('socket', None) to _find_pool
22:10:32 sean-k-mooney ah i see what you did
22:11:05 sean-k-mooney are you still ok with rewriting it again in the followup
22:11:23 sean-k-mooney that should work as is though
22:11:26 sean-k-mooney ill test it now
22:12:05 artom sean-k-mooney, yeah, we can improve in a follow up
22:12:15 artom For now, do the simple stupid thing that's easy to understand and review, and works ;)
22:15:33 sean-k-mooney ok i have the unit and funct test running for those locally now. im going to go get somethign to eat and when i get back ill test boot real vms and ill comment on the review
22:16:07 sean-k-mooney then ill rebase the vdpa change on top of both patches and test those
22:16:16 artom That's... optimistic ;)
22:16:36 artom Though I suppose if that was the only outstanding issue with the port NUMA policies, might as well aim for the moon ;)
22:17:27 sean-k-mooney i have some nits form stephen it looks like i mised but ya i think its the only issue
22:18:09 sean-k-mooney these https://review.opendev.org/c/openstack/nova/+/773792/11#message-8d43910738d79b4f530fd6b4176074ad6f53c543
22:20:56 openstackgerrit Merged openstack/os-resource-classes master: Fix hacking min version to 3.0.1 https://review.opendev.org/c/openstack/os-resource-classes/+/727557
22:58:54 dansmith melwitt: easy stats: https://review.opendev.org/c/openstack/nova/+/779815
22:59:27 dansmith converting the single-line glance policy I write from old json to new yaml, so I can write to it easier from a child job to change other stuff
23:01:11 melwitt +2
23:01:52 dansmith m'thanks
23:38:31 openstackgerrit sean mooney proposed openstack/nova master: Support per port numa policies with SR-IOV https://review.opendev.org/c/openstack/nova/+/773792
#openstack-nova - 2021-03-11
04:31:26 openstackgerrit melanie witt proposed openstack/nova master: Dynamically archive FK related records in archive_deleted_rows https://review.opendev.org/c/openstack/nova/+/773834
05:37:12 openstackgerrit Merged openstack/nova master: libvirt: Deprecate disable_native_luksv1 and rbd_volume_local_attach https://review.opendev.org/c/openstack/nova/+/778004
05:54:16 openstackgerrit Merged openstack/nova master: Follow up for QoS interface attach https://review.opendev.org/c/openstack/nova/+/777112
06:42:58 openstackgerrit Merged openstack/nova master: setup.cfg: Resolve warning https://review.opendev.org/c/openstack/nova/+/779449
06:57:40 openstackgerrit Merged openstack/nova master: Make nova-ceph-multistore use policy.yaml https://review.opendev.org/c/openstack/nova/+/779815
07:46:27 openstackgerrit Yongli He proposed openstack/nova master: smartnic support - reject server move and suspend https://review.opendev.org/c/openstack/nova/+/779913
07:50:03 openstackgerrit Yongli He proposed openstack/nova master: smartnic support - reject server move and suspend https://review.opendev.org/c/openstack/nova/+/779913
07:52:49 openstackgerrit Yongli He proposed openstack/nova master: smartnic support - reject server move and suspend https://review.opendev.org/c/openstack/nova/+/779913
08:05:31 yonglihe sean-k-mooney: alex_xu: could you please remove that -1 (base on discuss with alex): https://review.opendev.org/c/openstack/nova/+/771363
08:20:21 gibi yonglihe: hi, sorry I was not able to get back to the smartnic patches yesterday
08:26:05 yonglihe gibi: got, busy day.
08:26:48 gibi yonglihe: I see -1s on https://review.opendev.org/c/openstack/nova/+/771363/12 does the problem resolved?
08:27:18 yonglihe I post another patch to fix alex coments.
08:27:38 yonglihe and there are one unit test case will posted soon.
08:28:29 gibi OK
08:45:51 openstackgerrit Yongli He proposed openstack/nova master: smartnic support - new vnic type https://review.opendev.org/c/openstack/nova/+/771363
08:45:52 openstackgerrit Yongli He proposed openstack/nova master: smartnic support https://review.opendev.org/c/openstack/nova/+/758944
08:45:53 openstackgerrit Yongli He proposed openstack/nova master: smartnic support - reject server move and suspend https://review.opendev.org/c/openstack/nova/+/779913
08:48:34 yonglihe gibi alex_xu, sean-k-mooney, xinranwang: test cases added.
08:51:24 yonglihe gibi: all the problem resolved, https://review.opendev.org/c/openstack/nova/+/771363/13
08:51:34 gibi reading it ...
09:42:17 openstackgerrit Lee Yarwood proposed openstack/nova master: docs: Add note about rescuing bfv instances with the 2.87 microversion https://review.opendev.org/c/openstack/nova/+/779479
09:43:24 lyarwood whops
09:47:04 lyarwood Type 'yes' to confirm, other to cancel: yes
09:47:05 lyarwood Aborting.
09:47:10 openstackgerrit Lee Yarwood proposed openstack/nova master: docs: Add note about rescuing bfv instances with the 2.87 microversion https://review.opendev.org/c/openstack/nova/+/779479
09:47:16 lyarwood >.< git-review WHY DO YOU HATE ME?!
09:48:42 bauzas folks, I'll resume reviewing the uefi secure boot series starting with https://review.opendev.org/c/openstack/nova/+/779302/2
09:48:50 bauzas stephenfin: lyarwood: gibi: ^
09:48:55 stephenfin ta
09:49:10 gibi ack

Earlier   Later