)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32919,"name":"kiran pawar","display_name":"Kiran Pawar","email":"kinpaa@gmail.com","username":"kpdev"},"change_message_id":"4a8c7d6934228e23785ba76eaabd5d979cc6cbfd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"ecdad93b_d1bf5446","updated":"2025-01-27 07:03:20.000000000","message":"Added libvirt owners for review","commit_id":"30334e7d1a0fd8b55caa91bb56f1431a5764974f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d05d42c4b7faa4376b22314866f0c755279aa9a0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"1e98c356_21c70bae","updated":"2025-01-29 15:50:28.000000000","message":"this is not a bug its a feature request and not one that i belive we should enabel in nova.\n\nif we were to do this it would require a spec or a specless blueprint.\n\nthe direct reading of the cgroup file system should be done via the existing read_sys functions not directly via open\n\nhttps://github.com/openstack/nova/blob/master/nova/filesystem.py#L61-L81\n\nif we were to treat this as a vaild bug instead of a feature the correct fix woudl be to exted our exisgin cpu validation logic to stop the compute agent from starting if the effective cpu_shared_set or cpu_dedicate_set do not fullil our requiremnts. i.e. if the cgroup config would prevent the vm form booting.\n\nif that host cgoup config does not allow all cpus to be used by nova vms, we woudl condier nova to be miss configured by the installer. if we were to conditioner this a bug the bug is that nova did not exit an report an error telling the operator explictly that the configuration is invalid.\n\nthere is a lot of precendnt for failing early but the most relevent is that nova reports an error if a cpu in the cpu_share_set is offline.\nthat is never valid.\n\nim not against imporving the ux or docs. but i am not convice we shoudl proceed with the current aproch or that its in scope fo nova to adress this in general hence the -2. it partly procerual but partly based on scope of the project.\n\ni am ok to remove it if the core team agrees addign a error and block on start up is approprate or if we agree to add a feature to compute the cpu shared set  based on a number of critira in the 2025.2 release or later.","commit_id":"30334e7d1a0fd8b55caa91bb56f1431a5764974f"}],"nova/virt/libvirt/host.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9bf98829a24edc32fb9b5afc6776d32f4f11dcef","unresolved":true,"context_lines":[{"line_number":95,"context_line":"    \u0027/sys/fs/cgroup/cpuset/machine/cpuset.cpus\u0027,"},{"line_number":96,"context_line":"    \u0027/sys/fs/cgroup/machine.slice/cpuset.cpus\u0027,"},{"line_number":97,"context_line":"    \u0027/sys/fs/cgroup/machine/cpuset.cpus\u0027,"},{"line_number":98,"context_line":"]"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"# These are taken from the spec"},{"line_number":101,"context_line":"# https://github.com/qemu/qemu/blob/v5.2.0/docs/interop/firmware.json"}],"source_content_type":"text/x-python","patch_set":10,"id":"8331222b_e0930ec5","line":98,"updated":"2025-01-29 17:16:25.000000000","message":"my understanding is the cgroug heriacy is  not necessarily consistent between distributions. it can be affected by factors such as if you have systemd or not and which cgroup version you are using.\n\nhttps://libvirt.org/cgroups.html#systemd-cgroup-layout\nhttps://libvirt.org/cgroups.html#non-systemd-cgroups-layout\nhttps://libvirt.org/cgroups.html#legacy-cgroups-layout\n\nfor example, nova does not require the use of systemd so you must supprot both layout. you have trided to cater for that but I bleive with systemd you could deploy libvirt in its own slice outside of the general machine slice.\n\nfor example in the user.slice if it was not installed system wide but rans as a user level systemd service.\n\nhave you considered how this would work in a contaienrised deployment?\n\nfor example nova-comptue when contiareised may not be usign the same cgroups as libvirt\n\npodman/docker can change the cgroup hieracy of isntnace spawnd by libvirt.\n\nnormally the instance cgroup path looks like this\n\n\"/sys/fs/cgroup/devices/machine/qemu-1180-instance-000143fd.libvirt-qemu\"\n\nif you are runing in a podman contianer livbirt will by default be created in a diffent scope\n\"/sys/fs/cgroup/devices/machine.slice/libpod-6217866491a67688919906827eeb429f348744200d9bbccdc51794c5eb31c276.scope\"\n\nnow you can work around that by defining the contianer to use the host cgroup namespace isntead.\n\nthat is what is done in kolla and tripleo \n\nhttps://github.com/openstack/kolla-ansible/blob/master/ansible/roles/nova-cell/defaults/main.yml#L11\n\nhowever we do not typiclaly run the nova-comptue binary with     \ncgroupns_mode: \"host\"\n\nin that case ther emay not be a machine folder or slice.\njust looking a a random ubuntu podman contianer i see this\n\n\n```\n[17:08:00]⬢ [ubuntu-22.04] ➜ ls /sys/fs/cgroup/\ncgroup.controllers      cpu.stat             memory.reclaim\ncgroup.events           cpu.stat.local       memory.stat\ncgroup.freeze           cpu.weight           memory.swap.current\ncgroup.kill             cpu.weight.nice      memory.swap.events\ncgroup.max.depth        io.pressure          memory.swap.high\ncgroup.max.descendants  memory.current       memory.swap.max\ncgroup.pressure         memory.events        memory.swap.peak\ncgroup.procs            memory.events.local  memory.zswap.current\ncgroup.stat             memory.high          memory.zswap.max\ncgroup.subtree_control  memory.low           memory.zswap.writeback\ncgroup.threads          memory.max           pids.current\ncgroup.type             memory.min           pids.events\ncpu.idle                memory.numa_stat     pids.events.local\ncpu.max                 memory.oom.group     pids.max\ncpu.max.burst           memory.peak          pids.peak\ncpu.pressure            memory.pressure\n```\nthat will not match and of the search parths above.\n\nso in a contianerised envionment this approch wont work because we cant assume the nova agent is running with the same cgroup view as libvirt.\n\n\nthose are some of the complxities/open questions that come to mind, as reaons why nova should not try to do this itself.\n\nif libvirt exposed an api to query this info it may be reasonable to use that but i dont think we can have nova quity it form /sys/fs/cgroups directly.","commit_id":"30334e7d1a0fd8b55caa91bb56f1431a5764974f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9bf98829a24edc32fb9b5afc6776d32f4f11dcef","unresolved":true,"context_lines":[{"line_number":1710,"context_line":"        return self._has_cgroupsv1_cpu_controller() or \\"},{"line_number":1711,"context_line":"               self._has_cgroupsv2_cpu_controller()"},{"line_number":1712,"context_line":""},{"line_number":1713,"context_line":"    def _has_cgroupsv1_cpu_controller(self):"},{"line_number":1714,"context_line":"        LOG.debug(f\"Searching host: \u0027{self.get_hostname()}\u0027 \""},{"line_number":1715,"context_line":"                  \"for CPU controller through CGroups V1...\")"},{"line_number":1716,"context_line":"        try:"},{"line_number":1717,"context_line":"            with open(\"/proc/self/mounts\", \"r\") as fd:"},{"line_number":1718,"context_line":"                for line in fd.readlines():"},{"line_number":1719,"context_line":"                    # mount options and split options"},{"line_number":1720,"context_line":"                    bits \u003d line.split()[3].split(\",\")"},{"line_number":1721,"context_line":"                    if \"cpu\" in bits:"},{"line_number":1722,"context_line":"                        LOG.debug(\"CPU controller found on host.\")"},{"line_number":1723,"context_line":"                        return True"},{"line_number":1724,"context_line":"                LOG.debug(\"CPU controller missing on host.\")"},{"line_number":1725,"context_line":"                return False"},{"line_number":1726,"context_line":"        except IOError as ex:"},{"line_number":1727,"context_line":"            LOG.debug(f\"Search failed due to: \u0027{ex}\u0027. \""},{"line_number":1728,"context_line":"                      \"Maybe the host is not running under CGroups V1. \""},{"line_number":1729,"context_line":"                      \"Deemed host to be missing controller by this approach.\")"},{"line_number":1730,"context_line":"            return False"},{"line_number":1731,"context_line":""},{"line_number":1732,"context_line":"    def _has_cgroupsv2_cpu_controller(self):"},{"line_number":1733,"context_line":"        LOG.debug(f\"Searching host: \u0027{self.get_hostname()}\u0027 \""},{"line_number":1734,"context_line":"                  \"for CPU controller through CGroups V2...\")"},{"line_number":1735,"context_line":"        try:"},{"line_number":1736,"context_line":"            with open(\"/sys/fs/cgroup/cgroup.controllers\", \"r\") as fd:"},{"line_number":1737,"context_line":"                for line in fd.readlines():"},{"line_number":1738,"context_line":"                    bits \u003d line.split()"},{"line_number":1739,"context_line":"                    if \"cpu\" in bits:"},{"line_number":1740,"context_line":"                        LOG.debug(\"CPU controller found on host.\")"},{"line_number":1741,"context_line":"                        return True"},{"line_number":1742,"context_line":"                LOG.debug(\"CPU controller missing on host.\")"},{"line_number":1743,"context_line":"                return False"},{"line_number":1744,"context_line":"        except IOError as ex:"},{"line_number":1745,"context_line":"            LOG.debug(f\"Search failed due to: \u0027{ex}\u0027. \""},{"line_number":1746,"context_line":"                      \"Maybe the host is not running under CGroups V2. \""},{"line_number":1747,"context_line":"                      \"Deemed host to be missing controller by this approach.\")"},{"line_number":1748,"context_line":"            return False"},{"line_number":1749,"context_line":""},{"line_number":1750,"context_line":"    def list_schedulable_cpus(self) -\u003e set[int]:"},{"line_number":1751,"context_line":"        \"\"\"Returns a set of CPU cores that can be utilized to schedule"},{"line_number":1752,"context_line":"        vCPU, emulator and IO threads"},{"line_number":1753,"context_line":"        \"\"\""},{"line_number":1754,"context_line":"        for cpuset_path in CGROUP_CPUSET_PATHS:"},{"line_number":1755,"context_line":"            if os.path.exists(cpuset_path):"},{"line_number":1756,"context_line":"                with open(cpuset_path) as cpuset_cpus:"},{"line_number":1757,"context_line":"                    cgroup_cpus \u003d hardware.parse_cpu_spec(cpuset_cpus.read())"},{"line_number":1758,"context_line":""},{"line_number":1759,"context_line":"                    if len(cgroup_cpus):"},{"line_number":1760,"context_line":"                        return cgroup_cpus"},{"line_number":1761,"context_line":""},{"line_number":1762,"context_line":"                    LOG.debug(\u0027CGroup %(path)s has an empty CPU list,\u0027"},{"line_number":1763,"context_line":"                              \u0027skipping it.\u0027, {\u0027path\u0027: cpuset_path})"},{"line_number":1764,"context_line":""},{"line_number":1765,"context_line":"        LOG.info(\u0027CGroup cpuset controller configured but could not find \u0027"},{"line_number":1766,"context_line":"                 \u0027any libvirt cgroups, the shared cpuset will contain \u0027"},{"line_number":1767,"context_line":"                 \u0027all online cpus.\u0027)"},{"line_number":1768,"context_line":"        return self.get_online_cpus()"},{"line_number":1769,"context_line":""},{"line_number":1770,"context_line":"    def get_canonical_machine_type(self, arch, machine) -\u003e str:"},{"line_number":1771,"context_line":"        \"\"\"Resolve a machine type to its canonical representation."},{"line_number":1772,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"777a1889_d94a3087","line":1769,"range":{"start_line":1713,"start_character":2,"end_line":1769,"end_character":1},"updated":"2025-01-29 17:16:25.000000000","message":"as an aside, i see why you put this here but this file is not really intended for doing arbiary host cheks. it was introduced originally to lookup information about the host using the libvirt api\n\nthe other cgroup controller checks and sev checks should not be here either.\nthese really should be in a different class of module.\n\nwith that said I\u0027m not sure where to suggest moving them to so perhaps this is ok.\n\ni also see why you didnt use the read sys funciton\nhttps://github.com/openstack/nova/blob/master/nova/filesystem.py#L62\n\nsince the exting code eos not but that is really just tech debth.\nreading sysfs can in some cases return a EBUSY so we generaly need to account for that with a retry which read_sys does via the retry_if_busy decorator.","commit_id":"30334e7d1a0fd8b55caa91bb56f1431a5764974f"}],"releasenotes/notes/bug-2095591-84711c3c63555dc3.yaml":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9bf98829a24edc32fb9b5afc6776d32f4f11dcef","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Make Nova check if there is a cpuset cgroup activated for the respective"},{"line_number":5,"context_line":"    libvirt virtual machine cgroup and use the CPUs defined in it as the list"}],"source_content_type":"text/x-yaml","patch_set":10,"id":"19494d16_5bbc62e6","line":2,"range":{"start_line":2,"start_character":0,"end_line":2,"end_character":5},"updated":"2025-01-29 17:16:25.000000000","message":"as written this is a feature not a bug\n\na bugfix would stop the compute form starting and print an hard error.\n\nthis does not protect the cpu_share_set or cpu_dedicated_set form accidentally including a core we cant use so its  also incomplete.\n\ndynamically calculating the set based on hardcoded cgoup path while perhaps an option as a future feature  would need to protect against all misconfiguration too.\n\nright now it the human installer responsibility to configure this correctly.\nif we add this support we are making it novas and as a result nova need to also check the config options when used.","commit_id":"30334e7d1a0fd8b55caa91bb56f1431a5764974f"}]}