Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-16
15:56:56 bauzas dansmith: I see your point and I honestly have to double check
15:57:11 sean-k-mooney oh you were asking about the allcotaion fo the mdevs
15:57:27 dansmith sean-k-mooney: yes
15:57:38 bauzas dansmith: found why
15:57:40 sean-k-mooney ya it should get the exising ones and reuse them
15:57:45 bauzas dansmith: sec, giving you the link
15:59:03 gibi nova meeting starts in 2 minutes on #openstack-meeting-3
16:15:02 melwitt lbragstad: hey, would you be able to take a gander at the proposed doc patch for policy scopes https://review.opendev.org/#/c/720129/4/doc/source/configuration/policy-new-defaults.rst we want to have it clearly explain what are the action items for operators to be able to use scope types. i.e. will it involve creating new keystone roles and stuff like that
16:15:34 lbragstad melwitt sure - i can take a look
16:15:50 melwitt thanks
16:21:14 gmann melwitt: quota class policy is ready, fixed the johnthetubaguy comment of removing the non-working 'quota_class' from check_str - https://review.opendev.org/#/c/720042/1
16:21:18 gmann and its base pacth
16:21:56 melwitt gmann: ok, I wanted johnthetubaguy to look at it first and make sure he's ok with it since he found issue earlier
16:22:09 gmann melwitt: ok
16:32:39 gmann melwitt: thanks for nice feedback on doc patch, I will wait for other comments and update it together.
16:33:22 melwitt gmann: cool sounds good
16:54:38 bauzas dansmith: hah, found why it wasn't leaving a VGPU allocated when you revert https://github.com/openstack/nova/blob/e1359567e4985e9a671359d4c0d53404a8ba64ab/nova/virt/libvirt/driver.py#L6359
16:54:50 bauzas dansmith: we redefine the existing domain here ^
16:56:44 dansmith I don't think that answers my question
16:57:49 dansmith when we delete a domain, do we unallocate the mdev, or do we just leave it so the next spawn that needs one will find it?
17:00:17 bauzas dansmith: when we undefine the domain, the mdev is consider being freed
17:00:41 dansmith bauzas: "considered free" because it's not assigned to any other domain, but still allocated in sysfs yeah?
17:00:45 sean-k-mooney tframbo: so first the namespace should be hw not os
17:00:52 bauzas dansmith: yup, you're correct
17:01:22 sean-k-mooney that extra specs as you know alter the cpu feature flags reported to the guest and hide the fact its a vm form guest os
17:01:24 tframbo so it is add compat for "hw:hide_hypervisor_id”
17:01:25 tframbo ?
17:01:51 sean-k-mooney yes so we woudl deprecate hide_hypervisor_id and add "hw:hide_hypervisor_id”
17:02:18 sean-k-mooney make the diver accept both and then we could special case hide_hypervior_id so it is ignored
17:02:45 bauzas sean-k-mooney: wait, like I said, the bug is about a scheduling decision
17:02:51 bauzas sean-k-mooney: not a boot problem
17:02:58 sean-k-mooney you would use aggregate_instance_extra_specs:hide_hypervior_id in the flavor if you wanted to match on it in that filter
17:03:04 bauzas OVH and a couple of other folks already use this knob
17:03:13 bauzas which was merged a while ago
17:03:36 sean-k-mooney bauzas: yes i know but the current filter check all unamesapced extra specs and assert they are in the metadta
17:03:40 bauzas but here, what's wanted is to trick the scheduler to automatically accept hosts on such things
17:04:13 bauzas sean-k-mooney: I explained this can be achieved without amending with this filter
17:04:36 sean-k-mooney how
17:04:46 bauzas tbc : create a flavor with a random keypair
17:05:07 bauzas create aggregates with hosts having the same keypair in their metadata
17:05:11 bauzas and you're done
17:05:26 bauzas you don't need to use this specific property
17:05:44 sean-k-mooney your missing the point
17:06:00 bauzas which is ?
17:06:19 sean-k-mooney we cant remove hide_hypervisor_id form the current flavor without changing the behavior
17:06:44 sean-k-mooney and if it si set in the falvor then the AggregateInstanceExtraSpecsFilter will check it against the aggreate metadata
17:07:19 dansmith bauzas: so my point is, when we revert, we will allocate a second mdev, and then abandon the original one when we redefine.. but I guess like delete, that one then will get used the next time we go to spawn something that needs one
17:07:24 sean-k-mooney we will return false here https://github.com/openstack/nova/blob/46a3bcd80b41e99ec4923c7cf3d0f8dd8505e97c/nova/scheduler/filters/aggregate_instance_extra_specs.py#L62-L68
17:07:58 sean-k-mooney tframbo: the other workaround is to add the metadata key to the aggreate
17:08:22 bauzas dansmith: you're probably correct, we could have a window in time where we would allocate a new mdev and then releasing the other
17:08:51 bauzas which could lead to a capacity issue if you're out of available mdevs
17:08:59 sean-k-mooney tframbo: basically create two two new agggreats that conatin all hosts, then add hide_hypervisor_id=true to one and hide_hypervisor_id=false to the other
17:09:31 bauzas dansmith: s/probably/absolutely after re-thinking of it
17:09:35 dansmith bauzas: right
17:09:39 sean-k-mooney tframbo: if you do that we will get to here https://github.com/openstack/nova/blob/46a3bcd80b41e99ec4923c7cf3d0f8dd8505e97c/nova/scheduler/filters/aggregate_instance_extra_specs.py#L69-L71 and break then return ture and pass the host
17:10:07 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add new default roles in remaining servers policies https://review.opendev.org/720116
17:10:10 openstackgerrit Merged openstack/nova master: Fix servers policy for admin_or_owner https://review.opendev.org/718501
17:10:11 bauzas dansmith: this being said, I don't exactly know how to fix this, since this is the whole revert workflow
17:10:26 openstackgerrit Merged openstack/nova master: Add test coverage of existing server policies https://review.opendev.org/718348
17:10:32 bauzas and I guess the problem remains for all other resources
17:10:38 openstackgerrit Merged openstack/nova master: Introduce scope_types in servers Policies https://review.opendev.org/662968
17:10:48 openstackgerrit Merged openstack/nova master: Add new default roles in servers policies https://review.opendev.org/662971
17:10:55 openstackgerrit Merged openstack/nova master: Add test coverage of existing server attributes policies https://review.opendev.org/719726
17:11:04 openstackgerrit Merged openstack/nova stable/train: Reset the cell cache for database access in Service https://review.opendev.org/718934
17:11:16 openstackgerrit Merged openstack/nova stable/stein: Use placement stable version for functional job https://review.opendev.org/719121
17:11:51 bauzas dansmith: we could consider to special case the revert and find the existing mdev so we would pass it again instead of calling _allocate_mdevs() tho
17:12:05 dansmith bauzas: seems like that would be better
17:12:11 dansmith also avoids needing to grab the allocations right?
17:12:22 bauzas indeed
17:12:25 bauzas dansmith: <3 with love
17:12:44 bauzas that's what I call a top-notch review
17:15:33 sean-k-mooney tframbo: does my responce on the bug or above make sense too you
17:15:42 tframbo sean-k-mooney: yeah, we could also add the aggregate metadata "hide_hypervisor_id=true" to solve the bug.
17:16:02 sean-k-mooney yes
17:16:13 tframbo thank you sean-k-mooney and bauzas
17:16:23 sean-k-mooney do you also use the compute capability filter by the way
17:16:33 sean-k-mooney if not be aware it has the same behavior
17:16:52 tframbo I think the hide_hypervisor_id is bool before , sorry
17:17:57 sean-k-mooney tframbo: its is a bool that default to false if unset
17:18:11 sean-k-mooney so there isnt really a reason to ever set it to false explcitly
17:18:16 bauzas tframbo: sorry I dropped focus
17:18:21 sean-k-mooney unless you want to prevent images form settin git too true
17:20:58 bauzas sean-k-mooney: tframbo: so, again, the intent is to have the aggregate metadata key to be set
17:21:18 tframbo sean-k-mooney: yes ,we alse use the compute capability filter by the way
17:21:36 tframbo bauzas: yes
17:22:19 bauzas I honestly don't get the problem
17:22:20 sean-k-mooney tframbo: ok so the point i wanted to make in that case was that if you use both then you are required to not use unnamespaced keys or they can conflict
17:23:10 bauzas wait
17:23:26 bauzas the namespace is only for specifying that the key is for this filter
17:23:43 bauzas I just feel it's a configuration problem
17:23:43 sean-k-mooney bauzas: the problem is the AggregateInstanceExtraSpecsFilter require that an aggreate assocaited with a host must have all extra specs in the aggregate_instance_extra_specs as well as all non namespaced extra specs in the metadta
17:24:25 bauzas or prefixed by aggregate_instance_extra_specs https://github.com/openstack/nova/blob/46a3bcd80b41e99ec4923c7cf3d0f8dd8505e97c/nova/scheduler/filters/aggregate_instance_extra_specs.py#L54
17:24:29 bauzas and this is not a bug
17:24:30 sean-k-mooney bauzas: the filter also looks at non namespced extra specs and we only have 1 standard extra specs that falls into that catagory
17:25:09 bauzas this *works* with any random key
17:25:17 bauzas I don't get the point again
17:25:36 bauzas if you're using this flavor key for passing QEMU, fine
17:25:47 bauzas that's how it's intended to be used
17:25:53 sean-k-mooney hide_hypervisor_id is not a random custom key
17:25:59 bauzas but then, indeed, you need to have aggregates matching it

Earlier   Later