Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-01-13
15:26:03 sean-k-mooney yep i had this argument after we had already commited the VCPUs resouce class code in plament and no one wnated to change it
15:28:58 huaqiang efried,sean-k-mooney: do we have conclusion on adding a new cpu policy 'mixed' or just don't naming it as policy if we don't provided a policy?
15:30:02 huaqiang for later case we just create an instance through PCPU&VCPU and we don't assign any policy to it internally
15:30:41 sean-k-mooney that will triger alot of the "shared" policy code paths and break realtime cpus
15:31:37 sean-k-mooney given realtime guest is one of the primary usecase it think that would be bad unless you also modify the logic so that we take into account the realtime mask
15:32:09 sean-k-mooney i mean we shoudl do that anyway but the more things we make implcit the more edgecase you will have to handel interanlly
15:36:34 huaqiang the logic for creating a mixed instance follows the code path of 'dedicated' policy mostly.
15:37:06 sean-k-mooney yes both most of the code check expcitly check for dedicated then assuem if that is not set that it is shared
15:37:21 huaqiang but we have to create a internal variable to represent of mixed instance, not a policy but works paraelly with policies.
15:37:22 sean-k-mooney so if you dont set a policy it will be treated as shared
15:37:41 sean-k-mooney i think that would be a mistake
15:37:44 sean-k-mooney this is a policy
15:37:58 huaqiang I also think that is a problem
15:38:02 sean-k-mooney having yet another thing to check would just make the code more complcatedd
15:39:38 huaqiang How about tell user, the 'dedicated' is extended with the concept that not all CPUs are pinning CPUs.
15:39:49 huaqiang this was denied byefore
15:39:49 sean-k-mooney noop
15:40:13 sean-k-mooney i really hate that idea
15:40:55 huaqiang for inconsistent behavior
15:41:17 sean-k-mooney yes it would have different meaning on different versions of openstack
15:43:22 sean-k-mooney we could add teh mixed policy and make it optional
15:43:49 sean-k-mooney so we have an internal value to use and the user can choose not to set it and just use the placmenet syntax
15:44:32 sean-k-mooney i would still always recomend setting it but if we can infer the mixed case the it woudl not be required
15:45:10 sean-k-mooney however that woudl allwo you to do something like flavor.vcpu=8 hw:cpu_policy=mixed resouces:vcpu:2
15:45:29 sean-k-mooney and because we know its mixed we know the other 6 cpus must be pinned
15:46:03 huaqiang and the number of PCPU could be derived from the give information
15:46:10 sean-k-mooney you could also do flavor.vcpu=8 resouces:vcpu:2 resouces:PCPUS:6
15:46:21 sean-k-mooney yep
15:46:43 huaqiang if emulator thread policy is isolate
15:46:49 huaqiang then another PCPU is requred
15:46:58 sean-k-mooney yep
15:47:09 sean-k-mooney but again we can derive that form the info availble
15:47:15 huaqiang yes
15:47:36 huaqiang works for me
15:47:46 sean-k-mooney knowing its a mixed cpu explictly makes it eairer to derive thing in my view
15:48:06 sean-k-mooney but makeing it optional as long as you tell us how much of each type you want keeps it clean
15:48:13 sean-k-mooney if that is what you prefer
15:49:22 efried Yeah, I really don't understand why it needs to be explicitly stated by the user.
15:49:51 efried "will trigger X code paths by default" <== we will be changing those code paths as part of this feature, right?? That's what code changes are for??
15:50:14 efried Mixed means you have to take shared code paths in some places, dedicated in other places.
15:50:22 efried That's gonna happen internally regardless.
15:50:43 sean-k-mooney i think we only ever check for dedicated so i think you would always take the shared path
15:50:58 sean-k-mooney but yes it will be changed in anycase by this feature
15:51:20 efried I'm not refuting that we need "mixed" logic internally.
15:51:41 sean-k-mooney by the way im still looking for the code that allows resouce:PCPU=4 to actully give you a pinned guest
15:51:51 efried I'm saying, why require it to be stated by the user if it can be inferred 100% from the fact that they MIXED THEIR CPUs in the flavor?
15:52:11 sean-k-mooney well the field is an ovo filed and i dont like adding another seperate variable
15:52:28 efried an OVO field in what, the Flavor?
15:52:34 sean-k-mooney so if we extend the ovo filed defintion it will alther the api check implcitly to allow it
15:52:46 sean-k-mooney no in the image metadata
15:52:52 sean-k-mooney and in the numa objects
15:53:05 efried but it's not a required field.
15:53:29 sean-k-mooney yes it can be unset or set to shraed explcitly for floating instance or set to dedicated
15:53:43 efried Fine.
15:53:50 efried but don't require it.
15:54:32 efried I assume you can set it from either the flavor or the image?
15:54:38 sean-k-mooney the sematica are that if its not set in the flavor it means the image can requst pinning. so unset and set to shared are different
15:54:51 efried So we funnel those into the numa object
15:54:58 sean-k-mooney yes
15:55:16 sean-k-mooney and it becomes part of the numa constratin used in the numa toplogy filter and on the compute node
15:55:19 efried we logically evaluate cpu policy-ness from the flavor and the image to determine the value in the internal numa object
15:55:30 efried ? ^
15:55:53 sean-k-mooney we store it as cpu_polciy in the instnace numa topology
15:56:08 sean-k-mooney by reading the flavor and image properties yes
15:56:11 efried cool,
15:56:11 efried so what I'm saying is,
15:56:11 efried if neither the flavor nor the image dictates a cpu_policy, and the flavor specifies both VCPUs and PCPUs, we set the value internally to mixed.
15:56:23 efried but we do not require the flavor or image to say 'mixed'.
15:56:26 sean-k-mooney yep
15:56:34 sean-k-mooney ya we coudl do that
15:56:37 efried because they already did, by specifying VCPU+PCPU
15:56:40 efried and,
15:56:42 sean-k-mooney but i think we should not block it either
15:56:55 efried if they say VCPU+PCPU and specify cpu_policy=$anything_other_than_mixed, we have to fail.
15:57:11 sean-k-mooney am yes i think we can do that
15:57:32 sean-k-mooney infact i think we should already be doint that in the api somewhere
15:57:36 efried so if we're going to do that, I don't see why we even need to bother exposing the *ability* for them to say cpu_policy=mixed in the flavor/image.
15:57:45 efried but I won't die on a hill for that.
15:57:55 sean-k-mooney it would be more work to not expose it
15:57:59 efried fine
15:59:36 sean-k-mooney efried: huaqiang are we really sure we supprot enabling cpu_pinning by using PCPUs
15:59:40 sean-k-mooney https://github.com/openstack/nova/blob/0012fdfdc387da97d1d396e5ebda152d7c10b091/nova/objects/instance_numa.py#L111-L112
15:59:53 sean-k-mooney well i guess maybe if we have set that internally
16:00:02 sean-k-mooney i still need to find that code
16:03:06 huaqiang sean-k-mooney: in hardware.py I'll find it for yo
16:04:56 huaqiang https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1860
16:06:07 sean-k-mooney thanks ok and https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1840-L1843 is where we block the mixed case
16:06:22 sean-k-mooney so the transformation happens in the api
16:07:13 sean-k-mooney numa_get_constraints is invoked form teh api to validate teh request and it si used when constuction teh numa toplogy object that is passed to the schduler
16:07:37 huaqiang it will be permitted for mixed case
16:08:09 sean-k-mooney yes so basicaly at line 1840 you will replace that with a check that will set the polciy to mixed internally
16:08:17 openstackgerrit Iury Gregory Melo Ferreira proposed openstack/nova master: Add ironic hypervisor doc https://review.opendev.org/701998
16:08:28 sean-k-mooney then make the if on line 1845 an elif
16:08:44 sean-k-mooney so we only take it if its a non mixed case
16:13:04 huaqiang https://review.opendev.org/#/c/696009/1/nova/virt/hardware.py
16:13:19 huaqiang this is my poc code for this part
16:21:44 huaqiang sean-k-mooney, efried: It is too late for me, I'll go to sleep, and I caught the current idea is using an interal policy 'mixed' in code path, but without exposing it to end user for using in flavor or image properties.
16:27:24 sean-k-mooney huaqiang: good night o/
16:31:11 efried huaqiang: to be clear, I'm saying don't *require* it from the end user. I would prefer not to expose it, but if doing so (as optional) makes something significantly easier, I won't object to it.
17:06:00 openstackgerrit Balazs Gibizer proposed openstack/nova master: Remove extra instance.save() calls related to qos SRIOV ports https://review.opendev.org/702261

Earlier   Later