)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7b5f49b4a515f2c547788b22728addcf0ac5d5eb","unresolved":false,"context_lines":[{"line_number":10,"context_line":", an InternalError will be thrown"},{"line_number":11,"context_line":"and the VM state will be set to ERROR."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Actually we did nothing to VMs."},{"line_number":14,"context_line":"Also, we\u0027ve already catched this exception in compute.manager."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch checks the return code from libvirt"},{"line_number":17,"context_line":"and throws NotImplementedError if qga is not responsive."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_4c0be728","line":14,"range":{"start_line":13,"start_character":0,"end_line":14,"end_character":62},"updated":"2017-11-29 10:01:34.000000000","message":"I don\u0027t understand. Are you saying we catch the exception and we should not, or we do not catch the exception but we should?","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"},{"author":{"_account_id":19944,"name":"Chen Hanxiao","email":"chen_han_xiao@126.com","username":"chenhanxiao"},"change_message_id":"1a48284a076dd9d1e8bad342a0890d32e181ac48","unresolved":false,"context_lines":[{"line_number":10,"context_line":", an InternalError will be thrown"},{"line_number":11,"context_line":"and the VM state will be set to ERROR."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Actually we did nothing to VMs."},{"line_number":14,"context_line":"Also, we\u0027ve already catched this exception in compute.manager."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch checks the return code from libvirt"},{"line_number":17,"context_line":"and throws NotImplementedError if qga is not responsive."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_6f3fbd86","line":14,"range":{"start_line":13,"start_character":0,"end_line":14,"end_character":62},"in_reply_to":"ff82abbf_4c0be728","updated":"2017-11-29 10:22:01.000000000","message":"As qga is not responsive, we did nothing inside VM.\nLibvirt will report a VIR_ERR_AGENT_UNRESPONSIVE error.\nWe should catch this error.","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7b5f49b4a515f2c547788b22728addcf0ac5d5eb","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Actually we did nothing to VMs."},{"line_number":14,"context_line":"Also, we\u0027ve already catched this exception in compute.manager."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch checks the return code from libvirt"},{"line_number":17,"context_line":"and throws NotImplementedError if qga is not responsive."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I4603711c435e15593e4a979906a12b560737bd77"},{"line_number":20,"context_line":"Signed-off-by: Chen Hanxiao \u003cchenhx@certusnet.com.cn\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_6c126b64","line":17,"range":{"start_line":16,"start_character":0,"end_line":17,"end_character":56},"updated":"2017-11-29 10:01:34.000000000","message":"Could you provide a little more context. Why is it necessary to special case this instance? Surely an error is an error? Also, why is NotImplementedError a suitable exception type to throw?","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"},{"author":{"_account_id":19944,"name":"Chen Hanxiao","email":"chen_han_xiao@126.com","username":"chenhanxiao"},"change_message_id":"1a48284a076dd9d1e8bad342a0890d32e181ac48","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Actually we did nothing to VMs."},{"line_number":14,"context_line":"Also, we\u0027ve already catched this exception in compute.manager."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch checks the return code from libvirt"},{"line_number":17,"context_line":"and throws NotImplementedError if qga is not responsive."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: I4603711c435e15593e4a979906a12b560737bd77"},{"line_number":20,"context_line":"Signed-off-by: Chen Hanxiao \u003cchenhx@certusnet.com.cn\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ff82abbf_cfc12956","line":17,"range":{"start_line":16,"start_character":0,"end_line":17,"end_character":56},"in_reply_to":"ff82abbf_6c126b64","updated":"2017-11-29 10:22:01.000000000","message":"If we want to change password by qemu guest agent, we\u0027ll get:\n1) success\n2) qga not response to our command.\n   Such as qga is not installed, crashed, halted, etc. Indeed qga did nothing to VMs, so NotImplementedError is a suitable case.\n3) qga got our command but failed to exec it\n  Such as lack of permission, selinux policy, change user \u0027root\u0027 in windows, etc. At this time, we may get other error from libvirt, indicated that we did something inside VM, but failed.\nThen set vm_state to some suitable value is needed as we\u0027ve already done.","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"7b5f49b4a515f2c547788b22728addcf0ac5d5eb","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1834,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1835,"context_line":"                raise NotImplementedError()"},{"line_number":1836,"context_line":"            else:"},{"line_number":1837,"context_line":"                err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1838,"context_line":"                msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"},{"line_number":1839,"context_line":"                    \u0027\"%(user)s\": [Error Code %(error_code)s] %(ex)s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_8c3c8fe1","line":1836,"range":{"start_line":1836,"start_character":12,"end_line":1836,"end_character":17},"updated":"2017-11-29 10:01:34.000000000","message":"You don\u0027t need this, since the raise above is equivalent to a return. Can you remove it?","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"},{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"4c63fcc349f0215464c03c3edf6d5d6cac2e7027","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1834,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1835,"context_line":"                raise NotImplementedError()"},{"line_number":1836,"context_line":"            else:"},{"line_number":1837,"context_line":"                err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1838,"context_line":"                msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"},{"line_number":1839,"context_line":"                    \u0027\"%(user)s\": [Error Code %(error_code)s] %(ex)s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_e03bc134","line":1836,"range":{"start_line":1836,"start_character":12,"end_line":1836,"end_character":17},"in_reply_to":"ff82abbf_4f719975","updated":"2017-11-30 08:38:39.000000000","message":"I think what he means is keep the if, delete the else.","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"},{"author":{"_account_id":19944,"name":"Chen Hanxiao","email":"chen_han_xiao@126.com","username":"chenhanxiao"},"change_message_id":"1a48284a076dd9d1e8bad342a0890d32e181ac48","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1834,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1835,"context_line":"                raise NotImplementedError()"},{"line_number":1836,"context_line":"            else:"},{"line_number":1837,"context_line":"                err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1838,"context_line":"                msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"},{"line_number":1839,"context_line":"                    \u0027\"%(user)s\": [Error Code %(error_code)s] %(ex)s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_4f719975","line":1836,"range":{"start_line":1836,"start_character":12,"end_line":1836,"end_character":17},"in_reply_to":"ff82abbf_8c3c8fe1","updated":"2017-11-29 10:22:01.000000000","message":"I think we need this \u0027else\u0027.\nLibvirt will throw other errors, such as qga failed to change password due to perission/wrong usernames...\nWe need to check the error code.","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"},{"author":{"_account_id":19944,"name":"Chen Hanxiao","email":"chen_han_xiao@126.com","username":"chenhanxiao"},"change_message_id":"e858f6c7f1e82134d5c88ac1dbb33143beb37767","unresolved":false,"context_lines":[{"line_number":1833,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1834,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1835,"context_line":"                raise NotImplementedError()"},{"line_number":1836,"context_line":"            else:"},{"line_number":1837,"context_line":"                err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1838,"context_line":"                msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"},{"line_number":1839,"context_line":"                    \u0027\"%(user)s\": [Error Code %(error_code)s] %(ex)s\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ff82abbf_62825655","line":1836,"range":{"start_line":1836,"start_character":12,"end_line":1836,"end_character":17},"in_reply_to":"ff82abbf_e03bc134","updated":"2017-11-30 09:00:41.000000000","message":"Ahh, thanks for the clarification.\n@Stephen Finucane\nI misunderstand your comments. Will be fixed in v2.","commit_id":"6c964d4c8fe92ae28661c6994cca4a77e2766a3e"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"1b1a07582a933a9c6ef9eeb0ba653e582745847f","unresolved":false,"context_lines":[{"line_number":1835,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_AGENT_UNRESPONSIVE:"},{"line_number":1836,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1837,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1838,"context_line":"                raise NotImplementedError()"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"            err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1841,"context_line":"            msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_02deffd8","line":1838,"range":{"start_line":1838,"start_character":22,"end_line":1838,"end_character":41},"updated":"2017-12-12 03:39:05.000000000","message":"not sure this is correct as it will raise \n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3447\n\nso, at least I think we need update the warning and exception message as well","commit_id":"c32c6eb2cdfbcf3794ed16ba32c79ded2fba3e30"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"54e00543ffac537f4bdcc49646ef27f5b5f6192f","unresolved":false,"context_lines":[{"line_number":1835,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_AGENT_UNRESPONSIVE:"},{"line_number":1836,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1837,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1838,"context_line":"                raise NotImplementedError()"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"            err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1841,"context_line":"            msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_6a8a821b","line":1838,"range":{"start_line":1838,"start_character":22,"end_line":1838,"end_character":41},"in_reply_to":"df87a7cf_02deffd8","updated":"2017-12-19 18:05:34.000000000","message":"\u003e not sure this is correct as it will raise \n\u003e \n\u003e https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L3447\n\u003e \n\u003e so, at least I think we need update the warning and exception message as well\n\nAh, this is the bit I was missing when I first reviewed this. Per that code:\n\n\u003e set_admin_password is not implemented by this driver or *guest instance*\n\n(emphasis mine)\n\nThis would suggest this is indeed the correct path.","commit_id":"c32c6eb2cdfbcf3794ed16ba32c79ded2fba3e30"},{"author":{"_account_id":19944,"name":"Chen Hanxiao","email":"chen_han_xiao@126.com","username":"chenhanxiao"},"change_message_id":"206741f763d05fe68d700c04072102648d561a43","unresolved":false,"context_lines":[{"line_number":1835,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_AGENT_UNRESPONSIVE:"},{"line_number":1836,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1837,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1838,"context_line":"                raise NotImplementedError()"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"            err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1841,"context_line":"            msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_bdc17656","line":1838,"range":{"start_line":1838,"start_character":22,"end_line":1838,"end_character":41},"in_reply_to":"df87a7cf_02deffd8","updated":"2017-12-12 04:29:07.000000000","message":"As:\nhttps://github.com/libvirt/libvirt/blob/master/src/qemu/qemu_agent.c#L1136-L1138\nqga did nothing to guest and return, which is \u0027not implemented by this driver or guest instance\u0027\nSo NotImplementedError is suitable here.","commit_id":"c32c6eb2cdfbcf3794ed16ba32c79ded2fba3e30"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"f4d11680daa826de2d663cdcb133c089b5a38db9","unresolved":false,"context_lines":[{"line_number":1835,"context_line":"            if error_code \u003d\u003d libvirt.VIR_ERR_AGENT_UNRESPONSIVE:"},{"line_number":1836,"context_line":"                LOG.debug(\u0027Failed to set password: QEMU agent unresponsive\u0027,"},{"line_number":1837,"context_line":"                          instance_uuid\u003dinstance.uuid)"},{"line_number":1838,"context_line":"                raise NotImplementedError()"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"            err_msg \u003d encodeutils.exception_to_unicode(ex)"},{"line_number":1841,"context_line":"            msg \u003d (_(\u0027Error from libvirt while set password for username \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"df87a7cf_5d970a3b","line":1838,"range":{"start_line":1838,"start_character":22,"end_line":1838,"end_character":41},"in_reply_to":"df87a7cf_bdc17656","updated":"2017-12-12 04:39:23.000000000","message":"ok, I thought it\u0027s implemented but some internal error occurs.....anyway, not a big deal to distinguish","commit_id":"c32c6eb2cdfbcf3794ed16ba32c79ded2fba3e30"}]}
