| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-26 | |||
| 18:19:55 | mriedem | dansmith: that reminds me, was it you that didn't get my humpty dance reference while we were strolling through the death camp? | |
| 18:20:17 | mriedem | https://www.youtube.com/watch?v=PBsjggc5jHM | |
| 18:20:50 | adrianc | sean-k-mooney: to which source claim are you referring ? | |
| 18:21:11 | dansmith | mriedem: I knew you were talking about this song, I just didn't get how it related to, uh, mass murder | |
| 18:21:28 | mriedem | i don't remember | |
| 18:22:57 | artom | mriedem, you can pass the host to drop_move_claim | |
| 18:22:59 | adrianc | sean-k-mooney: if you mean removing the free_instance_allocations() in _post_live_migration then yes, but it will be another place we rely on the periodic resource_tracker job | |
| 18:23:32 | artom | So even if we call it from the source, we could drop the claim on the... wait, if we call it on the source, we just drop the resources on the source, since the instance is on the dest | |
| 18:24:19 | adrianc | artom: Hi, in regards to numa aware live migration, the plan is to converge for stein right ? as the SRIOV live migration will not mean much without it | |
| 18:24:28 | sean-k-mooney | adrianc: in option 1 we would have 2 vfs calimed with the same pci request uuid so if we do that i was wondering if we can avaoid relying on the periodic heal and proactivly release the vf on the source when we then migration completes | |
| 18:24:56 | sean-k-mooney | adrianc: well the sriov migration should be doable without the numa one | |
| 18:25:18 | mriedem | artom: see | |
| 18:25:53 | adrianc | sean-k-mooney: in the PS i am freeing the instance allocation on the source node. | |
| 18:26:52 | sean-k-mooney | adrianc: i ok ill read what you are currently doing and then ill respond to the question on the spec or updated it to match what you have implmeneted | |
| 18:27:20 | adrianc | sean-k-mooney: unless you request dedicated CPUs right ? (previous comment) | |
| 18:28:20 | sean-k-mooney | adrianc: yes but wwe shoudl treat these as seperate specs and seperate work items but makes sure they both work togeter in the end | |
| 18:28:38 | artom | mriedem, wait, so the cell conductor isn't involved at all? It's just superconductor and the source and dest? | |
| 18:28:55 | adrianc | sean-k-mooney: i agree, they do not depend, i was just wondering if its planned for stein as well :) | |
| 18:28:55 | sean-k-mooney | adrianc: in the simple case fo a floating instace with neutron sriov interface there is no numa affinity or numa topology for the guest | |
| 18:29:33 | sean-k-mooney | adrianc: the numa aware migration is proably more impactful to land in stien then sriov but hopefully both can land | |
| 18:31:24 | mriedem | artom: yes | |
| 18:31:41 | mriedem | superconductor orchestrates everything to find the correct dest host, then kicks things off with an rpc cast to the source | |
| 18:31:50 | mriedem | and then source/dest computes just rpc back and forth | |
| 18:32:00 | mriedem | there is no reschedule or anything within the cell conductor for live migration | |
| 18:32:00 | artom | OK, I need to eat, but I think I'm starting to understand the problem you're explaining, mriedem. Namely: we can't keep a claim context going, so... I guess we'll need to shove it in the migration context, like with cold migration? | |
| 18:32:13 | mriedem | i guess? | |
| 18:32:25 | mriedem | the instance.migration_context is still a bit of a mystery to me | |
| 18:32:29 | mriedem | but i also need to eat | |
| 18:32:34 | artom | You and everyone else | |
| 18:32:40 | mriedem | dansmith: comments on the numa live migration spec which maybe you can answer, | |
| 18:32:43 | mriedem | re: move claims | |
| 18:32:57 | artom | I think if Nikola came back today, he'd still know more than all of us combined | |
| 18:33:15 | mriedem | on that very hairy part of the code? i agree. | |
| 18:33:28 | mriedem | there are also TODOs in there from him about the move claim stuff for reize | |
| 18:33:29 | mriedem | *resize | |
| 18:34:04 | mriedem | https://github.com/openstack/nova/blob/594c653dc1a312d0364ad24c703e1a9b228133e1/nova/compute/manager.py#L3988 | |
| 18:34:41 | mriedem | anyway, turkey leftovers | |
| 18:36:05 | sean-k-mooney | mriedem: when you are back maybe you could weigh in on https://review.openstack.org/#/c/605116/6/specs/stein/approved/libvirt-neutron-sriov-livemigration.rst@111 also. | |
| 20:59:01 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove ironic/pike note from *_allocation_ratio help https://review.openstack.org/620154 | |
| 21:15:34 | mnaser | so while answering an ML post about all_tenants and friends, i found this TODO since 2015 -- https://github.com/openstack/nova/commit/be41910ac6be28060d9007778fb33766077de59b | |
| 21:15:51 | mnaser | do we just drop that part of the code at that point? given it's been uncommented for years now | |
| 21:19:40 | artom | mriedem, does the superconductor really rescheduler if the live migrations fails? I'm looking but I can't find anything in _execute, and in the conductor manager if there's a failure in _live_migrate it just sets an error. | |
| 21:20:00 | artom | Not sure it's super relevant to the spec, but for my own personal edification | |
| 21:21:39 | mriedem | mnaser: you mean drop it at this point? | |
| 21:22:19 | mnaser | mriedem: i think so? i mean it's just dead code for 5 years, do we want to muck around with microversion bumps and blah | |
| 21:23:25 | mriedem | mnaser: tbc, you're saying just ditch the commented out code since no one cares enough to change it with a new microversion | |
| 21:23:39 | mnaser | yep | |
| 21:24:29 | mriedem | shrug | |
| 21:24:33 | mriedem | i don't see anyone caring enough to change it | |
| 21:24:54 | mriedem | the fact you have to supply the all_tenants parameter to filter on project_id does always confuse me | |
| 21:25:00 | mnaser | well | |
| 21:25:03 | mriedem | but at least it's documented in the api-ref | |
| 21:25:03 | mnaser | the clients workaround it now.. | |
| 21:25:13 | mnaser | pretty sure you dont need to do that anymore with the cli | |
| 21:25:40 | mriedem | https://docs.openstack.org/python-novaclient/latest/cli/nova.html#nova-list | |
| 21:25:48 | mriedem | nova list --tenant will just implicitly add --all-tenants | |
| 21:25:59 | mriedem | if that's what you mean by workaround | |
| 21:26:20 | mnaser | yeah | |
| 21:26:41 | mnaser | https://github.com/openstack/python-openstackclient/blob/master/openstackclient/compute/v2/server.py#L1147-L1153 | |
| 21:26:44 | mnaser | same for osc | |
| 21:27:20 | mriedem | if you want to push a patch to remove the cruft, fine by me | |
| 21:27:23 | mriedem | i might even +2 that | |
| 21:27:52 | mriedem | anything to make that method smaller b/c god is it long | |
| 21:35:23 | mriedem | bnemec: have you ever heard of requests for something like a PostitiveIntOpt or PositiveFloatOpt in oslo.config? we have some options which can be set to 0.0 as the min, but really shouldn't be <= 0. | |
| 21:35:32 | mriedem | but we can't really describe that with just min | |
| 21:37:27 | dansmith | choices=range(1,1000) ? :P | |
| 21:37:36 | bnemec | mriedem: So an opt where min is a < comparison instead of a <=? | |
| 21:38:21 | mriedem | something like that | |
| 21:38:28 | mriedem | for context https://review.openstack.org/#/c/602804/9/nova/conf/compute.py | |
| 21:38:36 | mriedem | initial_cpu_allocation_ratio should never be 0.0 | |
| 21:38:39 | dansmith | we really just want a validation function parameter, right? | |
| 21:38:43 | mriedem | yeah | |
| 21:38:44 | dansmith | we wanted that for something else recently | |
| 21:38:53 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Drop cruft code for all_tenants behaviour https://review.openstack.org/620165 | |
| 21:38:53 | dansmith | validator=lambda str: ... | |
| 21:41:19 | bnemec | A validator callback seems like something we could do. | |
| 21:41:32 | bnemec | Alternatively, in this case min=0.000001 is probably also sane. | |
| 21:46:16 | bnemec | You could also create a custom type that did the validation in the constructor. | |
| 21:46:44 | bnemec | Subclass Float and put whatever logic you need in there: https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L409 | |
| 21:47:14 | mriedem | I wasn't sure how kosher subclassing oslo.config opt types was | |
| 21:47:16 | bnemec | Then create the opt as Opt(type=MyCustomType, ...). | |
| 21:47:55 | bnemec | They're part of the public API so I'd say they're fair game. | |
| 21:49:20 | mriedem | ok yeah that's probably cleanest | |
| 21:49:25 | bnemec | They danger might be creating a completely new class as a type, which could theoretically be done, but if we ever added to the type API you might get broken. | |
| 21:49:32 | bnemec | As long as you inherit from an existing type you should be okay though. | |
| 21:49:57 | bnemec | They all descend from a single ABC. | |
| 21:50:53 | mriedem | the adam of config opts? | |
| 21:51:14 | bnemec | Indeed. | |
| 21:51:30 | bnemec | Also, I think I was wrong. You want to do validation in __call__. | |
| 21:51:40 | bnemec | That's where we're doing it in the existing types: https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L830 | |
| 21:52:19 | mriedem | ah yeah https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L305 | |
| 21:52:51 | bnemec | Yeah, better example. :-) | |
| 22:03:19 | mriedem | welp https://review.openstack.org/#/c/613126/ kind of blows up https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/initial-allocation-ratios.html#manually-set-placement-allocation-ratios-are-overwritten | |
| 22:03:23 | mriedem | how am i not surprised | |
| 22:05:21 | mriedem | efried: you'll have some context on ^ | |
| 22:06:52 | mriedem | i'm not exactly sure what we should do about it, outside of passing an initial flag to update_provider_tree or something gross like that...although maybe upt can check the provider tree to see if it already has inventory with allocation_ratio set, and if so, don't provide a value unless CONF.*_allocation_ratio is not None | |
| 22:09:27 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Give drop_move_claim() correct docstring https://review.openstack.org/620170 | |
| 22:09:43 | artom | mriedem, ^^ related to the numa live migration spec. I *think* I'm right, and it might clarify the confusion around where we're removing usages. | |
| 22:10:31 | mriedem | artom: totally forgot you pinged me earlier, sec | |
| 22:10:52 | mriedem | artom: superconductor does not reschedule if live migration fails, no | |