)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"5d92fc85da744f9ee0c6fda655317255c72247ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"48f8ca88_5fd16325","updated":"2024-01-31 15:57:21.000000000","message":"recheck uknown bug","commit_id":"5616268290f897597bbdafafe562f656933fd4ea"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"93f1785336d866b24e3999dd8656e473c7e36cd5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"55944d56_81d4d0b2","updated":"2024-02-20 14:58:54.000000000","message":"bump please review","commit_id":"e57698b0666fd7df935ab784c859e83a40cbe509"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"4468f5a5d613c722c5bf5b8c2645f75720fea7f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ef46d0be_250cae20","updated":"2024-05-04 16:41:15.000000000","message":"please review","commit_id":"855a3e20adaf74f24d832a66a3d7a4186f460111"},{"author":{"_account_id":38176,"name":"Sei Sano","display_name":"say3no","email":"sei.sano@ntt.com","username":"say3no"},"change_message_id":"71b14ace629468c5293d756147c5119f73db04c7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"f790763a_3780f793","updated":"2026-02-16 22:57:08.000000000","message":"recheck","commit_id":"27ac343dfb3dd16b4cba994a28ab2688dd596e80"},{"author":{"_account_id":38176,"name":"Sei Sano","display_name":"say3no","email":"sei.sano@ntt.com","username":"say3no"},"change_message_id":"60d7c259c14ea0c2be223d9829050269a00c4012","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"be1d130e_3ef04bbe","updated":"2026-02-16 23:11:14.000000000","message":"PS 7,8(rebase) includes the following improvements on top of the original patch:\n\n1. Changed string matching (\"Guest agent is not responding\") to error code check (VIR_ERR_AGENT_UNRESPONSIVE) for robustness across libvirt versions\n2. Added else clause in the except block to handle unexpected LibvirtError - sets ret \u003d False (fail-safe) instead of leaving it as True\n3. Changed LOG.info to LOG.debug in _thresholdsCrossing to avoid excessive logging on every monitoring cycle\n4. Added unit tests for _qemuAgentGuestPing: normal response, None response, agent unresponsive, and unexpected error cases","commit_id":"c3c9f3fbfc5f724b1a95f13e959c8a1b63733fbb"},{"author":{"_account_id":38176,"name":"Sei Sano","display_name":"say3no","email":"sei.sano@ntt.com","username":"say3no"},"change_message_id":"c5850a2898b6d45df6fdf4d0e429aa03688b8712","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"2fefba87_1bf51df6","updated":"2026-10-09 08:54:20.000000000","message":"Thanks for checking this, Muroi-san. I wanted to answer each of your cases properly, so I verified them on a real environment.\n\nEnvironment and method\n- Single-node devstack, libvirt 10.0.0, connection URI qemu:///system\n- Three VMs:\n  - Ubuntu noble with qemu-guest-agent installed\n  - cirros with the guest agent channel but no agent inside (hw_qemu_guest_agent\u003dyes)\n  - cirros without the guest agent channel\n- For each case, I called both of the following with timeout\u003d2 and flags\u003d0, and recorded the return value, whether an exception was raised, and (error domain, error code, message):\n  - the low-level API used at qemu_utils.py:305: libvirtmod_qemu.virDomainQemuAgentCommand\n  - the high-level API: libvirt_qemu.qemuAgentCommand\n\nResults for your cases 1/2/3\n1. guest-ping succeeds: the string \u0027{\"return\":{}}\u0027 is returned.\n2. The libvirt connection is established but guest-ping fails: the low-level API does NOT raise. It returns None in all four variants I tried:\n   - no agent running in the guest: (QEMU, 86) \"QEMU guest agent is not connected\"\n   - agent hung (SIGSTOP inside the guest): (QEMU, 86) \"Guest agent not available for now\"\n   - agent returned an error: (QEMU, 1) \"The command guest-foo has not been found\"\n   - domain paused: (QEMU, 55) \"domain is not running\"\n3. libvirt connection failure: the result depends on which API is called (see below).\n\nAbout case 3: the result depends on the API\n- If libvirtd is down, libvirt.open() raises libvirtError. In checkGuests() this is caught by the outer except block, so no guest-ping is sent in that cycle.\n- If libvirtd goes down after the connection has been opened, guest-ping behaves as follows:\n  - low-level API: does not raise, returns None, with (RPC, 38) \"Cannot write data: Broken pipe\"\n  - high-level API: raises libvirtError with (RPC, 1) \"client socket is closed\"\n\nSummary\n- In every failure I tried, the low-level API returned None and never raised. This matches the libvirt-python source: libvirt-qemu-override.c returns None when the C function returns NULL.\n- So the \"except libvirt.libvirtError\" branch in PS9 is never reached in practice.\n- Conversely, if we keep the low-level API and remove \"if res is None\", every failure would be treated as a success, and detection would stop entirely, because None is the only failure signal the low-level API gives.\n- The high-level API is a thin wrapper that raises libvirtError, with error domain and code, when it gets None.\n- The same error code can come with different error domains: code 1 appeared for both RPC and QEMU, and \"domain is not running\" (55) appeared for both QEMU and DOMAIN (a domain stopped right after lookup).\n- Regarding the original motivation: the original implementation (since a680e32, 2018) already treated None as a failure correctly, so failures were not missed. What it could not do is tell the kinds of failures apart; everything was counted as a ping failure. I think the real value of this change is making that distinction.\n\nProposal\nBased on this, I\u0027d like to go with the whitelist approach:\n\na. Switch to the high-level API (libvirt_qemu.qemuAgentCommand). All failures are then received as libvirtError, and \"if res is None\" is no longer needed.\nb. Only the (error domain, error code) pairs that I confirmed are not guest failures are whitelisted. For these, the cycle is skipped: the failure count is neither incremented nor reset. Messages are not used because they vary across versions. The list:\n   - (QEMU, 55): domain paused\n   - (DOMAIN, 55): domain stopped\n   - (RPC, 38): connection to libvirtd lost\n   - (RPC, 1): connection to libvirtd lost\nc. Everything else counts as a ping failure, including 86 and any unknown error. On failure we log (error domain, code, message) so that gaps in the whitelist can be noticed. Even if the whitelist misses something, a notification is sent only when that failure persists beyond the threshold, so transient errors should rarely lead to false positives.\n\nUnit tests:\n\ne. Mock the high-level API, and build libvirtError from the actual (code, domain, message) values observed above, instead of exceptions that never happen in practice.\nf. Table-driven tests for the four whitelisted pairs: each is skipped, and neither the FSM nor the failure count changes.\ng. On the failure side, verify that the observed 86 (both messages), 74, and an unknown (domain, code) pair are counted as failures. Also verify that a successful response (\u0027{\"return\":{}}\u0027) resets the count.\n\nWhat do you think about this direction?","commit_id":"4c45f640447fcd81f19277f3ce9142d84dfad8f2"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"1c12b83384ddf1c96fb4b19a58474a3aa3241202","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"54ba12cb_3e8b5304","updated":"2026-10-13 05:15:11.000000000","message":"Thanks for the deep dive to the libvirtmod_qemu module. According to the investigation result, I\u0027m ok for the current error case handlings.","commit_id":"4c45f640447fcd81f19277f3ce9142d84dfad8f2"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"2f05aef4ba1a5855856b3d6f39b4f7a501967bbf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"94765768_6bab727f","updated":"2026-10-09 07:37:37.000000000","message":"Thanks. overall looks good to me. Added one quick question.","commit_id":"4c45f640447fcd81f19277f3ce9142d84dfad8f2"}],"masakarimonitors/introspectiveinstancemonitor/qemu_utils.py":[{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"2f05aef4ba1a5855856b3d6f39b4f7a501967bbf","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            res \u003d libvirtmod_qemu.virDomainQemuAgentCommand(domain._o,"},{"line_number":306,"context_line":"                \u0027{\"execute\": \"guest-ping\"}\u0027, timeout, flags)"},{"line_number":307,"context_line":"            if res is None:"},{"line_number":308,"context_line":"                ret \u003d False"},{"line_number":309,"context_line":"        except libvirt.libvirtError as e:"},{"line_number":310,"context_line":"            if e.get_error_code() \u003d\u003d libvirt.VIR_ERR_AGENT_UNRESPONSIVE:"}],"source_content_type":"text/x-python","patch_set":9,"id":"a01fa2d8_cca2d886","line":307,"updated":"2026-10-09 07:37:37.000000000","message":"Doublecheck for the return value variation of the `libvirtmod_qemu.virDomainQemuAgentCommand`.\n\n1. success guest-ping command\n2. failed guest-ping command and libvirt connection is established\n3. libvirt connection failure\n\nFrom my quick check result, #1 and #2 return some string values, not None.  And #3 raises libvirtError exception. The `res` seems to be always not None.\n\nI\u0027m ok just to remove the if-block if the response value pattern is correct from the original bug fix motivation.","commit_id":"4c45f640447fcd81f19277f3ce9142d84dfad8f2"},{"author":{"_account_id":8878,"name":"Masahito Muroi","email":"masahito.muroi@g.softbank.co.jp","username":"masa"},"change_message_id":"1c12b83384ddf1c96fb4b19a58474a3aa3241202","unresolved":true,"context_lines":[{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            res \u003d libvirtmod_qemu.virDomainQemuAgentCommand(domain._o,"},{"line_number":306,"context_line":"                \u0027{\"execute\": \"guest-ping\"}\u0027, timeout, flags)"},{"line_number":307,"context_line":"            if res is None:"},{"line_number":308,"context_line":"                ret \u003d False"},{"line_number":309,"context_line":"        except libvirt.libvirtError as e:"},{"line_number":310,"context_line":"            if e.get_error_code() \u003d\u003d libvirt.VIR_ERR_AGENT_UNRESPONSIVE:"}],"source_content_type":"text/x-python","patch_set":9,"id":"82436d7a_4efbff75","line":307,"in_reply_to":"a01fa2d8_cca2d886","updated":"2026-10-13 05:15:11.000000000","message":"@say3no@gmail.com Thanks for the detailed investigations.\n\n\u003e (QEMU, 55): domain paused\n\u003e (DOMAIN, 55): domain stopped\n\u003e (RPC, 38): connection to libvirtd lost\n\u003e (RPC, 1): connection to libvirtd lost\n\nThe four cases return `None`, don\u0027t it? If so I\u0027m okay for the current code.\n\nThe background of my comment is if #2 returns any string value neither `\u0027{\"return\":{}}\u0027`  nor None value, the string value is treated as `ret \u003d\u003d True`. So updating the if-condition with following sample pattern can handle the error case clearer.\n\n```\n# sample\ntry:\n    res \u003d libvirtmod_qemu.virDomainQemuAgentCommand(...)\n    if res \u003d\u003d \u0027{\"return\":{}}\u0027:\n        ret \u003d True\n    else:\n        ret \u003d False\nexcept libvirt.libvirtError as e:\n    ret \u003d False\n\n```","commit_id":"4c45f640447fcd81f19277f3ce9142d84dfad8f2"}]}