| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 14:47:04 | openstackgerrit | sahid proposed openstack/nova master: libvirt: move version to string in utils https://review.openstack.org/560455 | |
| 14:47:05 | openstackgerrit | sahid proposed openstack/nova master: libvirt: refactor get_base_config to accept host arg https://review.openstack.org/560456 | |
| 14:47:21 | efried | mriedem: Reviewed. | |
| 14:48:01 | mriedem | thanks | |
| 14:48:07 | efried | bhagyashris, jaypipes: Is there a bug for the double-reporting of DISK_GB? | |
| 14:50:08 | bhagyashris | efried: Don't know exactly. I have just fetch your patch https://review.openstack.org/560444 and checked inventory table records | |
| 14:50:26 | efried | bhagyashris: Oh - does it work?? | |
| 14:51:00 | bhagyashris | efried: but still it showing the DISK_GB record in inventory table as i configured the shared resource provider | |
| 14:51:17 | efried | bhagyashris: Yup, that's expected. The above patch should have been a straight swap. | |
| 14:51:20 | efried | bhagyashris: But this one.... | |
| 14:51:24 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: libvirt: Don't report DISK_GB if sharing https://review.openstack.org/560459 | |
| 14:51:28 | efried | jaypipes: bhagyashris ^ | |
| 14:51:41 | efried | ...should do the thing we were talking about. | |
| 14:51:53 | efried | bhagyashris: Since you seem to be set up to try this easily, would you mind...? | |
| 14:51:55 | openstackgerrit | Merged openstack/nova master: api-ref: Parameter verification for servers.inc (2/3) https://review.openstack.org/550648 | |
| 14:55:17 | openstackgerrit | Merged openstack/nova master: api-ref: Parameter verification for servers.inc (3/3) https://review.openstack.org/550659 | |
| 14:56:53 | bhagyashris | efried: so it's not working so don't know why it's not | |
| 14:57:24 | bhagyashris | efried: let me check with this patch change https://review.openstack.org/560459 | |
| 14:58:51 | openstackgerrit | Merged openstack/nova master: api-ref: Fix parameter order in rebuild https://review.openstack.org/529718 | |
| 15:11:03 | bhagyashris | efried: we will need to use that update_provider_tree() method in https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L890 | |
| 15:11:35 | efried | bhagyashris: Yup, see https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L881 | |
| 15:11:51 | bhagyashris | efried: so that it will get the proper inv_data and will update here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L892 | |
| 15:12:31 | efried | bhagyashris: Is it not getting invoked? | |
| 15:12:46 | efried | (Did you remember to restart your compute service? :) | |
| 15:13:03 | bhagyashris | yes i restart n-cpu | |
| 15:14:28 | bhagyashris | but it's giving problem in second patch now that the provider_tree has no attribute data | |
| 15:14:35 | efried | bhagyashris: Well, there *is* no get_inventory in the libvirt driver anymore, so if you're seeing inventory at all, it must have come from here. | |
| 15:15:03 | efried | bhagyashris: I didn't run any of this code, even in a unit test context, so I very well may have effed something up. Are you seeing an exception? | |
| 15:16:48 | bauzas | efried: jaypipes: thanks for the early reviews of https://review.openstack.org/#/c/552924/7 | |
| 15:17:14 | bauzas | efried: jaypipes: I think I need to understand more how numbered request groups would work | |
| 15:17:20 | bhagyashris | efried: I saw this exception at n-cpu Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager [None req-b17184a8-7d31-4c3d-9b8b-d1f1ddb6860e None None] Error updating resources for node openstack-VirtualBox.: AttributeError: 'ProviderData' object has no attribute 'data' | |
| 15:17:20 | bhagyashris | Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager Traceback (most recent call last): | |
| 15:17:20 | bhagyashris | Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager File "/opt/stack/nova/nova/compute/manager.py", line 7311, in update_available_resource_for_node | |
| 15:17:22 | bhagyashris | Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager rt.update_available_resource(context, nodename) | |
| 15:17:32 | efried | bhagyashris: okay, fixing... | |
| 15:17:57 | efried | whoops, that was a flub | |
| 15:18:13 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: libvirt: Don't report DISK_GB if sharing https://review.openstack.org/560459 | |
| 15:18:14 | efried | bhagyashris: ^ | |
| 15:20:26 | efried | bauzas: Have you memorized the granular spec? | |
| 15:20:44 | bauzas | efried: tbh, I need to look at it again | |
| 15:21:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: RT: replace _instance_in_resize_state with _is_trackable_migration https://review.openstack.org/560467 | |
| 15:21:09 | mriedem | this was a fun bit of history ^ | |
| 15:21:16 | efried | Resources specified *within* a single numbered request group are *guaranteed* to land on the *same* provider. | |
| 15:21:16 | efried | Resources specified in *different* numbered request groups may or may not land on same/different providers. | |
| 15:21:16 | efried | bauzas: Numbered request groups work like this: | |
| 15:21:42 | bauzas | efried: I thought asking resources1:STH=1&resources2:STH_ELSE=2 would make those two things on separate RPs | |
| 15:21:54 | efried | bauzas: That has been a source of confusion, but no. | |
| 15:21:58 | bauzas | ok, that's the second assumption I made | |
| 15:22:02 | bauzas | I made wrong | |
| 15:22:21 | bauzas | but may or may not is not really a big deal to me | |
| 15:22:31 | efried | bauzas: We will eventually need that semantic - and "deliberate sharding" would be a use case driving it - but we don't have it with the spec as currently written. | |
| 15:22:32 | bauzas | the first assumption is crucial to me | |
| 15:22:55 | bauzas | the second one (different groups leading or not on same RP) is not really important to me | |
| 15:23:09 | bauzas | what I want is affinity, not anti-affinity | |
| 15:23:15 | efried | bauzas: Perfect. | |
| 15:23:36 | efried | bauzas: In case you missed it, I summarized four use cases and what we can/can't do about them. It's at the top part of my review. | |
| 15:23:38 | bauzas | either customers care about NUMA specifics and then numbered req groups is the perfect answer | |
| 15:23:56 | bauzas | or, they don't care about NUMA specifics, and then they don't care where the resources land | |
| 15:23:57 | efried | (I know sometimes those don't get seen because folks dive right into the comments within the file) | |
| 15:24:11 | efried | bauzas: Good | |
| 15:24:11 | bauzas | efried: I saw your top-level comment, yup | |
| 15:24:21 | bauzas | thanks for that, appreciated | |
| 15:24:28 | bauzas | I sometimes do that to justify a -1 | |
| 15:24:41 | bauzas | when for example there are 30 comments but only one means -1 to me | |
| 15:24:57 | efried | bauzas: So the use case you *don't* care about is my third bullet. | |
| 15:25:05 | bauzas | correct | |
| 15:25:16 | bauzas | efried: I gave one example of NUMA affinity for vGPUs | |
| 15:25:23 | bauzas | https://devblogs.nvidia.com/performance-portability-gpus-cpus-openacc/ | |
| 15:25:34 | bauzas | efried: just for your context understanding ^ | |
| 15:25:36 | kashyap | mriedem: When you get a moment, a small update on this: https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bump_min_libvirt_and_qemu | |
| 15:25:56 | efried | bauzas: Right; that's my second bullet. If that's a critical use case, then we need to invent something more. | |
| 15:25:59 | kashyap | mriedem: There's about 14 patches that I need to queue in that drop the compat code. Right now there are only 4 | |
| 15:26:26 | efried | bauzas: Concept of "same subtree" | |
| 15:26:28 | kashyap | mriedem: I am resolving broken tests as I find them. 4 more to go (that are bit more involved, noted them here: https://review.openstack.org/#/c/558783/7) | |
| 15:26:41 | kashyap | mriedem: Finally, a question on unit tests for you here: https://review.openstack.org/#/c/560365/ | |
| 15:26:43 | bauzas | efried: why ? | |
| 15:26:53 | bauzas | efried: VGPU is a standard class | |
| 15:27:04 | bauzas | like VCPU and MEMORY_MB | |
| 15:27:07 | efried | Because VGPU and VCPU are in different resource providers. | |
| 15:27:20 | efried | bauzas: So you can't put them in the same numbered request group. | |
| 15:27:27 | bauzas | efried: that will depend on the operator | |
| 15:27:30 | kashyap | mriedem: Also, I can understand if you are of the view of: "I won't touch them until Zuul is +1 on them, and you've dropped all no-more-relevant compat code" :-) | |
| 15:27:35 | efried | bauzas: And if you put them in different request groups, they aren't guaranteed to land on the same NUMA node. | |
| 15:27:39 | bhagyashris | efried: it's working now :) | |
| 15:28:03 | efried | bhagyashris: Like, fully working? Like the DISK_GB is being reported by the sharing provider and not by the compute node provider? | |
| 15:28:08 | bauzas | efried: but if they want NUMA affinity for VGPUs, then the doc will say : enable the conf opt saying "I want VCPU and VGPU to be NUMA specific" | |
| 15:28:17 | bauzas | efried: in that case, they will be on the same RP | |
| 15:28:25 | bauzas | efried: which will make the thing work | |
| 15:28:39 | mriedem | kashyap: i'd prefer to not dig into those until tests are passing | |
| 15:29:05 | bauzas | efried: we could later add a new param that specifically request the notion of the "subtree" parenting, but I leave that out of that spec then | |
| 15:29:05 | efried | bauzas: It would. But saying that the VGPU inventory has to be on the same RP as the VCPU inventory - that's not a restriction that's going to remain viable for very long. | |
| 15:29:25 | kashyap | mriedem: Sure, that's perfectly fine. I will be able to full spend time on it only from Friday afternoon onwards. | |
| 15:29:39 | bauzas | efried: sure, but I don't want to boil the ocean and keep the NUMA spec minimalist | |
| 15:29:42 | bhagyashris | efried: yes it removes the CN RP DISK_GB inventory from inventory table and now it's reporting the resource against the CN and shared RP :) | |
| 15:29:48 | efried | bhagyashris: WOHOO! jaypipes ^^ | |
| 15:30:11 | bauzas | efried: I can leave a note explaining the caveat, and someone in the future (possibly being me) could write another spec describing the problem and the solution | |
| 15:30:14 | efried | bauzas: Okay, then the spec needs to state that restriction, for sure, cause that's significant. | |
| 15:30:41 | bauzas | efried: that's correct, I'll amend the spec accordingly and explicitely state the limitation, exactly like I did for huge pages | |
| 15:30:51 | bauzas | efried: thanks for your help! | |
| 15:31:02 | bhagyashris | efried: thanks for your inputs :) Now it's EOD for me leaving... | |