| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-11 | |||
| 14:00:42 | jaypipes | bhagyashris: efried can share some information with you about the aggregate tracking in the ProviderTree object | |
| 14:01:00 | efried | jaypipes: You're saying that if we find a sharing provider for a given RC, we should *remove* that RC from the compute node's inventory?? | |
| 14:01:19 | jaypipes | efried: I'm saying that was my original idea, yeah. | |
| 14:01:32 | bhagyashris | efried, jaypipes: There is a TODO note here : https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L212-L216 that the first allocation request object is use to claim resources | |
| 14:01:36 | jaypipes | (that's why I originally added the aggregate tracking in the reportclient to begin with) | |
| 14:02:27 | efried | bhagyashris: jaypipes: Do we have any evidence that the virt driver actually *used* the shared storage to make the disk? | |
| 14:02:31 | jaypipes | bhagyashris: yes, but that's more about the use case where we have both shared and local disk and can't decide which to consume from. what efried and I are discussing is how to signal to the compute node not to report local disk if it sees a sharing provider of disk. | |
| 14:02:34 | efried | or has that piece not been hooked up yet? | |
| 14:02:39 | bhagyashris | efried, jaypipes: yeah that is the best idea and that will not need to make any change at sch side | |
| 14:02:53 | jaypipes | efried: well, that's an entirely different question :) | |
| 14:03:01 | efried | But an important one, you'll agree. | |
| 14:03:08 | jaypipes | efried: and I see no reason why it wouldn't. | |
| 14:03:20 | bhagyashris | jaypipes: yeah totally agree with you point | |
| 14:03:20 | efried | Doesn't do us much good to have the alloc come from the right place if the storage still comes from the wrong place. | |
| 14:03:29 | jaypipes | efried: it's the same reason why pinning of CPU resources still works even though VCPU is tracked in placement. | |
| 14:03:37 | jaypipes | efried: in other words, assignment != allocation | |
| 14:04:30 | efried | jaypipes: So you're saying that by virtue of whatever existing code recognized the flavor request as wanting the storage to come from shared... is still working. | |
| 14:04:43 | jaypipes | efried: don't confuse a request for 10TB of disk with block device configuration information. | |
| 14:04:43 | efried | I don't know what that mechanism is, but if that's the case... fine. | |
| 14:05:08 | efried | ah, the bdm somehow identifies the source of the disk space? | |
| 14:05:51 | jaypipes | efried: no. nothing really knows whether the source of the disk space is shared. it's just that /var/lib/instances is on mounted shared storage. | |
| 14:06:14 | jaypipes | efried: that's pretty much the reason why shared storage resource accounting has been so wonky. | |
| 14:06:18 | efried | ohh. | |
| 14:06:26 | efried | So in this setup, there really *is* no local storage. | |
| 14:06:53 | jaypipes | efried: there totally *could* be. no way to know other than manually commenting out that code right now. | |
| 14:06:59 | openstackgerrit | Merged openstack/nova master: Update wording in @safe_connect placement warnings https://review.openstack.org/560039 | |
| 14:07:50 | bhagyashris | jaypipes: yeah that way I have tested | |
| 14:07:56 | jaypipes | efried: for the record, we used to determine if "the node was on shared storage" by literally attempting to write a file from two different compute hosts to a single location and checking if each node could read it. | |
| 14:08:13 | jaypipes | efried: IIRC, this was during live migration setup. | |
| 14:08:15 | efried | jaypipes: So here's where the virt driver would need to recognize whether the storage is shared or not, and if so, create* the sharing provider and put the DISK_GB inventory there instead of the compute node RP. | |
| 14:08:16 | efried | (*create, co-create, make sure it exists, etc.) | |
| 14:08:23 | jaypipes | efried: not sure if we still do that, but that's how it used to be. | |
| 14:08:49 | jaypipes | efried: no, the idea was never to have a compute node create the sharing provider. | |
| 14:08:58 | efried | don't focus on that bit. | |
| 14:09:04 | jaypipes | efried: the admin would do that, since they know they are setting up shared storage for a set of nodes. | |
| 14:09:27 | efried | well, okay; I'm also thinking about the PowerVM case, where we *do* want the driver to co-manage creation of the sharing provider. | |
| 14:10:09 | efried | because that would also be a non-hacky way to get rid of the compute node RP's DISK_GB inventory :) | |
| 14:10:45 | efried | Because we can't just blindly remove it (outside of the virt driver) based on the presence of an aggregated provider of the same RC. | |
| 14:11:06 | jaypipes | efried: I don't understand that last sentence. | |
| 14:11:11 | efried | We may legitimately have both shared and local at the same time. We may legitimately want the virt driver to do that management, etc. | |
| 14:11:50 | jaypipes | efried: why would you want the virt driver on a single compute node managing disk resources for multiple compute nodes? | |
| 14:11:52 | efried | jaypipes: I'm just saying if we're going to fiddle with the compute RP's DISK_GB inventory based on existence of sharing DISK_GB providers, that logic needs to be fully owned by the virt driver. We can't assume things outside of that scope. | |
| 14:12:46 | jaypipes | efried: sure. the process of fiddling with any resources that are owned by that compute node (and its children) should of course be controlled by the virt driver. | |
| 14:13:02 | efried | jaypipes: In the PowerVM case, it's because we can; and because we can, it means we don't need any outside agent (human or code) manitaining that sharing RP. | |
| 14:13:04 | jaypipes | efried: I don't agree that a single compute node should control inventory on shared disk providers, though. | |
| 14:13:24 | jaypipes | efried: then powervm can do that I guess. | |
| 14:13:32 | jaypipes | efried: obviously libvirt won't :) | |
| 14:13:35 | efried | It won't be a single compute node; it'll all the compute nodes attached to that sharing provider. | |
| 14:13:44 | efried | co-managing the SSP. | |
| 14:13:58 | jaypipes | efried: right, because it's a clustered hypervisor manager. | |
| 14:13:58 | efried | ...because they already co-manage the SSP (outside of the auspices of openstack) | |
| 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...? | |