Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-22
17:23:06 mdbooth mriedem: Reading back a bit more I was just going to ask exactly that.
17:23:30 mdbooth It sounds to me like the bug is in the setting of min_disk in the first place, then.
17:23:36 mdbooth It should be the min_disk of the original image.
17:23:57 mriedem so on my devstack right now, the single cirros image has min_disk=0
17:24:11 mriedem i can create a 1GB volume as the boot disk for my server that has that image in it,
17:24:21 mriedem snapshot it, and nova says the min_disk for the snapshot is now equal to the flavor.root_gb
17:24:27 mriedem 20 in my case
17:24:30 mriedem 20GB
17:25:01 mdbooth IMHO the bug is " and nova says the min_disk for the snapshot is now equal to the flavor.root_gb"
17:25:02 mriedem lemme create a volume from the image and see what the image meta shows in the volume
17:25:09 mriedem i agre
17:25:10 mriedem *agree
17:25:36 mdbooth IOW, I think the bug happens before the proposed fix
17:26:04 claudiub mriedem: regarding your comment on https://review.openstack.org/#/c/543971/5/nova/compute/manager.py , you mean the docstring which is on finish_resize (L4444), or a new one explaining the allocation cleanup part?
17:26:21 mriedem mdbooth: ok so i created a 1GB volume from a cirros image in devstack http://paste.openstack.org/show/682285/
17:26:30 mriedem | volume_image_metadata | {u'container_format': u'bare', u'min_ram': u'0', u'disk_format': u'qcow2', u'image_name': u'cirros-0.3.5-x86_64-disk', u'image_id': u'9491dd49-665c-46f4-9dbf-9a94aa1c9843', u'checksum': u'f8ab98ff5e73ebab884d80c9dc9c7290', u'min_disk': u'0', u'size': u'13267968'} |
17:26:42 mriedem the volume image meta shows the internal image min_disk is 0
17:27:16 mriedem mdbooth: what i'm not sure about, is if nova should say the volume-backed snapshot image min_disk should be the original image min_disk, or the volume size?
17:27:32 mriedem or, maybe we just don't care
17:27:38 mriedem and shouldn't even put min_disk in the snapshot
17:27:58 mriedem because if you use that thing to create another server, nova is going to boot from volume using the volume snapshot metadata embedded in the snapshot image
17:28:16 mriedem ie block_device_mapping='[{"guest_format": null, "boot_index": 0, "delete_on_termination": false, "no_device": null, "snapshot_id": "1158debc-9922-458d-9b46-8ef688334d65", "device_name": "/dev/vda", "disk_bus": "virtio", "image_id": null, "source_type": "snapshot", "tag": null, "device_type": "disk", "volume_id": null, "destination_type": "volume", "volume_size": 1}]'
17:28:31 mriedem so nova shouldn't really care about min_disk in this case
17:28:53 mriedem we're not going to create an ephemeral disk for the server, we're going to create a volume
17:29:28 mriedem here https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L684
17:29:33 openstackgerrit Merged openstack/nova-specs master: Explain how filters differ for /r_p vs. /a_c https://review.openstack.org/547056
17:29:58 mriedem claudiub: i meant on the new _finish_resize_helper method,
17:30:07 mdbooth I think min_disk should stay, and be the min_disk of the original image, because it's image metadata.
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 mdbooth min_disk is about the *workload*, not how big the disk is
17:31:38 mriedem anyway, that also works for me i think
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 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: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: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 dansmith jaypipes: nay
17:45:40 jaypipes oops.
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 openstack Launchpad bug 1039662 in OpenStack Compute (nova) "Snapshot queries original image which could be gone" [Medium,Fix released]
17:47:57 mriedem mdbooth: https://bugs.launchpad.net/nova/+bug/1039662
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: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: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?

Earlier   Later