)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2ac37c210a31ea2acde66a368737b7d7f23cef08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dbae5243_667ae728","updated":"2025-11-13 10:08:47.000000000","message":"Great find 👏 I\u0027d love to see a follow-up that would prevent access to any other driver attributes before the host was initialized. I\u0027m going to propose a WIP follow-up that I\u0027d be more than happy for someone to take over 🤞","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"9e4b95f0026ef49540c02ceb7e1071b3f8267dbe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e3f306b2_965d93ae","updated":"2025-11-21 09:06:24.000000000","message":"recheck\n```\n2025-11-20 17:45:10.134487 | compute1 |   \"msg\": \"Warning: Permanently added \u0027158.69.68.54\u0027 (ED25519) to the list of known hosts.\\r\\nrsync: [sender] link_stat \\\"/var/lib/zuul/builds/43482cbf708b4d8fada6beefe004e28b/work/ca-bundle.pem\\\" failed: No such file or directory (2)\\nrsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1338) [sender\u003d3.2.7]\\n\",\n```","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f69805e14801394fa4d87b671d5b0e2cd36bd6f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0cbad053_7dadadbc","updated":"2025-11-20 15:22:51.000000000","message":"recheck ssh timeout to the guest","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"}],"nova/compute/manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"504d07785d519e06d676a1540bbb4d30c7b040d7","unresolved":true,"context_lines":[{"line_number":1660,"context_line":"                    \u0027number of disk devices.\u0027)"},{"line_number":1661,"context_line":"            raise exception.InvalidConfiguration(msg)"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        self.driver.init_host(host\u003dself.host)"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"        if service_ref:"},{"line_number":1666,"context_line":"            # If we are an existing service, check to see if we need"}],"source_content_type":"text/x-python","patch_set":1,"id":"624afee2_0036f420","line":1663,"updated":"2025-11-13 15:19:53.000000000","message":"So the point of doing the check before basically anything in this method is to bail if we may be starting up with the wrong config before we have a chance to do anything to change the local state. I would have thought that `init_host()` could be doing stuff to instances (et al) but it looks like at least in the libvirt case we _currently_ don\u0027t. So I guess this is okay although it makes it easier to shove something else in here (or there) later which won\u0027t be prevented by our check.\n\nThe first thing we do in `init_host()` is call `self._host.initialize()` which I assume is the thing we need to be running after. Is there some reason we can\u0027t/shouldn\u0027t just move that to the end of `__init__()`? Seems like that would also avoid the concern over calling it before it is initialized (i.e. the patch after).\n\nI\u0027m sure you\u0027ve gone down this path and have a good reason, but.. I\u0027d like to know :)","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f705df2b9216d2761727a496b1c2e4dabe8e103","unresolved":true,"context_lines":[{"line_number":1660,"context_line":"                    \u0027number of disk devices.\u0027)"},{"line_number":1661,"context_line":"            raise exception.InvalidConfiguration(msg)"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        self.driver.init_host(host\u003dself.host)"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"        if service_ref:"},{"line_number":1666,"context_line":"            # If we are an existing service, check to see if we need"}],"source_content_type":"text/x-python","patch_set":1,"id":"641a3572_eae9bb1d","line":1663,"in_reply_to":"5d4524e1_6b0aa7b0","updated":"2025-11-17 15:27:17.000000000","message":"\u003e We use the virt drivers via the virt driver interface. That interface defines `init_host()` as a way to initialize the driver.\n\nI mean... that\u0027s definitely not what I think it\u0027s for, else it would be called `init_driver()` or, you know, `__init__()` :) To me, it\u0027s for syncing up the driver and/or nova itself with the current state of the host. Ironic uses it only to refresh their hash ring (I assume because it\u0027s called after initializing nova\u0027s service records). I don\u0027t know what vmware is using it for exactly. They have some session init there, but they clearly use the session in `__init__()` and they do query details from the vmware api during `__init__()` (before `init_host()` is called). zvm has nothing there. In fact, the base driver does say \"anything needed to function\" but mentions \"catching up\" specifically:\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/driver.py#L361\n\nGoing back to the first driver in the tree (xen), it looks to me like they do their initialization in `__init__()` and in `init_host()` they do some sanity checking and cleanup of stale resources:\n\nhttps://github.com/openstack/nova/blob/ussuri-em/nova/virt/xenapi/driver.py#L117\n\n\u003e While moving `self._host.initialize()` to `__init__()` in the libvirt driver would make it possible to call the `list_instance_uuids()` virt driver interface when implemented by the libvirt driver it would mean:\n\u003e * for some driver it might not be possible to call `list_instance_uuids()` before `init_host`\n\u003e * other methods on the driver interface like `spawn()` still fails if called before driver.init_host in any or some drivers making the interface confusing\n\u003e * those virt drivers (like libvirt driver) that implements additional environment check like hypervisor min version in `init_host` now would allow talking to the hypervisor before such additional checks. \n\nSure, calls that have side effects (on the host) seem quite different to me, and make sense to only call after we\u0027ve sync\u0027d with the current state of the hypervisor. However, I don\u0027t think that any of the other drivers we have in tree make the assumptions you imply are baked into the interface.\n\n\u003e So my problem with the suggestion is that it would arbitrarily broke the current interface contract of the virt driver interface.\n\nI\u0027m a bit concerned that we seem to have such a different view of role of `__init__()` and `init_host()`, which I guess is why this bug is here in the first place. Might be good to nail down the expectations for those two interfaces at some point. To me, having a \"finish initialization of the driver\" call doesn\u0027t make much sense unless there\u0027s a specific reason or delineation (i.e. `__init__()` makes the connection and `init_host()` things that actually affect the state of the system, like cleanup).\n \n\u003e What I can do to help a bit is to only move the _sanity_check_new_host() part of the currently moved block and keep the rest as in the baseline. Only _sanity_check_new_host() uses the virt interface so only that call needs to be after init_host.\n\nI mean, I think it\u0027s going to be more complicated that way as you\u0027ll need to replicate the condition above further down, and that\u0027s the important part to get done in sequence. So it seems not worth it to me.","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d56cbd036e036b8a6962e6023c1c4213fbadf7ab","unresolved":true,"context_lines":[{"line_number":1660,"context_line":"                    \u0027number of disk devices.\u0027)"},{"line_number":1661,"context_line":"            raise exception.InvalidConfiguration(msg)"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        self.driver.init_host(host\u003dself.host)"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"        if service_ref:"},{"line_number":1666,"context_line":"            # If we are an existing service, check to see if we need"}],"source_content_type":"text/x-python","patch_set":1,"id":"5d4524e1_6b0aa7b0","line":1663,"in_reply_to":"624afee2_0036f420","updated":"2025-11-17 08:43:06.000000000","message":"We use the virt drivers via the virt driver interface. That interface defines `init_host()` as a way to initialize the driver. While moving `self._host.initialize()` to `__init__()` in the libvirt driver would make it possible to call the `list_instance_uuids()` virt driver interface when implemented by the libvirt driver it would mean:\n* for some driver it might not be possible to call `list_instance_uuids()` before `init_host`\n* other methods on the driver interface like `spawn()` still fails if called before driver.init_host in any or some drivers making the interface confusing\n* those virt drivers (like libvirt driver) that implements additional environment check like hypervisor min version in `init_host` now would allow talking to the hypervisor before such additional checks. \n\nSo my problem with the suggestion is that it would arbitrarily broke the current interface contract of the virt driver interface.\n\nWhat I can do to help a bit is to only move the _sanity_check_new_host() part of the currently moved block and keep the rest as in the baseline. Only _sanity_check_new_host() uses the virt interface so only that call needs to be after init_host.","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"54cc2acfb1460cb2f80d901035f81e40fc60234b","unresolved":true,"context_lines":[{"line_number":1660,"context_line":"                    \u0027number of disk devices.\u0027)"},{"line_number":1661,"context_line":"            raise exception.InvalidConfiguration(msg)"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        self.driver.init_host(host\u003dself.host)"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"        if service_ref:"},{"line_number":1666,"context_line":"            # If we are an existing service, check to see if we need"}],"source_content_type":"text/x-python","patch_set":1,"id":"15996abf_9b5ec9ab","line":1663,"in_reply_to":"641a3572_eae9bb1d","updated":"2025-11-17 16:23:44.000000000","message":"I\u0027ve pushed a follow up to clarify the docs strings of __init__ and init_host in the virt interface https://review.opendev.org/c/openstack/nova/+/967398 based on the IRC discussion https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2025-11-17.log.html#openstack-nova.2025-11-17.log.html#t2025-11-17T15:27:41","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f68d1765878639cba6b6a6c0c91f9db479382ac0","unresolved":true,"context_lines":[{"line_number":1660,"context_line":"                    \u0027number of disk devices.\u0027)"},{"line_number":1661,"context_line":"            raise exception.InvalidConfiguration(msg)"},{"line_number":1662,"context_line":""},{"line_number":1663,"context_line":"        self.driver.init_host(host\u003dself.host)"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"        if service_ref:"},{"line_number":1666,"context_line":"            # If we are an existing service, check to see if we need"}],"source_content_type":"text/x-python","patch_set":1,"id":"a6bb5fdc_d9f08ac5","line":1663,"in_reply_to":"641a3572_eae9bb1d","updated":"2025-11-17 16:38:15.000000000","message":"for me init_host is more hten \"driver.init()\" in tha ti also think its responsible for doing things liek config validation and ensuring that the resouce tracker and other manager level sub components are initallise with potitally data form the driver but not necessarily.\n\nso init_host at the comptue manger lever is more then a thin wrapper over driver.init_host.\n\ndriver.init_host is an exection point ot allow the driver to initialise itself as part of the larger init_host pahse fo the agent start up.\n\nanyway i see gibi submitted a follow up https://review.opendev.org/c/openstack/nova/+/967398 so we can dicuss this more there i guess.","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d56cbd036e036b8a6962e6023c1c4213fbadf7ab","unresolved":true,"context_lines":[{"line_number":1678,"context_line":"        context \u003d nova.context.get_admin_context()"},{"line_number":1679,"context_line":"        nodes_by_uuid \u003d self._get_nodes(context)"},{"line_number":1680,"context_line":""},{"line_number":1681,"context_line":"        # NOTE(danms): Check for a possible host rename and abort"},{"line_number":1682,"context_line":"        # startup before we start mucking with instances we think are"},{"line_number":1683,"context_line":"        # ours."},{"line_number":1684,"context_line":"        self._check_for_host_rename(nodes_by_uuid)"},{"line_number":1685,"context_line":""},{"line_number":1686,"context_line":"        instances \u003d objects.InstanceList.get_by_host("},{"line_number":1687,"context_line":"            context, self.host,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ebdc5d44_d2d1dd5d","line":1684,"range":{"start_line":1681,"start_character":0,"end_line":1684,"end_character":50},"updated":"2025-11-17 08:43:06.000000000","message":"This is also put under init_host in the first place correctly as it needs to talk to the hypervisor via the virt interface.","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1f705df2b9216d2761727a496b1c2e4dabe8e103","unresolved":true,"context_lines":[{"line_number":1678,"context_line":"        context \u003d nova.context.get_admin_context()"},{"line_number":1679,"context_line":"        nodes_by_uuid \u003d self._get_nodes(context)"},{"line_number":1680,"context_line":""},{"line_number":1681,"context_line":"        # NOTE(danms): Check for a possible host rename and abort"},{"line_number":1682,"context_line":"        # startup before we start mucking with instances we think are"},{"line_number":1683,"context_line":"        # ours."},{"line_number":1684,"context_line":"        self._check_for_host_rename(nodes_by_uuid)"},{"line_number":1685,"context_line":""},{"line_number":1686,"context_line":"        instances \u003d objects.InstanceList.get_by_host("},{"line_number":1687,"context_line":"            context, self.host,"}],"source_content_type":"text/x-python","patch_set":1,"id":"a5939ec9_b4654e37","line":1684,"range":{"start_line":1681,"start_character":0,"end_line":1684,"end_character":50},"in_reply_to":"ebdc5d44_d2d1dd5d","updated":"2025-11-17 15:27:17.000000000","message":"Are you saying that is why I put this here? I\u0027m pretty sure I put this here because I needed `nodes_by_uuid` and we didn\u0027t pull that out until just above.","commit_id":"16abde56eebed7b95270516580f94c4ab0bcbe6b"}]}