| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 14:14:04 | efried | yeah, that. | |
| 14:14:18 | jaypipes | in any case... | |
| 14:14:27 | efried | So anyway, I'm saying I'm on board with the idea if we state it like this: | |
| 14:15:28 | efried | Because libvirt (somehow) knows that the storage it's looking at is shared, libvirt's impl of update_provider_tree shall remove the DISK_GB inventory from the compute node RP. | |
| 14:15:51 | efried | Perhaps the (somehow) is signalled by the operator having created the sharing provider and associated it via aggregate with the compute node. | |
| 14:16:12 | efried | libvirt's u_p_t will look for that and, if found, nix the compute RP's DISK_GB inventory. | |
| 14:16:18 | efried | Does that gel with your thinking? | |
| 14:16:44 | efried | zero part of this involves nova code outside of the libvirt driver. | |
| 14:17:41 | jaypipes | efried: that is precisely my thinking, yes. | |
| 14:18:17 | efried | jaypipes: Cool beans. bhagyashris, are you on the hook for proposing that code? I can help you understand how to make it work. | |
| 14:19:12 | bhagyashris | efried: ok means the CN RP DISK_GB inventory will be remove if the shared resource provider is configured | |
| 14:19:55 | efried | bhagyashris: Yes. The part I was unclear on at first was that that would be done within the libvirt compute driver's implementation of update_provider_tree. | |
| 14:20:05 | efried | ...which I bet hasn't been proposed yet. | |
| 14:21:57 | mriedem | fyi if you see test_resize_with_reschedule_then_live_migrate failing in functional tests, it's https://bugs.launchpad.net/nova/+bug/1762876 | |
| 14:21:57 | openstack | Launchpad bug 1762876 in OpenStack Compute (nova) "test_resize_with_reschedule_then_live_migrate intermittently failing; migration is not yet complete" [High,Confirmed] - Assigned to Matt Riedemann (mriedem) | |
| 14:21:58 | mriedem | i'm working on a fix | |
| 14:22:07 | mriedem | apparently the instance status goes to ACTIVE before the migration is completed | |
| 14:24:14 | bhagyashris | efried: ok. so for this is there any bp or bug reported? Actually i am not fully aware about update_provider_tree part but i will start to understand that part and will get to know | |
| 14:24:57 | bhagyashris | efried: I just tested few things and put my observations and analysis | |
| 14:25:03 | efried | bhagyashris: There's a blueprint around update_provider_tree. It's substantially completed, and ready for virt drivers to start using it. | |
| 14:25:13 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Make NetworkAPI an abstract base class https://review.openstack.org/560440 | |
| 14:25:33 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove :return from update_provider_tree docstring https://review.openstack.org/560442 | |
| 14:25:38 | efried | jaypipes: Fast approve-worthy ^ | |
| 14:27:15 | jaypipes | efried: done | |
| 14:27:24 | efried | jaypipes: Thanks. Not sure how I missed that. | |
| 14:27:58 | jaypipes | efried: you didn't. I did. | |
| 14:28:02 | efried | :) | |
| 14:30:42 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: libvirt: get_inventory => update_provider_tree https://review.openstack.org/560444 | |
| 14:30:45 | efried | jaypipes, bhagyashris: ^ | |
| 14:31:14 | efried | That would go under 'co-authorship candidate' in our fancy etherpad, cause I don't know from libvirt. | |
| 14:31:27 | efried | But now it can be used as the basis for what we discussed above. | |
| 14:33:50 | mriedem | gibi_awa1: my wedding gift to you is going to be bugs to discuss during the notifications meeting :) https://wiki.openstack.org/wiki/Meetings/NovaNotification#Agenda | |
| 14:45:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix race fail in test_resize_with_reschedule_then_live_migrate https://review.openstack.org/560454 | |
| 14:45:10 | mriedem | dansmith: efried: ^ fixes a regression i introduced | |
| 14:47:03 | openstackgerrit | sahid proposed openstack/nova master: libvirt: add support for virtio-net rx/tx queue sizes https://review.openstack.org/484997 | |
| 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. | |