Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-09
13:59:24 bauzas artom: stephenfin: https://specs.openstack.org/openstack/nova-specs/specs/queens/implemented/migration-allocations.html
13:59:58 artom stephenfin, other way around, I think
14:00:11 artom By default it always goes to different host
14:00:19 bauzas correct ^
14:00:20 stephenfin artom: yeah, sorry, never land on the _same_ host
14:00:27 artom And then if you set the allow_resize_to_same_host option, it can land on the same host
14:00:30 bauzas unless you modify an option $
14:00:35 bauzas yeah what artom said
14:00:36 artom (Or whatever it's called)
14:00:50 bauzas artom: that is the correct option name
14:02:01 stephenfin weeird
14:02:11 stephenfin there must be something else going on here so
14:02:18 artom stephenfin, so, dunno if this is relevant, but while working on the the NUMA LM func tests, I noticed there was an issue in how you started computes with different fakelibvirt connections for each
14:02:26 stephenfin shoot
14:02:47 artom stephenfin, https://review.opendev.org/#/c/672595/45/nova/tests/functional/libvirt/base.py
14:03:14 artom Again, no idea if that's what's causing your thing now, but could be relevant
14:04:01 stephenfin oh, fun
14:12:55 stephenfin artom: The other approach is this one https://review.opendev.org/#/c/678470/24/nova/tests/unit/virt/libvirt/fakelibvirt.py
14:13:11 stephenfin which I think accomplishes much the same thing
14:13:35 stephenfin there are quite a few in-flight changes with those files atm :)
14:15:09 openstackgerrit Brin Zhang proposed openstack/nova master: Follow-up: Add delete_on_termination to volume-attach API https://review.opendev.org/680655
14:15:28 artom stephenfin, and then you stub that connection into each compute manager I guess?
14:15:53 artom Oh, that's not your patch
14:15:54 artom Hrmm
14:17:02 kashyap aspiers: Any reason you know this method is in virt/hardware.py -- _get_flavor_image_meta()? It can be used by "non-hardware"-related features, too
14:17:15 aspiers kashyap: git blame?
14:17:17 kashyap Maybe a question for stephenfin, as he wrote that :-)
14:17:37 kashyap aspiers: Already did; 48e0c000fefe561ec5cfa1364ef2cac2f6ce9244
14:18:37 kashyap aspiers: stephenfin: Actually, disregard...it makes sense, because of the 'hw' key :D
14:35:19 dansmith mriedem: assume you'll take another trip through the numa patches today? I'm about to start into the forbidden garden of aggregates
14:35:53 bhagyashris mriedem: Hi
14:36:30 mriedem dansmith: i was going over them over the weekend but stopped short of the big one
14:36:38 mriedem waiting on ci results in the special job
14:36:42 dansmith ah cool
14:37:00 mriedem https://review.opendev.org/#/c/635229/ has new wizardry that is going to require stephenfin and/or sean-k-mooney
14:42:33 sean-k-mooney mriedem: is that the patch you had question relating to the xml generation on
14:44:36 sean-k-mooney mriedem: if there is a specific thing you wanted me to look at i can do it quickly otherwise im going to finish with stephenfin series then start on artoms
14:45:46 artom sean-k-mooney, the new allocation_cpu_tuning, to cover the case when an instance with no NUMA topology is on a host with vcpu_pin_set
14:45:59 sean-k-mooney ah right
14:46:16 sean-k-mooney this https://review.opendev.org/#/c/635229/60/nova/virt/libvirt/migration.py@101
14:46:21 artom So actually the field is new in https://review.opendev.org/#/c/634827/
14:46:47 artom sean-k-mooney, well, PS61 is the latest but yeah
14:47:20 dansmith efried: shilpasd: I found something else on the base patch for forbidden aggregates
14:47:45 dansmith I'm pretty sure we don't deepcopy() the default value, which is why I did default=None for the "aggregates" field right above it.
14:48:11 dansmith either way, it should be consistent I think, and I expect =None is more clearly safe than a default of something mutable (like mutable default arguments for methods in python)
14:48:41 sean-k-mooney mriedem: artom so the context for this is that since we added the vcpu_pin_set option, if you have used it we have relied on operators ensuring that all host in the same host aggrate/availablity zone (depending on how you choose to defien the region you can live migrate vm within) has vcpu_pin_set set to the same value
14:49:13 shilpasd dansmith: Thanks for review
14:49:56 dansmith shilpasd: does my comment make sense?
14:50:10 dansmith shilpasd: also, if you're not able to make this change right away I could probably just do it myself
14:50:18 sean-k-mooney mriedem: more specifically if you define vcpu_pin_set we copy the value into <vcpu cpuset=vcpu_pin_set> for all instnace but we never updated that on live migration
14:50:39 shilpasd dansmith: yes, your comment make sense, will have test cases with default config (i.e. no filter remove or append) and with remove and with append
14:50:53 sean-k-mooney so since artom is fixing it for numa instance we asked him to fix non numa instance too
14:51:03 mriedem sean-k-mooney: the config option help for that says nothing about that assumption/requirement https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.vcpu_pin_set
14:51:03 stephenfin TIL 'functional.libvirt.base.ServersTestBase' uses API v2.0 by default. That's two hours I won't get back \o/
14:51:30 dansmith shilpasd: oh you're talking about the later one
14:51:40 dansmith shilpasd: I +2d that already, so don't change it, and/or change it in a followup
14:51:53 dansmith shilpasd: I _just_ left a more complicated comment on the base patch, so go look at that real quick
14:52:00 sean-k-mooney mriedem: that is the entire orginal reason it was intoduced. to define the set of cpu that guest can run on. it was not just for pinned/numa instance
14:52:43 mriedem sean-k-mooney: ok....that doesn't mean much to me since i'm not familiar with when or why that was added,
14:52:50 mriedem i'm just saying, "if you have used it we have relied on operators ensuring that all host in the same host aggrate/availablity zone (depending on how you choose to defien the region you can live migrate vm within) has vcpu_pin_set set to the same value" is not documented
14:53:07 mriedem so undocumented assertions are kind of....hairy
14:53:31 artom mriedem, yep, which why with this new allocation_cpu_tuning stuff, it won't be necessary
14:53:47 artom Previously we hoped both dest and source had it set the same for LM to work
14:53:55 artom Now we handle them being different
14:53:56 mriedem i see there is a todo about it in https://docs.openstack.org/nova/latest/user/flavors.html?highlight=vcpu_pin_set
14:54:08 sean-k-mooney mriedem: ah ok its not documented upstream correct. it used to be documented in intels referece architecure as the way we advised people to partition cpu for the host/guest
14:54:47 openstackgerrit Stephen Finucane proposed openstack/nova master: Add reshaper for PCPU https://review.opendev.org/674895
14:55:24 sean-k-mooney mriedem: the behavior if you did not have them the same was that provide all the cores existed on the destinatinon, the migration would suceeed but the vm would use the wrong cores until it was hard rebooted
14:55:39 sean-k-mooney mriedem: if the core did not exist on the dest host then libvirt would reject the migration
14:55:59 sean-k-mooney so it was a self correcting problem once the guest hard rebooted
14:56:16 mriedem super
14:56:34 stephenfin mriedem: can we easily mutate policy in tests?
14:56:38 mriedem so tl;dr artom's series fixes that now so you don't have to have strict isolation and config in aggregates now?
14:56:41 mriedem stephenfin: yes
14:56:48 sean-k-mooney mriedem: yes
14:56:52 mriedem stephenfin: check functional tests thta use the PolicyFixture
14:56:57 stephenfin sweet. ta
14:57:05 artom mriedem, for that particular option, anyways
14:57:15 artom (vcpu_pin_set)
14:57:17 mriedem stephenfin: e.g. self.policy_fixture.set_rules({
14:57:17 mriedem servers_policies.ZERO_DISK_FLAVOR: base_policies.RULE_ADMIN_API},
14:57:17 mriedem overwrite=False)
14:58:39 shilpasd dansmith: yes cleared, will address for now 'complicated comment on the base patch'
14:59:11 dansmith heh okay
15:14:44 openstackgerrit Merged openstack/python-novaclient master: Microversion 2.78 - show server topology https://review.opendev.org/670790
15:15:41 sean-k-mooney artom: mriedem by the way we will need the <vcpu cpuset> xml update code for stephens pcup series too
15:16:16 sean-k-mooney so this is generally good to have anyway. it would be needed for non pinned guests when cpu_share_set is defiended
15:16:41 mriedem i don't really know anything about that series or how it's going to work but ok
15:16:53 sean-k-mooney *when cpu_shared_set is defined to different value on different hosts
15:17:37 sean-k-mooney mriedem: well we are just using cpu_shared_set instead of vcpu_pinned_set for the same behavior
15:17:52 sean-k-mooney e.g. defineing the set of core floating vms can float over
15:21:28 mriedem so do we expect instances using PCPU resources to be able to live migrate in Train with that series?
15:22:10 sean-k-mooney yes
15:22:57 sean-k-mooney if both seris merge we expect to be able to live migrate with PCPUs or VCPUs and have the xmls updated correctly in both cases
15:23:06 bauzas mriedem: this series will only support PCPU allocations once *all* computes are Train
15:23:22 sean-k-mooney mriedem: we defered instance have both pcpus and vcpus at the same time to U or later
15:23:27 bauzas mriedem: once the operator modifies an option
15:24:12 sean-k-mooney bauzas: correct. all of this is opt in. if you dont modify the configs for the PCPU seriese then you keep the old behavior
15:24:53 sean-k-mooney which is what we need for upgrades. and hopeful in U we can start enableing thing by defualt before removing the legacy options in V
15:25:10 mriedem bauzas: that wasn't what i asked

Earlier   Later