Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-05-12
16:09:44 opendevreview Merged openstack/nova-specs master: Fix typo https://review.opendev.org/c/openstack/nova-specs/+/841600
16:54:06 opendevreview Gorka Eguileor proposed openstack/nova master: Fix extending non LUKSv1 encrypted volumes https://review.opendev.org/c/openstack/nova/+/836064
16:58:11 geguileo stephenfin: I had to change the commit message, could you +2 it again, please: https://review.opendev.org/c/openstack/nova/+/836064
16:58:34 geguileo sean-k-mooney: ^
16:58:36 stephenfin done
16:58:45 geguileo stephenfin: wow, that was quick, thanks :-)
17:22:25 sean-k-mooney geguileo: ill take a look again shortly too
17:22:41 sean-k-mooney ah stephen put it back in the gate
17:22:43 sean-k-mooney cool
17:32:15 mnaser sean-k-mooney: do you think the `hw:mlock` thing deserves a full-on spec as well if we want to do that as well?
17:33:45 mnaser (i.e. https://review.opendev.org/c/openstack/nova/+/778347)
17:33:59 mnaser if so, i can try and ask ricolin to work on that as well
17:34:31 sean-k-mooney technically its a seperate feature which could be a seperate spec but im ok to combine the to unless others want it split out
17:35:14 sean-k-mooney i can unabandone that if you like if ricolin wants to take over the patch
17:36:07 sean-k-mooney my inital patch technically does not enfoce that you ahve a mem_page_size
17:36:16 sean-k-mooney so it can lead to OOM issues
17:36:23 sean-k-mooney but it was functional
17:37:04 sean-k-mooney it looks like artom and stephenfin had a prference for hw:locked_memory instead of hw:mlock
17:37:12 sean-k-mooney so it proably makes sense to at least make that change
17:37:52 sean-k-mooney i was ment ot re review the viommu spec today too sorry ricolin
17:38:21 sean-k-mooney ill try and get back to it tomorrow but likely wont get to it today
18:15:07 opendevreview Artom Lifshitz proposed openstack/nova stable/wallaby: DNM: Testing stuff https://review.opendev.org/c/openstack/nova/+/841626
23:29:13 ricolin sean-k-mooney: yeah, I can help with the mlock patch, and thanks for your review:)
#openstack-nova - 2022-05-13
07:04:10 opendevreview Wenping Song proposed openstack/os-traits master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/os-traits/+/841682
08:42:20 opendevreview Wenping Song proposed openstack/placement master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/placement/+/841690
08:42:39 opendevreview Wenping Song proposed openstack/placement master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/placement/+/841690
09:20:32 opendevreview Wenping Song proposed openstack/os-resource-classes master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/os-resource-classes/+/841700
09:46:43 opendevreview Wenping Song proposed openstack/os-resource-classes master: Update python testing as per zed cycle testing runtime https://review.opendev.org/c/openstack/os-resource-classes/+/841700
10:12:03 sean-k-mooney ricolin: +1 on the spec
10:12:23 sean-k-mooney ricolin: i have resotred the mlock patch too so feel free to updated it
10:12:29 ricolin thanks sean-k-mooney :)
10:15:28 sean-k-mooney ricolin: if you adress the extra spec name im baicly +2 ill read over it again when you respin but no rush
10:24:09 opendevreview Rico Lin proposed openstack/nova-specs master: Add vIOMMU device support for libvirt driver https://review.opendev.org/c/openstack/nova-specs/+/840310
10:25:12 ricolin sean-k-mooney: done ^^^ :)
10:28:37 sean-k-mooney +2 :) am milestone 1 i think is thursday. assuming gibi or others approve the spec can you submit the patch to os-traits to add the traits
10:28:55 sean-k-mooney if we merge that before tursday then the traits can be included in the m1 relases of os-traits
10:30:37 sean-k-mooney nova's unit tests need the traits to be in a release version of os-traits to pass if its not merge by then its not really a big deal we will just do another release whenever they land and the nova code look good to merge
10:51:22 stephenfin sean-k-mooney: gibi: melwitt: Reviewed that PCI in placement spec. Looks pretty good, though I'm convinced you're making an unnecessary rod for your own back in trying to port that dynamic PF/VF logic to placement land ;-)
11:00:54 sean-k-mooney stephenfin: ya it is complicating things
11:01:15 sean-k-mooney stephenfin: that use case is really only for sriov nics
11:01:23 sean-k-mooney as in for neuton ports
11:01:31 sean-k-mooney i dont think it really exists for pci alias
11:02:09 sean-k-mooney so we can kind of punt. technialy it applise to the alias too but i dont think that is a common usecase there
11:02:45 sean-k-mooney stephenfin: you might want to look over ricolin's spec for the viommu support
11:02:51 sean-k-mooney its pretty short
11:03:17 sean-k-mooney https://review.opendev.org/c/openstack/nova-specs/+/840310
11:04:00 stephenfin sure
11:06:36 gibi stephenfin: so you suggest that deciding to whitelist either the PF or its VFs are rarely used? I think the opposite. It is so easy to just whitelist everthing under 0000:81:* both PF and VF, without thingking to much, and then start using the compute for both direct and direct-physical ports, it just works, so I think there is a lot of deployment out there that might not need to whitelist both but
11:06:42 gibi they did
11:07:44 gibi if we start rejecting such config there will be a lot of pain figuring out a whitelist in these deployments that matches the current consumption
11:08:10 sean-k-mooney im not sure really. im tempted to say perhaps we sould only support that if you are not trackign devices in placment
11:08:45 sean-k-mooney and in the mvp implement supprot for only tracking deivces as PFs or VFs
11:09:03 sean-k-mooney and then we could add the mixed suport after if needed
11:09:04 stephenfin No, I'm suggesting people wanting to use both 'direct' and 'direct-physical' on a single host is likely rare. I think it's reasonable to insist that users that use wildcard device addresses (or vendor/device IDs) choose whether they want to consume the VFs or PFs
11:09:47 sean-k-mooney stephenfin: well your suggestiong they make that dissionc staticaly at deployment time
11:09:56 stephenfin At the moment, a user can dynamically choose whether they consume the PF or one or more of the VFs. I think moving to a static model makes sense now
11:09:59 stephenfin Yup
11:10:01 sean-k-mooney rather then dynmical as workloads are schulded
11:10:05 stephenfin exactly
11:10:32 sean-k-mooney i think pf passhtough is much less common then vf
11:10:39 gibi I have no problems forcing this to new deployments, but I still believe upgrade will be a pain
11:11:07 stephenfin *borderline non-existent (I say, with no actual evidence either way :) However, it seems like an odd thing to do, especially when we don't support nested virt)
11:11:10 gibi but yeah, we can push out the pain to the future by keepin the pci tracking in placement optional
11:11:57 sean-k-mooney stephenfin: well my evidence is that live migrtation, cold migration and unshelve have basicaly been broken since the feature was added until like 2 cylces ago
11:11:59 stephenfin I mean, it's been more than three years and people can still avoid tracking of pinned CPUs in placement
11:12:20 stephenfin so that can can be kicked endlessly down the road
11:12:44 sean-k-mooney so while i know we have some customer using PFs those custoemr also have static deployments
11:13:28 sean-k-mooney gibi: i guess its really up to you
11:13:36 sean-k-mooney if you want to include it in the mvp
11:13:40 gibi OK, I see an agreement forming. Let's implement PCI tracking with placement without the dynamic selection. Keep the everything working as today if the PCI tracking in placement is disabled
11:13:48 sean-k-mooney i think it could be a patch at the end of the seirse by the way
11:14:36 sean-k-mooney gibi: sound good to me that means the prefilter will not have an auto mode
11:14:42 gibi then when everythin (except the dynamic thing works with placemnet) deprecate the old way
11:14:46 gibi and wait for feedback
11:15:18 gibi yeah that means no auto mode
11:15:28 sean-k-mooney you will opt in on the compute with the new config option and opt in on schduler by enabling prefilter
11:15:29 gibi operator needs to first enable tracking in the compute config
11:15:36 gibi then enable prefiltering in the scheduler
11:15:41 sean-k-mooney yep
11:16:31 gibi and if somebody only opt in on a set of computes then enables the prefilter then we say sorry you lost the non enabled computes from the PCI scheduling
11:16:48 stephenfin I think lack of auto mode is a good thing. I called that out in the review as something weird (and I think melwitt had similar concerns)
11:16:54 stephenfin The less magic, the better
11:17:41 sean-k-mooney ack its nice form an opts perspectvie if an only if it means they dont have to do anything on upgrade
11:17:53 stephenfin sean-k-mooney: woah, that vIOMMU thing has got way bigger. I was proposing enabling by default on supported platforms with zero configurability. We now have...three knobs? :-O
11:18:40 stephenfin I've asked ricolin to clarify the need for each knob since it's not at all obvious from reading the spec. If they're really necessary, we'll need this info to document the extra specs.
11:18:46 gibi upgrading to Zed (if it lands) with default config will mean only the old PCI scheduling will be used by nova. So no real upgrade impact. Then operators needs to enable the new tracking
11:18:49 sean-k-mooney stephenfin: yep. the model, bit with which i dont like exposing but is required and locked memroy wich is technialy unrealted
11:19:23 sean-k-mooney but they need locked memroy for there specific hardware and you can only get that with realtim or sev today
11:19:42 sean-k-mooney gibi: yep with no config changes old behavior
11:19:58 gibi so we need to incentivise ops to enable the new behavior
11:20:00 stephenfin At risk of pre-empting discussion on the spec, why is the model necessary? Are these guest OS behavior implications or something? Is there not a sane default we can pick?
11:20:04 sean-k-mooney so no upgrade impact by defualt but we will ahve to document how to move form one to the other
11:20:39 sean-k-mooney stephenfin: the sane default would be virtio but it need a very new libvirt/qemu
11:21:03 sean-k-mooney we cant use intel because well there usecase is for aarch64 servers
11:21:33 sean-k-mooney we could choose dynmically i guess
11:21:47 stephenfin And I'm guessing we can't check what the libvirt version is and use 'virtio' if libvirt > 8.3.0 else 'intel'/'smmuv3' for Intel/ARM respectively?
11:21:49 sean-k-mooney based on the machine-type but i dont like the live migration implciations of that
11:22:01 stephenfin So long as we record the model used in system metadata, we should be fine
11:22:05 sean-k-mooney well we could but ya
11:22:09 stephenfin (i.e. so we don't change during rebuild)

Earlier   Later