)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"75f55cb1b0dd4f9b158824d9ee6efb330427a574","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"There are some open questions regarding this implementation in"},{"line_number":23,"context_line":"comparison with the original retry based impl:"},{"line_number":24,"context_line":"* what to do with synchronous errors from libvirt? retry?"},{"line_number":25,"context_line":"* what to do with asynch error from libvirt? (ie when"},{"line_number":26,"context_line":"  VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event is received) just fail?"},{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3439bd69_3c124f92","line":24,"range":{"start_line":24,"start_character":0,"end_line":24,"end_character":57},"updated":"2021-01-13 09:11:35.000000000","message":"For libvirt.VIR_ERR_DEVICE_MISSING and/or libvirt.VIR_ERR_INVALID_ARG (until we bump MIN_LIBVIRT_VERSION past 4.1.0) I\u0027d recommend continuing to fail and not retry.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"75f55cb1b0dd4f9b158824d9ee6efb330427a574","unresolved":true,"context_lines":[{"line_number":22,"context_line":"There are some open questions regarding this implementation in"},{"line_number":23,"context_line":"comparison with the original retry based impl:"},{"line_number":24,"context_line":"* what to do with synchronous errors from libvirt? retry?"},{"line_number":25,"context_line":"* what to do with asynch error from libvirt? (ie when"},{"line_number":26,"context_line":"  VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event is received) just fail?"},{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2d0f034e_c3ce0601","line":26,"range":{"start_line":25,"start_character":2,"end_line":26,"end_character":73},"updated":"2021-01-13 09:11:35.000000000","message":"I think we want to keep things in line with the original implementation and retry for the time being, assuming there\u0027s a timeout internal to libvirt etc for the detach. I\u0027ll ask around about that.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"520181287110bfa535285a5be23898b485771352","unresolved":true,"context_lines":[{"line_number":22,"context_line":"There are some open questions regarding this implementation in"},{"line_number":23,"context_line":"comparison with the original retry based impl:"},{"line_number":24,"context_line":"* what to do with synchronous errors from libvirt? retry?"},{"line_number":25,"context_line":"* what to do with asynch error from libvirt? (ie when"},{"line_number":26,"context_line":"  VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event is received) just fail?"},{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3177f7fc_2c56750b","line":26,"range":{"start_line":25,"start_character":2,"end_line":26,"end_character":73},"in_reply_to":"2d0f034e_c3ce0601","updated":"2021-01-13 10:24:39.000000000","message":"OK. I will add retry loop for this case based on the current retry loop parameters (sleeps an retry counts)","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"84f83073347e44c5551fd57d3073a26b58f078b1","unresolved":true,"context_lines":[{"line_number":22,"context_line":"There are some open questions regarding this implementation in"},{"line_number":23,"context_line":"comparison with the original retry based impl:"},{"line_number":24,"context_line":"* what to do with synchronous errors from libvirt? retry?"},{"line_number":25,"context_line":"* what to do with asynch error from libvirt? (ie when"},{"line_number":26,"context_line":"  VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event is received) just fail?"},{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3bf0d1e1_e8558d4b","line":26,"range":{"start_line":25,"start_character":2,"end_line":26,"end_character":73},"in_reply_to":"2d0f034e_c3ce0601","updated":"2021-01-13 09:21:42.000000000","message":"There\u0027s not a timeout within libvirtd VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED looks like it\u0027s raised synchronously after a direct failure to detach the device so we should likely retry our detach request before seeing this raised.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"75f55cb1b0dd4f9b158824d9ee6efb330427a574","unresolved":true,"context_lines":[{"line_number":24,"context_line":"* what to do with synchronous errors from libvirt? retry?"},{"line_number":25,"context_line":"* what to do with asynch error from libvirt? (ie when"},{"line_number":26,"context_line":"  VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event is received) just fail?"},{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."},{"line_number":30,"context_line":"* there was two detach attempts in the original code if the domain had  both"},{"line_number":31,"context_line":"  live and persistent configs. It was implemented by retrying the detach. The"},{"line_number":32,"context_line":"  current code has no such logic. Is it OK to simply send one single"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9078eb26_7ae5a92e","line":29,"range":{"start_line":27,"start_character":0,"end_line":29,"end_character":11},"updated":"2021-01-13 09:11:35.000000000","message":"As above, I assume there\u0027s an internal timeout within libvirtd so we\u0027d need to ensure our own is above that and then I\u0027d error out tbh.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"84f83073347e44c5551fd57d3073a26b58f078b1","unresolved":true,"context_lines":[{"line_number":24,"context_line":"* what to do with synchronous errors from libvirt? retry?"},{"line_number":25,"context_line":"* what to do with asynch error from libvirt? (ie when"},{"line_number":26,"context_line":"  VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event is received) just fail?"},{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."},{"line_number":30,"context_line":"* there was two detach attempts in the original code if the domain had  both"},{"line_number":31,"context_line":"  live and persistent configs. It was implemented by retrying the detach. The"},{"line_number":32,"context_line":"  current code has no such logic. Is it OK to simply send one single"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"e38cd01e_89712c5a","line":29,"range":{"start_line":27,"start_character":0,"end_line":29,"end_character":11},"in_reply_to":"9078eb26_7ae5a92e","updated":"2021-01-13 09:21:42.000000000","message":"As above, there isn\u0027t and we need to retry our request.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"520181287110bfa535285a5be23898b485771352","unresolved":true,"context_lines":[{"line_number":24,"context_line":"* what to do with synchronous errors from libvirt? retry?"},{"line_number":25,"context_line":"* what to do with asynch error from libvirt? (ie when"},{"line_number":26,"context_line":"  VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED event is received) just fail?"},{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."},{"line_number":30,"context_line":"* there was two detach attempts in the original code if the domain had  both"},{"line_number":31,"context_line":"  live and persistent configs. It was implemented by retrying the detach. The"},{"line_number":32,"context_line":"  current code has no such logic. Is it OK to simply send one single"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"f83289b7_5a4521f0","line":29,"range":{"start_line":27,"start_character":0,"end_line":29,"end_character":11},"in_reply_to":"9078eb26_7ae5a92e","updated":"2021-01-13 10:24:39.000000000","message":"OK, we can adjust our timeout to libvirt\u0027s one","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"75f55cb1b0dd4f9b158824d9ee6efb330427a574","unresolved":true,"context_lines":[{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."},{"line_number":30,"context_line":"* there was two detach attempts in the original code if the domain had  both"},{"line_number":31,"context_line":"  live and persistent configs. It was implemented by retrying the detach. The"},{"line_number":32,"context_line":"  current code has no such logic. Is it OK to simply send one single"},{"line_number":33,"context_line":"  detach request regardless of the domain being persistent \u0026\u0026 live?"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"TODO:"},{"line_number":36,"context_line":"* resolve the above questions"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8baef5da_11d0310b","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":67},"updated":"2021-01-13 09:11:35.000000000","message":"My understanding is that the events should only be emitted when both configs are updated now so we shouldn\u0027t technically need to always retry unless we see VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"84f83073347e44c5551fd57d3073a26b58f078b1","unresolved":true,"context_lines":[{"line_number":27,"context_line":"* is it OK to ignore the error if the event is not received within 60"},{"line_number":28,"context_line":"  seconds? The original code ignored the error if run out of retry"},{"line_number":29,"context_line":"  attempts."},{"line_number":30,"context_line":"* there was two detach attempts in the original code if the domain had  both"},{"line_number":31,"context_line":"  live and persistent configs. It was implemented by retrying the detach. The"},{"line_number":32,"context_line":"  current code has no such logic. Is it OK to simply send one single"},{"line_number":33,"context_line":"  detach request regardless of the domain being persistent \u0026\u0026 live?"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"TODO:"},{"line_number":36,"context_line":"* resolve the above questions"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"6caa3985_8b7f3ece","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":67},"in_reply_to":"8baef5da_11d0310b","updated":"2021-01-13 09:21:42.000000000","message":"As above, we need to retry until VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED and/or the device has been removed from both configs.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"520181287110bfa535285a5be23898b485771352","unresolved":true,"context_lines":[{"line_number":31,"context_line":"  live and persistent configs. It was implemented by retrying the detach. The"},{"line_number":32,"context_line":"  current code has no such logic. Is it OK to simply send one single"},{"line_number":33,"context_line":"  detach request regardless of the domain being persistent \u0026\u0026 live?"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"TODO:"},{"line_number":36,"context_line":"* resolve the above questions"},{"line_number":37,"context_line":"* do internal memory cleanup when a guest is destroyed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"67d2fa3b_57e2e630","line":34,"updated":"2021-01-13 10:24:39.000000000","message":"OK we also discussed things in IRC[1] based on Lee\u0027s above comments. So I\u0027m summarizing here:\n\n* the get_device_conf_func() we use to check if the device exists looks into the live domain as it uses XMLDesc(0)\n\n* sync device not found in case of the domain both live and persistent should cause a retry with persistent\u003dfalse if the device still exists in the domain\n\n* sync device not found error in case there is no two domains should be treated as error\n\n* async error with DeviceRemovalFailedEvent (VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED) should trigger a retry\n\n* before we retry it is safe to check if the device is still exists in the domain with get_device_conf_func to see if we really have to retry.\n\n* our async timeout should be bigger than the libivirt internal timeout\n\n\n\n[1] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2021-01-13.log.html#t2021-01-13T08:49:43","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":true,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"TODO:"},{"line_number":45,"context_line":"* make sure that the nova timeout value is bigger than the libvirt\u0027s"},{"line_number":46,"context_line":"  timeout value for the detach event"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Related-Bug: #1882521"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a82c354_0e9f3e5b","line":46,"updated":"2021-01-22 18:15:12.000000000","message":"+ split up the current patch to smaller pieces(see inline)","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aec07944bafbbe0f7f79ecf5176ac591c7d652e2","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"TODO:"},{"line_number":45,"context_line":"* make sure that the nova timeout value is bigger than the libvirt\u0027s"},{"line_number":46,"context_line":"  timeout value for the detach event"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Related-Bug: #1882521"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"5e62f2cf_66d8290f","line":46,"in_reply_to":"7a82c354_0e9f3e5b","updated":"2021-01-25 17:32:19.000000000","message":"Done","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"38831d05f76612fad6ba0d78c9f687d353fc9d87","unresolved":true,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"TODO:"},{"line_number":45,"context_line":"* make sure that the nova timeout value is bigger than the libvirt\u0027s"},{"line_number":46,"context_line":"  timeout value for the detach event"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Related-Bug: #1882521"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"3175de30_6ff3e05b","line":46,"updated":"2021-01-27 16:55:39.000000000","message":"I gave up finding any timeout handling in the libvirt codebase. So I assume the event is coming from QEMU and libvirt only translates it.","commit_id":"60142cb7777506a81e4712d1cbfd7cb08cb77762"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0da0b9c8448a3b637f50d3cc8047cc4716ad1f7b","unresolved":true,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"TODO:"},{"line_number":45,"context_line":"* make sure that the nova timeout value is bigger than the libvirt\u0027s"},{"line_number":46,"context_line":"  timeout value for the detach event"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Related-Bug: #1882521"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"af7ba2ad_9ebdd72d","line":46,"in_reply_to":"3175de30_6ff3e05b","updated":"2021-01-27 16:59:07.000000000","message":"i think the event is commint form the qemu monitor process but i have nerver actully looked myself.","commit_id":"60142cb7777506a81e4712d1cbfd7cb08cb77762"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"64dd8caa09eff769e0967165c00e90fa8df0a088","unresolved":true,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"TODO:"},{"line_number":45,"context_line":"* make sure that the nova timeout value is bigger than the libvirt\u0027s"},{"line_number":46,"context_line":"  timeout value for the detach event"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Related-Bug: #1882521"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":16,"id":"e59690e4_f4a99385","line":46,"in_reply_to":"af7ba2ad_9ebdd72d","updated":"2021-01-29 12:16:01.000000000","message":"I spent some time in the qemu codebase but honestly I did not find any timeout value there either. There might be no timeout at all. Which is fine I guess. So I will go and keep the current 20sec in nova. It is plenty and it only \"used\" if there is no event received. I see two examples in tempest where the first detach attempt result in no event, but then the event is sent after the retry. So there are cases where we still have to retry and there we will do it in 20sec.","commit_id":"60142cb7777506a81e4712d1cbfd7cb08cb77762"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Replace blind retry with libvirt event waiting in detach"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova so far applied a retry loop that tried to periodically detach the"},{"line_number":10,"context_line":"device from libvirt while the device was visible in the domain xml. This"},{"line_number":11,"context_line":"could lead to an issue where an already progressing detach on the"},{"line_number":12,"context_line":"libvirt side is interrupted by nova re-sending the detach request for"},{"line_number":13,"context_line":"the same device."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Also if there was both a persistent and a live domain the nova tried the"},{"line_number":16,"context_line":"detach from both at the same call. This lead to confusion about the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"e7d8efcf_eaba7186","line":13,"range":{"start_line":9,"start_character":0,"end_line":13,"end_character":16},"updated":"2021-02-16 15:55:48.000000000","message":"Might be worth referencing bug #1882521 here.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fb40f6b4ff939332797a91b51bc8a44b42896e8d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Replace blind retry with libvirt event waiting in detach"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova so far applied a retry loop that tried to periodically detach the"},{"line_number":10,"context_line":"device from libvirt while the device was visible in the domain xml. This"},{"line_number":11,"context_line":"could lead to an issue where an already progressing detach on the"},{"line_number":12,"context_line":"libvirt side is interrupted by nova re-sending the detach request for"},{"line_number":13,"context_line":"the same device."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Also if there was both a persistent and a live domain the nova tried the"},{"line_number":16,"context_line":"detach from both at the same call. This lead to confusion about the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"7325724a_2c356390","line":13,"range":{"start_line":9,"start_character":0,"end_line":13,"end_character":16},"in_reply_to":"e7d8efcf_eaba7186","updated":"2021-02-22 15:54:32.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":false,"context_lines":[{"line_number":30,"context_line":""},{"line_number":31,"context_line":"2) Changes the retry mechanism."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"   Detaching from the persistent domain is not retried. If libvirt"},{"line_number":34,"context_line":"   reports device not found, while both persistent and live detach"},{"line_number":35,"context_line":"   is needed, the error is ignored, and the process continues with"},{"line_number":36,"context_line":"   the live detach. In any other case the error considered as fatal."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"   Detaching from the live domain is changed to always wait for the"},{"line_number":39,"context_line":"   libvirt event. In case of timeout, the live detach is retried."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"b1e21b23_fdf5374b","line":36,"range":{"start_line":33,"start_character":0,"end_line":36,"end_character":68},"updated":"2021-02-16 15:55:48.000000000","message":"✔","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":false,"context_lines":[{"line_number":35,"context_line":"   is needed, the error is ignored, and the process continues with"},{"line_number":36,"context_line":"   the live detach. In any other case the error considered as fatal."},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"   Detaching from the live domain is changed to always wait for the"},{"line_number":39,"context_line":"   libvirt event. In case of timeout, the live detach is retried."},{"line_number":40,"context_line":"   But a failure event from libvirt considered fatal, based on the"},{"line_number":41,"context_line":"   information from the libvirt developers, so in this case the"},{"line_number":42,"context_line":"   detach is not retried."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Related-Bug: #1882521"},{"line_number":45,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"5bc4e6db_40680865","line":42,"range":{"start_line":38,"start_character":0,"end_line":42,"end_character":25},"updated":"2021-02-16 15:55:48.000000000","message":"✔","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"}],"nova/conf/libvirt.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":868,"context_line":"  :oslo.config:option:`libvirt.hw_machine_type`; see"},{"line_number":869,"context_line":"  :ref:`deploying-sev-capable-infrastructure` for more on this."},{"line_number":870,"context_line":"\"\"\"),"},{"line_number":871,"context_line":"    cfg.IntOpt(\u0027device_detach_attempts\u0027,"},{"line_number":872,"context_line":"               default\u003d8,"},{"line_number":873,"context_line":"               min\u003d1,"},{"line_number":874,"context_line":"               help\u003d\"\"\""},{"line_number":875,"context_line":"Maximum number of attempts the driver tries to detach a device in libvirt."},{"line_number":876,"context_line":""},{"line_number":877,"context_line":"Related options:"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"* :oslo.config:option:`libvirt.device_detach_timeout`"},{"line_number":880,"context_line":""},{"line_number":881,"context_line":"\"\"\"),"},{"line_number":882,"context_line":"    cfg.IntOpt(\u0027device_detach_timeout\u0027,"},{"line_number":883,"context_line":"               default\u003d20,"},{"line_number":884,"context_line":"               min\u003d1,"},{"line_number":885,"context_line":"               help\u003d\"\"\""},{"line_number":886,"context_line":"Maximum number of seconds the driver waits for the success or the failure"},{"line_number":887,"context_line":"event from libvirt for a given device detach attempt before it re-trigger the"},{"line_number":888,"context_line":"detach."},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"Related options:"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"* :oslo.config:option:`libvirt.device_detach_attempts`"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"\"\"\"),"},{"line_number":895,"context_line":"]"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"libvirt_imagebackend_opts \u003d ["}],"source_content_type":"text/x-python","patch_set":24,"id":"631e05ce_5c5614b8","line":894,"range":{"start_line":871,"start_character":0,"end_line":894,"end_character":5},"updated":"2021-03-08 10:25:01.000000000","message":"Need a releasenote for these.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":868,"context_line":"  :oslo.config:option:`libvirt.hw_machine_type`; see"},{"line_number":869,"context_line":"  :ref:`deploying-sev-capable-infrastructure` for more on this."},{"line_number":870,"context_line":"\"\"\"),"},{"line_number":871,"context_line":"    cfg.IntOpt(\u0027device_detach_attempts\u0027,"},{"line_number":872,"context_line":"               default\u003d8,"},{"line_number":873,"context_line":"               min\u003d1,"},{"line_number":874,"context_line":"               help\u003d\"\"\""},{"line_number":875,"context_line":"Maximum number of attempts the driver tries to detach a device in libvirt."},{"line_number":876,"context_line":""},{"line_number":877,"context_line":"Related options:"},{"line_number":878,"context_line":""},{"line_number":879,"context_line":"* :oslo.config:option:`libvirt.device_detach_timeout`"},{"line_number":880,"context_line":""},{"line_number":881,"context_line":"\"\"\"),"},{"line_number":882,"context_line":"    cfg.IntOpt(\u0027device_detach_timeout\u0027,"},{"line_number":883,"context_line":"               default\u003d20,"},{"line_number":884,"context_line":"               min\u003d1,"},{"line_number":885,"context_line":"               help\u003d\"\"\""},{"line_number":886,"context_line":"Maximum number of seconds the driver waits for the success or the failure"},{"line_number":887,"context_line":"event from libvirt for a given device detach attempt before it re-trigger the"},{"line_number":888,"context_line":"detach."},{"line_number":889,"context_line":""},{"line_number":890,"context_line":"Related options:"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"* :oslo.config:option:`libvirt.device_detach_attempts`"},{"line_number":893,"context_line":""},{"line_number":894,"context_line":"\"\"\"),"},{"line_number":895,"context_line":"]"},{"line_number":896,"context_line":""},{"line_number":897,"context_line":"libvirt_imagebackend_opts \u003d ["}],"source_content_type":"text/x-python","patch_set":24,"id":"0b167d09_8079ddb5","line":894,"range":{"start_line":871,"start_character":0,"end_line":894,"end_character":5},"in_reply_to":"631e05ce_5c5614b8","updated":"2021-04-14 16:33:36.000000000","message":"Good point. Done.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"}],"nova/tests/unit/virt/libvirt/fakelibvirt.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8b6d09d9e4996778e36058625694ed1a9e497f05","unresolved":true,"context_lines":[{"line_number":1121,"context_line":"            self._def[\u0027devices\u0027][\u0027nics\u0027] +\u003d [nic_info]"},{"line_number":1122,"context_line":"            result \u003d True"},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"        print(\u0027attached device\u0027, xml)"},{"line_number":1125,"context_line":"        return result"},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":"    def attachDeviceFlags(self, xml, flags):"}],"source_content_type":"text/x-python","patch_set":16,"id":"a062c94d_c9c97fcf","line":1124,"updated":"2021-01-26 17:17:11.000000000","message":"drop it","commit_id":"60142cb7777506a81e4712d1cbfd7cb08cb77762"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"44c31e7cc4b727b80772916cfbec51188d855815","unresolved":false,"context_lines":[{"line_number":1121,"context_line":"            self._def[\u0027devices\u0027][\u0027nics\u0027] +\u003d [nic_info]"},{"line_number":1122,"context_line":"            result \u003d True"},{"line_number":1123,"context_line":""},{"line_number":1124,"context_line":"        print(\u0027attached device\u0027, xml)"},{"line_number":1125,"context_line":"        return result"},{"line_number":1126,"context_line":""},{"line_number":1127,"context_line":"    def attachDeviceFlags(self, xml, flags):"}],"source_content_type":"text/x-python","patch_set":16,"id":"b1264a0e_9d88aa61","line":1124,"in_reply_to":"a062c94d_c9c97fcf","updated":"2021-01-29 12:45:59.000000000","message":"Done","commit_id":"60142cb7777506a81e4712d1cbfd7cb08cb77762"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":1134,"context_line":"        self.attachDevice(xml)"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"    def detachDevice(self, xml):"},{"line_number":1137,"context_line":"        # NOTE(gibi): this should handle nics similarly to attachDevice()"},{"line_number":1138,"context_line":"        disk_info \u003d _parse_disk_info(etree.fromstring(xml))"},{"line_number":1139,"context_line":"        attached_disk_info \u003d None"},{"line_number":1140,"context_line":"        for attached_disk in self._def[\u0027devices\u0027][\u0027disks\u0027]:"}],"source_content_type":"text/x-python","patch_set":18,"id":"22c563f0_7d46b269","line":1137,"range":{"start_line":1137,"start_character":10,"end_line":1137,"end_character":14},"updated":"2021-02-09 17:38:06.000000000","message":"Probably more of a TODO?","commit_id":"ac45794d9b7a90d6725df678dc46f37debb3cb62"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        self.attachDevice(xml)"},{"line_number":1135,"context_line":""},{"line_number":1136,"context_line":"    def detachDevice(self, xml):"},{"line_number":1137,"context_line":"        # NOTE(gibi): this should handle nics similarly to attachDevice()"},{"line_number":1138,"context_line":"        disk_info \u003d _parse_disk_info(etree.fromstring(xml))"},{"line_number":1139,"context_line":"        attached_disk_info \u003d None"},{"line_number":1140,"context_line":"        for attached_disk in self._def[\u0027devices\u0027][\u0027disks\u0027]:"}],"source_content_type":"text/x-python","patch_set":18,"id":"b84de3be_503d49dd","line":1137,"range":{"start_line":1137,"start_character":10,"end_line":1137,"end_character":14},"in_reply_to":"22c563f0_7d46b269","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"ac45794d9b7a90d6725df678dc46f37debb3cb62"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a571262d6792828b3125b10dcf8e5a6f5498872f","unresolved":true,"context_lines":[{"line_number":1283,"context_line":"        attached_disk_info \u003d None"},{"line_number":1284,"context_line":"        for attached_disk in self._def[\u0027devices\u0027][\u0027disks\u0027]:"},{"line_number":1285,"context_line":"            if attached_disk[\u0027target_dev\u0027] \u003d\u003d disk_info.get(\u0027target_dev\u0027):"},{"line_number":1286,"context_line":"                attached_disk_info \u003d attached_disk"},{"line_number":1287,"context_line":""},{"line_number":1288,"context_line":"        if attached_disk_info:"},{"line_number":1289,"context_line":"            self._def[\u0027devices\u0027][\u0027disks\u0027].remove(attached_disk_info)"}],"source_content_type":"text/x-python","patch_set":26,"id":"55fa672b_2555c1c4","line":1286,"updated":"2021-04-15 11:26:03.000000000","message":"nit: break","commit_id":"c04e34c7292ee3da718d2fac9bd9b86f52deca84"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3b13f7a153a74def22ea17338c8c648215794523","unresolved":false,"context_lines":[{"line_number":1283,"context_line":"        attached_disk_info \u003d None"},{"line_number":1284,"context_line":"        for attached_disk in self._def[\u0027devices\u0027][\u0027disks\u0027]:"},{"line_number":1285,"context_line":"            if attached_disk[\u0027target_dev\u0027] \u003d\u003d disk_info.get(\u0027target_dev\u0027):"},{"line_number":1286,"context_line":"                attached_disk_info \u003d attached_disk"},{"line_number":1287,"context_line":""},{"line_number":1288,"context_line":"        if attached_disk_info:"},{"line_number":1289,"context_line":"            self._def[\u0027devices\u0027][\u0027disks\u0027].remove(attached_disk_info)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5e19cdc3_ea891ae4","line":1286,"in_reply_to":"55fa672b_2555c1c4","updated":"2021-04-16 14:12:40.000000000","message":"Done","commit_id":"c04e34c7292ee3da718d2fac9bd9b86f52deca84"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":9635,"context_line":"                self.assertXmlEqual(xml, call.args[0])"},{"line_number":9636,"context_line":"                self.assertEqual("},{"line_number":9637,"context_line":"                    {\"flags\": fakelibvirt.VIR_DOMAIN_AFFECT_LIVE},"},{"line_number":9638,"context_line":"                    call.kwargs)"},{"line_number":9639,"context_line":""},{"line_number":9640,"context_line":"                mock_disconnect_volume.assert_called_with("},{"line_number":9641,"context_line":"                    self.context, connection_info, instance, encryption\u003dNone)"}],"source_content_type":"text/x-python","patch_set":18,"id":"1a9729f1_c9fd8870","line":9638,"updated":"2021-02-09 17:38:06.000000000","message":"running commentary on why you\u0027re doing these assertions would be appreciated","commit_id":"ac45794d9b7a90d6725df678dc46f37debb3cb62"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":9635,"context_line":"                self.assertXmlEqual(xml, call.args[0])"},{"line_number":9636,"context_line":"                self.assertEqual("},{"line_number":9637,"context_line":"                    {\"flags\": fakelibvirt.VIR_DOMAIN_AFFECT_LIVE},"},{"line_number":9638,"context_line":"                    call.kwargs)"},{"line_number":9639,"context_line":""},{"line_number":9640,"context_line":"                mock_disconnect_volume.assert_called_with("},{"line_number":9641,"context_line":"                    self.context, connection_info, instance, encryption\u003dNone)"}],"source_content_type":"text/x-python","patch_set":18,"id":"e647d20f_abaf7929","line":9638,"in_reply_to":"1a9729f1_c9fd8870","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"ac45794d9b7a90d6725df678dc46f37debb3cb62"}],"nova/virt/event.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"class LibvirtEvents(InstanceEvent):"},{"line_number":126,"context_line":"    \"\"\"Base class for virt events that are specific to libvirt and therefore"},{"line_number":127,"context_line":"    handled in the libvirt driver level instead of propagatig it up to the"},{"line_number":128,"context_line":"    compute manager."},{"line_number":129,"context_line":"    \"\"\""},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"48e190d8_74e6f4a8","line":127,"range":{"start_line":127,"start_character":51,"end_line":127,"end_character":61},"updated":"2021-01-13 01:51:47.000000000","message":"propagating","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":12,"id":"be6b04ef_b4258a29","updated":"2021-01-22 18:15:12.000000000","message":"move the changes from here to the previous patch","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aec07944bafbbe0f7f79ecf5176ac591c7d652e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"12bece8d_2b9b35c5","in_reply_to":"be6b04ef_b4258a29","updated":"2021-01-25 17:32:19.000000000","message":"Done","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":true,"context_lines":[{"line_number":1037,"context_line":"        self.boot_order \u003d None"},{"line_number":1038,"context_line":"        self.mirror \u003d None"},{"line_number":1039,"context_line":"        self.encryption \u003d None"},{"line_number":1040,"context_line":"        self.alias \u003d None"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":"    def _format_iotune(self, dev):"},{"line_number":1043,"context_line":"        iotune \u003d etree.Element(\"iotune\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"d11c53a2_78f286ee","line":1040,"range":{"start_line":1040,"start_character":13,"end_line":1040,"end_character":20},"updated":"2021-01-22 18:15:12.000000000","message":"this change can be done in a separate patch","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4e6b1319fe02e23f383dbeb2e4d838632bf5af55","unresolved":true,"context_lines":[{"line_number":2080,"context_line":"        LOG.debug(\u0027Attempting initial detach for device %s\u0027,"},{"line_number":2081,"context_line":"                  alternative_device_name)"},{"line_number":2082,"context_line":""},{"line_number":2083,"context_line":"        # XXX(gibi): we don\u0027t have a good ID for the event. The"},{"line_number":2084,"context_line":"        # alternate_device_name here is \u0027vdb\u0027 but when we receive the event"},{"line_number":2085,"context_line":"        # from libvirt it contains device name as \u0027virtio-disk1\u0027"},{"line_number":2086,"context_line":"        event_name \u003d f\"{instance_uuid}-{alternative_device_name}\""},{"line_number":2087,"context_line":"        try:"},{"line_number":2088,"context_line":"            # So we will issue an detach to libvirt and we will wait for an"},{"line_number":2089,"context_line":"            # event from libvirt about the result. We need to set up the event"},{"line_number":2090,"context_line":"            # handling before the detach to avoid missing the event if libvirt"}],"source_content_type":"text/x-python","patch_set":1,"id":"8340b5e5_6624611e","line":2087,"range":{"start_line":2083,"start_character":0,"end_line":2087,"end_character":1},"updated":"2021-01-12 11:33:25.000000000","message":"Jan 12 11:08:53 aio nova-compute[361320]:     \u003cdisk type\u003d\u0027block\u0027 device\u003d\u0027disk\u0027\u003e\nJan 12 11:08:53 aio nova-compute[361320]:       \u003cdriver name\u003d\u0027qemu\u0027 type\u003d\u0027raw\u0027 cache\u003d\u0027none\u0027 io\u003d\u0027native\u0027/\u003e\nJan 12 11:08:53 aio nova-compute[361320]:       \u003csource dev\u003d\u0027/dev/sda\u0027 index\u003d\u00271\u0027/\u003e\nJan 12 11:08:53 aio nova-compute[361320]:       \u003cbackingStore/\u003e\nJan 12 11:08:53 aio nova-compute[361320]:       \u003ctarget dev\u003d\u0027vdb\u0027 bus\u003d\u0027virtio\u0027/\u003e\nJan 12 11:08:53 aio nova-compute[361320]:       \u003cserial\u003e5d182d2a-fdeb-4a76-abd1-e6d74a038fbe\u003c/serial\u003e\nJan 12 11:08:53 aio nova-compute[361320]:       \u003calias name\u003d\u0027virtio-disk1\u0027/\u003e\n\n\nSo we have the necessary name used by the event in the domain as alias name. just need some enhancement on our disk model to contain it.","commit_id":"a28a4b43e09a174c15b41d8e1b48793bd2b12a1c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":218,"context_line":"        self._lock \u003d threading.Lock()"},{"line_number":219,"context_line":"        # Ongoing device detaches in libvirt where we wait for the events"},{"line_number":220,"context_line":"        # about success or failure. It is a dict of threading.Event objects"},{"line_number":221,"context_line":"        # keyed by \u003cinstance.uuid\u003e-\u003cdevice-name\u003e string."},{"line_number":222,"context_line":"        self._waiters: ty.Dict[str, threading.Event] \u003d {}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def create_waiter("}],"source_content_type":"text/x-python","patch_set":2,"id":"8005776b_8829a040","line":221,"range":{"start_line":221,"start_character":36,"end_line":221,"end_character":47},"updated":"2021-01-13 01:51:47.000000000","message":"device_name","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":218,"context_line":"        self._lock \u003d threading.Lock()"},{"line_number":219,"context_line":"        # Ongoing device detaches in libvirt where we wait for the events"},{"line_number":220,"context_line":"        # about success or failure. It is a dict of threading.Event objects"},{"line_number":221,"context_line":"        # keyed by \u003cinstance.uuid\u003e-\u003cdevice-name\u003e string."},{"line_number":222,"context_line":"        self._waiters: ty.Dict[str, threading.Event] \u003d {}"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def create_waiter("}],"source_content_type":"text/x-python","patch_set":2,"id":"95d38d2b_e61b08e6","line":221,"range":{"start_line":221,"start_character":20,"end_line":221,"end_character":33},"updated":"2021-01-13 01:51:47.000000000","message":"instance_uuid","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":230,"context_line":"        libvirt event is received."},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        :param instance_uuid: The UUID of the instance."},{"line_number":233,"context_line":"        :param device-name: The device name alias used by libvirt for this"},{"line_number":234,"context_line":"            device."},{"line_number":235,"context_line":"        :returns: A threading.Event() object to wait on unit the libvirt event"},{"line_number":236,"context_line":"            is received."}],"source_content_type":"text/x-python","patch_set":2,"id":"5ada1294_734f65e6","line":233,"range":{"start_line":233,"start_character":15,"end_line":233,"end_character":26},"updated":"2021-01-13 01:51:47.000000000","message":"device_name","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":232,"context_line":"        :param instance_uuid: The UUID of the instance."},{"line_number":233,"context_line":"        :param device-name: The device name alias used by libvirt for this"},{"line_number":234,"context_line":"            device."},{"line_number":235,"context_line":"        :returns: A threading.Event() object to wait on unit the libvirt event"},{"line_number":236,"context_line":"            is received."},{"line_number":237,"context_line":"        \"\"\""},{"line_number":238,"context_line":"        event_name \u003d f\"{instance_uuid}-{device_name}\""}],"source_content_type":"text/x-python","patch_set":2,"id":"edef372e_aa81e6cf","line":235,"range":{"start_line":235,"start_character":56,"end_line":235,"end_character":60},"updated":"2021-01-13 01:51:47.000000000","message":"until","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e19e162451699016f8e09a60496ea5f714b1f17d","unresolved":true,"context_lines":[{"line_number":247,"context_line":"                # event for both requests so if the detach libvirt event for"},{"line_number":248,"context_line":"                # this device is received then both waiting detach requests"},{"line_number":249,"context_line":"                # will be unblocked."},{"line_number":250,"context_line":"                LOG.warning("},{"line_number":251,"context_line":"                    \u0027!!! Device detach event handle already exists for \u0027"},{"line_number":252,"context_line":"                    \u0027instance %s and device %s. This should not happen. \u0027"},{"line_number":253,"context_line":"                    \u0027Driver will notify all handlers waiting for this event.\u0027,"},{"line_number":254,"context_line":"                    instance_uuid, device_name)"},{"line_number":255,"context_line":"            else:"},{"line_number":256,"context_line":"                threading_event \u003d threading.Event()"},{"line_number":257,"context_line":"                self._waiters[event_name] \u003d threading_event"}],"source_content_type":"text/x-python","patch_set":2,"id":"cf444336_6ae4536b","line":254,"range":{"start_line":250,"start_character":14,"end_line":254,"end_character":47},"updated":"2021-01-14 20:30:51.000000000","message":"ok so we can use this log message to assert that we dont race right.\n\nyou could do this slighly differently\n\nnew_event \u003d threading.Event()\nthreading_event \u003d self._waiters.set_default(event_name, new_event)\nif threading_event is not new_event:\n  LOG.warning(...)\nreturn threading_event\n\n\"is not\"  will do an identity check not equality so ti will compare the adress of the event.\n\nif set default set the value because it did not exist then thread_event will be the same\nif not it will be the existing waiter tread event.\ni think that would be more idiomatic but what you have is perfectly valid.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":263,"context_line":"        instance_uuid: str,"},{"line_number":264,"context_line":"        device_name: str"},{"line_number":265,"context_line":"    ) -\u003e bool:"},{"line_number":266,"context_line":"        \"\"\"Ublocks the client waiting for this event."},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        :param instance_uuid: The UUID of the instance."},{"line_number":269,"context_line":"        :param device-name: The device name alias used by libvirt for this"}],"source_content_type":"text/x-python","patch_set":2,"id":"bc58cbf4_755beb06","line":266,"range":{"start_line":266,"start_character":11,"end_line":266,"end_character":18},"updated":"2021-01-13 01:51:47.000000000","message":"Unblocks?","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":266,"context_line":"        \"\"\"Ublocks the client waiting for this event."},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        :param instance_uuid: The UUID of the instance."},{"line_number":269,"context_line":"        :param device-name: The device name alias used by libvirt for this"},{"line_number":270,"context_line":"            device."},{"line_number":271,"context_line":"        :returns: True if there was a client waiting and False otherwise."},{"line_number":272,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":2,"id":"876fcecf_4c4be406","line":269,"range":{"start_line":269,"start_character":15,"end_line":269,"end_character":26},"updated":"2021-01-13 01:51:47.000000000","message":"device_name","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e19e162451699016f8e09a60496ea5f714b1f17d","unresolved":true,"context_lines":[{"line_number":279,"context_line":"        if threading_event is None:"},{"line_number":280,"context_line":"            return False"},{"line_number":281,"context_line":"        else:"},{"line_number":282,"context_line":"            threading_event.set()"},{"line_number":283,"context_line":"            return True"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def remove_waiter(self, instance_uuid: str, device_name: str) -\u003e None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7d64c975_4f605c2f","line":282,"range":{"start_line":282,"start_character":12,"end_line":282,"end_character":33},"updated":"2021-01-14 20:30:51.000000000","message":"this is basically thread.notify right?\n\ne.g. it notifys all greanthread that are waiting on the event that the event has not been triggered/completed and they can resume","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":285,"context_line":"    def remove_waiter(self, instance_uuid: str, device_name: str) -\u003e None:"},{"line_number":286,"context_line":"        \"\"\"Removes the waiter without notifying it."},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        It is not an error if the waiter does not exists."},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        :param instance_uuid: The UUID of the instance."},{"line_number":291,"context_line":"        :param device-name: The device name alias used by libvirt for this"}],"source_content_type":"text/x-python","patch_set":2,"id":"656ae27a_354b7fb8","line":288,"range":{"start_line":288,"start_character":50,"end_line":288,"end_character":56},"updated":"2021-01-13 01:51:47.000000000","message":"exist","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":288,"context_line":"        It is not an error if the waiter does not exists."},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"        :param instance_uuid: The UUID of the instance."},{"line_number":291,"context_line":"        :param device-name: The device name alias used by libvirt for this"},{"line_number":292,"context_line":"            device."},{"line_number":293,"context_line":"        \"\"\""},{"line_number":294,"context_line":"        event_name \u003d f\"{instance_uuid}-{device_name}\""}],"source_content_type":"text/x-python","patch_set":2,"id":"a9b24060_5ba27640","line":291,"range":{"start_line":291,"start_character":15,"end_line":291,"end_character":26},"updated":"2021-01-13 01:51:47.000000000","message":"device_name","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e19e162451699016f8e09a60496ea5f714b1f17d","unresolved":true,"context_lines":[{"line_number":296,"context_line":"        with self._lock:"},{"line_number":297,"context_line":"            self._waiters.pop(event_name, None)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def remove_all_waiters(self, instance_uuid):"},{"line_number":300,"context_line":"        # XXX(gibi): to avoid old, invalid entreis remove all waiters for an"},{"line_number":301,"context_line":"        # instance when the instance is destroyed"},{"line_number":302,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"50bca799_f59f71ac","line":299,"range":{"start_line":299,"start_character":8,"end_line":299,"end_character":26},"updated":"2021-01-14 20:30:51.000000000","message":"so we could do this in a few ways.\n\nwe could as written we don\u0027t actually have an map of instance to a list of events\nso if we want to just use the uuid we have 2 options\n\nadd\ninstance_event\u003d default_dict(list)\nand append the event to that in create_waiter.\n\nthe other option for finding the events is  to to a liner search under a lock and pop all events that start with the instance uuid. to avoid a concurrent modification issue we would have to do that in two passes adding all  the event to be removed to a list and then popping them.\n\nin both cases we have two ways to deal with the pending events.\neither call set() on each event, or do nothing and have all instance of wait use a time-out.\n\nif we call set we have to differentiate between a completion with success vs as a result of remove all.\nif we go the time-out route you can check the return value of wait to determine if it succeeded or was cancelled.\n\n\ni think the time-out and wait is probably the way to go so this function would just delete the events.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":297,"context_line":"            self._waiters.pop(event_name, None)"},{"line_number":298,"context_line":""},{"line_number":299,"context_line":"    def remove_all_waiters(self, instance_uuid):"},{"line_number":300,"context_line":"        # XXX(gibi): to avoid old, invalid entreis remove all waiters for an"},{"line_number":301,"context_line":"        # instance when the instance is destroyed"},{"line_number":302,"context_line":"        pass"},{"line_number":303,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d27f9aa0_6a2f452e","line":300,"range":{"start_line":300,"start_character":43,"end_line":300,"end_character":50},"updated":"2021-01-13 01:51:47.000000000","message":"entries","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e19e162451699016f8e09a60496ea5f714b1f17d","unresolved":true,"context_lines":[{"line_number":299,"context_line":"    def remove_all_waiters(self, instance_uuid):"},{"line_number":300,"context_line":"        # XXX(gibi): to avoid old, invalid entreis remove all waiters for an"},{"line_number":301,"context_line":"        # instance when the instance is destroyed"},{"line_number":302,"context_line":"        pass"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"# For information about when MIN_LIBVIRT_VERSION and"}],"source_content_type":"text/x-python","patch_set":2,"id":"6f458224_2c4fa7e6","line":302,"range":{"start_line":302,"start_character":8,"end_line":302,"end_character":12},"updated":"2021-01-14 20:30:51.000000000","message":"i would make this \"raise NotImplemented\" for now","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":2169,"context_line":"        supports_device_missing_error_code: bool \u003d False"},{"line_number":2170,"context_line":"    ) -\u003e bool:"},{"line_number":2171,"context_line":"        \"\"\"Detaches a device from the guest and wait for the libvirt event"},{"line_number":2172,"context_line":"        singalling the finish of the detach."},{"line_number":2173,"context_line":""},{"line_number":2174,"context_line":"        :param get_device_conf_func: function which returns the configuration"},{"line_number":2175,"context_line":"            for device from the domain"}],"source_content_type":"text/x-python","patch_set":2,"id":"c68bb37a_94166358","line":2172,"range":{"start_line":2172,"start_character":8,"end_line":2172,"end_character":18},"updated":"2021-01-13 01:51:47.000000000","message":"signaling","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":2179,"context_line":"            solely for error messages."},{"line_number":2180,"context_line":"        :param supports_device_missing_error_code: does the installed version"},{"line_number":2181,"context_line":"            of libvirt provide the VIR_ERR_DEVICE_MISSING error code."},{"line_number":2182,"context_line":"        :returns: True if the detach was successfull. Returns False if the"},{"line_number":2183,"context_line":"            detach was unsuccessfully asynchronously."},{"line_number":2184,"context_line":"        :raises exception.DeviceNotFound: if the device was not found in the"},{"line_number":2185,"context_line":"            domain before the detach or if libvirt reported the device is"}],"source_content_type":"text/x-python","patch_set":2,"id":"d5f39744_782f5aa5","line":2182,"range":{"start_line":2182,"start_character":41,"end_line":2182,"end_character":52},"updated":"2021-01-13 01:51:47.000000000","message":"successful","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":2180,"context_line":"        :param supports_device_missing_error_code: does the installed version"},{"line_number":2181,"context_line":"            of libvirt provide the VIR_ERR_DEVICE_MISSING error code."},{"line_number":2182,"context_line":"        :returns: True if the detach was successfull. Returns False if the"},{"line_number":2183,"context_line":"            detach was unsuccessfully asynchronously."},{"line_number":2184,"context_line":"        :raises exception.DeviceNotFound: if the device was not found in the"},{"line_number":2185,"context_line":"            domain before the detach or if libvirt reported the device is"},{"line_number":2186,"context_line":"            missing synchronously."}],"source_content_type":"text/x-python","patch_set":2,"id":"e067433a_88af8b48","line":2183,"range":{"start_line":2183,"start_character":23,"end_line":2183,"end_character":37},"updated":"2021-01-13 01:51:47.000000000","message":"unsuccessful","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":2228,"context_line":"            # we should still try to detach from the live config, so continue."},{"line_number":2229,"context_line":"            # XXX(gibi): so a persistent\u003dTrue and live\u003dTrue domain might needed"},{"line_number":2230,"context_line":"            # two guest.detach_device call in the past. Should we replicate"},{"line_number":2231,"context_line":"            # that here too? How?"},{"line_number":2232,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse) as ctx:"},{"line_number":2233,"context_line":"                errcode \u003d ex.get_error_code()"},{"line_number":2234,"context_line":"                # TODO(lyarwood): Remove libvirt.VIR_ERR_OPERATION_FAILED"}],"source_content_type":"text/x-python","patch_set":2,"id":"0d8c117c_ff39e8f1","line":2231,"updated":"2021-01-13 01:51:47.000000000","message":"I should really rewrite this comment :/ but the scenario I was trying to describe is one where the persistent config was successfully updated to remove the volume in the past, but the live config still has the volume showing as attached.\n\nRemoval of the volume from the persistent config always succeeds (unless it is not found/already detached). But the removal of the volume from the live config can be refused by the guest. We have had customer scenarios where they tried to detach a volume from an instance but there was some file open on the volume or other issue that prevented detach from the live config.\n\nIn this case, they will try to detach the volume at a later time (in a new, separate request). We need to make sure we handle this case, where if we detect a device not found for the persistent config when we call with persistent\u003dTrue, we should not short-circuit and leave the method. We need to try again with persistent\u003dFalse and live\u003dTrue to try to detach from only the live config.\n\nThe old bug was that the customer became unable to ever detach the volume from the guest\u0027s live config if it was refused once in the past because the method used to short-circuit and return upon detecting the volume already detached from the persistent config.","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e3531a677b97e3718d2dbfc62158215346b1537d","unresolved":true,"context_lines":[{"line_number":2228,"context_line":"            # we should still try to detach from the live config, so continue."},{"line_number":2229,"context_line":"            # XXX(gibi): so a persistent\u003dTrue and live\u003dTrue domain might needed"},{"line_number":2230,"context_line":"            # two guest.detach_device call in the past. Should we replicate"},{"line_number":2231,"context_line":"            # that here too? How?"},{"line_number":2232,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse) as ctx:"},{"line_number":2233,"context_line":"                errcode \u003d ex.get_error_code()"},{"line_number":2234,"context_line":"                # TODO(lyarwood): Remove libvirt.VIR_ERR_OPERATION_FAILED"}],"source_content_type":"text/x-python","patch_set":2,"id":"0ce335a9_5e0e58c9","line":2231,"in_reply_to":"0d8c117c_ff39e8f1","updated":"2021-01-13 08:41:04.000000000","message":"Thanks this helps me understand the case we need to handle. As we discussed on IRC before, the old code relied on get_device_conf_func() to return None after a successful detach to exit from the retry loop. So I guess I can use the same logic here. So L2197 is OK to exit early from the detach code. But at L2260 and L2292 (where we got not found from libvirt for a persistent\u003dtrue live\u003dtrue domain) we need to call detach again forcing persistent\u003dfalse. Still I have to assume that get_device_conf_func considers not just the peristent but the live domain, otherwise the early exit happens (and happened in the past too).\n\nI will add a single retry for the case you described. thanks","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":2245,"context_line":"                        # This will be raised if the live domain"},{"line_number":2246,"context_line":"                        # detach fails because the device is not found"},{"line_number":2247,"context_line":"                        if not persistent or not live:"},{"line_number":2248,"context_line":"                            # XXX(gibi): error case, it was (and is) bubled"},{"line_number":2249,"context_line":"                            # up to the caller"},{"line_number":2250,"context_line":"                            LOG.debug("},{"line_number":2251,"context_line":"                                \u0027!!! Libvirt failed to detach device %s from \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"ec0b89ee_427b76d3","line":2248,"range":{"start_line":2248,"start_character":69,"end_line":2248,"end_character":75},"updated":"2021-01-13 01:51:47.000000000","message":"bubbled","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":2275,"context_line":"                        # This will be raised if the persistent domain"},{"line_number":2276,"context_line":"                        # detach fails because the device is not found"},{"line_number":2277,"context_line":"                        if not persistent or not live:"},{"line_number":2278,"context_line":"                            # XXX(gibi): error case, it was (and is) bubled up"},{"line_number":2279,"context_line":"                            # to the caller"},{"line_number":2280,"context_line":"                            LOG.debug("},{"line_number":2281,"context_line":"                                \u0027!!! Libvirt failed to detach device %s from \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"c368636d_5d8f0267","line":2278,"range":{"start_line":2278,"start_character":69,"end_line":2278,"end_character":75},"updated":"2021-01-13 01:51:47.000000000","message":"bubbled","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1cb7ca9c6c3bfe0262e5eb039bd1826209db9d5a","unresolved":true,"context_lines":[{"line_number":2300,"context_line":"                # Re-raise the original exception if we\u0027re not raising"},{"line_number":2301,"context_line":"                # DeviceNotFound instead. This will avoid logging of a"},{"line_number":2302,"context_line":"                # \"Original exception being dropped\" traceback."},{"line_number":2303,"context_line":"                # XXX(gibi): error case, it was (and it is) bubled up to the"},{"line_number":2304,"context_line":"                # caller"},{"line_number":2305,"context_line":"                ctx.reraise \u003d True"},{"line_number":2306,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5e90667f_3862e735","line":2303,"range":{"start_line":2303,"start_character":60,"end_line":2303,"end_character":66},"updated":"2021-01-13 01:51:47.000000000","message":"bubbled","commit_id":"1bce9297416de616fc0aaa00383d12e16318928b"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"96d1634e1c88a444fe262c52421fa1dd39babe73","unresolved":true,"context_lines":[{"line_number":2299,"context_line":"                \u0027instance %s: %s\u0027, device_name_for_logging, instance_uuid,"},{"line_number":2300,"context_line":"                str(ex))"},{"line_number":2301,"context_line":"            raise"},{"line_number":2302,"context_line":"        finally:"},{"line_number":2303,"context_line":"            # clean up the libvirt event handler. If the detach was a success"},{"line_number":2304,"context_line":"            # it is a noop"},{"line_number":2305,"context_line":"            self._detach_event_handler.remove_waiter(instance_uuid, dev.alias)"},{"line_number":2306,"context_line":""},{"line_number":2307,"context_line":"        LOG.debug("},{"line_number":2308,"context_line":"            \u0027!!! Start waiting for the detach event from libvirt for \u0027"},{"line_number":2309,"context_line":"            \u0027device %s with device alias %s for instance %s\u0027,"},{"line_number":2310,"context_line":"            device_name_for_logging, dev.alias, instance_uuid)"},{"line_number":2311,"context_line":"        # We issued the detach without any exception so we can wait for"},{"line_number":2312,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2313,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"},{"line_number":2314,"context_line":"        # incoming libvirt DeviceRemovedEvent or DeviceRemovalFailedEvent"},{"line_number":2315,"context_line":"        event_received \u003d waiter.wait(timeout\u003d60)"},{"line_number":2316,"context_line":""},{"line_number":2317,"context_line":"        if not event_received:"},{"line_number":2318,"context_line":"            LOG.debug("}],"source_content_type":"text/x-python","patch_set":6,"id":"36ef304f_9db6ae6c","line":2315,"range":{"start_line":2302,"start_character":2,"end_line":2315,"end_character":48},"updated":"2021-01-17 10:54:35.000000000","message":"this is wrong, we clean up the waiter before we start waiting","commit_id":"f31a736a6ec4ca300200b1c686c3568cc1bd008e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f3c48c87ea996aa5d23c05ad7824da2f4f6a361f","unresolved":false,"context_lines":[{"line_number":2299,"context_line":"                \u0027instance %s: %s\u0027, device_name_for_logging, instance_uuid,"},{"line_number":2300,"context_line":"                str(ex))"},{"line_number":2301,"context_line":"            raise"},{"line_number":2302,"context_line":"        finally:"},{"line_number":2303,"context_line":"            # clean up the libvirt event handler. If the detach was a success"},{"line_number":2304,"context_line":"            # it is a noop"},{"line_number":2305,"context_line":"            self._detach_event_handler.remove_waiter(instance_uuid, dev.alias)"},{"line_number":2306,"context_line":""},{"line_number":2307,"context_line":"        LOG.debug("},{"line_number":2308,"context_line":"            \u0027!!! Start waiting for the detach event from libvirt for \u0027"},{"line_number":2309,"context_line":"            \u0027device %s with device alias %s for instance %s\u0027,"},{"line_number":2310,"context_line":"            device_name_for_logging, dev.alias, instance_uuid)"},{"line_number":2311,"context_line":"        # We issued the detach without any exception so we can wait for"},{"line_number":2312,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2313,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"},{"line_number":2314,"context_line":"        # incoming libvirt DeviceRemovedEvent or DeviceRemovalFailedEvent"},{"line_number":2315,"context_line":"        event_received \u003d waiter.wait(timeout\u003d60)"},{"line_number":2316,"context_line":""},{"line_number":2317,"context_line":"        if not event_received:"},{"line_number":2318,"context_line":"            LOG.debug("}],"source_content_type":"text/x-python","patch_set":6,"id":"e869a93a_2dae5ab8","line":2315,"range":{"start_line":2302,"start_character":2,"end_line":2315,"end_character":48},"in_reply_to":"36ef304f_9db6ae6c","updated":"2021-01-18 08:06:33.000000000","message":"Done","commit_id":"f31a736a6ec4ca300200b1c686c3568cc1bd008e"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"49558c547b07c5f929c31c4f88de0fa7aa4c2e86","unresolved":true,"context_lines":[{"line_number":2327,"context_line":"                device_name_for_logging, instance_uuid)"},{"line_number":2328,"context_line":"            return True"},{"line_number":2329,"context_line":"        else:"},{"line_number":2330,"context_line":"            # Either we get a DeviceRemovalFailedEvent from libvirt or timed"},{"line_number":2331,"context_line":"            # out waiting for the event (or libvirt lied to use about"},{"line_number":2332,"context_line":"            # success?!)"},{"line_number":2333,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"9d1afb1c_0aeb1d45","line":2330,"range":{"start_line":2330,"start_character":30,"end_line":2330,"end_character":54},"updated":"2021-01-18 11:54:01.000000000","message":"Just summarizing discussion from IRC chat with Gibi, Lee (and DanPB on #virt, OFTC) on the event VIR_DOMAIN_EVENT_ID_DEVICE_REMOVAL_FAILED:\n\n- There\u0027s no libvirt timeout before that event is emitted; it\u0027s emitted once we hit the failure\n- The said _REMOVAL_FAILED event is emitted when ACPI refuses the device unplug explicitly.\n- However, if the guest OS is just slow or not responding, then we don\u0027t get the _REMOVAL_FAILED event.\n- The _REMOVAL_FAILED event is an unconditional failure — i.e. no need to retry if we get this event\n\nBased on the above, I agree w/ the logic Gibi suggested: If we the _REMOVAL_FAILED event, we fail;  if we get timeout, we retry; if we get a sync failure about device missing [i.e. VIR_ERR_DEVICE_MISSING] then we retry with \u0027persistent\u003dFalse\u0027","commit_id":"03d6c0e1f02ee0517a2cce946c91e03f1e794156"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"7752dccea1a6dae0de90e293d303ba8745d64cd7","unresolved":true,"context_lines":[{"line_number":2327,"context_line":"                device_name_for_logging, instance_uuid)"},{"line_number":2328,"context_line":"            return True"},{"line_number":2329,"context_line":"        else:"},{"line_number":2330,"context_line":"            # Either we get a DeviceRemovalFailedEvent from libvirt or timed"},{"line_number":2331,"context_line":"            # out waiting for the event (or libvirt lied to use about"},{"line_number":2332,"context_line":"            # success?!)"},{"line_number":2333,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"1bb76b70_3a8602a1","line":2330,"range":{"start_line":2330,"start_character":30,"end_line":2330,"end_character":54},"in_reply_to":"48eb94d7_e0ed30a4","updated":"2021-01-18 17:27:14.000000000","message":"I did some further tests by modifying this patch to see how libvirt behaves if the device detached from the persistent config independently from detached from the live config. \n\nIn a simple scenario[1]:\n1) if I detach from the live config first then I got an event as expected. Then I detach from the persistent config and no event is received. \n\n2) if I detach the persistent config first then no event, then detach from the live config and I get an event as expected.\n\nIn a more complex scenario[2] something weird happens:\n1) if I detach form live first then no event and no error is reported, then I detach from the persistent config, no event, no error, then I detach from the live config again, and there I get an event as expected.\n\n2) if I detach from the persistent first then I see when I saw in the simple scenario.\n\nFrom this I assume:\n* detaching only from the persistent config does not generate the event\n* there is something unexpected in the complex scenario and maybe it is in the test so I have to dig deeper there.\n\n\n[1] https://github.com/openstack/tempest/blob/d76d371e1d81bc0e78afdeb23f7128ff2eae09f5/tempest/api/compute/volumes/test_attach_volume.py#L67\n[2] https://github.com/openstack/tempest/blob/3442eaf87919e6ec9809eb15bcc327e94984dbc5/tempest/api/compute/servers/test_server_rescue_negative.py#L136","commit_id":"03d6c0e1f02ee0517a2cce946c91e03f1e794156"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f764592659d9b58c8452c48e8dc9c6969369c276","unresolved":true,"context_lines":[{"line_number":2327,"context_line":"                device_name_for_logging, instance_uuid)"},{"line_number":2328,"context_line":"            return True"},{"line_number":2329,"context_line":"        else:"},{"line_number":2330,"context_line":"            # Either we get a DeviceRemovalFailedEvent from libvirt or timed"},{"line_number":2331,"context_line":"            # out waiting for the event (or libvirt lied to use about"},{"line_number":2332,"context_line":"            # success?!)"},{"line_number":2333,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"48eb94d7_e0ed30a4","line":2330,"range":{"start_line":2330,"start_character":30,"end_line":2330,"end_character":54},"in_reply_to":"9d1afb1c_0aeb1d45","updated":"2021-01-18 12:47:31.000000000","message":"So, here\u0027s more notes based on further chat with Peter Krempa and DanPB:\n\nPeter: \n\n- The _REMOVAL_FAILED event is emitted in certain cases based on an ACPI event from the guest, so in that case the \"timeout\" will depend on how quickly the guest responds, bypassing the libvirt timeout.  \n- And the error is fatal — but it\u0027s not for every device, this is just for memory devices IIRC But it\u0027s not for every device, this is just for memory devices (based on Peter\u0027s recollection).  And libvirt might extend it in some cases to other devices, but it was not wired up last time I\u0027ve checked.\n\nDanPB:\n\n- Unplug is an async operation, so libvirt will wait a short while for completion, but then return leaving it in progress.\n- Suggest to deal with live and inactive config separately: removing it [i.e. the device being unplugged] from inactive config will essentially always succeed.  \n- I.e. issue a separate \u0027detachDevice\u0027 call for inactive config and separate one for the live unplug, so you know exactly which part worked and which didn\u0027t.  After issuing the  \u0027detachDevice\u0027 for the live config, you can just wait for VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED  instead of retrying.\n\nFWIW, I agree with the deal separately w/ active vs. inactive; and avoid retry altogether, since unplug is fundamentally an asynchronous operation.","commit_id":"03d6c0e1f02ee0517a2cce946c91e03f1e794156"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"46cd970a4b7e121e4057969a51f13275ea3f0973","unresolved":true,"context_lines":[{"line_number":2327,"context_line":"                device_name_for_logging, instance_uuid)"},{"line_number":2328,"context_line":"            return True"},{"line_number":2329,"context_line":"        else:"},{"line_number":2330,"context_line":"            # Either we get a DeviceRemovalFailedEvent from libvirt or timed"},{"line_number":2331,"context_line":"            # out waiting for the event (or libvirt lied to use about"},{"line_number":2332,"context_line":"            # success?!)"},{"line_number":2333,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":7,"id":"a04d9bdc_1b6a74b3","line":2330,"range":{"start_line":2330,"start_character":30,"end_line":2330,"end_character":54},"in_reply_to":"9d1afb1c_0aeb1d45","updated":"2021-01-18 12:00:43.000000000","message":"thanks Kashyap","commit_id":"03d6c0e1f02ee0517a2cce946c91e03f1e794156"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f4d636c709594f668292c3923d2a2314eac1f0b8","unresolved":true,"context_lines":[{"line_number":2238,"context_line":"                    \u0027instance %s from the persistent domain config. Libvirt \u0027"},{"line_number":2239,"context_line":"                    \u0027did not report any error but the device is still in\u0027"},{"line_number":2240,"context_line":"                    \u0027the config.\u0027, device_name_for_logging,"},{"line_number":2241,"context_line":"                    persistent_dev.alias, instance_uuid)"},{"line_number":2242,"context_line":"                success \u003d False"},{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        if live and live_dev:"}],"source_content_type":"text/x-python","patch_set":9,"id":"2fd6c682_5c82c453","line":2241,"range":{"start_line":2241,"start_character":20,"end_line":2241,"end_character":40},"updated":"2021-01-19 17:42:44.000000000","message":"this is always None, alias only provided in the live config","commit_id":"9e17af5813a53411901938e7b74b72e88caf8f68"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":false,"context_lines":[{"line_number":2238,"context_line":"                    \u0027instance %s from the persistent domain config. Libvirt \u0027"},{"line_number":2239,"context_line":"                    \u0027did not report any error but the device is still in\u0027"},{"line_number":2240,"context_line":"                    \u0027the config.\u0027, device_name_for_logging,"},{"line_number":2241,"context_line":"                    persistent_dev.alias, instance_uuid)"},{"line_number":2242,"context_line":"                success \u003d False"},{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        if live and live_dev:"}],"source_content_type":"text/x-python","patch_set":9,"id":"aade9599_d144d89a","line":2241,"range":{"start_line":2241,"start_character":20,"end_line":2241,"end_character":40},"in_reply_to":"2fd6c682_5c82c453","updated":"2021-01-22 18:15:12.000000000","message":"Done","commit_id":"9e17af5813a53411901938e7b74b72e88caf8f68"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f4d636c709594f668292c3923d2a2314eac1f0b8","unresolved":true,"context_lines":[{"line_number":2336,"context_line":"        if not event_received:"},{"line_number":2337,"context_line":"            # This should not happen. But it does at least during the cleanup"},{"line_number":2338,"context_line":"            # of the tempest test case"},{"line_number":2339,"context_line":"            # ServerRescueNegativeTestJSON. test_rescued_vm_detach_volume"},{"line_number":2340,"context_line":"            LOG.warning("},{"line_number":2341,"context_line":"                \u0027!!! Waiting for libvirt event about the detach of \u0027"},{"line_number":2342,"context_line":"                \u0027device %s with device alias %s from instance %s is timed \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"94c2ad90_460ef308","line":2339,"range":{"start_line":2339,"start_character":43,"end_line":2339,"end_character":44},"updated":"2021-01-19 17:42:44.000000000","message":"nit","commit_id":"9e17af5813a53411901938e7b74b72e88caf8f68"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":false,"context_lines":[{"line_number":2336,"context_line":"        if not event_received:"},{"line_number":2337,"context_line":"            # This should not happen. But it does at least during the cleanup"},{"line_number":2338,"context_line":"            # of the tempest test case"},{"line_number":2339,"context_line":"            # ServerRescueNegativeTestJSON. test_rescued_vm_detach_volume"},{"line_number":2340,"context_line":"            LOG.warning("},{"line_number":2341,"context_line":"                \u0027!!! Waiting for libvirt event about the detach of \u0027"},{"line_number":2342,"context_line":"                \u0027device %s with device alias %s from instance %s is timed \u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"3bce4811_59cb0aba","line":2339,"range":{"start_line":2339,"start_character":43,"end_line":2339,"end_character":44},"in_reply_to":"94c2ad90_460ef308","updated":"2021-01-22 18:15:12.000000000","message":"Done","commit_id":"9e17af5813a53411901938e7b74b72e88caf8f68"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f4d636c709594f668292c3923d2a2314eac1f0b8","unresolved":true,"context_lines":[{"line_number":2358,"context_line":"    ):"},{"line_number":2359,"context_line":"        \"\"\"Detaches a device from the guest without waiting for libvirt events"},{"line_number":2360,"context_line":""},{"line_number":2361,"context_line":"        It only handles synchronos errors (i.e. exceptions) but not wait for"},{"line_number":2362,"context_line":"        any event from libvirt."},{"line_number":2363,"context_line":""},{"line_number":2364,"context_line":"        :param dev: the device configuration to be detached"}],"source_content_type":"text/x-python","patch_set":9,"id":"1dc25661_0bf852e7","line":2361,"range":{"start_line":2361,"start_character":64,"end_line":2361,"end_character":67},"updated":"2021-01-19 17:42:44.000000000","message":"does not","commit_id":"9e17af5813a53411901938e7b74b72e88caf8f68"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":false,"context_lines":[{"line_number":2358,"context_line":"    ):"},{"line_number":2359,"context_line":"        \"\"\"Detaches a device from the guest without waiting for libvirt events"},{"line_number":2360,"context_line":""},{"line_number":2361,"context_line":"        It only handles synchronos errors (i.e. exceptions) but not wait for"},{"line_number":2362,"context_line":"        any event from libvirt."},{"line_number":2363,"context_line":""},{"line_number":2364,"context_line":"        :param dev: the device configuration to be detached"}],"source_content_type":"text/x-python","patch_set":9,"id":"7c571781_95821a4d","line":2361,"range":{"start_line":2361,"start_character":64,"end_line":2361,"end_character":67},"in_reply_to":"1dc25661_0bf852e7","updated":"2021-01-22 18:15:12.000000000","message":"Done","commit_id":"9e17af5813a53411901938e7b74b72e88caf8f68"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":true,"context_lines":[{"line_number":213,"context_line":"patch_tpool_proxy()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"class AsyncDeviceDetachEventsHandler:"},{"line_number":217,"context_line":"    def __init__(self):"},{"line_number":218,"context_line":"        self._lock \u003d threading.Lock()"},{"line_number":219,"context_line":"        # Ongoing device detaches in libvirt where we wait for the events"}],"source_content_type":"text/x-python","patch_set":12,"id":"5eba8e1a_1a10ae8c","line":216,"range":{"start_line":216,"start_character":6,"end_line":216,"end_character":36},"updated":"2021-01-22 18:15:12.000000000","message":"move the addition of this class to a separate patch and add separate unit test coverage to it","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aec07944bafbbe0f7f79ecf5176ac591c7d652e2","unresolved":true,"context_lines":[{"line_number":213,"context_line":"patch_tpool_proxy()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"class AsyncDeviceDetachEventsHandler:"},{"line_number":217,"context_line":"    def __init__(self):"},{"line_number":218,"context_line":"        self._lock \u003d threading.Lock()"},{"line_number":219,"context_line":"        # Ongoing device detaches in libvirt where we wait for the events"}],"source_content_type":"text/x-python","patch_set":12,"id":"2ee2c724_0e75d0b8","line":216,"range":{"start_line":216,"start_character":6,"end_line":216,"end_character":36},"in_reply_to":"376b5aab_2c8d7525","updated":"2021-01-25 17:32:19.000000000","message":"I\u0027ve split this out to a separate patch","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"0046f7849fb880b71b0475a181d0301ff2b07b36","unresolved":true,"context_lines":[{"line_number":213,"context_line":"patch_tpool_proxy()"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"class AsyncDeviceDetachEventsHandler:"},{"line_number":217,"context_line":"    def __init__(self):"},{"line_number":218,"context_line":"        self._lock \u003d threading.Lock()"},{"line_number":219,"context_line":"        # Ongoing device detaches in libvirt where we wait for the events"}],"source_content_type":"text/x-python","patch_set":12,"id":"376b5aab_2c8d7525","line":216,"range":{"start_line":216,"start_character":6,"end_line":216,"end_character":36},"in_reply_to":"5eba8e1a_1a10ae8c","updated":"2021-01-25 09:41:49.000000000","message":"Yeah, splitting out sounds good :-)  I see you\u0027re annotating the change with some to-dos here.\n\nMy threading knowledge is not super-strong, but I\u0027m slowly parsing the code.","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":true,"context_lines":[{"line_number":2199,"context_line":"        else:"},{"line_number":2200,"context_line":"            # Let the generic driver code dispatch the event to the compute"},{"line_number":2201,"context_line":"            # manager"},{"line_number":2202,"context_line":"            super().emit_event(event)"},{"line_number":2203,"context_line":""},{"line_number":2204,"context_line":"    def _detach_with_retry("},{"line_number":2205,"context_line":"        self,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9deeb4de_734cf6d3","line":2202,"updated":"2021-01-22 18:15:12.000000000","message":"emit_event and handle_*events changes can be done in a separate patch","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aec07944bafbbe0f7f79ecf5176ac591c7d652e2","unresolved":false,"context_lines":[{"line_number":2199,"context_line":"        else:"},{"line_number":2200,"context_line":"            # Let the generic driver code dispatch the event to the compute"},{"line_number":2201,"context_line":"            # manager"},{"line_number":2202,"context_line":"            super().emit_event(event)"},{"line_number":2203,"context_line":""},{"line_number":2204,"context_line":"    def _detach_with_retry("},{"line_number":2205,"context_line":"        self,"}],"source_content_type":"text/x-python","patch_set":12,"id":"4eff5400_8192e338","line":2202,"in_reply_to":"9deeb4de_734cf6d3","updated":"2021-01-25 17:32:19.000000000","message":"Done","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e0960b9320be6b8b31cbf0e4168fbd4c384f98ab","unresolved":true,"context_lines":[{"line_number":2287,"context_line":"                if not persistent_dev:"},{"line_number":2288,"context_line":"                    LOG.debug("},{"line_number":2289,"context_line":"                        \u0027!!! Successfully detached device %s from instance %s \u0027"},{"line_number":2290,"context_line":"                        \u0027from the pegrsistent domain config.\u0027,"},{"line_number":2291,"context_line":"                        device_name_for_logging, instance_uuid)"},{"line_number":2292,"context_line":"                else:"},{"line_number":2293,"context_line":"                    # Based on the libvirt devs this should never happen"}],"source_content_type":"text/x-python","patch_set":12,"id":"b3684dba_f37378c9","line":2290,"range":{"start_line":2290,"start_character":34,"end_line":2290,"end_character":45},"updated":"2021-01-25 08:03:52.000000000","message":"persistent","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2424,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2425,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"},{"line_number":2426,"context_line":"        # incoming libvirt DeviceRemovedEvent or DeviceRemovalFailedEvent"},{"line_number":2427,"context_line":"        event \u003d self._detach_event_handler.wait(waiter, timeout\u003d20)"},{"line_number":2428,"context_line":""},{"line_number":2429,"context_line":"        if not event:"},{"line_number":2430,"context_line":"            # This should not happen based on information from the libvirt"}],"source_content_type":"text/x-python","patch_set":15,"id":"98b14f4a_985c925d","line":2427,"range":{"start_line":2427,"start_character":64,"end_line":2427,"end_character":66},"updated":"2021-02-16 15:55:48.000000000","message":"lets make this configurable","commit_id":"a0f2e819c3c9e6a78206c7161395d4b24e390387"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":1344,"context_line":"    def destroy(self, context, instance, network_info, block_device_info\u003dNone,"},{"line_number":1345,"context_line":"                destroy_disks\u003dTrue):"},{"line_number":1346,"context_line":"        self._destroy(instance)"},{"line_number":1347,"context_line":"        # NOTE(gibi): if there was device detach in progress then we need to"},{"line_number":1348,"context_line":"        # unblock the waiting threads and clean up."},{"line_number":1349,"context_line":"        self._detach_event_handler.cleanup_waiters(instance.uuid)"},{"line_number":1350,"context_line":"        self.cleanup(context, instance, network_info, block_device_info,"},{"line_number":1351,"context_line":"                     destroy_disks)"},{"line_number":1352,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"01baf35c_d4c4a361","line":1349,"range":{"start_line":1347,"start_character":0,"end_line":1349,"end_character":65},"updated":"2021-02-16 15:55:48.000000000","message":"FWIW the only reason we need this is because detach_interface doesn\u0027t lock by instance.uuid. We hit a gate issue last week where a call to detach_interface raced a call to destroy because of this.\n\nhttps://bugs.launchpad.net/nova/+bug/1914664/comments/4\n\nThis change is fine but I\u0027d also like to remove the need for it by locking detach_interface by instance.uuid.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":true,"context_lines":[{"line_number":1344,"context_line":"    def destroy(self, context, instance, network_info, block_device_info\u003dNone,"},{"line_number":1345,"context_line":"                destroy_disks\u003dTrue):"},{"line_number":1346,"context_line":"        self._destroy(instance)"},{"line_number":1347,"context_line":"        # NOTE(gibi): if there was device detach in progress then we need to"},{"line_number":1348,"context_line":"        # unblock the waiting threads and clean up."},{"line_number":1349,"context_line":"        self._detach_event_handler.cleanup_waiters(instance.uuid)"},{"line_number":1350,"context_line":"        self.cleanup(context, instance, network_info, block_device_info,"},{"line_number":1351,"context_line":"                     destroy_disks)"},{"line_number":1352,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"a1f36943_29b8fa25","line":1349,"range":{"start_line":1347,"start_character":0,"end_line":1349,"end_character":65},"in_reply_to":"01baf35c_d4c4a361","updated":"2021-03-04 17:05:55.000000000","message":"I\u0027m supportive to add the missing locking. \n\nStill I think it is a small price to pay here to avoid an ever growing list of unused waiters of already deleted instances leaking memory in nova-compute. So I would like to keep this cleanup call to avoid later changes creating a memory leak.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2129,"context_line":"        event: ty.Union["},{"line_number":2130,"context_line":"            libvirtevent.DeviceRemovedEvent,"},{"line_number":2131,"context_line":"            libvirtevent.DeviceRemovalFailedEvent"},{"line_number":2132,"context_line":"        ],"},{"line_number":2133,"context_line":"    ) -\u003e None:"},{"line_number":2134,"context_line":""},{"line_number":2135,"context_line":"        had_clients \u003d self._detach_event_handler.notify_waiters(event)"}],"source_content_type":"text/x-python","patch_set":19,"id":"99e73c15_ed053a2b","line":2132,"updated":"2021-02-09 17:38:06.000000000","message":"Same comment as before about using the base class rather than the union of subclasses","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2129,"context_line":"        event: ty.Union["},{"line_number":2130,"context_line":"            libvirtevent.DeviceRemovedEvent,"},{"line_number":2131,"context_line":"            libvirtevent.DeviceRemovalFailedEvent"},{"line_number":2132,"context_line":"        ],"},{"line_number":2133,"context_line":"    ) -\u003e None:"},{"line_number":2134,"context_line":""},{"line_number":2135,"context_line":"        had_clients \u003d self._detach_event_handler.notify_waiters(event)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7bc21366_2bd5519d","line":2132,"in_reply_to":"99e73c15_ed053a2b","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2134,"context_line":""},{"line_number":2135,"context_line":"        had_clients \u003d self._detach_event_handler.notify_waiters(event)"},{"line_number":2136,"context_line":""},{"line_number":2137,"context_line":"        if had_clients:"},{"line_number":2138,"context_line":"            LOG.debug("},{"line_number":2139,"context_line":"                \"Received event %s from libvirt while the driver is \""},{"line_number":2140,"context_line":"                \"waiting for it so it is dispatched.\","}],"source_content_type":"text/x-python","patch_set":19,"id":"391bece3_7c27916d","line":2137,"updated":"2021-02-09 17:38:06.000000000","message":"Or\n\n  if self._detach_event_handler.notify_waiters(event):","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":true,"context_lines":[{"line_number":2134,"context_line":""},{"line_number":2135,"context_line":"        had_clients \u003d self._detach_event_handler.notify_waiters(event)"},{"line_number":2136,"context_line":""},{"line_number":2137,"context_line":"        if had_clients:"},{"line_number":2138,"context_line":"            LOG.debug("},{"line_number":2139,"context_line":"                \"Received event %s from libvirt while the driver is \""},{"line_number":2140,"context_line":"                \"waiting for it so it is dispatched.\","}],"source_content_type":"text/x-python","patch_set":19,"id":"7332a50b_50bb2704","line":2137,"in_reply_to":"391bece3_7c27916d","updated":"2021-03-04 17:05:55.000000000","message":"meh, the current form reads better to me.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2137,"context_line":"        if had_clients:"},{"line_number":2138,"context_line":"            LOG.debug("},{"line_number":2139,"context_line":"                \"Received event %s from libvirt while the driver is \""},{"line_number":2140,"context_line":"                \"waiting for it so it is dispatched.\","},{"line_number":2141,"context_line":"                event,"},{"line_number":2142,"context_line":"            )"},{"line_number":2143,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":19,"id":"81c84aad_816cf85e","line":2140,"range":{"start_line":2140,"start_character":31,"end_line":2140,"end_character":40},"updated":"2021-02-09 17:38:06.000000000","message":"waiting for it; dispatched.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2137,"context_line":"        if had_clients:"},{"line_number":2138,"context_line":"            LOG.debug("},{"line_number":2139,"context_line":"                \"Received event %s from libvirt while the driver is \""},{"line_number":2140,"context_line":"                \"waiting for it so it is dispatched.\","},{"line_number":2141,"context_line":"                event,"},{"line_number":2142,"context_line":"            )"},{"line_number":2143,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":19,"id":"91d6fea3_b8b1295f","line":2140,"range":{"start_line":2140,"start_character":31,"end_line":2140,"end_character":40},"in_reply_to":"81c84aad_816cf85e","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2143,"context_line":"        else:"},{"line_number":2144,"context_line":"            LOG.debug("},{"line_number":2145,"context_line":"                \"Received event %s from libvirt but the driver is not \""},{"line_number":2146,"context_line":"                \"waiting for it so it is ignored.\","},{"line_number":2147,"context_line":"                event,"},{"line_number":2148,"context_line":"            )"},{"line_number":2149,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"509d06e2_e0f51916","line":2146,"range":{"start_line":2146,"start_character":24,"end_line":2146,"end_character":49},"updated":"2021-02-09 17:38:06.000000000","message":"waiting for it; ignored.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2143,"context_line":"        else:"},{"line_number":2144,"context_line":"            LOG.debug("},{"line_number":2145,"context_line":"                \"Received event %s from libvirt but the driver is not \""},{"line_number":2146,"context_line":"                \"waiting for it so it is ignored.\","},{"line_number":2147,"context_line":"                event,"},{"line_number":2148,"context_line":"            )"},{"line_number":2149,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"f443decd_01b4714c","line":2146,"range":{"start_line":2146,"start_character":24,"end_line":2146,"end_character":49},"in_reply_to":"509d06e2_e0f51916","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2172,"context_line":"            # manager"},{"line_number":2173,"context_line":"            super().emit_event(event)"},{"line_number":2174,"context_line":""},{"line_number":2175,"context_line":"    def _detach_with_retry("},{"line_number":2176,"context_line":"        self,"},{"line_number":2177,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2178,"context_line":"        instance_uuid: str,"}],"source_content_type":"text/x-python","patch_set":19,"id":"60c765a6_57423226","line":2175,"updated":"2021-02-09 17:38:06.000000000","message":"This really feels like two methods, one for live and one for persistent domain. Could we reasonably split them, even if we had a caller calling both?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2172,"context_line":"            # manager"},{"line_number":2173,"context_line":"            super().emit_event(event)"},{"line_number":2174,"context_line":""},{"line_number":2175,"context_line":"    def _detach_with_retry("},{"line_number":2176,"context_line":"        self,"},{"line_number":2177,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2178,"context_line":"        instance_uuid: str,"}],"source_content_type":"text/x-python","patch_set":19,"id":"a102b9c7_08fd6e69","line":2175,"in_reply_to":"19af57f8_1515d87c","updated":"2021-03-04 17:05:55.000000000","message":"the ugly thing is that the error handling during the persistent detach depends on the fact if live detach is requested or not. So even if I split this to two they will be interconnected logically.\n\nI now pulled out the core of the try-except for persistent detach and keep the exception handling here as this depends on the live part. Let me know if you would like to see a bigger extraction even if the resulting functions would be interdependent.\n\nI was able to pull the live par out cleanly.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2172,"context_line":"            # manager"},{"line_number":2173,"context_line":"            super().emit_event(event)"},{"line_number":2174,"context_line":""},{"line_number":2175,"context_line":"    def _detach_with_retry("},{"line_number":2176,"context_line":"        self,"},{"line_number":2177,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2178,"context_line":"        instance_uuid: str,"}],"source_content_type":"text/x-python","patch_set":19,"id":"19af57f8_1515d87c","line":2175,"in_reply_to":"60c765a6_57423226","updated":"2021-02-16 15:55:48.000000000","message":"+1","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2177,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2178,"context_line":"        instance_uuid: str,"},{"line_number":2179,"context_line":"        # to properly typehint this param we would need typing.Protocol but"},{"line_number":2180,"context_line":"        # that is only available since python 3.8"},{"line_number":2181,"context_line":"        get_device_conf_func,"},{"line_number":2182,"context_line":"        device_name_for_logging: str,"},{"line_number":2183,"context_line":"        live: bool,"}],"source_content_type":"text/x-python","patch_set":19,"id":"34b19399_008b4e1b","line":2180,"updated":"2021-02-09 17:38:06.000000000","message":"Can\u0027t you use ty.Callable?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2177,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2178,"context_line":"        instance_uuid: str,"},{"line_number":2179,"context_line":"        # to properly typehint this param we would need typing.Protocol but"},{"line_number":2180,"context_line":"        # that is only available since python 3.8"},{"line_number":2181,"context_line":"        get_device_conf_func,"},{"line_number":2182,"context_line":"        device_name_for_logging: str,"},{"line_number":2183,"context_line":"        live: bool,"}],"source_content_type":"text/x-python","patch_set":19,"id":"4082b6c0_86eb6631","line":2180,"in_reply_to":"34b19399_008b4e1b","updated":"2021-03-04 17:05:55.000000000","message":"good point, at least we can say that. Done.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2181,"context_line":"        get_device_conf_func,"},{"line_number":2182,"context_line":"        device_name_for_logging: str,"},{"line_number":2183,"context_line":"        live: bool,"},{"line_number":2184,"context_line":"        num_retry: int \u003d 8,"},{"line_number":2185,"context_line":"    ) -\u003e None:"},{"line_number":2186,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2187,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"c6c24f48_bbcb864a","line":2184,"updated":"2021-02-09 17:38:06.000000000","message":"If this isn\u0027t ever set, I suggest we simply hardcode it inline. We can make it configurable in the future if needed, I suspect?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2181,"context_line":"        get_device_conf_func,"},{"line_number":2182,"context_line":"        device_name_for_logging: str,"},{"line_number":2183,"context_line":"        live: bool,"},{"line_number":2184,"context_line":"        num_retry: int \u003d 8,"},{"line_number":2185,"context_line":"    ) -\u003e None:"},{"line_number":2186,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2187,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9f54e7e1_79ccd5ab","line":2184,"in_reply_to":"47fbd7c4_e0b61f0c","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2181,"context_line":"        get_device_conf_func,"},{"line_number":2182,"context_line":"        device_name_for_logging: str,"},{"line_number":2183,"context_line":"        live: bool,"},{"line_number":2184,"context_line":"        num_retry: int \u003d 8,"},{"line_number":2185,"context_line":"    ) -\u003e None:"},{"line_number":2186,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2187,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"47fbd7c4_e0b61f0c","line":2184,"in_reply_to":"c6c24f48_bbcb864a","updated":"2021-02-16 15:55:48.000000000","message":"Could we make this configurable now to get it over with?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2189,"context_line":"        event signalling the finish of the detach process."},{"line_number":2190,"context_line":""},{"line_number":2191,"context_line":"        If the live detach times out then it will retry the detach. Detach from"},{"line_number":2192,"context_line":"        the persistent config is not retried as it is:"},{"line_number":2193,"context_line":"        * synchronous and no event is sent from libvirt"},{"line_number":2194,"context_line":"        * it is always expected to succeed if the device is in the domain"},{"line_number":2195,"context_line":"          config"}],"source_content_type":"text/x-python","patch_set":19,"id":"646717da_3d2adc6c","line":2192,"updated":"2021-02-09 17:38:06.000000000","message":"nit: newline under here","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2189,"context_line":"        event signalling the finish of the detach process."},{"line_number":2190,"context_line":""},{"line_number":2191,"context_line":"        If the live detach times out then it will retry the detach. Detach from"},{"line_number":2192,"context_line":"        the persistent config is not retried as it is:"},{"line_number":2193,"context_line":"        * synchronous and no event is sent from libvirt"},{"line_number":2194,"context_line":"        * it is always expected to succeed if the device is in the domain"},{"line_number":2195,"context_line":"          config"}],"source_content_type":"text/x-python","patch_set":19,"id":"4f7a59d7_d7ec0332","line":2192,"in_reply_to":"646717da_3d2adc6c","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2282,"context_line":"                    raise"},{"line_number":2283,"context_line":""},{"line_number":2284,"context_line":"        if live and live_dev:"},{"line_number":2285,"context_line":"            for attempt in range(num_retry):"},{"line_number":2286,"context_line":"                LOG.debug("},{"line_number":2287,"context_line":"                    \u0027(%s/%s): Attempting to detach device %s with device \u0027"},{"line_number":2288,"context_line":"                    \u0027alias %s from instance %s from the live domain config.\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"a39d76ce_41a55ba5","line":2285,"range":{"start_line":2285,"start_character":12,"end_line":2285,"end_character":44},"updated":"2021-02-16 15:55:48.000000000","message":"+1 for keeping it simple and dropping the RetryDecorator, that really didn\u0027t help when trying to understand what the hell the code was doing before.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2282,"context_line":"                    raise"},{"line_number":2283,"context_line":""},{"line_number":2284,"context_line":"        if live and live_dev:"},{"line_number":2285,"context_line":"            for attempt in range(num_retry):"},{"line_number":2286,"context_line":"                LOG.debug("},{"line_number":2287,"context_line":"                    \u0027(%s/%s): Attempting to detach device %s with device \u0027"},{"line_number":2288,"context_line":"                    \u0027alias %s from instance %s from the live domain config.\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"6ff50e11_3642ca10","line":2285,"range":{"start_line":2285,"start_character":12,"end_line":2285,"end_character":44},"in_reply_to":"a39d76ce_41a55ba5","updated":"2021-03-04 17:05:55.000000000","message":"Ack","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2302,"context_line":"                    # we are done"},{"line_number":2303,"context_line":"                    return"},{"line_number":2304,"context_line":""},{"line_number":2305,"context_line":"                else:"},{"line_number":2306,"context_line":"                    LOG.debug("},{"line_number":2307,"context_line":"                        \u0027Failed to detach device %s with device alias %s \u0027"},{"line_number":2308,"context_line":"                        \u0027from instance %s from the live domain config. \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"c877709a_f7d62d3f","line":2305,"range":{"start_line":2305,"start_character":16,"end_line":2305,"end_character":21},"updated":"2021-02-16 15:55:48.000000000","message":"You don\u0027t need this.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2302,"context_line":"                    # we are done"},{"line_number":2303,"context_line":"                    return"},{"line_number":2304,"context_line":""},{"line_number":2305,"context_line":"                else:"},{"line_number":2306,"context_line":"                    LOG.debug("},{"line_number":2307,"context_line":"                        \u0027Failed to detach device %s with device alias %s \u0027"},{"line_number":2308,"context_line":"                        \u0027from instance %s from the live domain config. \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"b4ef56c5_143185ef","line":2305,"range":{"start_line":2305,"start_character":16,"end_line":2305,"end_character":21},"in_reply_to":"c877709a_f7d62d3f","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2317,"context_line":"                device_name_for_logging, live_dev.alias, instance_uuid)"},{"line_number":2318,"context_line":"            raise exception.DeviceDetachFailed("},{"line_number":2319,"context_line":"                device\u003ddevice_name_for_logging,"},{"line_number":2320,"context_line":"                reason\u003d\"Run out of retry attempts\")"},{"line_number":2321,"context_line":""},{"line_number":2322,"context_line":"    def _detach_from_live_and_wait_for_event("},{"line_number":2323,"context_line":"        self,"}],"source_content_type":"text/x-python","patch_set":19,"id":"1b83738b_96abb52d","line":2320,"range":{"start_line":2320,"start_character":24,"end_line":2320,"end_character":49},"updated":"2021-02-16 15:55:48.000000000","message":"We log a nice descriptive warning above only to raise this, I can\u0027t recall what this looks like in the logs but maybe some pointers to the device, alias etc would also be useful here.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2317,"context_line":"                device_name_for_logging, live_dev.alias, instance_uuid)"},{"line_number":2318,"context_line":"            raise exception.DeviceDetachFailed("},{"line_number":2319,"context_line":"                device\u003ddevice_name_for_logging,"},{"line_number":2320,"context_line":"                reason\u003d\"Run out of retry attempts\")"},{"line_number":2321,"context_line":""},{"line_number":2322,"context_line":"    def _detach_from_live_and_wait_for_event("},{"line_number":2323,"context_line":"        self,"}],"source_content_type":"text/x-python","patch_set":19,"id":"e096f9fc_ad4178c4","line":2320,"range":{"start_line":2320,"start_character":24,"end_line":2320,"end_character":49},"in_reply_to":"1b83738b_96abb52d","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2323,"context_line":"        self,"},{"line_number":2324,"context_line":"        dev: ty.Union["},{"line_number":2325,"context_line":"            vconfig.LibvirtConfigGuestDisk,"},{"line_number":2326,"context_line":"            vconfig.LibvirtConfigGuestInterface],"},{"line_number":2327,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2328,"context_line":"        instance_uuid: str,"},{"line_number":2329,"context_line":"        device_name_for_logging: str,"}],"source_content_type":"text/x-python","patch_set":19,"id":"8fbb35df_7a5ab2e2","line":2326,"updated":"2021-02-09 17:38:06.000000000","message":"ditto (use base class)","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":true,"context_lines":[{"line_number":2323,"context_line":"        self,"},{"line_number":2324,"context_line":"        dev: ty.Union["},{"line_number":2325,"context_line":"            vconfig.LibvirtConfigGuestDisk,"},{"line_number":2326,"context_line":"            vconfig.LibvirtConfigGuestInterface],"},{"line_number":2327,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2328,"context_line":"        instance_uuid: str,"},{"line_number":2329,"context_line":"        device_name_for_logging: str,"}],"source_content_type":"text/x-python","patch_set":19,"id":"7b7baccc_bc2d9349","line":2326,"in_reply_to":"8fbb35df_7a5ab2e2","updated":"2021-03-04 17:05:55.000000000","message":"I cannot do that. This function only works for these two types as they have an alias field. The base class LibvirtConfigGuestDevice does not have it. Also I\u0027m not sure if the libvirt call behind Guest.detach_device() interface works with every child of LibvirtConfigGuestDevice. What I know from the baseline code is that Guest.detach_device() was used with these the two specific types that are listed here now. \n\nI think this shows how having a dynamic language and the resulting ducktyping promote bad OOP hierarchies. Sure it can be fixed by adding an \"interface\" defining the alias field maybe implemented with a mixin class. Or by adding a step in the hierarchy between LibvirtConfigGuestDevice and LibvirtConfigGuestDisk, something like LibvirtConfigGuestDeviceWithAlias. But we are not forced to do that as ducktyping helps. (Btw, I\u0027m also not even sure if libvirt itself define alias for every type of LibvirtConfigGuestDevice)\n\nI kept the union here now. Please let me know how you feel about it.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2360,"context_line":"            self._detach_event_handler.remove_waiter(waiter)"},{"line_number":2361,"context_line":"            raise"},{"line_number":2362,"context_line":""},{"line_number":2363,"context_line":"        LOG.debug("},{"line_number":2364,"context_line":"            \u0027Start waiting for the detach event from libvirt for \u0027"},{"line_number":2365,"context_line":"            \u0027device %s with device alias %s for instance %s\u0027,"},{"line_number":2366,"context_line":"            device_name_for_logging, dev.alias, instance_uuid)"},{"line_number":2367,"context_line":"        # We issued the detach without any exception so we can wait for"},{"line_number":2368,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2369,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"}],"source_content_type":"text/x-python","patch_set":19,"id":"30addb41_73c2b86d","line":2366,"range":{"start_line":2363,"start_character":0,"end_line":2366,"end_character":62},"updated":"2021-02-16 15:55:48.000000000","message":"I know this is going to be tiny but isn\u0027t there a chance that we could race an event from libvirt while n-cpu dumps this into the logs?\n\nCould you move this ahead of the _detach_sync call and reword to something like `About to detach and start waiting ...`","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2360,"context_line":"            self._detach_event_handler.remove_waiter(waiter)"},{"line_number":2361,"context_line":"            raise"},{"line_number":2362,"context_line":""},{"line_number":2363,"context_line":"        LOG.debug("},{"line_number":2364,"context_line":"            \u0027Start waiting for the detach event from libvirt for \u0027"},{"line_number":2365,"context_line":"            \u0027device %s with device alias %s for instance %s\u0027,"},{"line_number":2366,"context_line":"            device_name_for_logging, dev.alias, instance_uuid)"},{"line_number":2367,"context_line":"        # We issued the detach without any exception so we can wait for"},{"line_number":2368,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2369,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"}],"source_content_type":"text/x-python","patch_set":19,"id":"c610d9f5_cda4ee1e","line":2366,"range":{"start_line":2363,"start_character":0,"end_line":2366,"end_character":62},"in_reply_to":"30addb41_73c2b86d","updated":"2021-03-04 17:05:55.000000000","message":"We cannot race. The event handler already created at L2352 to avoid any race. See also the comment above that line. What happens in reality is that qemu makes the detach sync if it going fast enough. So in most of my tests the event already received _before_ _detach_sync() returns. But no problem, in this case the _detach_event_handler.wait() will return immediately[1] as the threading.Event will already be set, before the wait call.\n\nThe log maybe misleading stating that we will start waiting but at this point in the execution we have no idea how much wait will actually happen.\n\n[1] https://docs.python.org/3/library/threading.html#threading.Event.wait","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":false,"context_lines":[{"line_number":2360,"context_line":"            self._detach_event_handler.remove_waiter(waiter)"},{"line_number":2361,"context_line":"            raise"},{"line_number":2362,"context_line":""},{"line_number":2363,"context_line":"        LOG.debug("},{"line_number":2364,"context_line":"            \u0027Start waiting for the detach event from libvirt for \u0027"},{"line_number":2365,"context_line":"            \u0027device %s with device alias %s for instance %s\u0027,"},{"line_number":2366,"context_line":"            device_name_for_logging, dev.alias, instance_uuid)"},{"line_number":2367,"context_line":"        # We issued the detach without any exception so we can wait for"},{"line_number":2368,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2369,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"}],"source_content_type":"text/x-python","patch_set":19,"id":"c284ff72_f1a56aee","line":2366,"range":{"start_line":2363,"start_character":0,"end_line":2366,"end_character":62},"in_reply_to":"c610d9f5_cda4ee1e","updated":"2021-03-08 10:25:01.000000000","message":"ACK thanks for spelling that out.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2368,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2369,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"},{"line_number":2370,"context_line":"        # incoming libvirt DeviceRemovedEvent or DeviceRemovalFailedEvent"},{"line_number":2371,"context_line":"        event \u003d self._detach_event_handler.wait(waiter, timeout\u003d20)"},{"line_number":2372,"context_line":""},{"line_number":2373,"context_line":"        if not event:"},{"line_number":2374,"context_line":"            # This should not happen based on information from the libvirt"}],"source_content_type":"text/x-python","patch_set":19,"id":"384c319d_853ec51b","line":2371,"range":{"start_line":2371,"start_character":56,"end_line":2371,"end_character":66},"updated":"2021-02-16 15:55:48.000000000","message":"Again would be super if we could make this configurable now instead of later.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2368,"context_line":"        # a libvirt event to arrive to notify us about the result"},{"line_number":2369,"context_line":"        # NOTE(gibi): we expect that this call will be unblocked by an"},{"line_number":2370,"context_line":"        # incoming libvirt DeviceRemovedEvent or DeviceRemovalFailedEvent"},{"line_number":2371,"context_line":"        event \u003d self._detach_event_handler.wait(waiter, timeout\u003d20)"},{"line_number":2372,"context_line":""},{"line_number":2373,"context_line":"        if not event:"},{"line_number":2374,"context_line":"            # This should not happen based on information from the libvirt"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a174586_a2bec043","line":2371,"range":{"start_line":2371,"start_character":56,"end_line":2371,"end_character":66},"in_reply_to":"384c319d_853ec51b","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2372,"context_line":""},{"line_number":2373,"context_line":"        if not event:"},{"line_number":2374,"context_line":"            # This should not happen based on information from the libvirt"},{"line_number":2375,"context_line":"            # developers. But it does at least during the cleanup of the"},{"line_number":2376,"context_line":"            # tempest test case"},{"line_number":2377,"context_line":"            # ServerRescueNegativeTestJSON.test_rescued_vm_detach_volume"},{"line_number":2378,"context_line":"            # Log a warning and let the upper layer detect that the device is"},{"line_number":2379,"context_line":"            # still attached and retry"},{"line_number":2380,"context_line":"            LOG.warning("},{"line_number":2381,"context_line":"                \u0027Waiting for libvirt event about the detach of \u0027"},{"line_number":2382,"context_line":"                \u0027device %s with device alias %s from instance %s is timed \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"d953ec95_1663cc32","line":2379,"range":{"start_line":2375,"start_character":26,"end_line":2379,"end_character":38},"updated":"2021-02-16 15:55:48.000000000","message":"Interesting, n-api should reject the request in the test to detach the volume so that must be happening during cleanup? I wonder if the unrescue cleanup isn\u0027t waiting until the instance is ACTIVE?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":true,"context_lines":[{"line_number":2372,"context_line":""},{"line_number":2373,"context_line":"        if not event:"},{"line_number":2374,"context_line":"            # This should not happen based on information from the libvirt"},{"line_number":2375,"context_line":"            # developers. But it does at least during the cleanup of the"},{"line_number":2376,"context_line":"            # tempest test case"},{"line_number":2377,"context_line":"            # ServerRescueNegativeTestJSON.test_rescued_vm_detach_volume"},{"line_number":2378,"context_line":"            # Log a warning and let the upper layer detect that the device is"},{"line_number":2379,"context_line":"            # still attached and retry"},{"line_number":2380,"context_line":"            LOG.warning("},{"line_number":2381,"context_line":"                \u0027Waiting for libvirt event about the detach of \u0027"},{"line_number":2382,"context_line":"                \u0027device %s with device alias %s from instance %s is timed \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"fb128e6f_35407e25","line":2379,"range":{"start_line":2375,"start_character":26,"end_line":2379,"end_character":38},"in_reply_to":"d953ec95_1663cc32","updated":"2021-03-04 17:05:55.000000000","message":"As far as I understand the tempest test waits until the instance reaches RESCUE state and then issue the detach, which expected to fail, then the test goes to cleanup. Unrescues the instance and the detach the volume. But the unrescue also waits until the instance goes to ACTIVE[1]. So I don\u0027t understand what really happens in qemu leading to the missing event. Interestingly the next retry succeeds with the event received.\n\nIt is easy to reproduce this case, by replacing this log with some exception raise and run the tempest test.\n\nAnyhow even if we would not have any know cases when this happen it would be good to keep this log here so that we have logs about future cases when something goes south in qemu.\n\n[1] https://github.com/openstack/tempest/blob/3442eaf87919e6ec9809eb15bcc327e94984dbc5/tempest/api/compute/servers/test_server_rescue_negative.py#L66","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2377,"context_line":"            # ServerRescueNegativeTestJSON.test_rescued_vm_detach_volume"},{"line_number":2378,"context_line":"            # Log a warning and let the upper layer detect that the device is"},{"line_number":2379,"context_line":"            # still attached and retry"},{"line_number":2380,"context_line":"            LOG.warning("},{"line_number":2381,"context_line":"                \u0027Waiting for libvirt event about the detach of \u0027"},{"line_number":2382,"context_line":"                \u0027device %s with device alias %s from instance %s is timed \u0027"},{"line_number":2383,"context_line":"                \u0027out.\u0027, device_name_for_logging, dev.alias, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":19,"id":"6e4cc790_bdc0fe39","line":2380,"range":{"start_line":2380,"start_character":16,"end_line":2380,"end_character":23},"updated":"2021-02-16 15:55:48.000000000","message":"error?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2377,"context_line":"            # ServerRescueNegativeTestJSON.test_rescued_vm_detach_volume"},{"line_number":2378,"context_line":"            # Log a warning and let the upper layer detect that the device is"},{"line_number":2379,"context_line":"            # still attached and retry"},{"line_number":2380,"context_line":"            LOG.warning("},{"line_number":2381,"context_line":"                \u0027Waiting for libvirt event about the detach of \u0027"},{"line_number":2382,"context_line":"                \u0027device %s with device alias %s from instance %s is timed \u0027"},{"line_number":2383,"context_line":"                \u0027out.\u0027, device_name_for_logging, dev.alias, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3bc6568a_607a4556","line":2380,"range":{"start_line":2380,"start_character":16,"end_line":2380,"end_character":23},"in_reply_to":"6e4cc790_bdc0fe39","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2384,"context_line":""},{"line_number":2385,"context_line":"        if isinstance(event, libvirtevent.DeviceRemovalFailedEvent):"},{"line_number":2386,"context_line":"            # Based on the libvirt developers this signals a permanent failure"},{"line_number":2387,"context_line":"            LOG.warning("},{"line_number":2388,"context_line":"                \u0027Received DeviceRemovalFailedEvent from libvirt for the \u0027"},{"line_number":2389,"context_line":"                \u0027detach of device %s with device alias %s from instance %s \u0027,"},{"line_number":2390,"context_line":"                device_name_for_logging, dev.alias, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":19,"id":"9cb9ef66_ef0d6b33","line":2387,"range":{"start_line":2387,"start_character":16,"end_line":2387,"end_character":23},"updated":"2021-02-16 15:55:48.000000000","message":"error?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2384,"context_line":""},{"line_number":2385,"context_line":"        if isinstance(event, libvirtevent.DeviceRemovalFailedEvent):"},{"line_number":2386,"context_line":"            # Based on the libvirt developers this signals a permanent failure"},{"line_number":2387,"context_line":"            LOG.warning("},{"line_number":2388,"context_line":"                \u0027Received DeviceRemovalFailedEvent from libvirt for the \u0027"},{"line_number":2389,"context_line":"                \u0027detach of device %s with device alias %s from instance %s \u0027,"},{"line_number":2390,"context_line":"                device_name_for_logging, dev.alias, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":19,"id":"20dfcbb3_8a50d29e","line":2387,"range":{"start_line":2387,"start_character":16,"end_line":2387,"end_character":23},"in_reply_to":"9cb9ef66_ef0d6b33","updated":"2021-03-04 17:05:55.000000000","message":"Done","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1cf76f40fa954ebdce2672b20c16ec0565a2179a","unresolved":true,"context_lines":[{"line_number":2399,"context_line":"            vconfig.LibvirtConfigGuestInterface],"},{"line_number":2400,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2401,"context_line":"        instance_uuid: str,"},{"line_number":2402,"context_line":"        device_name_for_logging: str,"},{"line_number":2403,"context_line":"        persistent: bool,"},{"line_number":2404,"context_line":"        live: bool,"},{"line_number":2405,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":19,"id":"66415e4a_9deeff50","line":2402,"range":{"start_line":2402,"start_character":19,"end_line":2402,"end_character":31},"updated":"2021-02-09 17:38:06.000000000","message":"I know you\u0027re being explicit, but this does feel _slightly_ superfluous. Maybe drop it and simply call out its purpose in the docs?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"43d36ac63ae47b12fbd194a49723ee46e218dc23","unresolved":false,"context_lines":[{"line_number":2399,"context_line":"            vconfig.LibvirtConfigGuestInterface],"},{"line_number":2400,"context_line":"        guest: libvirt_guest.Guest,"},{"line_number":2401,"context_line":"        instance_uuid: str,"},{"line_number":2402,"context_line":"        device_name_for_logging: str,"},{"line_number":2403,"context_line":"        persistent: bool,"},{"line_number":2404,"context_line":"        live: bool,"},{"line_number":2405,"context_line":"    ):"}],"source_content_type":"text/x-python","patch_set":19,"id":"814adc18_47da19e8","line":2402,"range":{"start_line":2402,"start_character":19,"end_line":2402,"end_character":31},"in_reply_to":"66415e4a_9deeff50","updated":"2021-03-04 17:05:55.000000000","message":"I inherited the name from the baseline but I renamed it now. What should be noted here is that that this device name is not the same as the device alias used by libvirt. So we have two names for the same device just for fun. :)","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":false,"context_lines":[{"line_number":2442,"context_line":""},{"line_number":2443,"context_line":"    def detach_volume(self, context, connection_info, instance, mountpoint,"},{"line_number":2444,"context_line":"                      encryption\u003dNone):"},{"line_number":2445,"context_line":"        disk_dev \u003d mountpoint.rpartition(\"/\")[2]"},{"line_number":2446,"context_line":"        try:"},{"line_number":2447,"context_line":"            guest \u003d self._host.get_guest(instance)"},{"line_number":2448,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"f2ab193d_cbf00447","line":2445,"range":{"start_line":2445,"start_character":8,"end_line":2445,"end_character":48},"updated":"2021-02-16 15:55:48.000000000","message":"Oh how I hate this.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2446,"context_line":"        try:"},{"line_number":2447,"context_line":"            guest \u003d self._host.get_guest(instance)"},{"line_number":2448,"context_line":""},{"line_number":2449,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":2450,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":2451,"context_line":"            # NOTE(lyarwood): The volume must be detached from the VM before"},{"line_number":2452,"context_line":"            # detaching any attached encryptors or disconnecting the underlying"},{"line_number":2453,"context_line":"            # volume in _disconnect_volume. Otherwise, the encryptor or volume"}],"source_content_type":"text/x-python","patch_set":19,"id":"c3a907a3_dfd4340f","line":2450,"range":{"start_line":2449,"start_character":0,"end_line":2450,"end_character":69},"updated":"2021-02-16 15:55:48.000000000","message":"Can you move this down into _detach_with_retry given you\u0027re passing guest down anyway.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e8d258767671de5d3801b595176a8bce7510ff20","unresolved":false,"context_lines":[{"line_number":2446,"context_line":"        try:"},{"line_number":2447,"context_line":"            guest \u003d self._host.get_guest(instance)"},{"line_number":2448,"context_line":""},{"line_number":2449,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":2450,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":2451,"context_line":"            # NOTE(lyarwood): The volume must be detached from the VM before"},{"line_number":2452,"context_line":"            # detaching any attached encryptors or disconnecting the underlying"},{"line_number":2453,"context_line":"            # volume in _disconnect_volume. Otherwise, the encryptor or volume"}],"source_content_type":"text/x-python","patch_set":19,"id":"a98fc505_6f077d09","line":2450,"range":{"start_line":2449,"start_character":0,"end_line":2450,"end_character":69},"in_reply_to":"c3a907a3_dfd4340f","updated":"2021-03-05 13:41:38.000000000","message":"Done in https://review.opendev.org/c/openstack/nova/+/778918","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":false,"context_lines":[{"line_number":2484,"context_line":"            else:"},{"line_number":2485,"context_line":"                raise"},{"line_number":2486,"context_line":""},{"line_number":2487,"context_line":"        self._disconnect_volume(context, connection_info, instance,"},{"line_number":2488,"context_line":"                                encryption\u003dencryption)"},{"line_number":2489,"context_line":""},{"line_number":2490,"context_line":"    def _resize_attached_volume(self, new_size, block_device, instance):"},{"line_number":2491,"context_line":"        LOG.debug(\u0027Resizing target device %(dev)s to %(size)u\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"11c9886d_8c1fe1ad","line":2488,"range":{"start_line":2487,"start_character":0,"end_line":2488,"end_character":54},"updated":"2021-02-16 15:55:48.000000000","message":"This really should be a in finally block.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2644,"context_line":"                                         instance.flavor,"},{"line_number":2645,"context_line":"                                         CONF.libvirt.virt_type)"},{"line_number":2646,"context_line":"        interface \u003d guest.get_interface_by_cfg(cfg)"},{"line_number":2647,"context_line":"        try:"},{"line_number":2648,"context_line":"            # NOTE(mriedem): When deleting an instance and using Neutron,"},{"line_number":2649,"context_line":"            # we can be racing against Neutron deleting the port and"},{"line_number":2650,"context_line":"            # sending the vif-deleted event which then triggers a call to"},{"line_number":2651,"context_line":"            # detach the interface, so if the interface is not found then"},{"line_number":2652,"context_line":"            # we can just log it as a warning."},{"line_number":2653,"context_line":"            if not interface:"},{"line_number":2654,"context_line":"                mac \u003d vif.get(\u0027address\u0027)"},{"line_number":2655,"context_line":"                # The interface is gone so just log it as a warning."},{"line_number":2656,"context_line":"                LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"},{"line_number":2657,"context_line":"                            \u0027the device is no longer found on the guest.\u0027,"},{"line_number":2658,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2659,"context_line":"                return"},{"line_number":2660,"context_line":""},{"line_number":2661,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":2662,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"}],"source_content_type":"text/x-python","patch_set":19,"id":"9e46dd54_551dc605","line":2659,"range":{"start_line":2647,"start_character":0,"end_line":2659,"end_character":22},"updated":"2021-02-16 15:55:48.000000000","message":"You have this built into _detach_with_retry now so I wonder if we can drop this?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49fc881a8005a11a2d34d5b0cc5bac74ff50aa01","unresolved":false,"context_lines":[{"line_number":2644,"context_line":"                                         instance.flavor,"},{"line_number":2645,"context_line":"                                         CONF.libvirt.virt_type)"},{"line_number":2646,"context_line":"        interface \u003d guest.get_interface_by_cfg(cfg)"},{"line_number":2647,"context_line":"        try:"},{"line_number":2648,"context_line":"            # NOTE(mriedem): When deleting an instance and using Neutron,"},{"line_number":2649,"context_line":"            # we can be racing against Neutron deleting the port and"},{"line_number":2650,"context_line":"            # sending the vif-deleted event which then triggers a call to"},{"line_number":2651,"context_line":"            # detach the interface, so if the interface is not found then"},{"line_number":2652,"context_line":"            # we can just log it as a warning."},{"line_number":2653,"context_line":"            if not interface:"},{"line_number":2654,"context_line":"                mac \u003d vif.get(\u0027address\u0027)"},{"line_number":2655,"context_line":"                # The interface is gone so just log it as a warning."},{"line_number":2656,"context_line":"                LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"},{"line_number":2657,"context_line":"                            \u0027the device is no longer found on the guest.\u0027,"},{"line_number":2658,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2659,"context_line":"                return"},{"line_number":2660,"context_line":""},{"line_number":2661,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":2662,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"}],"source_content_type":"text/x-python","patch_set":19,"id":"332ece28_955985bc","line":2659,"range":{"start_line":2647,"start_character":0,"end_line":2659,"end_character":22},"in_reply_to":"9e46dd54_551dc605","updated":"2021-03-05 15:58:17.000000000","message":"Good point. It can be combined with the DeviceNotFound exception handler below. Basically this is now duplicate code, as you said, the detach_with_retry does the same check and raises DeviceNotFound which is already caught below and a warning is logged.\n\nDone in https://review.opendev.org/c/openstack/nova/+/778978","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2658,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2659,"context_line":"                return"},{"line_number":2660,"context_line":""},{"line_number":2661,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":2662,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":2663,"context_line":"            get_dev \u003d functools.partial(guest.get_interface_by_cfg, cfg)"},{"line_number":2664,"context_line":"            self._detach_with_retry("},{"line_number":2665,"context_line":"                guest,"}],"source_content_type":"text/x-python","patch_set":19,"id":"925af304_b6773674","line":2662,"range":{"start_line":2661,"start_character":0,"end_line":2662,"end_character":69},"updated":"2021-02-16 15:55:48.000000000","message":"As with detach_volume can you move this down into _detach_with_retry","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"695e1d7f8ea5b6948124c2164533163ab6e5b815","unresolved":false,"context_lines":[{"line_number":2658,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2659,"context_line":"                return"},{"line_number":2660,"context_line":""},{"line_number":2661,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":2662,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":2663,"context_line":"            get_dev \u003d functools.partial(guest.get_interface_by_cfg, cfg)"},{"line_number":2664,"context_line":"            self._detach_with_retry("},{"line_number":2665,"context_line":"                guest,"}],"source_content_type":"text/x-python","patch_set":19,"id":"f5a3c007_a768809f","line":2662,"range":{"start_line":2661,"start_character":0,"end_line":2662,"end_character":69},"in_reply_to":"4cfefbf3_3d583465","updated":"2021-03-05 13:42:24.000000000","message":"I mean in https://review.opendev.org/c/openstack/nova/+/778918","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e8d258767671de5d3801b595176a8bce7510ff20","unresolved":false,"context_lines":[{"line_number":2658,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2659,"context_line":"                return"},{"line_number":2660,"context_line":""},{"line_number":2661,"context_line":"            state \u003d guest.get_power_state(self._host)"},{"line_number":2662,"context_line":"            live \u003d state in (power_state.RUNNING, power_state.PAUSED)"},{"line_number":2663,"context_line":"            get_dev \u003d functools.partial(guest.get_interface_by_cfg, cfg)"},{"line_number":2664,"context_line":"            self._detach_with_retry("},{"line_number":2665,"context_line":"                guest,"}],"source_content_type":"text/x-python","patch_set":19,"id":"4cfefbf3_3d583465","line":2662,"range":{"start_line":2661,"start_character":0,"end_line":2662,"end_character":69},"in_reply_to":"925af304_b6773674","updated":"2021-03-05 13:41:38.000000000","message":"Done in XXX","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2669,"context_line":"                live\u003dlive,"},{"line_number":2670,"context_line":"            )"},{"line_number":2671,"context_line":"        except exception.DeviceDetachFailed:"},{"line_number":2672,"context_line":"            # We failed to detach the device even with the retry loop, so let\u0027s"},{"line_number":2673,"context_line":"            # dump some debug information to the logs before raising back up."},{"line_number":2674,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2675,"context_line":"                devname \u003d self.vif_driver.get_vif_devname(vif)"},{"line_number":2676,"context_line":"                interface \u003d guest.get_interface_by_cfg(cfg)"},{"line_number":2677,"context_line":"                if interface:"},{"line_number":2678,"context_line":"                    LOG.warning("},{"line_number":2679,"context_line":"                        \u0027Failed to detach interface %(devname)s after \u0027"},{"line_number":2680,"context_line":"                        \u0027repeated attempts. Final interface xml:\\n\u0027"},{"line_number":2681,"context_line":"                        \u0027%(interface_xml)s\\nFinal guest xml:\\n%(guest_xml)s\u0027,"},{"line_number":2682,"context_line":"                        {\u0027devname\u0027: devname,"},{"line_number":2683,"context_line":"                         \u0027interface_xml\u0027: interface.to_xml(),"},{"line_number":2684,"context_line":"                         \u0027guest_xml\u0027: guest.get_xml_desc()},"},{"line_number":2685,"context_line":"                        instance\u003dinstance)"},{"line_number":2686,"context_line":"        except exception.DeviceNotFound:"},{"line_number":2687,"context_line":"            # The interface is gone so just log it as a warning."},{"line_number":2688,"context_line":"            LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"eefbc00a_e07250da","line":2685,"range":{"start_line":2672,"start_character":0,"end_line":2685,"end_character":42},"updated":"2021-02-16 15:55:48.000000000","message":"This duplicates logging within your change, I think we can drop it.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"9b6cc35df47eda019587875c06a9720e3ba2f97f","unresolved":true,"context_lines":[{"line_number":2669,"context_line":"                live\u003dlive,"},{"line_number":2670,"context_line":"            )"},{"line_number":2671,"context_line":"        except exception.DeviceDetachFailed:"},{"line_number":2672,"context_line":"            # We failed to detach the device even with the retry loop, so let\u0027s"},{"line_number":2673,"context_line":"            # dump some debug information to the logs before raising back up."},{"line_number":2674,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2675,"context_line":"                devname \u003d self.vif_driver.get_vif_devname(vif)"},{"line_number":2676,"context_line":"                interface \u003d guest.get_interface_by_cfg(cfg)"},{"line_number":2677,"context_line":"                if interface:"},{"line_number":2678,"context_line":"                    LOG.warning("},{"line_number":2679,"context_line":"                        \u0027Failed to detach interface %(devname)s after \u0027"},{"line_number":2680,"context_line":"                        \u0027repeated attempts. Final interface xml:\\n\u0027"},{"line_number":2681,"context_line":"                        \u0027%(interface_xml)s\\nFinal guest xml:\\n%(guest_xml)s\u0027,"},{"line_number":2682,"context_line":"                        {\u0027devname\u0027: devname,"},{"line_number":2683,"context_line":"                         \u0027interface_xml\u0027: interface.to_xml(),"},{"line_number":2684,"context_line":"                         \u0027guest_xml\u0027: guest.get_xml_desc()},"},{"line_number":2685,"context_line":"                        instance\u003dinstance)"},{"line_number":2686,"context_line":"        except exception.DeviceNotFound:"},{"line_number":2687,"context_line":"            # The interface is gone so just log it as a warning."},{"line_number":2688,"context_line":"            LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"ffe61c25_56054fc0","line":2685,"range":{"start_line":2672,"start_character":0,"end_line":2685,"end_character":42},"in_reply_to":"156a4cb0_499b8932","updated":"2021-02-16 16:14:37.000000000","message":"_detach_from_live_and_wait_for_event - we just check the config again here and dump some additional metadata that I\u0027m not convinced would actually be useful in debugging why the interface is still attached.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"918211ba20e360b56a7e1b4ff817c533a87f76f7","unresolved":true,"context_lines":[{"line_number":2669,"context_line":"                live\u003dlive,"},{"line_number":2670,"context_line":"            )"},{"line_number":2671,"context_line":"        except exception.DeviceDetachFailed:"},{"line_number":2672,"context_line":"            # We failed to detach the device even with the retry loop, so let\u0027s"},{"line_number":2673,"context_line":"            # dump some debug information to the logs before raising back up."},{"line_number":2674,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2675,"context_line":"                devname \u003d self.vif_driver.get_vif_devname(vif)"},{"line_number":2676,"context_line":"                interface \u003d guest.get_interface_by_cfg(cfg)"},{"line_number":2677,"context_line":"                if interface:"},{"line_number":2678,"context_line":"                    LOG.warning("},{"line_number":2679,"context_line":"                        \u0027Failed to detach interface %(devname)s after \u0027"},{"line_number":2680,"context_line":"                        \u0027repeated attempts. Final interface xml:\\n\u0027"},{"line_number":2681,"context_line":"                        \u0027%(interface_xml)s\\nFinal guest xml:\\n%(guest_xml)s\u0027,"},{"line_number":2682,"context_line":"                        {\u0027devname\u0027: devname,"},{"line_number":2683,"context_line":"                         \u0027interface_xml\u0027: interface.to_xml(),"},{"line_number":2684,"context_line":"                         \u0027guest_xml\u0027: guest.get_xml_desc()},"},{"line_number":2685,"context_line":"                        instance\u003dinstance)"},{"line_number":2686,"context_line":"        except exception.DeviceNotFound:"},{"line_number":2687,"context_line":"            # The interface is gone so just log it as a warning."},{"line_number":2688,"context_line":"            LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"156a4cb0_499b8932","line":2685,"range":{"start_line":2672,"start_character":0,"end_line":2685,"end_character":42},"in_reply_to":"eefbc00a_e07250da","updated":"2021-02-16 16:07:46.000000000","message":"where is the duplicationion?\n_detach_with_retry?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49fc881a8005a11a2d34d5b0cc5bac74ff50aa01","unresolved":false,"context_lines":[{"line_number":2669,"context_line":"                live\u003dlive,"},{"line_number":2670,"context_line":"            )"},{"line_number":2671,"context_line":"        except exception.DeviceDetachFailed:"},{"line_number":2672,"context_line":"            # We failed to detach the device even with the retry loop, so let\u0027s"},{"line_number":2673,"context_line":"            # dump some debug information to the logs before raising back up."},{"line_number":2674,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":2675,"context_line":"                devname \u003d self.vif_driver.get_vif_devname(vif)"},{"line_number":2676,"context_line":"                interface \u003d guest.get_interface_by_cfg(cfg)"},{"line_number":2677,"context_line":"                if interface:"},{"line_number":2678,"context_line":"                    LOG.warning("},{"line_number":2679,"context_line":"                        \u0027Failed to detach interface %(devname)s after \u0027"},{"line_number":2680,"context_line":"                        \u0027repeated attempts. Final interface xml:\\n\u0027"},{"line_number":2681,"context_line":"                        \u0027%(interface_xml)s\\nFinal guest xml:\\n%(guest_xml)s\u0027,"},{"line_number":2682,"context_line":"                        {\u0027devname\u0027: devname,"},{"line_number":2683,"context_line":"                         \u0027interface_xml\u0027: interface.to_xml(),"},{"line_number":2684,"context_line":"                         \u0027guest_xml\u0027: guest.get_xml_desc()},"},{"line_number":2685,"context_line":"                        instance\u003dinstance)"},{"line_number":2686,"context_line":"        except exception.DeviceNotFound:"},{"line_number":2687,"context_line":"            # The interface is gone so just log it as a warning."},{"line_number":2688,"context_line":"            LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"1273e263_ab3263d9","line":2685,"range":{"start_line":2672,"start_character":0,"end_line":2685,"end_character":42},"in_reply_to":"ffe61c25_56054fc0","updated":"2021-03-05 15:58:17.000000000","message":"Correct. We log plenty of information already at failure in _detach_with_retry. Done in https://review.opendev.org/c/openstack/nova/+/778978","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2690,"context_line":"                        {\u0027mac\u0027: vif.get(\u0027address\u0027)}, instance\u003dinstance)"},{"line_number":2691,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":2692,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":2693,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_NO_DOMAIN:"},{"line_number":2694,"context_line":"                LOG.warning(\"During detach_interface, instance disappeared.\","},{"line_number":2695,"context_line":"                            instance\u003dinstance)"},{"line_number":2696,"context_line":"            else:"},{"line_number":2697,"context_line":"                # NOTE(mriedem): When deleting an instance and using Neutron,"},{"line_number":2698,"context_line":"                # we can be racing against Neutron deleting the port and"}],"source_content_type":"text/x-python","patch_set":19,"id":"6f135595_67d8fc22","line":2695,"range":{"start_line":2693,"start_character":0,"end_line":2695,"end_character":46},"updated":"2021-02-16 15:55:48.000000000","message":"This smells like something we should move to _detach_sync","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49fc881a8005a11a2d34d5b0cc5bac74ff50aa01","unresolved":false,"context_lines":[{"line_number":2690,"context_line":"                        {\u0027mac\u0027: vif.get(\u0027address\u0027)}, instance\u003dinstance)"},{"line_number":2691,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":2692,"context_line":"            error_code \u003d ex.get_error_code()"},{"line_number":2693,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_NO_DOMAIN:"},{"line_number":2694,"context_line":"                LOG.warning(\"During detach_interface, instance disappeared.\","},{"line_number":2695,"context_line":"                            instance\u003dinstance)"},{"line_number":2696,"context_line":"            else:"},{"line_number":2697,"context_line":"                # NOTE(mriedem): When deleting an instance and using Neutron,"},{"line_number":2698,"context_line":"                # we can be racing against Neutron deleting the port and"}],"source_content_type":"text/x-python","patch_set":19,"id":"fddc6f98_454d621a","line":2695,"range":{"start_line":2693,"start_character":0,"end_line":2695,"end_character":46},"in_reply_to":"6f135595_67d8fc22","updated":"2021-03-05 15:58:17.000000000","message":"Hm, yes that would be good to catch and log there. Done in https://review.opendev.org/c/openstack/nova/+/778978","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2698,"context_line":"                # we can be racing against Neutron deleting the port and"},{"line_number":2699,"context_line":"                # sending the vif-deleted event which then triggers a call to"},{"line_number":2700,"context_line":"                # detach the interface, so we might have failed because the"},{"line_number":2701,"context_line":"                # network device no longer exists. Libvirt will fail with"},{"line_number":2702,"context_line":"                # \"operation failed: no matching network device was found\""},{"line_number":2703,"context_line":"                # which unfortunately does not have a unique error code so we"},{"line_number":2704,"context_line":"                # need to look up the interface by config and if it\u0027s not found"},{"line_number":2705,"context_line":"                # then we can just log it as a warning rather than tracing an"},{"line_number":2706,"context_line":"                # error."}],"source_content_type":"text/x-python","patch_set":19,"id":"99250689_abaccc11","line":2703,"range":{"start_line":2701,"start_character":51,"end_line":2703,"end_character":71},"updated":"2021-02-16 15:55:48.000000000","message":"Looking at the libvirt source it suggests this is actually VIR_ERR_DEVICE_MISSING now so I think we can drop this?\n\nhttps://github.com/libvirt/libvirt/blob/945132f842101535aee545413f3082a62a9a3453/src/conf/domain_conf.c#L16515-L16555","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"49fc881a8005a11a2d34d5b0cc5bac74ff50aa01","unresolved":false,"context_lines":[{"line_number":2698,"context_line":"                # we can be racing against Neutron deleting the port and"},{"line_number":2699,"context_line":"                # sending the vif-deleted event which then triggers a call to"},{"line_number":2700,"context_line":"                # detach the interface, so we might have failed because the"},{"line_number":2701,"context_line":"                # network device no longer exists. Libvirt will fail with"},{"line_number":2702,"context_line":"                # \"operation failed: no matching network device was found\""},{"line_number":2703,"context_line":"                # which unfortunately does not have a unique error code so we"},{"line_number":2704,"context_line":"                # need to look up the interface by config and if it\u0027s not found"},{"line_number":2705,"context_line":"                # then we can just log it as a warning rather than tracing an"},{"line_number":2706,"context_line":"                # error."}],"source_content_type":"text/x-python","patch_set":19,"id":"2a1ce1e2_b46e0a7a","line":2703,"range":{"start_line":2701,"start_character":51,"end_line":2703,"end_character":71},"in_reply_to":"99250689_abaccc11","updated":"2021-03-05 15:58:17.000000000","message":"Good point this is already covered in the _detach_sync() and transformed to DeviceNotFound handled above with a similar warning log we have here. I guess this else: branch handled more than just the missing device case. But for those unrealated cases we are fine propagating the libvirtError and fail hard with http 500.\n\nDone in  https://review.opendev.org/c/openstack/nova/+/778978","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":true,"context_lines":[{"line_number":2723,"context_line":"                LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"},{"line_number":2724,"context_line":"                            \u0027the device is no longer found on the guest.\u0027,"},{"line_number":2725,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2726,"context_line":"        finally:"},{"line_number":2727,"context_line":"            # NOTE(gibi): we need to unplug the vif _after_ the detach is done"},{"line_number":2728,"context_line":"            # on the libvirt side as otherwise libvirt will still manage the"},{"line_number":2729,"context_line":"            # device that our unplug code trying to reset. This can cause a"},{"line_number":2730,"context_line":"            # race and leave the detached device configured. Also even if we"},{"line_number":2731,"context_line":"            # are failed to detach due to race conditions the unplug is"},{"line_number":2732,"context_line":"            # necessary for the same reason"},{"line_number":2733,"context_line":"            self.vif_driver.unplug(instance, vif)"},{"line_number":2734,"context_line":""},{"line_number":2735,"context_line":"    def _create_snapshot_metadata(self, image_meta, instance,"},{"line_number":2736,"context_line":"                                  img_fmt, snp_name):"}],"source_content_type":"text/x-python","patch_set":19,"id":"7e25dada_3cd5a0e8","line":2733,"range":{"start_line":2726,"start_character":0,"end_line":2733,"end_character":49},"updated":"2021-02-16 15:55:48.000000000","message":"Nice, FWIW we should really refactor detach_volume to call disconnect_volume like this.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"9b6cc35df47eda019587875c06a9720e3ba2f97f","unresolved":false,"context_lines":[{"line_number":2723,"context_line":"                LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"},{"line_number":2724,"context_line":"                            \u0027the device is no longer found on the guest.\u0027,"},{"line_number":2725,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2726,"context_line":"        finally:"},{"line_number":2727,"context_line":"            # NOTE(gibi): we need to unplug the vif _after_ the detach is done"},{"line_number":2728,"context_line":"            # on the libvirt side as otherwise libvirt will still manage the"},{"line_number":2729,"context_line":"            # device that our unplug code trying to reset. This can cause a"},{"line_number":2730,"context_line":"            # race and leave the detached device configured. Also even if we"},{"line_number":2731,"context_line":"            # are failed to detach due to race conditions the unplug is"},{"line_number":2732,"context_line":"            # necessary for the same reason"},{"line_number":2733,"context_line":"            self.vif_driver.unplug(instance, vif)"},{"line_number":2734,"context_line":""},{"line_number":2735,"context_line":"    def _create_snapshot_metadata(self, image_meta, instance,"},{"line_number":2736,"context_line":"                                  img_fmt, snp_name):"}],"source_content_type":"text/x-python","patch_set":19,"id":"d24b1a20_5b6b9663","line":2733,"range":{"start_line":2726,"start_character":0,"end_line":2733,"end_character":49},"in_reply_to":"48071875_f9b474dc","updated":"2021-02-16 16:14:37.000000000","message":"I was going to say the finally block but thinking about it should we really unplug here if the detach failed and the device is still attached to the domain?","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3ed000620fdd8330c67548b6331142e18a34b366","unresolved":false,"context_lines":[{"line_number":2723,"context_line":"                LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"},{"line_number":2724,"context_line":"                            \u0027the device is no longer found on the guest.\u0027,"},{"line_number":2725,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2726,"context_line":"        finally:"},{"line_number":2727,"context_line":"            # NOTE(gibi): we need to unplug the vif _after_ the detach is done"},{"line_number":2728,"context_line":"            # on the libvirt side as otherwise libvirt will still manage the"},{"line_number":2729,"context_line":"            # device that our unplug code trying to reset. This can cause a"},{"line_number":2730,"context_line":"            # race and leave the detached device configured. Also even if we"},{"line_number":2731,"context_line":"            # are failed to detach due to race conditions the unplug is"},{"line_number":2732,"context_line":"            # necessary for the same reason"},{"line_number":2733,"context_line":"            self.vif_driver.unplug(instance, vif)"},{"line_number":2734,"context_line":""},{"line_number":2735,"context_line":"    def _create_snapshot_metadata(self, image_meta, instance,"},{"line_number":2736,"context_line":"                                  img_fmt, snp_name):"}],"source_content_type":"text/x-python","patch_set":19,"id":"bc452558_9dad8bfd","line":2733,"range":{"start_line":2726,"start_character":0,"end_line":2733,"end_character":49},"in_reply_to":"7e25dada_3cd5a0e8","updated":"2021-02-16 15:58:50.000000000","message":"Apologies this shouldn\u0027t be unresolved.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"918211ba20e360b56a7e1b4ff817c533a87f76f7","unresolved":false,"context_lines":[{"line_number":2723,"context_line":"                LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"},{"line_number":2724,"context_line":"                            \u0027the device is no longer found on the guest.\u0027,"},{"line_number":2725,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2726,"context_line":"        finally:"},{"line_number":2727,"context_line":"            # NOTE(gibi): we need to unplug the vif _after_ the detach is done"},{"line_number":2728,"context_line":"            # on the libvirt side as otherwise libvirt will still manage the"},{"line_number":2729,"context_line":"            # device that our unplug code trying to reset. This can cause a"},{"line_number":2730,"context_line":"            # race and leave the detached device configured. Also even if we"},{"line_number":2731,"context_line":"            # are failed to detach due to race conditions the unplug is"},{"line_number":2732,"context_line":"            # necessary for the same reason"},{"line_number":2733,"context_line":"            self.vif_driver.unplug(instance, vif)"},{"line_number":2734,"context_line":""},{"line_number":2735,"context_line":"    def _create_snapshot_metadata(self, image_meta, instance,"},{"line_number":2736,"context_line":"                                  img_fmt, snp_name):"}],"source_content_type":"text/x-python","patch_set":19,"id":"48071875_f9b474dc","line":2733,"range":{"start_line":2726,"start_character":0,"end_line":2733,"end_character":49},"in_reply_to":"bc452558_9dad8bfd","updated":"2021-02-16 16:07:46.000000000","message":"you mean in a finally block or in the reverse order to attach.\n\ne.g. in attach we plug before updating the libvirt xml  so in detach we unplug after updating it.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"61600b7c5c1e14aacc70e4e7aff0080e6c4352e3","unresolved":false,"context_lines":[{"line_number":2723,"context_line":"                LOG.warning(\u0027Detaching interface %(mac)s failed because \u0027"},{"line_number":2724,"context_line":"                            \u0027the device is no longer found on the guest.\u0027,"},{"line_number":2725,"context_line":"                            {\u0027mac\u0027: mac}, instance\u003dinstance)"},{"line_number":2726,"context_line":"        finally:"},{"line_number":2727,"context_line":"            # NOTE(gibi): we need to unplug the vif _after_ the detach is done"},{"line_number":2728,"context_line":"            # on the libvirt side as otherwise libvirt will still manage the"},{"line_number":2729,"context_line":"            # device that our unplug code trying to reset. This can cause a"},{"line_number":2730,"context_line":"            # race and leave the detached device configured. Also even if we"},{"line_number":2731,"context_line":"            # are failed to detach due to race conditions the unplug is"},{"line_number":2732,"context_line":"            # necessary for the same reason"},{"line_number":2733,"context_line":"            self.vif_driver.unplug(instance, vif)"},{"line_number":2734,"context_line":""},{"line_number":2735,"context_line":"    def _create_snapshot_metadata(self, image_meta, instance,"},{"line_number":2736,"context_line":"                                  img_fmt, snp_name):"}],"source_content_type":"text/x-python","patch_set":19,"id":"3af635e3_9942b141","line":2733,"range":{"start_line":2726,"start_character":0,"end_line":2733,"end_character":49},"in_reply_to":"d24b1a20_5b6b9663","updated":"2021-02-16 17:07:29.000000000","message":"i think so.\ni think we alwasy want to tear down the netowrking even if the vm is not updated.\ni belive that was the old behavior anyway we jsut consolidated it into one code path\nwhen we changed the order.","commit_id":"d8bc9ef4ba95d36b9a5efa7806730b899c2207b2"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":1971,"context_line":"        # VolumeEncryptionNotSupported being thrown when we incorrectly build"},{"line_number":1972,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1973,"context_line":"        if (encryption and self._allow_native_luksv1(encryption\u003dencryption) and"},{"line_number":1974,"context_line":"                not connection_info[\u0027data\u0027].get(\u0027device_path\u0027)):"},{"line_number":1975,"context_line":"            return"},{"line_number":1976,"context_line":"        if encryption:"},{"line_number":1977,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"}],"source_content_type":"text/x-python","patch_set":24,"id":"42d8de1d_1e1a5265","line":1974,"range":{"start_line":1974,"start_character":0,"end_line":1974,"end_character":64},"updated":"2021-03-08 10:25:01.000000000","message":"nit - Valid but unrelated.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":1971,"context_line":"        # VolumeEncryptionNotSupported being thrown when we incorrectly build"},{"line_number":1972,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1973,"context_line":"        if (encryption and self._allow_native_luksv1(encryption\u003dencryption) and"},{"line_number":1974,"context_line":"                not connection_info[\u0027data\u0027].get(\u0027device_path\u0027)):"},{"line_number":1975,"context_line":"            return"},{"line_number":1976,"context_line":"        if encryption:"},{"line_number":1977,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"}],"source_content_type":"text/x-python","patch_set":24,"id":"1ee39b28_94f2eca3","line":1974,"range":{"start_line":1974,"start_character":0,"end_line":1974,"end_character":64},"in_reply_to":"42d8de1d_1e1a5265","updated":"2021-04-14 16:33:36.000000000","message":"I blame my IDE. Reverted.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":2195,"context_line":"                        event,"},{"line_number":2196,"context_line":"                    )"},{"line_number":2197,"context_line":"                else:"},{"line_number":2198,"context_line":"                    LOG.debug("},{"line_number":2199,"context_line":"                        \"Received event %s from libvirt but the driver is not \""},{"line_number":2200,"context_line":"                        \"waiting for it; ignored.\","},{"line_number":2201,"context_line":"                        event,"}],"source_content_type":"text/x-python","patch_set":24,"id":"8f2f5cbc_9593bdb1","line":2198,"range":{"start_line":2198,"start_character":24,"end_line":2198,"end_character":29},"updated":"2021-03-08 10:25:01.000000000","message":"I wonder if this needs to be a warning? I guess it suggests someone or something is detaching devices outside of Nova\u0027s control?","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":2195,"context_line":"                        event,"},{"line_number":2196,"context_line":"                    )"},{"line_number":2197,"context_line":"                else:"},{"line_number":2198,"context_line":"                    LOG.debug("},{"line_number":2199,"context_line":"                        \"Received event %s from libvirt but the driver is not \""},{"line_number":2200,"context_line":"                        \"waiting for it; ignored.\","},{"line_number":2201,"context_line":"                        event,"}],"source_content_type":"text/x-python","patch_set":24,"id":"537c77d8_fa391c08","line":2198,"range":{"start_line":2198,"start_character":24,"end_line":2198,"end_character":29},"in_reply_to":"1aecac24_15e0d145","updated":"2021-04-14 16:33:36.000000000","message":"Done","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99ea45264a2e60018d19dba5ac2a90a8a5cbd0ad","unresolved":true,"context_lines":[{"line_number":2195,"context_line":"                        event,"},{"line_number":2196,"context_line":"                    )"},{"line_number":2197,"context_line":"                else:"},{"line_number":2198,"context_line":"                    LOG.debug("},{"line_number":2199,"context_line":"                        \"Received event %s from libvirt but the driver is not \""},{"line_number":2200,"context_line":"                        \"waiting for it; ignored.\","},{"line_number":2201,"context_line":"                        event,"}],"source_content_type":"text/x-python","patch_set":24,"id":"1aecac24_15e0d145","line":2198,"range":{"start_line":2198,"start_character":24,"end_line":2198,"end_character":29},"in_reply_to":"8f2f5cbc_9593bdb1","updated":"2021-03-19 08:44:03.000000000","message":"Yeah we could make it a warning, as it should not really happen in normal circumstances.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":2223,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2224,"context_line":""},{"line_number":2225,"context_line":"        If live detach is requested then this call will wait for the libvirt"},{"line_number":2226,"context_line":"        event signalling the finish of the detach process."},{"line_number":2227,"context_line":""},{"line_number":2228,"context_line":"        If the live detach times out then it will retry the detach. Detach from"},{"line_number":2229,"context_line":"        the persistent config is not retried as it is:"}],"source_content_type":"text/x-python","patch_set":24,"id":"a1805712_86f9035c","line":2226,"range":{"start_line":2226,"start_character":29,"end_line":2226,"end_character":35},"updated":"2021-03-08 10:25:01.000000000","message":"supernit - end","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":2223,"context_line":"        \"\"\"Detaches a device from the guest"},{"line_number":2224,"context_line":""},{"line_number":2225,"context_line":"        If live detach is requested then this call will wait for the libvirt"},{"line_number":2226,"context_line":"        event signalling the finish of the detach process."},{"line_number":2227,"context_line":""},{"line_number":2228,"context_line":"        If the live detach times out then it will retry the detach. Detach from"},{"line_number":2229,"context_line":"        the persistent config is not retried as it is:"}],"source_content_type":"text/x-python","patch_set":24,"id":"1afd62d9_0024d58a","line":2226,"range":{"start_line":2226,"start_character":29,"end_line":2226,"end_character":35},"in_reply_to":"a1805712_86f9035c","updated":"2021-04-14 16:33:36.000000000","message":"Done","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":false,"context_lines":[{"line_number":2260,"context_line":"            # nothing to do"},{"line_number":2261,"context_line":"            return"},{"line_number":2262,"context_line":""},{"line_number":2263,"context_line":"        if persistent:"},{"line_number":2264,"context_line":"            persistent_dev \u003d get_device_conf_func(from_persistent_config\u003dTrue)"},{"line_number":2265,"context_line":"        else:"},{"line_number":2266,"context_line":"            persistent_dev \u003d None"},{"line_number":2267,"context_line":""},{"line_number":2268,"context_line":"        if live:"},{"line_number":2269,"context_line":"            live_dev \u003d get_device_conf_func()"}],"source_content_type":"text/x-python","patch_set":24,"id":"cc0382c8_504acd87","line":2266,"range":{"start_line":2263,"start_character":0,"end_line":2266,"end_character":33},"updated":"2021-03-08 10:25:01.000000000","message":"supersupernit - feel free to ignore.\n\n        persistent_dev \u003d None\n        if persistent:\n            persistent_dev \u003d get_device_conf_func(from_persistent_config\u003dTrue)","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":2260,"context_line":"            # nothing to do"},{"line_number":2261,"context_line":"            return"},{"line_number":2262,"context_line":""},{"line_number":2263,"context_line":"        if persistent:"},{"line_number":2264,"context_line":"            persistent_dev \u003d get_device_conf_func(from_persistent_config\u003dTrue)"},{"line_number":2265,"context_line":"        else:"},{"line_number":2266,"context_line":"            persistent_dev \u003d None"},{"line_number":2267,"context_line":""},{"line_number":2268,"context_line":"        if live:"},{"line_number":2269,"context_line":"            live_dev \u003d get_device_conf_func()"}],"source_content_type":"text/x-python","patch_set":24,"id":"947cdbd4_d5ce600a","line":2266,"range":{"start_line":2263,"start_character":0,"end_line":2266,"end_character":33},"in_reply_to":"cc0382c8_504acd87","updated":"2021-04-14 16:33:36.000000000","message":"I have a strange attraction to complete if-then-else constructs :D but true that what you suggest is equivalent and somehow more aligned with the nova style (or even more pythonic). Done.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":false,"context_lines":[{"line_number":2265,"context_line":"        else:"},{"line_number":2266,"context_line":"            persistent_dev \u003d None"},{"line_number":2267,"context_line":""},{"line_number":2268,"context_line":"        if live:"},{"line_number":2269,"context_line":"            live_dev \u003d get_device_conf_func()"},{"line_number":2270,"context_line":"        else:"},{"line_number":2271,"context_line":"            live_dev \u003d None"},{"line_number":2272,"context_line":""},{"line_number":2273,"context_line":"        if live and live_dev is None:"},{"line_number":2274,"context_line":"            # caller requested a live detach but device is not present"}],"source_content_type":"text/x-python","patch_set":24,"id":"636e753d_e2c0ea91","line":2271,"range":{"start_line":2268,"start_character":0,"end_line":2271,"end_character":27},"updated":"2021-03-08 10:25:01.000000000","message":"supersupernit - As above.\n\n        live_dev \u003d None\n        if live:\n            live_dev \u003d get_device_conf_func()","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":2265,"context_line":"        else:"},{"line_number":2266,"context_line":"            persistent_dev \u003d None"},{"line_number":2267,"context_line":""},{"line_number":2268,"context_line":"        if live:"},{"line_number":2269,"context_line":"            live_dev \u003d get_device_conf_func()"},{"line_number":2270,"context_line":"        else:"},{"line_number":2271,"context_line":"            live_dev \u003d None"},{"line_number":2272,"context_line":""},{"line_number":2273,"context_line":"        if live and live_dev is None:"},{"line_number":2274,"context_line":"            # caller requested a live detach but device is not present"}],"source_content_type":"text/x-python","patch_set":24,"id":"72f2a215_e239ea4e","line":2271,"range":{"start_line":2268,"start_character":0,"end_line":2271,"end_character":27},"in_reply_to":"636e753d_e2c0ea91","updated":"2021-04-14 16:33:36.000000000","message":"Done.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":2270,"context_line":"        else:"},{"line_number":2271,"context_line":"            live_dev \u003d None"},{"line_number":2272,"context_line":""},{"line_number":2273,"context_line":"        if live and live_dev is None:"},{"line_number":2274,"context_line":"            # caller requested a live detach but device is not present"},{"line_number":2275,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2276,"context_line":""},{"line_number":2277,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2278,"context_line":"            # caller requested detach from the persistent domain but device is"}],"source_content_type":"text/x-python","patch_set":24,"id":"10be73be_900ed97f","line":2275,"range":{"start_line":2273,"start_character":0,"end_line":2275,"end_character":62},"updated":"2021-03-08 10:25:01.000000000","message":"I\u0027m not sure about this, shouldn\u0027t we check this *after* the _detach_from_persistent attempt so we always try to remove the persistent device if present?","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"99ea45264a2e60018d19dba5ac2a90a8a5cbd0ad","unresolved":true,"context_lines":[{"line_number":2270,"context_line":"        else:"},{"line_number":2271,"context_line":"            live_dev \u003d None"},{"line_number":2272,"context_line":""},{"line_number":2273,"context_line":"        if live and live_dev is None:"},{"line_number":2274,"context_line":"            # caller requested a live detach but device is not present"},{"line_number":2275,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2276,"context_line":""},{"line_number":2277,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2278,"context_line":"            # caller requested detach from the persistent domain but device is"}],"source_content_type":"text/x-python","patch_set":24,"id":"53399c13_4e3cba9e","line":2275,"range":{"start_line":2273,"start_character":0,"end_line":2275,"end_character":62},"in_reply_to":"10be73be_900ed97f","updated":"2021-03-19 08:44:03.000000000","message":"This is the same logic as in the baseline[1].\n\nThe libvirt documentation [2] is not super clear what happens when the domain is defined but not running. Will a simple XMLDesc return the non running definition or return nothing. If it returns the non running definition then our whole naming convention here is wrong and misleading. As if the domain is stopped then get_device_conf_func() returns the persistent dev instead of returning None for the live dev. One can argue that if the client requested detach from the live domain then we should fail if the domain is not alive. \n\nThe next patch in the series actually encapsulate the aliveness check into the detach function[3] so that makes the situation clearer as it clarifies the whole what-was-the-callers-intention. The caller intention becomes: detach the device for every form (live and persistent) of the domain. And then your comment is totally valid in a sense that we need to detach from the persistent domain if the device is not in the live domain. \n\n[1] https://review.opendev.org/c/openstack/nova/+/770246/24/nova/virt/libvirt/guest.py#b450\n[2] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetXMLDesc\n[3] https://review.opendev.org/c/openstack/nova/+/778918/1/nova/virt/libvirt/driver.py#2267","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"f6c33a661254464ec28bbe19910c30ec390dc07c","unresolved":true,"context_lines":[{"line_number":2270,"context_line":"        else:"},{"line_number":2271,"context_line":"            live_dev \u003d None"},{"line_number":2272,"context_line":""},{"line_number":2273,"context_line":"        if live and live_dev is None:"},{"line_number":2274,"context_line":"            # caller requested a live detach but device is not present"},{"line_number":2275,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2276,"context_line":""},{"line_number":2277,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2278,"context_line":"            # caller requested detach from the persistent domain but device is"}],"source_content_type":"text/x-python","patch_set":24,"id":"c9ac7b47_9890f6ee","line":2275,"range":{"start_line":2273,"start_character":0,"end_line":2275,"end_character":62},"in_reply_to":"53399c13_4e3cba9e","updated":"2021-03-19 08:59:41.000000000","message":"By default no XMLDesc doesn\u0027t return the inactive config, you need to provide VIR_DOMAIN_XML_INACTIVE [1] as we do in guest.get_disk when from_persistent_config\u003dTrue. So if the domain isn\u0027t running it should list anything but then again I\u0027m not sure if we\u0027d even get this far in that case?\n\nAnyway yeah in terms of the next change it becomes clear that we need to move this check below the persistent_dev removal.\n\n[1] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainXMLFlags","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":2270,"context_line":"        else:"},{"line_number":2271,"context_line":"            live_dev \u003d None"},{"line_number":2272,"context_line":""},{"line_number":2273,"context_line":"        if live and live_dev is None:"},{"line_number":2274,"context_line":"            # caller requested a live detach but device is not present"},{"line_number":2275,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2276,"context_line":""},{"line_number":2277,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2278,"context_line":"            # caller requested detach from the persistent domain but device is"}],"source_content_type":"text/x-python","patch_set":24,"id":"0f17a1eb_a5752d0a","line":2275,"range":{"start_line":2273,"start_character":0,"end_line":2275,"end_character":62},"in_reply_to":"5e0db2da_84308f41","updated":"2021-04-14 16:33:36.000000000","message":"let me do this is in the next patch where it make sense to me a bit more.\n\nDone in the subsequent patch","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"28ca8c757e0c931b17829c7e5cb44da2b7b44871","unresolved":true,"context_lines":[{"line_number":2270,"context_line":"        else:"},{"line_number":2271,"context_line":"            live_dev \u003d None"},{"line_number":2272,"context_line":""},{"line_number":2273,"context_line":"        if live and live_dev is None:"},{"line_number":2274,"context_line":"            # caller requested a live detach but device is not present"},{"line_number":2275,"context_line":"            raise exception.DeviceNotFound(device\u003ddevice_name)"},{"line_number":2276,"context_line":""},{"line_number":2277,"context_line":"        if not live and persistent_dev is None:"},{"line_number":2278,"context_line":"            # caller requested detach from the persistent domain but device is"}],"source_content_type":"text/x-python","patch_set":24,"id":"5e0db2da_84308f41","line":2275,"range":{"start_line":2273,"start_character":0,"end_line":2275,"end_character":62},"in_reply_to":"c9ac7b47_9890f6ee","updated":"2021-03-19 09:09:06.000000000","message":"\u003e it should list anything\n\n_shouldn\u0027t_ list anything sorry.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":2312,"context_line":"        get_device_conf_func,"},{"line_number":2313,"context_line":"        device_name: str,"},{"line_number":2314,"context_line":"    ):"},{"line_number":2315,"context_line":"        LOG.debug("},{"line_number":2316,"context_line":"            \u0027Attempting to detach device %s from instance %s from \u0027"},{"line_number":2317,"context_line":"            \u0027the persistent domain config.\u0027, device_name, instance_uuid)"},{"line_number":2318,"context_line":""},{"line_number":2319,"context_line":"        self._detach_sync("},{"line_number":2320,"context_line":"            persistent_dev, guest, instance_uuid, device_name,"}],"source_content_type":"text/x-python","patch_set":24,"id":"cacf9b64_cb7c07f2","line":2317,"range":{"start_line":2315,"start_character":0,"end_line":2317,"end_character":72},"updated":"2021-03-08 10:25:01.000000000","message":"nit - Can you just use instance\u003dinstance and drop the uuid from the actual log string? Ditto for all of the log calls below.","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":true,"context_lines":[{"line_number":2312,"context_line":"        get_device_conf_func,"},{"line_number":2313,"context_line":"        device_name: str,"},{"line_number":2314,"context_line":"    ):"},{"line_number":2315,"context_line":"        LOG.debug("},{"line_number":2316,"context_line":"            \u0027Attempting to detach device %s from instance %s from \u0027"},{"line_number":2317,"context_line":"            \u0027the persistent domain config.\u0027, device_name, instance_uuid)"},{"line_number":2318,"context_line":""},{"line_number":2319,"context_line":"        self._detach_sync("},{"line_number":2320,"context_line":"            persistent_dev, guest, instance_uuid, device_name,"}],"source_content_type":"text/x-python","patch_set":24,"id":"ca57e178_b656786f","line":2317,"range":{"start_line":2315,"start_character":0,"end_line":2317,"end_character":72},"in_reply_to":"cacf9b64_cb7c07f2","updated":"2021-04-14 16:33:36.000000000","message":"The instance object is not passed to this call just the instance_uuid. Should I change the signature and pass the whole instance even it is only the uuid is used?","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":2324,"context_line":"        persistent_dev \u003d get_device_conf_func("},{"line_number":2325,"context_line":"            from_persistent_config\u003dTrue)"},{"line_number":2326,"context_line":"        if not persistent_dev:"},{"line_number":2327,"context_line":"            LOG.debug("},{"line_number":2328,"context_line":"                \u0027Successfully detached device %s from instance %s \u0027"},{"line_number":2329,"context_line":"                \u0027from the persistent domain config.\u0027,"},{"line_number":2330,"context_line":"                device_name, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"075ea886_2cc72291","line":2327,"range":{"start_line":2327,"start_character":16,"end_line":2327,"end_character":21},"updated":"2021-03-08 10:25:01.000000000","message":"info for success?","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":2324,"context_line":"        persistent_dev \u003d get_device_conf_func("},{"line_number":2325,"context_line":"            from_persistent_config\u003dTrue)"},{"line_number":2326,"context_line":"        if not persistent_dev:"},{"line_number":2327,"context_line":"            LOG.debug("},{"line_number":2328,"context_line":"                \u0027Successfully detached device %s from instance %s \u0027"},{"line_number":2329,"context_line":"                \u0027from the persistent domain config.\u0027,"},{"line_number":2330,"context_line":"                device_name, instance_uuid)"}],"source_content_type":"text/x-python","patch_set":24,"id":"572e9cd5_2bc37604","line":2327,"range":{"start_line":2327,"start_character":16,"end_line":2327,"end_character":21},"in_reply_to":"075ea886_2cc72291","updated":"2021-04-14 16:33:36.000000000","message":"Done","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d03920967287443586ac2cc4ccf23a84f12d4a1d","unresolved":true,"context_lines":[{"line_number":2360,"context_line":"            # make sure the dev is really gone"},{"line_number":2361,"context_line":"            live_dev \u003d get_device_conf_func()"},{"line_number":2362,"context_line":"            if not live_dev:"},{"line_number":2363,"context_line":"                LOG.debug("},{"line_number":2364,"context_line":"                    \u0027Successfully detached device %s from instance %s \u0027"},{"line_number":2365,"context_line":"                    \u0027from the live domain config.\u0027, device_name, instance_uuid)"},{"line_number":2366,"context_line":"                # we are done"}],"source_content_type":"text/x-python","patch_set":24,"id":"165976ab_216fbb71","line":2363,"range":{"start_line":2363,"start_character":20,"end_line":2363,"end_character":25},"updated":"2021-03-08 10:25:01.000000000","message":"As above what about using info for success?","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"adbdb39c85d3fc86301351fdf048c1819754d04e","unresolved":false,"context_lines":[{"line_number":2360,"context_line":"            # make sure the dev is really gone"},{"line_number":2361,"context_line":"            live_dev \u003d get_device_conf_func()"},{"line_number":2362,"context_line":"            if not live_dev:"},{"line_number":2363,"context_line":"                LOG.debug("},{"line_number":2364,"context_line":"                    \u0027Successfully detached device %s from instance %s \u0027"},{"line_number":2365,"context_line":"                    \u0027from the live domain config.\u0027, device_name, instance_uuid)"},{"line_number":2366,"context_line":"                # we are done"}],"source_content_type":"text/x-python","patch_set":24,"id":"1ccc4941_507a10d6","line":2363,"range":{"start_line":2363,"start_character":20,"end_line":2363,"end_character":25},"in_reply_to":"165976ab_216fbb71","updated":"2021-04-14 16:33:36.000000000","message":"Done","commit_id":"20f7c54ab734fec0478b4b61bca8699e89bc6482"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3fbb40564e58f5c3a996f9ee93c0246d0dcc284c","unresolved":true,"context_lines":[{"line_number":2497,"context_line":"        \"\"\""},{"line_number":2498,"context_line":"        try:"},{"line_number":2499,"context_line":"            guest.detach_device(dev, persistent\u003dpersistent, live\u003dlive)"},{"line_number":2500,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":2501,"context_line":"            if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_DEVICE_MISSING:"},{"line_number":2502,"context_line":"                LOG.debug("},{"line_number":2503,"context_line":"                    \u0027Libvirt failed to detach device %s from instance %s \u0027"},{"line_number":2504,"context_line":"                    \u0027synchronously (persistent\u003d%s, live\u003d%s) with error: %s.\u0027,"},{"line_number":2505,"context_line":"                    device_name, instance_uuid, persistent, live, str(ex))"},{"line_number":2506,"context_line":"                raise exception.DeviceNotFound(device\u003ddevice_name) from ex"},{"line_number":2507,"context_line":""},{"line_number":2508,"context_line":"            LOG.warning("},{"line_number":2509,"context_line":"                \u0027Unexpected libvirt error while detaching device %s from \u0027"}],"source_content_type":"text/x-python","patch_set":26,"id":"6d7ec8df_8aab72b4","line":2506,"range":{"start_line":2500,"start_character":0,"end_line":2506,"end_character":74},"updated":"2021-04-15 12:10:12.000000000","message":"https://review.opendev.org/c/openstack/nova/+/785682/3/nova/virt/libvirt/guest.py - This would be added here btw.","commit_id":"c04e34c7292ee3da718d2fac9bd9b86f52deca84"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3b13f7a153a74def22ea17338c8c648215794523","unresolved":false,"context_lines":[{"line_number":2497,"context_line":"        \"\"\""},{"line_number":2498,"context_line":"        try:"},{"line_number":2499,"context_line":"            guest.detach_device(dev, persistent\u003dpersistent, live\u003dlive)"},{"line_number":2500,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":2501,"context_line":"            if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_DEVICE_MISSING:"},{"line_number":2502,"context_line":"                LOG.debug("},{"line_number":2503,"context_line":"                    \u0027Libvirt failed to detach device %s from instance %s \u0027"},{"line_number":2504,"context_line":"                    \u0027synchronously (persistent\u003d%s, live\u003d%s) with error: %s.\u0027,"},{"line_number":2505,"context_line":"                    device_name, instance_uuid, persistent, live, str(ex))"},{"line_number":2506,"context_line":"                raise exception.DeviceNotFound(device\u003ddevice_name) from ex"},{"line_number":2507,"context_line":""},{"line_number":2508,"context_line":"            LOG.warning("},{"line_number":2509,"context_line":"                \u0027Unexpected libvirt error while detaching device %s from \u0027"}],"source_content_type":"text/x-python","patch_set":26,"id":"975b0a73_27680c98","line":2506,"range":{"start_line":2500,"start_character":0,"end_line":2506,"end_character":74},"in_reply_to":"6d7ec8df_8aab72b4","updated":"2021-04-16 14:12:40.000000000","message":"So your fix states that we should retry if libvirt reports \"already in the process of unplug\". \n\nIn the new logic:\n\n* if this is the detach from the persistent domain then we never retry. Therefore we can never hit the case when a detach is already ongoing from this domain.\n\n* if this is the detach from the live domain then we can only hit the \"already in the process of unplug\" case if there was a previous detach attempt in the callers retry loop and there we timed out waiting for the libvirt event. Based on this and the fact that qemu reports that the detach is still ongoing, I think the code can assume that a libvirt event will be emitted eventually. Not because of the current detach attempt but because of the previous one. In case of a timeout in the previous attempt the caller stopped waiting for the event. So the code can return from here and the caller can start waiting for such event again. \n\nThis also shows an almost race condition in the live detach case:\n1) first detach attempt triggered the detach in libvirt and started waiting for the event\n2) the driver timed out waiting for the event\n3) the driver checked the domain and saw that the device is still attached so decided to retry\n4) libvirt sent the detach event but the driver ignored it as nobody waited for it\n5) the driver starts the next retry step and issues the detach to libvirt again. Then libvirt reports that the device is missing, the driver raises DeviceNotFound that the client ignores.\n\nSo I think I will simply return form here in case of \"already in the process of unplug\" is reported.","commit_id":"c04e34c7292ee3da718d2fac9bd9b86f52deca84"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5d35da74f80421c8099bc38d7a792f7c57b6e1ec","unresolved":true,"context_lines":[{"line_number":2500,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":2501,"context_line":"            code \u003d ex.get_error_code()"},{"line_number":2502,"context_line":"            msg \u003d ex.get_error_message()"},{"line_number":2503,"context_line":"            print(msg)"},{"line_number":2504,"context_line":"            if code \u003d\u003d libvirt.VIR_ERR_DEVICE_MISSING:"},{"line_number":2505,"context_line":"                LOG.debug("},{"line_number":2506,"context_line":"                    \u0027Libvirt failed to detach device %s from instance %s \u0027"}],"source_content_type":"text/x-python","patch_set":27,"id":"04f20ff9_d940c67a","line":2503,"range":{"start_line":2503,"start_character":12,"end_line":2503,"end_character":22},"updated":"2021-04-16 15:40:33.000000000","message":"wuh wuh","commit_id":"2fc32c7a9756b147b1b1ba205ef20bcfe2f20df1"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"36584cbb12560e33fc80687adaf552b930753ba5","unresolved":false,"context_lines":[{"line_number":2500,"context_line":"        except libvirt.libvirtError as ex:"},{"line_number":2501,"context_line":"            code \u003d ex.get_error_code()"},{"line_number":2502,"context_line":"            msg \u003d ex.get_error_message()"},{"line_number":2503,"context_line":"            print(msg)"},{"line_number":2504,"context_line":"            if code \u003d\u003d libvirt.VIR_ERR_DEVICE_MISSING:"},{"line_number":2505,"context_line":"                LOG.debug("},{"line_number":2506,"context_line":"                    \u0027Libvirt failed to detach device %s from instance %s \u0027"}],"source_content_type":"text/x-python","patch_set":27,"id":"594757b3_ae1d0246","line":2503,"range":{"start_line":2503,"start_character":12,"end_line":2503,"end_character":22},"in_reply_to":"04f20ff9_d940c67a","updated":"2021-04-18 08:27:45.000000000","message":"Done","commit_id":"2fc32c7a9756b147b1b1ba205ef20bcfe2f20df1"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0c22dd973414d3c7de018e52a494c9d57e22dc0","unresolved":false,"context_lines":[{"line_number":2510,"context_line":"            # NOTE(lyarwood): https://bugzilla.redhat.com/1878659"},{"line_number":2511,"context_line":"            # Ignore this known QEMU bug for the time being allowing"},{"line_number":2512,"context_line":"            # our retry logic to handle it."},{"line_number":2513,"context_line":"            # NOTE(gibi): This can only happen in case of detaching from the"},{"line_number":2514,"context_line":"            # live domain as we never retry a detach from the persistent"},{"line_number":2515,"context_line":"            # domain so we cannot hit an already running detach there."},{"line_number":2516,"context_line":"            # In case of detaching from the live domain this error can happen"},{"line_number":2517,"context_line":"            # if the caller timed out during the first detach attempt then saw"},{"line_number":2518,"context_line":"            # that the device is still attached and therefore looped over and"},{"line_number":2519,"context_line":"            # and retried the detach. In this case the previous attempt stopped"},{"line_number":2520,"context_line":"            # waiting for the libvirt event. Also libvirt reports that there is"},{"line_number":2521,"context_line":"            # a detach ongoing, so the current attempt expects that a"},{"line_number":2522,"context_line":"            # libvirt event will be still emitted. Therefore we simply return"},{"line_number":2523,"context_line":"            # from here. Then the caller will wait for such event."},{"line_number":2524,"context_line":"            if (code \u003d\u003d libvirt.VIR_ERR_INTERNAL_ERROR and msg and"},{"line_number":2525,"context_line":"                    \u0027already in the process of unplug\u0027 in msg"},{"line_number":2526,"context_line":"            ):"},{"line_number":2527,"context_line":"                LOG.debug("},{"line_number":2528,"context_line":"                    \u0027Ignoring QEMU rejecting our request to detach device %s \u0027"},{"line_number":2529,"context_line":"                    \u0027from instance %s as it is caused by a previous request \u0027"},{"line_number":2530,"context_line":"                    \u0027still being in progress.\u0027, device_name, instance_uuid)"},{"line_number":2531,"context_line":"                return"},{"line_number":2532,"context_line":""},{"line_number":2533,"context_line":"            LOG.warning("},{"line_number":2534,"context_line":"                \u0027Unexpected libvirt error while detaching device %s from \u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"ca193773_9f5e6373","line":2531,"range":{"start_line":2513,"start_character":0,"end_line":2531,"end_character":22},"updated":"2021-04-19 10:45:51.000000000","message":"ACK thanks for adding this in!","commit_id":"e56cc4f439846558fc13298c2360d7cdd473cc89"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"23c4c9e1336259071424a15fe19fa95fc66cc005","unresolved":true,"context_lines":[{"line_number":314,"context_line":"        config.parse_str(self._domain.XMLDesc(0))"},{"line_number":315,"context_line":"        return config"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def get_disk(self, device, from_persistent_config\u003dFalse):"},{"line_number":318,"context_line":"        \"\"\"Returns the disk mounted at device"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        :returns LivirtConfigGuestDisk: mounted at device or None"}],"source_content_type":"text/x-python","patch_set":12,"id":"b03c6536_a11abe34","line":317,"range":{"start_line":317,"start_character":31,"end_line":317,"end_character":53},"updated":"2021-01-22 18:15:12.000000000","message":"these changes can be done is a separate patch","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aec07944bafbbe0f7f79ecf5176ac591c7d652e2","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        config.parse_str(self._domain.XMLDesc(0))"},{"line_number":315,"context_line":"        return config"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def get_disk(self, device, from_persistent_config\u003dFalse):"},{"line_number":318,"context_line":"        \"\"\"Returns the disk mounted at device"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        :returns LivirtConfigGuestDisk: mounted at device or None"}],"source_content_type":"text/x-python","patch_set":12,"id":"37740c29_78b2d6e2","line":317,"range":{"start_line":317,"start_character":31,"end_line":317,"end_character":53},"in_reply_to":"b03c6536_a11abe34","updated":"2021-01-25 17:32:19.000000000","message":"Done","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aec07944bafbbe0f7f79ecf5176ac591c7d652e2","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        config.parse_str(self._domain.XMLDesc(0))"},{"line_number":315,"context_line":"        return config"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def get_disk(self, device, from_persistent_config\u003dFalse):"},{"line_number":318,"context_line":"        \"\"\"Returns the disk mounted at device"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        :returns LivirtConfigGuestDisk: mounted at device or None"}],"source_content_type":"text/x-python","patch_set":12,"id":"6b280342_fab8a460","line":317,"range":{"start_line":317,"start_character":31,"end_line":317,"end_character":53},"in_reply_to":"b03c6536_a11abe34","updated":"2021-01-25 17:32:19.000000000","message":"Done","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"0046f7849fb880b71b0475a181d0301ff2b07b36","unresolved":true,"context_lines":[{"line_number":353,"context_line":"        \"\"\"Returns all devices for a guest"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        :param devtype: a LibvirtConfigGuestDevice subclass class"},{"line_number":356,"context_line":"        :param from_persistent_config: query the device from the persistent"},{"line_number":357,"context_line":"            domain instead of the live domain configuration"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"        :returns: a list of LibvirtConfigGuestDevice instances"}],"source_content_type":"text/x-python","patch_set":12,"id":"2c7f54f9_d084fdaa","line":356,"updated":"2021-01-25 09:41:49.000000000","message":"Nit: You might want to add in the brackets: \"... persistent domain (i.e. inactive XML configuration that\u0027ll be used on next start of the domain)\"\n\n(Not worth a respin; add it only if you\u0027re going to resping for other reasons.)","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"aec07944bafbbe0f7f79ecf5176ac591c7d652e2","unresolved":false,"context_lines":[{"line_number":353,"context_line":"        \"\"\"Returns all devices for a guest"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        :param devtype: a LibvirtConfigGuestDevice subclass class"},{"line_number":356,"context_line":"        :param from_persistent_config: query the device from the persistent"},{"line_number":357,"context_line":"            domain instead of the live domain configuration"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":"        :returns: a list of LibvirtConfigGuestDevice instances"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba0d5c10_e31add13","line":356,"in_reply_to":"2c7f54f9_d084fdaa","updated":"2021-01-25 17:32:19.000000000","message":"Done","commit_id":"e5073577e40590e50dab17c9a3406a2aab5e2c26"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8b6d09d9e4996778e36058625694ed1a9e497f05","unresolved":true,"context_lines":[{"line_number":358,"context_line":"        :returns: a list of LibvirtConfigGuestDevice instances"},{"line_number":359,"context_line":"        \"\"\""},{"line_number":360,"context_line":"        flags \u003d 0"},{"line_number":361,"context_line":"        flags |\u003d libvirt.VIR_DOMAIN_XML_INACTIVE"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"        flags \u003d 0"},{"line_number":364,"context_line":"        if from_persistent_config:"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a3cb1d9_757e109f","line":361,"updated":"2021-01-26 17:17:11.000000000","message":"bad rebase drop","commit_id":"60142cb7777506a81e4712d1cbfd7cb08cb77762"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"44c31e7cc4b727b80772916cfbec51188d855815","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        :returns: a list of LibvirtConfigGuestDevice instances"},{"line_number":359,"context_line":"        \"\"\""},{"line_number":360,"context_line":"        flags \u003d 0"},{"line_number":361,"context_line":"        flags |\u003d libvirt.VIR_DOMAIN_XML_INACTIVE"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"        flags \u003d 0"},{"line_number":364,"context_line":"        if from_persistent_config:"}],"source_content_type":"text/x-python","patch_set":16,"id":"a707e169_6e4bc9f4","line":361,"in_reply_to":"5a3cb1d9_757e109f","updated":"2021-01-29 12:45:59.000000000","message":"Done","commit_id":"60142cb7777506a81e4712d1cbfd7cb08cb77762"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b0d12dfb40a772b1d62fe6324d23bd505679c4ed","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                devs.append(dev)"},{"line_number":378,"context_line":"        return devs"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def detach_device_with_retry(self, get_device_conf_func, device, live,"},{"line_number":381,"context_line":"                                 max_retry_count\u003d7, inc_sleep_time\u003d10,"},{"line_number":382,"context_line":"                                 max_sleep_time\u003d60,"},{"line_number":383,"context_line":"                                 alternative_device_name\u003dNone):"},{"line_number":384,"context_line":"        \"\"\"Detaches a device from the guest. After the initial detach request,"},{"line_number":385,"context_line":"        a function is returned which can be used to ensure the device is"},{"line_number":386,"context_line":"        successfully removed from the guest domain (retrying the removal as"},{"line_number":387,"context_line":"        necessary)."},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        :param get_device_conf_func: function which takes device as a parameter"},{"line_number":390,"context_line":"                                     and returns the configuration for device"},{"line_number":391,"context_line":"        :param device: device to detach"},{"line_number":392,"context_line":"        :param live: bool to indicate whether it affects the guest in running"},{"line_number":393,"context_line":"                     state"},{"line_number":394,"context_line":"        :param max_retry_count: number of times the returned function will"},{"line_number":395,"context_line":"                                retry a detach before failing"},{"line_number":396,"context_line":"        :param inc_sleep_time: incremental time to sleep in seconds between"},{"line_number":397,"context_line":"                               detach retries"},{"line_number":398,"context_line":"        :param max_sleep_time: max sleep time in seconds beyond which the sleep"},{"line_number":399,"context_line":"                               time will not be incremented using param"},{"line_number":400,"context_line":"                               inc_sleep_time. On reaching this threshold,"},{"line_number":401,"context_line":"                               max_sleep_time will be used as the sleep time."},{"line_number":402,"context_line":"        :param alternative_device_name: This is an alternative identifier for"},{"line_number":403,"context_line":"            the device if device is not an ID, used solely for error messages."},{"line_number":404,"context_line":"        \"\"\""},{"line_number":405,"context_line":"        alternative_device_name \u003d alternative_device_name or device"},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"        def _try_detach_device(conf, persistent\u003dFalse, live\u003dFalse):"},{"line_number":408,"context_line":"            # Raise DeviceNotFound if the device isn\u0027t found during detach"},{"line_number":409,"context_line":"            try:"},{"line_number":410,"context_line":"                self.detach_device(conf, persistent\u003dpersistent, live\u003dlive)"},{"line_number":411,"context_line":"                if get_device_conf_func(device) is None:"},{"line_number":412,"context_line":"                    LOG.debug(\u0027Successfully detached device %s from guest. \u0027"},{"line_number":413,"context_line":"                              \u0027Persistent? %s. Live? %s\u0027,"},{"line_number":414,"context_line":"                              device, persistent, live)"},{"line_number":415,"context_line":""},{"line_number":416,"context_line":"            except libvirt.libvirtError as ex:"},{"line_number":417,"context_line":"                with excutils.save_and_reraise_exception(reraise\u003dFalse) as ctx:"},{"line_number":418,"context_line":"                    if ex.get_error_code() \u003d\u003d libvirt.VIR_ERR_DEVICE_MISSING:"},{"line_number":419,"context_line":"                        raise exception.DeviceNotFound("},{"line_number":420,"context_line":"                            device\u003dalternative_device_name)"},{"line_number":421,"context_line":"                    # Re-raise the original exception if we\u0027re not raising"},{"line_number":422,"context_line":"                    # DeviceNotFound instead. This will avoid logging of a"},{"line_number":423,"context_line":"                    # \"Original exception being dropped\" traceback."},{"line_number":424,"context_line":"                    ctx.reraise \u003d True"},{"line_number":425,"context_line":""},{"line_number":426,"context_line":"        conf \u003d get_device_conf_func(device)"},{"line_number":427,"context_line":"        if conf is None:"},{"line_number":428,"context_line":"            raise exception.DeviceNotFound(device\u003dalternative_device_name)"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        persistent \u003d self.has_persistent_configuration()"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        LOG.debug(\u0027Attempting initial detach for device %s\u0027,"},{"line_number":433,"context_line":"                  alternative_device_name)"},{"line_number":434,"context_line":"        try:"},{"line_number":435,"context_line":"            _try_detach_device(conf, persistent, live)"},{"line_number":436,"context_line":"        except exception.DeviceNotFound:"},{"line_number":437,"context_line":"            # NOTE(melwitt): There are effectively two configs for an instance."},{"line_number":438,"context_line":"            # The persistent config (affects instance upon next boot) and the"},{"line_number":439,"context_line":"            # live config (affects running instance). When we detach a device,"},{"line_number":440,"context_line":"            # we need to detach it from both configs if the instance has a"},{"line_number":441,"context_line":"            # persistent config and a live config. If we tried to detach the"},{"line_number":442,"context_line":"            # device with persistent\u003dTrue and live\u003dTrue and it was not found,"},{"line_number":443,"context_line":"            # we should still try to detach from the live config, so continue."},{"line_number":444,"context_line":"            if persistent and live:"},{"line_number":445,"context_line":"                pass"},{"line_number":446,"context_line":"            else:"},{"line_number":447,"context_line":"                raise"},{"line_number":448,"context_line":"        LOG.debug(\u0027Start retrying detach until device %s is gone.\u0027,"},{"line_number":449,"context_line":"                  alternative_device_name)"},{"line_number":450,"context_line":""},{"line_number":451,"context_line":"        @loopingcall.RetryDecorator(max_retry_count\u003dmax_retry_count,"},{"line_number":452,"context_line":"                                    inc_sleep_time\u003dinc_sleep_time,"},{"line_number":453,"context_line":"                                    max_sleep_time\u003dmax_sleep_time,"},{"line_number":454,"context_line":"                                    exceptions\u003dexception.DeviceDetachFailed)"},{"line_number":455,"context_line":"        def _do_wait_and_retry_detach():"},{"line_number":456,"context_line":"            config \u003d get_device_conf_func(device)"},{"line_number":457,"context_line":"            if config is not None:"},{"line_number":458,"context_line":"                # Device is already detached from persistent config"},{"line_number":459,"context_line":"                # and only the live config needs to be updated."},{"line_number":460,"context_line":"                _try_detach_device(config, persistent\u003dFalse, live\u003dlive)"},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"                reason \u003d _(\"Unable to detach the device from the live config.\")"},{"line_number":463,"context_line":"                raise exception.DeviceDetachFailed("},{"line_number":464,"context_line":"                    device\u003dalternative_device_name, reason\u003dreason)"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"        return _do_wait_and_retry_detach"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"    def detach_device(self, conf, persistent\u003dFalse, live\u003dFalse):"},{"line_number":469,"context_line":"        \"\"\"Detaches device to the guest."},{"line_number":470,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"933e20e5_16a8aff9","side":"PARENT","line":467,"range":{"start_line":380,"start_character":1,"end_line":467,"end_character":0},"updated":"2021-02-16 15:55:48.000000000","message":"🎉","commit_id":"624db21de53f740a5a97b41d349c19165b07b44b"}],"releasenotes/notes/libvirt-event-based-device-detach-23ac037004d753b1.yaml":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a571262d6792828b3125b10dcf8e5a6f5498872f","unresolved":true,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    To fix `device detach issues`__ in the libvirt driver the detach logic has"},{"line_number":5,"context_line":"    been changed from a sleep based retry loop to waiting for libvir domain"},{"line_number":6,"context_line":"    events. During this change we also introduced two new config options to"},{"line_number":7,"context_line":"    allow fine tuning the retry logic. For details see the description of the"},{"line_number":8,"context_line":"    new ``[libvirt]device_detach_attempts`` and"}],"source_content_type":"text/x-yaml","patch_set":26,"id":"ef32d07a_b9a91615","line":5,"range":{"start_line":5,"start_character":62,"end_line":5,"end_character":68},"updated":"2021-04-15 11:26:03.000000000","message":"libvirt","commit_id":"c04e34c7292ee3da718d2fac9bd9b86f52deca84"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3b13f7a153a74def22ea17338c8c648215794523","unresolved":false,"context_lines":[{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    To fix `device detach issues`__ in the libvirt driver the detach logic has"},{"line_number":5,"context_line":"    been changed from a sleep based retry loop to waiting for libvir domain"},{"line_number":6,"context_line":"    events. During this change we also introduced two new config options to"},{"line_number":7,"context_line":"    allow fine tuning the retry logic. For details see the description of the"},{"line_number":8,"context_line":"    new ``[libvirt]device_detach_attempts`` and"}],"source_content_type":"text/x-yaml","patch_set":26,"id":"d9e03f3c_0220382d","line":5,"range":{"start_line":5,"start_character":62,"end_line":5,"end_character":68},"in_reply_to":"ef32d07a_b9a91615","updated":"2021-04-16 14:12:40.000000000","message":"Done","commit_id":"c04e34c7292ee3da718d2fac9bd9b86f52deca84"}]}