| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-22 | |||
| 20:39:15 | cfriesen | but yeah, up to the virt driver | |
| 20:39:59 | rybridges | mlavalle: {"versions": [{"min_version": "1.0", "max_version": "1.4", "id": "v1.0"} | |
| 20:40:02 | jaypipes | cfriesen, sean-k-mooney: ok, I'm going to update my spec and attempt as best as possible to recap the above decisions. | |
| 20:40:10 | cfriesen | we'd have to persist the mapping somewhere to preserve it over live migration. and it'd be nice to keep it over cold migration/evacuate too | |
| 20:40:24 | jaypipes | cfriesen: instance metadata... | |
| 20:40:45 | jaypipes | cfriesen: just like how we save device metadata/tags today, right? | |
| 20:41:00 | cfriesen | should work, I think | |
| 20:42:06 | sean-k-mooney | cfriesen: you say per numa node but you realsie libvirt does not map guest cores to virtual numa nodes right | |
| 20:42:53 | sean-k-mooney | actully you kind of can. | |
| 20:42:55 | sean-k-mooney | <numa> | |
| 20:42:57 | sean-k-mooney | <cell id='0' cpus='0-3' memory='512000' unit='KiB'/> | |
| 20:43:04 | sean-k-mooney | </numa> | |
| 20:43:38 | sean-k-mooney | cpus in the cell id is the guest logical cpu | |
| 20:45:48 | sean-k-mooney | jaypipes: the gotcha with the metadata preseting the mapping is that on live migration we would want to make sure the mapping did not change as the running workload would not likely call the metadata api again | |
| 20:46:16 | cfriesen | sean-k-mooney: yeah, that's doable | |
| 20:46:41 | sean-k-mooney | that said NFV + live migration does not mix no matter how much telcos want it to | |
| 20:47:00 | cfriesen | sean-k-mooney: it just means that NUMATopologyFilter needs to check for that case | |
| 20:47:19 | jaypipes | sean-k-mooney: we'd still want to update the instance metadata to reflect the pinning on the destination host, though, so when the workload rebooted, it was able to configure itself. | |
| 20:47:26 | sean-k-mooney | cfriesen: meaning the numa toplogy filter would have to query the metatdata service | |
| 20:47:55 | cfriesen | jaypipes: no, we'd need to keep the "which vcpus are dedicated" mapping the same over the live migration | |
| 20:47:59 | sean-k-mooney | cfriesen: maybe put that in its own filter. | |
| 20:48:22 | cfriesen | jaypipes: we don't need to store the virtual-to-physical mapping in the metadata | |
| 20:49:08 | sean-k-mooney | cfriesen: ok i should have left 2 hours ago. letse leave the edgecase to live migration of snowflake vnf to another spec? | |
| 20:49:24 | cfriesen | sean-k-mooney: lol...me too. got an issue to debug | |
| 20:49:55 | sean-k-mooney | cfriesen: it has no effect on placement just the numatoplogy+everything_else_super filter | |
| 20:50:26 | sean-k-mooney | jaypipes: are you ok writing this up in the spec? | |
| 20:50:31 | cfriesen | sean-k-mooney: correct | |
| 20:52:02 | sean-k-mooney | ok im off tomorow but if you ping me ill proably check irc at some point | |
| 20:57:01 | cfriesen | if we report the "which vcpus are dedicated" mapping via metadata, what happens if the virtual routing is such that the guest has no access to the metadata server? will the config drive get suitably updated? | |
| 20:57:31 | sean-k-mooney | config drive is read only so no | |
| 20:57:58 | sean-k-mooney | you have to proxy to metadata via dhcp server if its an isolated neutron network | |
| 20:58:18 | sean-k-mooney | config dirive might get update on cold migrate or hard reboot | |
| 20:58:48 | cfriesen | sean-k-mooney: readonly is fine, the guest isn't going to be updating it | |
| 20:59:16 | cfriesen | and the mapping of which guest cpus are shared/dedicated shouldn't change dynamically once booted | |
| 20:59:29 | sean-k-mooney | yes but qemu can unplug it and update it on livemigrate either | |
| 20:59:42 | cfriesen | we need to keep it constant over live migrate | |
| 20:59:49 | cfriesen | so no problem | |
| 21:01:08 | mlavalle | rybridges: the microversion required on the Neutron side is 1.1. So it seems right. At the time of creating your subnet, do you you see any tracebacks in the Neutron server log? If so, please share it in a paste | |
| 21:01:30 | sean-k-mooney | sure. it just means more complicated code so when we calluate the pinning for the dest we have to maintian the shared/dedicated logical core mappings which is technical debt but ok | |
| 21:02:53 | dansmith | efried: excellent use of proper ITU phonetics | |
| 21:02:53 | cfriesen | sean-k-mooney: not really technical debt. logically the guest shouldn't need to be constantly checking which guest cpus are shared/dedicated. should only need to happen at boot time, or ideally on creation/rebuild | |
| 21:04:06 | sean-k-mooney | cfriesen: yes but it does mean that this is yes another thing the fit_instance_to_host function needs to enforce | |
| 21:04:38 | cfriesen | sean-k-mooney: yes, but it's required as soon as you allow mixed shared/dedicated in one instance. | |
| 21:05:40 | sean-k-mooney | cfriesen: yes which we dont allow today so once its added that fucntion need to be extended to allow passing in a set of mappings and the validate that the would still be correct for the new host. | |
| 21:08:41 | sean-k-mooney | cfriesen: again its doable but https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4364-L4510 is not exactly the most plesant code to debug currently. anyway this time im really leaving | |
| 21:08:44 | sean-k-mooney | o/ | |
| 21:17:50 | cfriesen | jaypipes: so is your overall goal to get rid of the current hw:numa_mem.X=Y, hw:numa_cpu.X=Y, hw:mem_page_size=2048, etc. and specify it all explicitly as placement resources? | |
| 21:18:23 | cfriesen | jaypipes: because the alternative would be to keep them as-is and have nova calculate the allocation | |
| 21:18:43 | dansmith | cfriesen: fwiw, I have't been following all of this today, | |
| 21:19:00 | dansmith | but I'm highly allergic to plans for us trying to automate that fitting of things onto numa nodes | |
| 21:19:13 | dansmith | I don't like that it's manual, but it's a headache I don't really want to own | |
| 21:19:34 | dansmith | so convincing me is going to take some damn fine, simple code and a lot of tests | |
| 21:20:05 | cfriesen | dansmith: so are you suggesting we keep what we have now or require explicit resource specification in the flavor? | |
| 21:20:55 | dansmith | I would expect we'd take the thing we have now and turn it into a granular resource request based on the total, and the pieces you specified per node | |
| 21:21:29 | dansmith | cfriesen: I don't really have a suggestion, I'm just saying, every time I think I could genericify that someone brings up a case they want to support which is hard | |
| 21:22:21 | dansmith | and I expect people that need/use that level of fine-grained control over what the system looks like are doing it for suuuuper specific reasons | |
| 21:22:54 | openstackgerrit | Patricia Domingues proposed openstack/nova master: load up the volume drivers by checking architecture https://review.openstack.org/541393 | |
| 21:23:17 | dansmith | like "I have a custom irreplaceable application that only runs on windows nt 4.0 which has a never-going-to-be-fixed bug where it requires 2MB of memory on the same node as the cdrom's IDE controller or my application fails to read from the light pen" | |
| 21:25:17 | cfriesen | dansmith: nice. | |
| 21:28:59 | cfriesen | If we're going to allow shared/dedicated CPUs within a single instance then we'll need to add some way of specifying at a minimum how many "shared" or "dedicated" vcpus we want in each virtual numa node. Whether that's explicitly via resources or via some other thing that nova factors into the resource allocation request is sort of up in the air. | |
| 21:30:32 | cfriesen | But if we keep the existing fields we have now (which would be nice to avoid breaking people) nova will need to translate that into allocation requests for VCPUS or PCPUS (to use jay's terminology) | |
| 21:31:36 | melwitt | bug possibilities for stable branches | |
| 21:31:36 | melwitt | dansmith, mriedem: I was just looking at https://blueprints.launchpad.net/nova/+spec/libvirt-cpu-model-extra-flags again. is the ability to enable various cpu flags a feature that we would want anyway regardless of the meltdown mitigation? wondering why one of the choices isn't just "enable_pcid = True/False" and leave it at that, for the backport and going forward? that way there's no migration path nor concern about opening up more | |
| 21:31:44 | melwitt | sigh | |
| 21:32:04 | dansmith | melwitt: yeah I think people have wanted that for other reasons | |
| 21:32:17 | dansmith | melwitt: like, choose a lower base cpu but add in avx2 | |
| 21:32:22 | dansmith | or something like that | |
| 21:32:31 | melwitt | okay, I see | |
| 21:32:55 | rybridges | mlavalle: We are not seeing any stacktraces in neutron-server.log or in nova-api.log or in nova-placement-api.log. It looks like the association is not working on this line: https://github.com/openstack/neutron/blob/stable/ocata/neutron/services/segments/plugin.py#L225 Nothing after that line is being executed. But we dont see errors or stack traces. When we look at the resource_providers in | |
| 21:32:57 | rybridges | placement, the segment is there and registered as a resource provider, but the aggregate is not associated with it. | |
| 21:33:05 | dansmith | melwitt: you can already configure your cpu model to be different and break live migration, so this wouldn't be any different | |
| 21:35:23 | melwitt | okay. if it's a feature that has utility going forward (specifying various flags) then I'm inclined to favor the idea of backporting it as the full-fledged feature being that the risk is lower than the upgrade pain for operators to go from a [workarounds] option -> cpu_model_extra_flags | |
| 21:35:58 | dansmith | melwitt: did you see my latest suggestion on the patch? | |
| 21:36:12 | melwitt | no, looking now | |
| 21:36:39 | dansmith | melwitt: backporting it with a restriction that pcid is the only thing you can put in that option would eliminate the general-purpose use of it without causing the deployment pain | |
| 21:36:53 | melwitt | ah, I see. that's a nice idea | |
| 21:37:42 | melwitt | I think that addresses all of the concerns | |
| 21:37:46 | dansmith | me too | |
| 21:37:55 | melwitt | noice | |
| 21:38:22 | melwitt | what do you think of that mriedem? | |
| 21:39:40 | mriedem | so the backport has the choices kwarg with a single item? | |
| 21:39:52 | mriedem | and we drop choices in master? | |
| 21:39:56 | dansmith | that's one way yeah | |
| 21:40:05 | dansmith | well, I'm asking right now if we want a list anyway | |
| 21:40:25 | dansmith | or some sort of reasonable validation | |
| 21:40:26 | dansmith | if we can get the list of all options from libvirt or something | |
| 21:41:23 | mriedem | cpu_map.xml comes back in the libvirt host capabilities stuff we already have i think | |
| 21:41:26 | mriedem | we dump it on startup to the logs | |
| 21:41:31 | melwitt | yeah, I was thinking maybe we have choices and there's only one choice in the backport and going forward there will be a set of choices we know are good? or are there just too many to reasonable do it that way | |
| 21:41:42 | dansmith | yeah, so I would think we should validate what they give us anyway, so we don't just generate broken xml | |
| 21:41:48 | dansmith | but that would be later than in conf choices | |
| 21:42:33 | mriedem | http://logs.openstack.org/84/534384/9/check/tempest-full/a66bf1a/controller/logs/screen-n-cpu.txt.gz#_Mar_22_13_34_01_146931 | |
| 21:43:07 | mriedem | i would say, (1) backportable version has just a single choice, pcied or whatever, and then (2) another patch, master-only, drops choices and we validate on startup of the driver based on the host capabilities | |
| 21:43:09 | mriedem | not in config | |
| 21:43:23 | dansmith | that's cool, | |
| 21:43:38 | dansmith | although I think just landing it with the validation and restricted choice would be fine too | |
| 21:43:48 | dansmith | but if you like it being smaller with no validation then that's cool | |
| 21:44:21 | mriedem | oh i don't care if we add the validation in the backport change too | |
| 21:44:30 | mriedem | if that's reliable | |
| 21:44:41 | mriedem | i'm not sure what in ^ is the thing we would use to validate | |