| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-22 | |||
| 17:30:33 | mdbooth | Same as hw_* says the image needs certain hardware | |
| 17:30:42 | mdbooth | min_disk says it needs so much disk | |
| 17:31:06 | mriedem | for the image sure, | |
| 17:31:17 | mriedem | so the min_disk on the image could be 100MB, and the volume could be 1GB | |
| 17:31:38 | mriedem | anyway, that also works for me i think | |
| 17:31:38 | mdbooth | min_disk is about the *workload*, not how big the disk is | |
| 17:31:49 | mdbooth | We already know exactly how many bits are physically required to unpack it. | |
| 17:32:06 | mdbooth | min_disk says how much space is required to operate | |
| 17:32:33 | mdbooth | IOW we shouldn't be arbitrarily setting it where it wasn't set before. | |
| 17:33:55 | mdbooth | If we're going to exclude it, it should be in the context of a generic ability to filter arbitrary image metadata when creating a snapshot. | |
| 17:34:13 | mdbooth | Or entirely disabling copying image metadata, more likely. | |
| 17:35:11 | mdbooth | But I don't think min_disk can be treated specially by excluding it if it's set. | |
| 17:35:34 | mriedem | well we do have the non_inheritable_image_properties config option, which sucks since it's config-based API behavior | |
| 17:36:18 | mdbooth | Yeah | |
| 17:36:26 | mriedem | i guess that's just more about stuff you put into your images which shouldn't apply across snapshots, like signatures | |
| 17:36:42 | mdbooth | For now, I think the solution is to find whatever's setting it where it wasn't set before and kill that. | |
| 17:37:01 | mdbooth | Hopefully it's not cinder :/ | |
| 17:37:44 | mriedem | not following you - setting it where it wasn't set before? | |
| 17:37:45 | mriedem | that's nova | |
| 17:37:52 | mriedem | https://review.openstack.org/#/c/542646/2/nova/compute/api.py | |
| 17:38:01 | mriedem | oh sec | |
| 17:38:04 | openstackgerrit | Claudiu Belu proposed openstack/nova master: hyper-v: autospec classes before they are instantiated https://review.openstack.org/342211 | |
| 17:38:33 | mriedem | mdbooth: nova.utils.get_image_from_system_metadata | |
| 17:39:15 | mriedem | but yeah i'm not exactly where where that min_disk value is coming from the flavor.root_gb | |
| 17:39:18 | mriedem | *sure | |
| 17:40:01 | mdbooth | I think there might have been some code Jenga involved. | |
| 17:40:20 | jaypipes | cfriesen: ok, now I'm confused... I thought "the VM's I/O threads" and "the VM's vCPU threads" were the same thing :( | |
| 17:43:17 | mriedem | mdbooth: found it | |
| 17:43:18 | cfriesen | jaypipes: you have emulator threads, IO threads, and vCPU threads. | |
| 17:43:41 | mriedem | mdbooth: https://github.com/openstack/nova/blob/master/nova/utils.py#L989 | |
| 17:43:45 | jaypipes | cfriesen: oh? is this something new in libvirt? | |
| 17:44:23 | mriedem | mdbooth: that says if the image min_disk is not set, or 0, just store it as the flavor root disk in the instance system_metadata, which we then use later to build the image meta for the snapshot | |
| 17:44:23 | cfriesen | jaypipes: no. if you're just running pure userspace code in the guest then there's no work for the IO threads. they're used for reading from virtual disk and such | |
| 17:44:24 | dansmith | jaypipes: no :) | |
| 17:44:47 | mdbooth | mriedem: Yep, I think that's it. | |
| 17:45:21 | mdbooth | mriedem: I have a suspicion that at some point in the past it was really hard to change min_disk due to assumptions about it dotted around the code. | |
| 17:45:28 | jaypipes | cfriesen, dansmith: I've always just referred to the qemu-kvm processes that perform work for a guest as "I/O threads".. :( | |
| 17:45:40 | jaypipes | oops. | |
| 17:45:40 | dansmith | jaypipes: nay | |
| 17:45:44 | cfriesen | how does placement deal with the difference between "shared" vcpus and "dedicated" vcpus? | |
| 17:45:54 | jaypipes | cfriesen: it doesn't. and it won't. | |
| 17:46:01 | mdbooth | Although my grepping suggests it's no longer misused. | |
| 17:46:39 | mriedem | mdbooth: it doesn't explain why it uses the flavor root_gb https://review.openstack.org/#/c/43782/ | |
| 17:46:46 | cfriesen | jaypipes: so how does it deal with a host that has X pCPUs, which lets it run X dedicated vCPUs or X*cpu_allocation_ratio shared vCPUs? | |
| 17:46:56 | jaypipes | cfriesen: it doesn't. | |
| 17:47:05 | cfriesen | okay, so do we not track cpu usage in placement? | |
| 17:47:23 | jaypipes | cfriesen: we do not track cpu pinning or numa topology in placement, no. | |
| 17:47:28 | openstackgerrit | Claudiu Belu proposed openstack/nova master: compute: Cleans up allocations after failed resize https://review.openstack.org/543971 | |
| 17:47:46 | jaypipes | cfriesen: all of that is done in the highly-coupled NUMA topology stuff in nova/virt/hardware.py and the NUMATopologyFilter. | |
| 17:47:57 | mriedem | mdbooth: https://bugs.launchpad.net/nova/+bug/1039662 | |
| 17:47:57 | openstack | Launchpad bug 1039662 in OpenStack Compute (nova) "Snapshot queries original image which could be gone" [Medium,Fix released] | |
| 17:48:27 | mriedem | mdbooth: so yeah, i don't have any clue here why nova decided that if min_disk isn't set, we should set it to the flavor root_gb | |
| 17:48:27 | claudiub | mriedem: if you're feeling generous with the reviews, can you take a look at the autospec stuff? :D https://review.openstack.org/#/q/topic:bug/1735588+(status:open+OR+status:merged)+project:openstack/nova | |
| 17:48:37 | jaypipes | cfriesen: also, allocation ratio isn't considered in any CPU pinning scenarios at all. | |
| 17:48:44 | mriedem | claudiub: i'm not that generous | |
| 17:48:48 | cfriesen | jaypipes: okay, so if you use up all the pCPUs running dedicated vCPUs placement will still think there are vcpus available? | |
| 17:48:51 | jaypipes | cfriesen: it's an entirely different system of accounting that uses the vcpu_pin_set CONF option. | |
| 17:48:56 | mdbooth | mriedem: Sounds like the right place to fix it to me. | |
| 17:49:31 | jaypipes | cfriesen: it was an entirely different system of accounting that was added in the hardware.py module and has ZERO to do with how any other resources are tracked in Nova. | |
| 17:49:33 | claudiub | mriedem: then, if I may abuse your not that generous-ness, can you take a look at the autospec stuff? :D | |
| 17:54:18 | cfriesen | jaypipes: in the context of the "shared/dedicated on one host", I just added some comments. If we're going to allow both on one host then I think it would make sense to have the usage numbers ("vcpus_used" in "nova hypervisor-show" for example) accurately reflect what's on the system. | |
| 17:54:59 | jaypipes | cfriesen: yeah, I saw your comments. this is definitely something to talk about next week... | |
| 17:55:05 | jaypipes | cfriesen: will you be there? | |
| 17:55:18 | cfriesen | yep, starting monday afternoon | |
| 17:55:35 | jaypipes | cfriesen: also, keep in mind vcpus_used has absolutely nothing to do with pCPUs. | |
| 17:55:43 | mriedem | umm wtf - flavor doesn't show up in the server response? http://paste.openstack.org/show/682314/ | |
| 17:55:49 | jaypipes | cfriesen: ne'er the two shall meet. ;) | |
| 17:56:14 | jaypipes | mriedem: lol. | |
| 17:56:20 | mriedem | oh i bet i know what it is, | |
| 17:56:29 | mriedem | osc can't handle microversion>=2.47 responses | |
| 17:56:33 | mriedem | where the flavor is a dict instead of a string | |
| 17:56:59 | jaypipes | mriedem: an awkward dict? | |
| 17:57:06 | cfriesen | jaypipes: hypervisor-show has "vcpus" and "vcpus_used". in our case we made it such that "vcpus" is len(vcpu_pin_set). This makes it easy to see how "full" your hypervisor is. | |
| 17:57:16 | mriedem | yup http://paste.openstack.org/show/682315/ | |
| 17:57:49 | jaypipes | cfriesen: that is an abuse. | |
| 17:57:55 | jaypipes | cfriesen: and completely incorrect :) | |
| 17:58:48 | mriedem | cfriesen: since you delt it, maybe you'd like to fix this https://bugs.launchpad.net/python-openstackclient/+bug/1751104 | |
| 17:58:48 | openstack | Launchpad bug 1751104 in python-openstackclient "openstack server show with --os-compute-api-version >= 2.47 doesn't show the flavor" [Undecided,New] | |
| 17:59:43 | cfriesen | ooo, fun | |
| 18:00:12 | cfriesen | yah, I think I can take that. | |
| 18:02:00 | mriedem | mdbooth: i think removing that would be wrong, and it's likely there for non-volume-backed instances anyway (like everything in nova) | |
| 18:02:29 | mriedem | mdbooth: i created a 20gb flavor instance from the devstack cirros image which has min_disk=0, and the resulting snapshot has min_disk=20 to match the flavor root_gb | |
| 18:03:28 | mdbooth | Right. So the 20GB restriction here should be because that's how many bits are physically required, which is represented in 'size'. | |
| 18:03:54 | mdbooth | IMHO min_disk should still be unset. | |
| 18:04:05 | mriedem | for the volume-backed snapshot you mean? | |
| 18:04:10 | mriedem | i.e. exactly what this patch is doing? | |
| 18:04:10 | mdbooth | Yeah. | |
| 18:05:11 | mdbooth | That patch is arbitrarily popping min_disk out of image metadata. | |
| 18:05:25 | mdbooth | I don't think it should have been in there in the first place. | |
| 18:05:40 | mdbooth | min_disk is analogous to min_ram here | |
| 18:09:46 | mriedem | mdbooth: i don't really agree, | |
| 18:09:57 | mriedem | for a volume-backed instance, the disk is what matters (or doesn't) | |
| 18:10:31 | mriedem | if i have a 20gb flavor instance and snapshot it to get a 20gb image, then the image should say that you need a flavor of a minimum of 20gb to boot an instance with that image, right? | |
| 18:10:52 | mriedem | but that's not true with a volume, since the flavor.root_gb isn't used for the volume | |
| 18:14:08 | dansmith | mriedem: min_disk should be the size of the volume in that case when you snapshot I think | |
| 18:14:15 | dansmith | flavor.root_gb if ephemeral | |
| 18:14:30 | mriedem | dansmith: that's what i originally asked in the patch | |
| 18:14:36 | mriedem | rather than pop off min_disk, | |
| 18:14:40 | mriedem | why not just set it to root_bdm.size | |
| 18:15:21 | mriedem | good point though | |