| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-18 | |||
| 09:58:37 | stephenfin | If cpu_dedicated_set is not defined, then cpu_shared_set should only be used for emulator threads | |
| 09:59:46 | stephenfin | However, I think I addressed this in a previous email by saying it was extremely unlikely that cpu_shared_set would be defined without vcpu_pin_set, since it wouldn't offer any advantage (the emulator thread host cores would overlap with the pinned CPU host cores) | |
| 10:01:04 | bhagyashris__ | stephenfin: no I am talking about if operator sets only ``cpu_shared_set`` in Train on fresh compute node then it should report the VCPU inventory as per the specs | |
| 10:01:24 | bhagyashris__ | Right? | |
| 10:01:54 | stephenfin | Then the spec needs a modification. They also need to set cpu_dedicated_set, even if it's to an empty string | |
| 10:02:36 | stephenfin | Because otherwise we can't tell the difference between a freshly deployed node and an upgraded node | |
| 10:03:15 | bhagyashris__ | stephenfin: ok means on train operator can not set the ``cpu_shared_set`` alone . If it's sets then raised the error that ``cpu_dsedoicated_set`` should be set | |
| 10:03:26 | stephenfin | Not an error, a warning | |
| 10:03:32 | bhagyashris__ | Ok ok | |
| 10:04:03 | stephenfin | Stating that we are ignoring that value and are instead relying on 'vcpu_pin_set', if configured, or reporting all host cores if not | |
| 10:04:17 | stephenfin | From the spec | |
| 10:04:19 | stephenfin | "If the [compute] cpu_dedicated_set config option is not set, a warning will be logged stating that reserved_host_cpus is deprecated and that the operator should set both [compute] cpu_shared_set and [compute] cpu_dedicated_set." | |
| 10:04:29 | bhagyashris__ | Then in that case the inventory reported at placement side will be PCPU=0 and VCPU=0 | |
| 10:05:15 | stephenfin | No, it'll be PCPU = (len(vcpu_pin_set) | len($total_host_cores)) | |
| 10:05:33 | stephenfin | and VCPU = (len(vcpu_pin_set) | len($total_host_cores)) * cpu_allocation_ratio | |
| 10:05:59 | stephenfin | i.e. the exact same behavior if cpu_shared_set wasn't defined | |
| 10:06:08 | stephenfin | and cpu_dedicated_set wasn't defined | |
| 10:06:11 | openstackgerrit | zhaixiaojun proposed openstack/python-novaclient master: Modify the url of upper_constraints_file https://review.opendev.org/665934 | |
| 10:08:55 | bhagyashris__ | means if only the ``cpu_shared_set`` is define then in that case inventory should be reported as PCPU = (len(vcpu_pin_set) | len($total_host_cores)) and VCPU = (len(vcpu_pin_set) | len($total_host_cores)) * cpu_allocation_ratio | |
| 10:09:07 | bhagyashris__ | ? | |
| 10:10:18 | stephenfin | What's the question? | |
| 10:10:19 | bhagyashris__ | stephenfin: ^ | |
| 10:10:43 | stephenfin | You seem to just be saying exactly what I just said :) | |
| 10:11:47 | bhagyashris__ | stephenfin: my question is if the operator set only ``cpu_shared_set`` in Train on fresh compute node then we will raise the Warning but what inventory will reported at placement side? | |
| 10:12:06 | stephenfin | What I just said | |
| 10:12:09 | stephenfin | PCPU = (len(vcpu_pin_set) | len($total_host_cores)) | |
| 10:12:11 | stephenfin | VCPU = (len(vcpu_pin_set) | len($total_host_cores)) * cpu_allocation_ratio | |
| 10:12:28 | bhagyashris__ | Ok got it | |
| 10:12:58 | sean-k-mooney | actully that is not correct | |
| 10:13:42 | bhagyashris__ | But if operator want to use that not as shared then in that case reporting both PCPU and VCPU the inventory is wrong IMO | |
| 10:13:51 | sean-k-mooney | if vcpu_pin_set is not defiend and cpu_dedicated_set is not defiend but cpu_shared_set is then we should only report a singel inventory of VCPU=len(cpu_share_set) | |
| 10:14:10 | stephenfin | Ohhh, you're right | |
| 10:14:19 | sean-k-mooney | and set the inventory allcoation raito as neeeded | |
| 10:14:33 | stephenfin | Sorry bhagyashris__ | |
| 10:14:51 | bhagyashris__ | stephenfin, sean-k-mooney : yeah thats whats my understanding from the spec | |
| 10:15:02 | stephenfin | Yeah, I guess we'd written off the combination of cpu_shared_set being defined and vcpu_pin_set being undefined as "not gonna happen" | |
| 10:15:30 | sean-k-mooney | yes so we dont actly multiple the capasity by the allocation raition since the inventory has an allcoation ration filed seperatly as well | |
| 10:16:26 | sean-k-mooney | stephenfin: cpu_share_set and vcpu_pin_set makes sense when doign cpu pinning | |
| 10:17:00 | sean-k-mooney | as the cpu_shared_set is used for emulator trheads as you know but in that case we report vcpu and pcu based on vcpu_pin_set | |
| 10:17:09 | stephenfin | Agreed. I'm saying cpu_shared_set *without* vcpu_pin_set *doesn't* makes sense when doing CPU pinning | |
| 10:17:30 | sean-k-mooney | well i guess it could | |
| 10:17:44 | sean-k-mooney | no it doesnt | |
| 10:18:11 | sean-k-mooney | you never want teh emulator thread to overlap with the cpu that you can pin too | |
| 10:18:35 | sean-k-mooney | so it only makes sense if you have use vcpu_pin_set to restic what cpus you can pin too | |
| 10:18:45 | stephenfin | Correct | |
| 10:18:53 | stephenfin | It's just annoying that we don't have a check to validate that at the moment | |
| 10:19:00 | stephenfin | I wonder if we should add one and backport it? | |
| 10:19:23 | stephenfin | i.e. WARNING: You've set cpu_shared_set without vcpu_pin_set. This is dumb. Stop it. | |
| 10:19:58 | sean-k-mooney | so if cpu_shared_set was defined on its own then it would only make sense in train to model a host that resultss floating vms being confined to a subset of hosts | |
| 10:20:07 | sean-k-mooney | :) | |
| 10:20:27 | sean-k-mooney | i like how bitchy the warning is :) | |
| 10:20:29 | sean-k-mooney | +1 | |
| 10:22:13 | sean-k-mooney | stephenfin: maybe it makes sense to write up a bunch of different configuration in an eterpad with the config args and the expect placmenet inventories which we can use as a set of functional test to implement | |
| 10:28:04 | bhagyashris__ | stephenfin, sean-k-mooney : thank you for you opinion ! I will try to push the patches ASAP. And incase any of any doubts I will ping. | |
| 10:47:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: hacking: Resolve W605 (invalid escape sequence) https://review.opendev.org/652104 | |
| 10:47:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Use consistent URL regex substitution https://review.opendev.org/665949 | |
| 11:04:13 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Integrate 'pre-commit' and deprecate 'tox -e fast8' https://review.opendev.org/665518 | |
| 11:22:44 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove redundant group host setup https://review.opendev.org/665954 | |
| 11:33:32 | openstackgerrit | Edward Hope-Morley proposed openstack/nova stable/stein: Fix python3 compatibility of rbd get_fsid https://review.opendev.org/664512 | |
| 12:29:21 | openstackgerrit | Hamdy Khader proposed openstack/os-vif master: [Follow Up] OVS DPDK port representors support https://review.opendev.org/665965 | |
| 12:49:40 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Grab fresh info from the driver during nova start/stop actions https://review.opendev.org/665975 | |
| 13:03:21 | yonglihe | sean-k-mooney: clean up orphan instances, Checking instance nova metadata added: https://review.opendev.org/#/c/627765/ | |
| 13:03:43 | yonglihe | sean-k-mooney: You might want to take a look | |
| 13:03:58 | sean-k-mooney | cool jsut responding to a review but ill look at it next | |
| 13:04:37 | yonglihe | Thanks. | |
| 13:07:14 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Recalculate 'RequestSpec.numa_topology' on resize https://review.opendev.org/662522 | |
| 13:07:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Add '_setup_compute_services' helper https://review.opendev.org/663102 | |
| 13:07:15 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Cleanup of '_test_resize' helper test https://review.opendev.org/664245 | |
| 13:07:16 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Update docstring of 'revert_resize' function https://review.opendev.org/665976 | |
| 13:10:16 | jangutter | hamdyk: I don't understand https://review.opendev.org/#/c/665965 - I don't see how it's required. | |
| 13:10:23 | yonglihe | add spec "show-server-numa-topology", Got Alex's +2. Hope I can got another +2, thanks guys. https://review.opendev.org/#/c/658716/ | |
| 13:13:22 | hamdyk | jangutter: when we use hostdev (unmanaged device by libvirt), MAC will need to be set outside of Nova | |
| 13:13:41 | jangutter | hamdyk: yes, which is why it's not used? | |
| 13:14:14 | jangutter | hamdyk: for kernel TC offloads, <interface type='hostdev'> is used. | |
| 13:14:24 | hamdyk | we updated the Nova patch | |
| 13:15:10 | sean-k-mooney | right but if you want to delegate the mac managment to os-vif it has to work form multiple vendors and cant depend on the presence of a biforcated driver | |
| 13:15:11 | hamdyk | jangutter: take a look: https://review.opendev.org/#/c/658785/ | |
| 13:15:50 | hamdyk | sean-k-mooney: correct, so now we are setting the MAC for bifurcated drivers | |
| 13:16:23 | sean-k-mooney | do you have a follow up patch that set it for non biforcated driver like i40e | |
| 13:16:25 | hamdyk | sean-k-mooney: and maybe later for non-bifurcated | |
| 13:17:17 | jangutter | hamdyk: you realise that introduces a race condition, right? | |
| 13:17:35 | sean-k-mooney | jangutter: it actully should not. | |
| 13:17:50 | sean-k-mooney | jangutter: we wait for os-vif to plug the vfs before we start teh vm | |
| 13:17:59 | hamdyk | actually we are looking into this (setting MAC for non-biforcated driver) any insights would be great | |
| 13:18:03 | jangutter | sean-k-mooney: ah, so libvirt always pauses? | |
| 13:18:35 | sean-k-mooney | no we call os-vif.plug before we defien the domain in libvirt | |
| 13:18:41 | hamdyk | yes, libvirt waits for event that plugging was successful | |
| 13:18:53 | sean-k-mooney | so we fully complete plug before we ever define the domain in libvirt | |
| 13:18:58 | yonglihe | mriedem: You have all context about this, It may easier for you than other cores. Hope you could have a time slot. Thanks, https://review.opendev.org/#/c/658716/ | |
| 13:19:06 | jangutter | sean-k-mooney: but on reboot? The domain is already defined? | |
| 13:19:32 | sean-k-mooney | oh hard reboot we destroy and redefine the domain | |
| 13:19:53 | sean-k-mooney | on soft reboot we do not unplug the vf for qemu or ovs | |
| 13:20:43 | hamdyk | so we are good with the patch? | |
| 13:20:58 | sean-k-mooney | not really | |
| 13:21:11 | sean-k-mooney | i would like to check with the ovs folks first | |
| 13:21:48 | sean-k-mooney | unless we can do it in a vendor neutral way i would prefer to do it in libvirt but im open to being conviced | |
| 13:22:00 | jangutter | sean-k-mooney, hamdyk: what happens on upgrade? This would cause an interface type=hostdev to morph to unmanaged PCI passthrough. | |
| 13:22:32 | sean-k-mooney | if the ovs or dpdk folks are open to adding a way to set the mac via ovs in the future the im oke with the patch | |
| 13:22:53 | sean-k-mooney | jangutter: only for this once case correct | |
| 13:23:02 | sean-k-mooney | this code path will not be taken by standard sriov | |