)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e950818c6b0ba2754a711ac52500e4ea176dac5","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Libvirt driver will respond with a meaningful error on any"},{"line_number":10,"context_line":"command, when the connection to the libvirt service is broken."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Fixes bug #1240344"},{"line_number":13,"context_line":"Change-Id: I823fefaffcf969ecf9eb853ac99dc18cef92d137"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATH%2F%2FQdU%3D","line":12,"updated":"2013-10-18 22:58:34.000000000","message":"This should be:\n\n Closes-bug: #1240344","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":8076,"name":"Vladik Romanovsky","email":"vladik.romanovsky@gmail.com","username":"vladik"},"change_message_id":"ea3cd6692319b90214e0b86653db0afbab78b231","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Libvirt driver will respond with a meaningful error on any"},{"line_number":10,"context_line":"command, when the connection to the libvirt service is broken."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Fixes bug #1240344"},{"line_number":13,"context_line":"Change-Id: I823fefaffcf969ecf9eb853ac99dc18cef92d137"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATH%2F%2FQYY%3D","line":12,"in_reply_to":"AAAATH%2F%2FQdU%3D","updated":"2013-10-18 23:55:50.000000000","message":"Done","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"}],"nova/exception.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e950818c6b0ba2754a711ac52500e4ea176dac5","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    msg_fmt \u003d _(\"Insufficient compute resources.\")"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"class HypervisorUnavailable(NovaException):"},{"line_number":392,"context_line":"    msg_fmt \u003d _(\"Connection to the hypervisor is broken on host: %(host)s\")"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def __init__(self, host\u003dNone, error_code\u003d1, error_domain\u003d13):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FQdw%3D","line":391,"updated":"2013-10-18 22:58:34.000000000","message":"AFAICT, this is only used internally to the libvirt driver, so it can and should be declared there instead of in this global exceptions file.","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba800aa130e2e940d355312ebe0ebb853a1204dc","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    msg_fmt \u003d _(\"Insufficient compute resources.\")"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"class HypervisorUnavailable(NovaException):"},{"line_number":392,"context_line":"    msg_fmt \u003d _(\"Connection to the hypervisor is broken on host: %(host)s\")"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def __init__(self, host\u003dNone, error_code\u003d1, error_domain\u003d13):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FLhg%3D","line":391,"in_reply_to":"AAAATH%2F%2FQaw%3D","updated":"2013-10-21 21:18:42.000000000","message":"Well, my comment is that it *isn\u0027t* used anywhere else in this patch. If it\u0027s going to be used elsewhere, then that\u0027s fine.","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":8076,"name":"Vladik Romanovsky","email":"vladik.romanovsky@gmail.com","username":"vladik"},"change_message_id":"ea3cd6692319b90214e0b86653db0afbab78b231","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    msg_fmt \u003d _(\"Insufficient compute resources.\")"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"class HypervisorUnavailable(NovaException):"},{"line_number":392,"context_line":"    msg_fmt \u003d _(\"Connection to the hypervisor is broken on host: %(host)s\")"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def __init__(self, host\u003dNone, error_code\u003d1, error_domain\u003d13):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FQaw%3D","line":391,"in_reply_to":"AAAATH%2F%2FQdw%3D","updated":"2013-10-18 23:55:50.000000000","message":"Hi,\nI think that this can be used by other drivers as well.. why wouldn\u0027t it?\nAnd be used in many general case, such as migration.\nFor example in the dependant patch: \nhttps://review.openstack.org/#/c/50629/4/nova/conductor/manager.py","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e950818c6b0ba2754a711ac52500e4ea176dac5","unresolved":false,"context_lines":[{"line_number":391,"context_line":"class HypervisorUnavailable(NovaException):"},{"line_number":392,"context_line":"    msg_fmt \u003d _(\"Connection to the hypervisor is broken on host: %(host)s\")"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def __init__(self, host\u003dNone, error_code\u003d1, error_domain\u003d13):"},{"line_number":395,"context_line":"        self.error_code \u003d error_code"},{"line_number":396,"context_line":"        self.error_domain \u003d error_domain"},{"line_number":397,"context_line":"        super(HypervisorUnavailable, self).__init__(self.msg_fmt %"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FQeU%3D","line":394,"updated":"2013-10-18 22:58:34.000000000","message":"What are these magic numbers?","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba800aa130e2e940d355312ebe0ebb853a1204dc","unresolved":false,"context_lines":[{"line_number":391,"context_line":"class HypervisorUnavailable(NovaException):"},{"line_number":392,"context_line":"    msg_fmt \u003d _(\"Connection to the hypervisor is broken on host: %(host)s\")"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def __init__(self, host\u003dNone, error_code\u003d1, error_domain\u003d13):"},{"line_number":395,"context_line":"        self.error_code \u003d error_code"},{"line_number":396,"context_line":"        self.error_domain \u003d error_domain"},{"line_number":397,"context_line":"        super(HypervisorUnavailable, self).__init__(self.msg_fmt %"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FLhQ%3D","line":394,"in_reply_to":"AAAATH%2F%2FQZ8%3D","updated":"2013-10-21 21:18:42.000000000","message":"My complaint is that they are random magic numbers and should be constants, yes. As it turns out, these are libvirt-specific constants, and thus inappropriate defaults to be in the general exceptions file.\n\nIMHO, this general exception should have none of it, and you can subclass it in libvirt/driver to include these extra bits of information that you\u0027re going to use only in there. Leaking hypervisor-specific error codes outside the driver doesn\u0027t really make much sense to me.","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":8076,"name":"Vladik Romanovsky","email":"vladik.romanovsky@gmail.com","username":"vladik"},"change_message_id":"ea3cd6692319b90214e0b86653db0afbab78b231","unresolved":false,"context_lines":[{"line_number":391,"context_line":"class HypervisorUnavailable(NovaException):"},{"line_number":392,"context_line":"    msg_fmt \u003d _(\"Connection to the hypervisor is broken on host: %(host)s\")"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def __init__(self, host\u003dNone, error_code\u003d1, error_domain\u003d13):"},{"line_number":395,"context_line":"        self.error_code \u003d error_code"},{"line_number":396,"context_line":"        self.error_domain \u003d error_domain"},{"line_number":397,"context_line":"        super(HypervisorUnavailable, self).__init__(self.msg_fmt %"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FQZ8%3D","line":394,"in_reply_to":"AAAATH%2F%2FQeU%3D","updated":"2013-10-18 23:55:50.000000000","message":"error_code\u003d1 is usually exit_failure.. 13 came from libvirt error_domain\nI thought that there are good defaults, but I can use something else or did you mean that it should be some constant?","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"4e950818c6b0ba2754a711ac52500e4ea176dac5","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        raise exception.HypervisorUnavailable("},{"line_number":315,"context_line":"                host\u003dself.hostname,"},{"line_number":316,"context_line":"                error_code\u003dlibvirt.VIR_ERR_INTERNAL_ERROR,"},{"line_number":317,"context_line":"                error_domain\u003dlibvirt.VIR_FROM_REMOTE)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FQds%3D","line":317,"updated":"2013-10-18 22:58:34.000000000","message":"While sneaky, I feel like this is a bit of a hack to fix the problem as an afterthought. I would really like to see a more integrated approach at some point.","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ba800aa130e2e940d355312ebe0ebb853a1204dc","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        raise exception.HypervisorUnavailable("},{"line_number":315,"context_line":"                host\u003dself.hostname,"},{"line_number":316,"context_line":"                error_code\u003dlibvirt.VIR_ERR_INTERNAL_ERROR,"},{"line_number":317,"context_line":"                error_domain\u003dlibvirt.VIR_FROM_REMOTE)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FLgc%3D","line":317,"in_reply_to":"AAAATH%2F%2FQVo%3D","updated":"2013-10-21 21:18:42.000000000","message":"I mean I don\u0027t like the entire approach of replacing the self._wrapped_conn with something that is not a LibvirtConnection, nor even inherits from it. I understand that this is the least amount of code required to fix the problem, but that is kindof my point. I think being more careful in the driver where we assume self._wrapped_connection is always live would be better.\n\nWe\u0027re already wrapping it with a property, right? Why not just raise the exception from _get_connection() instead of setting self._wrapped_conn to this fake objects?","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":8076,"name":"Vladik Romanovsky","email":"vladik.romanovsky@gmail.com","username":"vladik"},"change_message_id":"9f68db917f45d3348b48aff6c5fd191ab172a3e4","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        raise exception.HypervisorUnavailable("},{"line_number":315,"context_line":"                host\u003dself.hostname,"},{"line_number":316,"context_line":"                error_code\u003dlibvirt.VIR_ERR_INTERNAL_ERROR,"},{"line_number":317,"context_line":"                error_domain\u003dlibvirt.VIR_FROM_REMOTE)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FQVo%3D","line":317,"in_reply_to":"AAAATH%2F%2FQYg%3D","updated":"2013-10-19 01:13:57.000000000","message":"Regarding the LibvirtConnectionBroken class.. I could create a libvirt connection class and register all the methods in the driver with a wrapper that would throw and exception when libvirt is not connected. However, I have seen that previous attempts to implement that failed.\n\nAlso, its always possible to use a decorator for all the commands, but I don\u0027t see why is it better than this class, which is being set only when libvirt reports that the connection is broken. \n\nI think that, this approach has minimum code changes and fixes the problem.","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"},{"author":{"_account_id":8076,"name":"Vladik Romanovsky","email":"vladik.romanovsky@gmail.com","username":"vladik"},"change_message_id":"ea3cd6692319b90214e0b86653db0afbab78b231","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        raise exception.HypervisorUnavailable("},{"line_number":315,"context_line":"                host\u003dself.hostname,"},{"line_number":316,"context_line":"                error_code\u003dlibvirt.VIR_ERR_INTERNAL_ERROR,"},{"line_number":317,"context_line":"                error_domain\u003dlibvirt.VIR_FROM_REMOTE)"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"class LibvirtDriver(driver.ComputeDriver):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATH%2F%2FQYg%3D","line":317,"in_reply_to":"AAAATH%2F%2FQds%3D","updated":"2013-10-18 23:55:50.000000000","message":"Are you referring to the whole LibvirtConnectionBroken class or to the defaults in error_code and error_domain?\n\nIf its the defaults, will propagating the error_code and error_domain from _close_callback satisfy your concern?\n\nIf its about the class, could you please elaborate?","commit_id":"a0b6b8d6981b8aa7b657059f3d136643f1d690bc"}]}