Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-16
15:40:17 alex_xu gibi: yea, I will let you know whatever the result coming out
15:40:46 openstackgerrit Lee Yarwood proposed openstack/nova master: images: Make JSON the default output format of calls to qemu-img info https://review.opendev.org/711679
15:40:47 lyarwood ^ now with added unit tests, thanks again all.
15:45:18 sean-k-mooney bauzas: for the revert case we have not undeifed the domain on the source node when we trigger revert
15:46:02 sean-k-mooney so im not sure the allocation are truly needed as we should not need to regenerate the xml
15:46:39 sean-k-mooney that said we might do anyway depending on how we restart the vm
15:46:55 bauzas sean-k-mooney: we regenerate the XML https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L10259
15:47:04 sean-k-mooney if we just call power on it call hard reboot which would
15:48:03 sean-k-mooney ya ok so that is why we need it in this case.
15:48:45 bauzas yuuuuuup
15:49:02 bauzas that does sound crazy, but we blindy recreate the guest when reverting
15:49:21 bauzas sean-k-mooney: see my functtest, you'll see evidence of it
15:49:27 sean-k-mooney im not sure if we really need to regenerate the xml in that case i cant think of why but lets not get distracted by that for now
15:49:28 dansmith sean-k-mooney: right that was my exact concern
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

Earlier   Later