Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-16
15:50:04 sean-k-mooney oh i know why i think
15:50:08 dansmith where does allocate_mdevs persist? does it look at all the other mdevs currently being used?
15:50:24 dansmith meaning, when we undefine the original and re-define with a new one, will the old one be available again, or might we leak something?
15:50:34 sean-k-mooney we regenerate the xml becasue snapshots
15:50:58 sean-k-mooney this code https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L10249-L10254
15:51:09 bauzas dansmith: mdevs aren't persisted
15:51:34 bauzas mdevs can be even pre-provisioned
15:51:39 dansmith sean-k-mooney: also maybe because you can attach a volume to the new one before revert
15:51:54 dansmith bauzas: how do we select one that isn't in use? that's what I'm getting at
15:51:59 bauzas "allocating a mdev" just means modifying the guest XML domain to use a specific sysfs object
15:52:05 bauzas dansmith: oh this, sec
15:52:16 dansmith right, I know, but we need to make sure we hand it one not in use
15:52:38 bauzas dansmith: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7206
15:52:59 dansmith bauzas: so we create a new one in sysfs?
15:53:17 sean-k-mooney ya i guess that is ture although im realy not sure why we allow volume or interface attach in resize_verify or whatever that state is called
15:53:23 dansmith bauzas: how does the old one get reaped when we redefine?
15:53:27 bauzas dansmith: which is made by comparing all the existing mdevs and the ones that are in use by each domain
15:53:28 bauzas https://github.com/openstack/nova/blob/e1359567e4985e9a671359d4c0d53404a8ba64ab/nova/virt/libvirt/driver.py#L7088-L7102
15:53:32 sean-k-mooney but given that we do we cant assume the current xml is correct
15:53:36 dansmith sean-k-mooney: we do, but it's crazy
15:54:16 dansmith bauzas: right so when we revert, we'll see that our original one is in use (by us) and allocate a new one right?
15:54:51 bauzas dansmith: during the resize, before the confirm, we have a doubled allocation of mdevs
15:55:02 bauzas (and a double placement allocation)
15:55:27 bauzas dansmith: when we revert, the target mdev allocation disappears because we undefine the domain
15:55:35 dansmith bauzas: but that doubling is on two different hosts
15:55:38 sean-k-mooney https://github.com/openstack/nova/blob/e1359567e4985e9a671359d4c0d53404a8ba64ab/nova/virt/libvirt/driver.py#L6349
15:56:04 sean-k-mooney it look like the domain creation fucntion will overwrite existing domains
15:56:18 dansmith bauzas: I'm talking about when we revert, don't we hit the "no free mdevs, allocate a new one" in _allocate_mdevs() since our original mdev is still in use by the libvirt domain we're about to redefine?
15:56:20 bauzas dansmith: yeah https://review.opendev.org/#/c/712741/2/nova/tests/functional/libvirt/test_vgpu.py@224
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

Earlier   Later