)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0406be5d7114ab53f0477e46a9b8f8ab8180088b","unresolved":false,"context_lines":[{"line_number":4647,"context_line":"        is_shared_instance_path \u003d True"},{"line_number":4648,"context_line":"        if migrate_data:"},{"line_number":4649,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4650,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"},{"line_number":4651,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4652,"context_line":"                    \u0027is_shared_instance_path\u0027, True)"},{"line_number":4653,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_6fb55430","line":4650,"updated":"2014-05-02 17:15:39.000000000","message":"Should this default to False?","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"e2d830f3d9b1b74c2396d12a9afc45ef463ac3a3","unresolved":false,"context_lines":[{"line_number":4647,"context_line":"        is_shared_instance_path \u003d True"},{"line_number":4648,"context_line":"        if migrate_data:"},{"line_number":4649,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4650,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"},{"line_number":4651,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4652,"context_line":"                    \u0027is_shared_instance_path\u0027, True)"},{"line_number":4653,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_92b478d2","line":4650,"in_reply_to":"9ad9bd40_6fb55430","updated":"2014-05-02 18:17:25.000000000","message":"No, I don\u0027t think so. If migrate_data is not populated, we should fall on the safe side and default to the option that will _not_ cause the instance disks to be destroyed.","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0406be5d7114ab53f0477e46a9b8f8ab8180088b","unresolved":false,"context_lines":[{"line_number":4789,"context_line":"        is_shared_instance_path \u003d False"},{"line_number":4790,"context_line":"        if migrate_data:"},{"line_number":4791,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4792,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"},{"line_number":4793,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4794,"context_line":"                    \u0027is_shared_instance_path\u0027, True)"},{"line_number":4795,"context_line":"        if block_migration or ("}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_afd75c22","line":4792,"updated":"2014-05-02 17:15:39.000000000","message":"Same comment as above.. I believe this should default to False","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"a06a85510a225f3b4e51508e14b1ba04c0f86776","unresolved":false,"context_lines":[{"line_number":4789,"context_line":"        is_shared_instance_path \u003d False"},{"line_number":4790,"context_line":"        if migrate_data:"},{"line_number":4791,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4792,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"},{"line_number":4793,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4794,"context_line":"                    \u0027is_shared_instance_path\u0027, True)"},{"line_number":4795,"context_line":"        if block_migration or ("}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_a354d054","line":4792,"in_reply_to":"9ad9bd40_1245c8b2","updated":"2014-05-02 19:58:14.000000000","message":"Hmm, interesting, that\u0027s a fair point. Yeah, I think that makes sense, thx.","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"e2d830f3d9b1b74c2396d12a9afc45ef463ac3a3","unresolved":false,"context_lines":[{"line_number":4789,"context_line":"        is_shared_instance_path \u003d False"},{"line_number":4790,"context_line":"        if migrate_data:"},{"line_number":4791,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4792,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"},{"line_number":4793,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4794,"context_line":"                    \u0027is_shared_instance_path\u0027, True)"},{"line_number":4795,"context_line":"        if block_migration or ("}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_1245c8b2","line":4792,"in_reply_to":"9ad9bd40_afd75c22","updated":"2014-05-02 18:17:25.000000000","message":"On the contrary, I think I should change lines 4781 to default to True.\n\nAlternatively, I\u0027m considering whether the default for both flags should be (not block_migration): there\u0027s a pre-check for live migration that raises an exception if a block migration is attempted when either of these flags is True, so it should be relatively safe to assume that if block_migration is True than neither flag can be True.","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0406be5d7114ab53f0477e46a9b8f8ab8180088b","unresolved":false,"context_lines":[{"line_number":4826,"context_line":"                            context, instance)"},{"line_number":4827,"context_line":"        self.driver.rollback_live_migration_at_destination("},{"line_number":4828,"context_line":"                        context, instance, network_info, block_device_info,"},{"line_number":4829,"context_line":"                        destroy_disks, migrate_data)"},{"line_number":4830,"context_line":"        self._notify_about_instance_usage("},{"line_number":4831,"context_line":"                        context, instance, \"live_migration.rollback.dest.end\","},{"line_number":4832,"context_line":"                        network_info\u003dnetwork_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_8feff886","line":4829,"updated":"2014-05-02 17:15:39.000000000","message":"Please use kwarg notation (destroy_disks\u003ddestroy_disks) for  destroy_disks and migrate_data arguments. This makes it clearer that they are kwargs, not args, and helps to prevent subtle, difficult-to-debug argument ordering bugs that can pop up if a new arg is added to the called function.","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2069960bd89e79ad25daabc733f6dd6e59b8f241","unresolved":false,"context_lines":[{"line_number":4786,"context_line":"        # newly created instance-xxx dir on the destination as a part of its"},{"line_number":4787,"context_line":"        # rollback process"},{"line_number":4788,"context_line":"        is_shared_block_storage \u003d False"},{"line_number":4789,"context_line":"        is_shared_instance_path \u003d False"},{"line_number":4790,"context_line":"        if migrate_data:"},{"line_number":4791,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4792,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_43cb041e","line":4789,"updated":"2014-05-02 20:04:56.000000000","message":"Per your comment on the previous patch, perhaps you want to do:\n\n is_shared_block_storage \u003d is_shared_instance_path \u003d not block_migration\n\n?","commit_id":"92cd036c36cf3c3d6c46b225a2872db825b7f9bd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3f1af9ab9604a437200a2a908571a163d8c27961","unresolved":false,"context_lines":[{"line_number":4599,"context_line":"        do_cleanup \u003d block_migration or not is_shared_instance_path"},{"line_number":4600,"context_line":"        destroy_disks \u003d not is_shared_block_storage"},{"line_number":4601,"context_line":""},{"line_number":4602,"context_line":"        return (do_cleanup, destroy_disks)"},{"line_number":4603,"context_line":""},{"line_number":4604,"context_line":"    @wrap_exception()"},{"line_number":4605,"context_line":"    @wrap_instance_fault"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_be0cb5ee","line":4602,"updated":"2014-05-05 18:06:47.000000000","message":"nice little DRY up here, thx :)","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"bb1cd3243baa553f355ac6d7cccf29a1fccadabb","unresolved":false,"context_lines":[{"line_number":4598,"context_line":"                                   block_migration, migrate_data)"},{"line_number":4599,"context_line":""},{"line_number":4600,"context_line":"    def _live_migration_cleanup_flags(self, block_migration, migrate_data):"},{"line_number":4601,"context_line":"        \u0027\u0027\u0027Determine whether disks or intance path need to be cleaned up after"},{"line_number":4602,"context_line":"        live migration (at source on success, at destination on rollback)"},{"line_number":4603,"context_line":""},{"line_number":4604,"context_line":"        Block migration needs empty image at destination host before migration"}],"source_content_type":"text/x-python","patch_set":13,"id":"7adec928_a47f6d65","line":4601,"updated":"2014-05-23 05:48:21.000000000","message":"We use double quotes for doc strings. Also, instance.","commit_id":"aae89669a7d7a573c937042da00391b1c947cdc3"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"6f8d189f0f6813e13b02e93453e57ea2dad46d7d","unresolved":false,"context_lines":[{"line_number":562,"context_line":"class ComputeManager(manager.Manager):"},{"line_number":563,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    target \u003d messaging.Target(version\u003d\u00273.27\u0027)"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"    def __init__(self, compute_driver\u003dNone, *args, **kwargs):"},{"line_number":568,"context_line":"        \"\"\"Load configuration options and connect to the hypervisor.\"\"\""}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_633356ab","line":565,"updated":"2014-06-04 08:59:04.000000000","message":"We need to bump the version here as well?","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"56bf93d0d2a3c4a1265639bd128ea87f844b2cf5","unresolved":false,"context_lines":[{"line_number":562,"context_line":"class ComputeManager(manager.Manager):"},{"line_number":563,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    target \u003d messaging.Target(version\u003d\u00273.27\u0027)"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"    def __init__(self, compute_driver\u003dNone, *args, **kwargs):"},{"line_number":568,"context_line":"        \"\"\"Load configuration options and connect to the hypervisor.\"\"\""}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_6a65d41c","line":565,"in_reply_to":"1ae5cdf2_633356ab","updated":"2014-06-04 17:47:23.000000000","message":"We should.","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"02074d532be86681cbcc9c4c6f471ed33e4625bd","unresolved":false,"context_lines":[{"line_number":562,"context_line":"class ComputeManager(manager.Manager):"},{"line_number":563,"context_line":"    \"\"\"Manages the running instances from creation to destruction.\"\"\""},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    target \u003d messaging.Target(version\u003d\u00273.27\u0027)"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"    def __init__(self, compute_driver\u003dNone, *args, **kwargs):"},{"line_number":568,"context_line":"        \"\"\"Load configuration options and connect to the hypervisor.\"\"\""}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_8d96ba3d","line":565,"in_reply_to":"1ae5cdf2_6a65d41c","updated":"2014-06-06 18:37:52.000000000","message":"Done","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cb5dff1193b3217b8e01e15f9140aefad89bfc10","unresolved":false,"context_lines":[{"line_number":4665,"context_line":"            self.driver.cleanup(ctxt, instance, network_info)"},{"line_number":4666,"context_line":"        else:"},{"line_number":4667,"context_line":"            # self.driver.destroy() usually performs  vif unplugging"},{"line_number":4668,"context_line":"            # but we must do it explicitly here when block_migration"},{"line_number":4669,"context_line":"            # is false, as the network devices at the source must be"},{"line_number":4670,"context_line":"            # torn down"},{"line_number":4671,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_6d2f853f","side":"PARENT","line":4668,"updated":"2014-06-05 18:21:25.000000000","message":"heheh nice","commit_id":"422b9dc4ff9311c427b176a183f068f86244275f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f6ebcd93905c23207b0a62958e9c3a386bc05f07","unresolved":false,"context_lines":[{"line_number":4613,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4614,"context_line":"                    \u0027is_shared_block_storage\u0027, is_shared_block_storage)"},{"line_number":4615,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4616,"context_line":"                    \u0027is_shared_instance_path\u0027, is_shared_instance_path)"},{"line_number":4617,"context_line":""},{"line_number":4618,"context_line":"        # No instance booting at source host, but instance dir"},{"line_number":4619,"context_line":"        # must be deleted for preparing next block migration"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_f4ee758a","line":4616,"updated":"2014-06-10 17:19:58.000000000","message":"In nova/virt/driver.py the \u0027migrate_data\u0027 parameter is documented as being private to the virt driver implementation\n\n  \" :param migrate_data: implementation specific params\"\n\nSo, IMHO, it is very wrong for this common nova/compute/manager.py to be looking at its contents at all.\n\nIn general I feel like this entire method is really describing the logic / limitations of the current libvirt driver and is not going to necessarily work with VMWare / XenAPI.\n\nI think the logic to decide on cleanup should be entirely within the virt driver not the compute manager.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"dc1f67401e55df1f082c6a95e50ac459a47d4a80","unresolved":false,"context_lines":[{"line_number":4613,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4614,"context_line":"                    \u0027is_shared_block_storage\u0027, is_shared_block_storage)"},{"line_number":4615,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4616,"context_line":"                    \u0027is_shared_instance_path\u0027, is_shared_instance_path)"},{"line_number":4617,"context_line":""},{"line_number":4618,"context_line":"        # No instance booting at source host, but instance dir"},{"line_number":4619,"context_line":"        # must be deleted for preparing next block migration"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_dacee92c","line":4616,"in_reply_to":"1ae5cdf2_ca49d2b8","updated":"2014-06-11 08:20:15.000000000","message":"FWIW - I agree with Dan and Matt in principle completeley - whenever I\u0027ve done this kind of fixes, I\u0027ve started with making libvirt specific stuff go back down where they belong (See default_device_names in build_and_run_instance as a good example)) - so I\u0027ve complained about this patch and the way it is structured before.\n\nLooking at the mechanics of how code lands in Nova right now - IMHO this patch is on the right side of the horrible_code/useful_code line and as such I\u0027m more for landing it than not","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"c5c02794903b563c92641b38680bbb6d9b150944","unresolved":false,"context_lines":[{"line_number":4613,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4614,"context_line":"                    \u0027is_shared_block_storage\u0027, is_shared_block_storage)"},{"line_number":4615,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4616,"context_line":"                    \u0027is_shared_instance_path\u0027, is_shared_instance_path)"},{"line_number":4617,"context_line":""},{"line_number":4618,"context_line":"        # No instance booting at source host, but instance dir"},{"line_number":4619,"context_line":"        # must be deleted for preparing next block migration"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_54cc6d89","line":4616,"in_reply_to":"1ae5cdf2_dacee92c","updated":"2014-06-11 09:11:33.000000000","message":"IMHO if we\u0027re going to continue down this path, then we need to admit that \u0027migration_data\u0027 is not arbitrary driver specific data and actually document it properly as a standard part of these APIs, not a driver-free-for-all. Then we can make use of RPC versioning when we need to change the data keys that are included in this dict.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e020b0b741ae492193c082e8d91f8e3f7c744f3c","unresolved":false,"context_lines":[{"line_number":4613,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4614,"context_line":"                    \u0027is_shared_block_storage\u0027, is_shared_block_storage)"},{"line_number":4615,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4616,"context_line":"                    \u0027is_shared_instance_path\u0027, is_shared_instance_path)"},{"line_number":4617,"context_line":""},{"line_number":4618,"context_line":"        # No instance booting at source host, but instance dir"},{"line_number":4619,"context_line":"        # must be deleted for preparing next block migration"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_8a598a5d","line":4616,"in_reply_to":"1ae5cdf2_f4ee758a","updated":"2014-06-10 18:14:45.000000000","message":"Agree, can\u0027t we push something down to the virt driver interface and have the compute manager talk to the virt driver about what to do?\n\nHowever, there is precedent in the compute manager for checking values in the migrate_data dict in live/post/rollback migration methods here.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"7bc5747a23ae0e21b0dca7195e71d661ab4b629c","unresolved":false,"context_lines":[{"line_number":4613,"context_line":"            is_shared_block_storage \u003d migrate_data.get("},{"line_number":4614,"context_line":"                    \u0027is_shared_block_storage\u0027, is_shared_block_storage)"},{"line_number":4615,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4616,"context_line":"                    \u0027is_shared_instance_path\u0027, is_shared_instance_path)"},{"line_number":4617,"context_line":""},{"line_number":4618,"context_line":"        # No instance booting at source host, but instance dir"},{"line_number":4619,"context_line":"        # must be deleted for preparing next block migration"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_ca49d2b8","line":4616,"in_reply_to":"1ae5cdf2_f4ee758a","updated":"2014-06-10 19:09:47.000000000","message":"The fact that compute manager code for live migrations is poorly structured and biased with libvirt specific logic is something I\u0027m painfully aware of, as it is one of the primary reasons this bugfix has to touch so much code outside of libvirt driver.\n\nThat being said, it is a problem that already exists in the current version of this code, and it is not the problem this patch is trying to solve. That would require a blueprint, not a bug, and a lot more than just one commit to fix. This patch does not make this problem better or worse, but it does make this code path less broken. I don\u0027t think it\u0027s fair to reject it only because it doesn\u0027t solve *all* live migrations problems in one go.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0406be5d7114ab53f0477e46a9b8f8ab8180088b","unresolved":false,"context_lines":[{"line_number":751,"context_line":""},{"line_number":752,"context_line":"    def rollback_live_migration_at_destination(self, ctxt, instance, host,"},{"line_number":753,"context_line":"                                               destroy_disks\u003dTrue,"},{"line_number":754,"context_line":"                                               migrate_data\u003dNone):"},{"line_number":755,"context_line":"        # NOTE(russellb) Havana compat"},{"line_number":756,"context_line":"        version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"},{"line_number":757,"context_line":"        instance_p \u003d jsonutils.to_primitive(instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_2a3dfabb","line":754,"updated":"2014-05-02 17:15:39.000000000","message":"This will need a corresponding RPC version increment to account for the changed signature and payload.","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cfa0e817bb57a4c0993ccf7895c50d79a1a1d5f2","unresolved":false,"context_lines":[{"line_number":755,"context_line":"                                               destroy_disks\u003dTrue,"},{"line_number":756,"context_line":"                                               migrate_data\u003dNone):"},{"line_number":757,"context_line":"        # NOTE(russellb) Havana compat"},{"line_number":758,"context_line":"        version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"},{"line_number":759,"context_line":"        instance_p \u003d jsonutils.to_primitive(instance)"},{"line_number":760,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":761,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_32c4875e","line":758,"updated":"2014-05-02 21:05:51.000000000","message":"This needs to be:\n\n if self.client.can_send_version(\u00273.26\u0027):\n     version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)\t            \n else:\n     # NOTE(russellb) Havana compat\n     version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)","commit_id":"92cd036c36cf3c3d6c46b225a2872db825b7f9bd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"11cb58a648ab37e46580152e2205e6b2c7d24ecc","unresolved":false,"context_lines":[{"line_number":755,"context_line":"                                               destroy_disks\u003dTrue,"},{"line_number":756,"context_line":"                                               migrate_data\u003dNone):"},{"line_number":757,"context_line":"        # NOTE(russellb) Havana compat"},{"line_number":758,"context_line":"        version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"},{"line_number":759,"context_line":"        instance_p \u003d jsonutils.to_primitive(instance)"},{"line_number":760,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":761,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_357d51b2","line":758,"in_reply_to":"9ad9bd40_32c4875e","updated":"2014-05-02 22:11:36.000000000","message":"Sorry :( should have been:\n\n if self.client.can_send_version(\u00273.26\u0027):\n     version \u003d \u00273.26\u0027\n else:\n     # NOTE(russellb) Havana compat\n     version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)","commit_id":"92cd036c36cf3c3d6c46b225a2872db825b7f9bd"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3138ba8c6c87730807f0409d83bd989f6062725d","unresolved":false,"context_lines":[{"line_number":763,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":764,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":765,"context_line":"                   instance\u003dinstance_p, destroy_disks\u003ddestroy_disks,"},{"line_number":766,"context_line":"                   migrate_data\u003dmigrate_data)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    def run_instance(self, ctxt, instance, host, request_spec,"},{"line_number":769,"context_line":"                     filter_properties, requested_networks,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_3b398541","line":766,"updated":"2014-05-06 09:21:33.000000000","message":"This is not correct - you need to NOT SEND destroy_disks and migrate_data if version is lower than 3.26. Here - you send them in all cases but just change the version you claim to be sending.","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"807ca71e0dd89e7c570d3453ef66d54092a5f052","unresolved":false,"context_lines":[{"line_number":763,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":764,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":765,"context_line":"                   instance\u003dinstance_p, destroy_disks\u003ddestroy_disks,"},{"line_number":766,"context_line":"                   migrate_data\u003dmigrate_data)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    def run_instance(self, ctxt, instance, host, request_spec,"},{"line_number":769,"context_line":"                     filter_properties, requested_networks,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_dfe5d709","line":766,"in_reply_to":"9ad9bd40_3b398541","updated":"2014-05-06 16:46:35.000000000","message":"Nikola, I don\u0027t believe that is how the RPCAPI versioning works... the call to self.client.prepare() will pack the version in the context. If it received by a compute node that does not speak the 3.26 RPC API version, then the destroy_disks and migrate_data kwargs won\u0027t be considered and the compute node will behave according to version prior to 3.26.\n\nLooking in nova/compute/rpcapi.py, I don\u0027t see anywhere where the the code in rpcapi.py needs to have conditionals in it to change the call signature of the cctxt.cast() or cctxt.call().","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6917e96115441a79cd7fb6e899bacbfbbf0ca105","unresolved":false,"context_lines":[{"line_number":763,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":764,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":765,"context_line":"                   instance\u003dinstance_p, destroy_disks\u003ddestroy_disks,"},{"line_number":766,"context_line":"                   migrate_data\u003dmigrate_data)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    def run_instance(self, ctxt, instance, host, request_spec,"},{"line_number":769,"context_line":"                     filter_properties, requested_networks,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_c0ded86c","line":766,"in_reply_to":"9ad9bd40_602d0c95","updated":"2014-05-06 18:35:32.000000000","message":"Chatted with Dan on IRC about this. Sorry for my misunderstanding.\n\nDmitry, you will want to do something like this:\n\n version \u003d \u00273.26\u0027\n kw \u003d {\u0027instance\u0027: instance_p}\n if self.client.can_send_version(version):\n     kw.update({\u0027destroy_disks\u0027: destroy_disks,\n            \u0027migrate_data\u0027: migrate_data})\n ...\n return cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027, **kw)","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"d62eaf44baefd45ed8633ae187852f8d5ffef550","unresolved":false,"context_lines":[{"line_number":763,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":764,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":765,"context_line":"                   instance\u003dinstance_p, destroy_disks\u003ddestroy_disks,"},{"line_number":766,"context_line":"                   migrate_data\u003dmigrate_data)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    def run_instance(self, ctxt, instance, host, request_spec,"},{"line_number":769,"context_line":"                     filter_properties, requested_networks,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_00e360a4","line":766,"in_reply_to":"9ad9bd40_c0ded86c","updated":"2014-05-06 19:45:34.000000000","message":"Done","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7adfc89901585b35389852d1d69f355786d50132","unresolved":false,"context_lines":[{"line_number":763,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":764,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":765,"context_line":"                   instance\u003dinstance_p, destroy_disks\u003ddestroy_disks,"},{"line_number":766,"context_line":"                   migrate_data\u003dmigrate_data)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    def run_instance(self, ctxt, instance, host, request_spec,"},{"line_number":769,"context_line":"                     filter_properties, requested_networks,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_602d0c95","line":766,"in_reply_to":"9ad9bd40_dfe5d709","updated":"2014-05-06 18:28:48.000000000","message":"Nikola is correct here. If you pass an argument to a remote method that isn\u0027t expecting it, we\u0027ll still try to dispatch it with an unknown keyword arg and it will explode. There is no magic here to help us avoid that.","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cbdbf2e6be690a82794fc80b4fe3133919b63ae6","unresolved":false,"context_lines":[{"line_number":770,"context_line":"        if self.client.can_send_version(\u00273.27\u0027):"},{"line_number":771,"context_line":"            version \u003d \u00273.27\u0027"},{"line_number":772,"context_line":"            msg_kwargs.update(destroy_disks\u003ddestroy_disks,"},{"line_number":773,"context_line":"                              migrate_data\u003dmigrate_data)"},{"line_number":774,"context_line":""},{"line_number":775,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":776,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_8d810d4d","line":773,"updated":"2014-05-07 15:26:10.000000000","message":"This looks correct now - IMHO it is better to write this method higher-version to lower, and then drop (del msg_kwargs[\u0027new_stuff\u0027]) as it is easier to read, however - a minor nit really.","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"6f8d189f0f6813e13b02e93453e57ea2dad46d7d","unresolved":false,"context_lines":[{"line_number":774,"context_line":"            # NOTE(russellb) Havana compat"},{"line_number":775,"context_line":"            version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"},{"line_number":776,"context_line":"            instance \u003d jsonutils.to_primitive(instance)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":779,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":780,"context_line":"                   **msg_kwargs)"}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_151ba8ca","line":777,"updated":"2014-06-04 08:59:04.000000000","message":"Whitespace not related to the patch itself should be avoided. Consider if you have to rebase yet again.","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"02074d532be86681cbcc9c4c6f471ed33e4625bd","unresolved":false,"context_lines":[{"line_number":774,"context_line":"            # NOTE(russellb) Havana compat"},{"line_number":775,"context_line":"            version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"},{"line_number":776,"context_line":"            instance \u003d jsonutils.to_primitive(instance)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":779,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":780,"context_line":"                   **msg_kwargs)"}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_cdb362e7","line":777,"in_reply_to":"1ae5cdf2_151ba8ca","updated":"2014-06-06 18:37:52.000000000","message":"Arguably it is related: the patch makes the if statement above hairy enough to need more separation from surrounding code. But I\u0027m more interested in getting this code approved than in arguing whitespace :)","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"30ca680e82ffc3cb9651aa35baa161f9d52009bc","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        else:"},{"line_number":791,"context_line":"            # NOTE(russellb) Havana compat"},{"line_number":792,"context_line":"            version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"},{"line_number":793,"context_line":"            instance \u003d jsonutils.to_primitive(instance)"},{"line_number":794,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":795,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":796,"context_line":"                   **msg_kwargs)"}],"source_content_type":"text/x-python","patch_set":27,"id":"1ae5cdf2_f0544a55","line":793,"updated":"2014-06-20 16:45:13.000000000","message":"This needs to be adjusted as the instance here in the Havana compat case is now ignored. How about:\n\n    msg_kwargs[\u0027instance\u0027] \u003d jsonutils.to_primitive(instance)","commit_id":"257ac43260c6a9b841d9846d140f3a91ba1945f2"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"07e1b50232bb2ae88cadd8c2eb4359c847159cfd","unresolved":false,"context_lines":[{"line_number":790,"context_line":"        else:"},{"line_number":791,"context_line":"            # NOTE(russellb) Havana compat"},{"line_number":792,"context_line":"            version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"},{"line_number":793,"context_line":"            instance \u003d jsonutils.to_primitive(instance)"},{"line_number":794,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003dversion)"},{"line_number":795,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":796,"context_line":"                   **msg_kwargs)"}],"source_content_type":"text/x-python","patch_set":27,"id":"1ae5cdf2_618b0ec0","line":793,"in_reply_to":"1ae5cdf2_f0544a55","updated":"2014-06-20 18:23:14.000000000","message":"Nice catch! Done.","commit_id":"257ac43260c6a9b841d9846d140f3a91ba1945f2"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"d0302a6be1b9eaa44201a672a4cb7887829b8574","unresolved":false,"context_lines":[{"line_number":786,"context_line":"            msg_kwargs.update(destroy_disks\u003ddestroy_disks,"},{"line_number":787,"context_line":"                              migrate_data\u003dmigrate_data)"},{"line_number":788,"context_line":"        elif self.client.can_send_version(\u00273.26\u0027):"},{"line_number":789,"context_line":"            version \u003d \u00273.26\u0027"},{"line_number":790,"context_line":"        else:"},{"line_number":791,"context_line":"            # NOTE(russellb) Havana compat"},{"line_number":792,"context_line":"            version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.0\u0027)"}],"source_content_type":"text/x-python","patch_set":28,"id":"1ae5cdf2_8ba714a7","line":789,"updated":"2014-06-24 01:54:49.000000000","message":"I\u0027m confused here. Earlier you throw an exception if the destination node can\u0027t support version 3.30, but now we\u0027re willing to fall back to 3.26. Can you explain why that\u0027s ok?","commit_id":"adb3bebff02522063d8633a9e96ad5d99efd4d08"},{"author":{"_account_id":1812,"name":"p-draigbrady","email":"P@draigBrady.com","username":"p-draigbrady"},"change_message_id":"6015873c9afe01c2001718cacd475e0142b1a990","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                                           block_migration, disk_over_commit):"},{"line_number":379,"context_line":"        self._check_live_migration_api_version(destination)"},{"line_number":380,"context_line":"        # NOTE(russellb) Havana compat"},{"line_number":381,"context_line":"        version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.38\u0027)"},{"line_number":382,"context_line":"        cctxt \u003d self.client.prepare(server\u003ddestination, version\u003dversion)"},{"line_number":383,"context_line":"        return cctxt.call(ctxt, \u0027check_can_live_migrate_destination\u0027,"},{"line_number":384,"context_line":"                          instance\u003dinstance,"}],"source_content_type":"text/x-python","patch_set":29,"id":"1ae5cdf2_9427d5c6","line":381,"updated":"2014-06-26 03:01:51.000000000","message":"I\u0027m still a bit confused with the version checks TBH.\nAren\u0027t the _get_compat_version() checks redundant if we\u0027re already checking the client can send 3.30 in _check_live_migration_api_version() ?\n\nAlso how does this impact upgrades?\nI.E. for Icehouse -\u003e Juno, if you have new compute nodes prepared running Juno, will one now not be able to live migrate VMs from icehouse compute nodes?\nOr will that be supported once the API nodes are updated first?\n\nthanks","commit_id":"df99ff0135a2238d40daf32163860c07c360266e"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"b774120165726430b9b0ed1473ae33356e9799b2","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                                           block_migration, disk_over_commit):"},{"line_number":379,"context_line":"        self._check_live_migration_api_version(destination)"},{"line_number":380,"context_line":"        # NOTE(russellb) Havana compat"},{"line_number":381,"context_line":"        version \u003d self._get_compat_version(\u00273.0\u0027, \u00272.38\u0027)"},{"line_number":382,"context_line":"        cctxt \u003d self.client.prepare(server\u003ddestination, version\u003dversion)"},{"line_number":383,"context_line":"        return cctxt.call(ctxt, \u0027check_can_live_migrate_destination\u0027,"},{"line_number":384,"context_line":"                          instance\u003dinstance,"}],"source_content_type":"text/x-python","patch_set":29,"id":"1ae5cdf2_c4550653","line":381,"in_reply_to":"1ae5cdf2_9427d5c6","updated":"2014-06-27 01:42:19.000000000","message":"\u003e Aren\u0027t the _get_compat_version() checks redundant\n\nIndeed they are, that was just me missing a couple of spots I needed to mop up in the previous patch set. Thanks for noticing!\n\n\u003e will one now not be able to live migrate VMs from icehouse compute nodes\n\nYes. You won\u0027t be able to migrate VMs off Icehouse computes, or onto Icehouse computes. As noted in lines 293-295, this is the only reliable way to prevent a scenario resulting in destruction of ephemeral disks on shared storage in either post-migration or pre-rollback.","commit_id":"df99ff0135a2238d40daf32163860c07c360266e"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"66d8a608b1f664d2dfd1899d36d6319894a3a069","unresolved":false,"context_lines":[{"line_number":776,"context_line":"    def rollback_live_migration_at_destination(self, ctxt, instance, host,"},{"line_number":777,"context_line":"                                               destroy_disks\u003dTrue,"},{"line_number":778,"context_line":"                                               migrate_data\u003dNone):"},{"line_number":779,"context_line":"        self._check_live_migration_api_version(host)"},{"line_number":780,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003d\u00273.30\u0027)"},{"line_number":781,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":782,"context_line":"                   instance\u003dinstance,"}],"source_content_type":"text/x-python","patch_set":30,"id":"baada198_1bdd3f15","line":779,"updated":"2014-06-30 15:59:06.000000000","message":"I guess this is OK to have here but in reality this should never happen as it would have failed on one of the previous rpc calls. Probably best to leave it here tho.","commit_id":"6eba979d76f451031ec6c184e7b66190092acbae"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"a719722cff24d0170a4a9cb5d6694c0589f370a3","unresolved":false,"context_lines":[{"line_number":776,"context_line":"    def rollback_live_migration_at_destination(self, ctxt, instance, host,"},{"line_number":777,"context_line":"                                               destroy_disks\u003dTrue,"},{"line_number":778,"context_line":"                                               migrate_data\u003dNone):"},{"line_number":779,"context_line":"        self._check_live_migration_api_version(host)"},{"line_number":780,"context_line":"        cctxt \u003d self.client.prepare(server\u003dhost, version\u003d\u00273.30\u0027)"},{"line_number":781,"context_line":"        cctxt.cast(ctxt, \u0027rollback_live_migration_at_destination\u0027,"},{"line_number":782,"context_line":"                   instance\u003dinstance,"}],"source_content_type":"text/x-python","patch_set":30,"id":"baada198_8d2bd12f","line":779,"in_reply_to":"baada198_1bdd3f15","updated":"2014-06-30 17:26:00.000000000","message":"It\u0027s better to have it than not, for the benefit of the slim probability of someone upgrading Nova on destination in the middle of the migration: after pre_live_migration() but before _rollback_live_migration().","commit_id":"6eba979d76f451031ec6c184e7b66190092acbae"}],"nova/exception.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ecc84b1d6ad6f03b2977f90456ba9cc17e8ea01e","unresolved":false,"context_lines":[{"line_number":1553,"context_line":""},{"line_number":1554,"context_line":""},{"line_number":1555,"context_line":"class NoLiveMigrationForConfigDriveInLibVirt(NovaException):"},{"line_number":1556,"context_line":"    msg_fmt \u003d _(\"Live migration of instances with config drives is not \""},{"line_number":1557,"context_line":"                \"supported in libvirt.\")"},{"line_number":1558,"context_line":""},{"line_number":1559,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"7adec928_a5d94e85","line":1556,"updated":"2014-05-22 06:42:45.000000000","message":"The message here could be a bit more precise but fix it if you need to respin only.","commit_id":"13db90e0f18461d593604269b0fcdb0794f98670"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cb5dff1193b3217b8e01e15f9140aefad89bfc10","unresolved":false,"context_lines":[{"line_number":1561,"context_line":""},{"line_number":1562,"context_line":""},{"line_number":1563,"context_line":"class NoLiveMigrationForConfigDriveInLibVirt(NovaException):"},{"line_number":1564,"context_line":"    msg_fmt \u003d _(\"Live migration of instances with config drives is not \""},{"line_number":1565,"context_line":"                \"supported in libvirt.\")"},{"line_number":1566,"context_line":""},{"line_number":1567,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_adaa6d82","line":1564,"updated":"2014-06-05 18:21:25.000000000","message":"Well maybe say something like \"in this configuration\" since it is kind of supported just not for all setups. But only if you have to respin again","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"66d8a608b1f664d2dfd1899d36d6319894a3a069","unresolved":false,"context_lines":[{"line_number":1579,"context_line":"class NoLiveMigrationForConfigDriveInLibVirt(NovaException):"},{"line_number":1580,"context_line":"    msg_fmt \u003d _(\"Live migration of instances with config drives is not \""},{"line_number":1581,"context_line":"                \"supported in libvirt unless libvirt instance path and \""},{"line_number":1582,"context_line":"                \"drive data is shared across compute nodes.\")"},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":""},{"line_number":1585,"context_line":"class LiveMigrationWithOldNovaNotSafe(NovaException):"}],"source_content_type":"text/x-python","patch_set":30,"id":"baada198_bb08d32e","line":1582,"updated":"2014-06-30 15:59:06.000000000","message":"Nice! Tnx","commit_id":"6eba979d76f451031ec6c184e7b66190092acbae"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e635c4c3bc942978650dd6a53874bffc3932e421","unresolved":false,"context_lines":[{"line_number":1578,"context_line":""},{"line_number":1579,"context_line":"class NoLiveMigrationForConfigDriveInLibVirt(NovaException):"},{"line_number":1580,"context_line":"    msg_fmt \u003d _(\"Live migration of instances with config drives is not \""},{"line_number":1581,"context_line":"                \"supported in libvirt unless libvirt instance path and \""},{"line_number":1582,"context_line":"                \"drive data is shared across compute nodes.\")"},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"baada198_f1158a49","line":1581,"updated":"2014-07-03 12:01:19.000000000","message":"Why is this exception in the global exceptions? It should be a driver specific exception.","commit_id":"73739035536a5fdad211edd9c2d4adf160bbc2fa"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"3bfc8c5bc90018ab199eea5881118089eec79c72","unresolved":false,"context_lines":[{"line_number":1578,"context_line":""},{"line_number":1579,"context_line":"class NoLiveMigrationForConfigDriveInLibVirt(NovaException):"},{"line_number":1580,"context_line":"    msg_fmt \u003d _(\"Live migration of instances with config drives is not \""},{"line_number":1581,"context_line":"                \"supported in libvirt unless libvirt instance path and \""},{"line_number":1582,"context_line":"                \"drive data is shared across compute nodes.\")"},{"line_number":1583,"context_line":""},{"line_number":1584,"context_line":""}],"source_content_type":"text/x-python","patch_set":32,"id":"baada198_9c9d45bf","line":1581,"in_reply_to":"baada198_f1158a49","updated":"2014-07-03 12:02:32.000000000","message":"This is not related to this patch in any case.","commit_id":"73739035536a5fdad211edd9c2d4adf160bbc2fa"}],"nova/tests/compute/test_rpcapi.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cfa0e817bb57a4c0993ccf7895c50d79a1a1d5f2","unresolved":false,"context_lines":[{"line_number":626,"context_line":"    def test_rollback_live_migration_at_destination(self):"},{"line_number":627,"context_line":"        self._test_compute_api(\u0027rollback_live_migration_at_destination\u0027,"},{"line_number":628,"context_line":"                \u0027cast\u0027, instance\u003dself.fake_instance, host\u003d\u0027host\u0027,"},{"line_number":629,"context_line":"                destroy_disks\u003dTrue, migrate_data\u003dNone)"},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"        # NOTE(russellb) Havana compat"},{"line_number":632,"context_line":"        self.flags(compute\u003d\u0027havana\u0027, group\u003d\u0027upgrade_levels\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_d2e7db10","line":629,"updated":"2014-05-02 21:05:51.000000000","message":"this should have version\u003d3.26","commit_id":"92cd036c36cf3c3d6c46b225a2872db825b7f9bd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cfa0e817bb57a4c0993ccf7895c50d79a1a1d5f2","unresolved":false,"context_lines":[{"line_number":632,"context_line":"        self.flags(compute\u003d\u0027havana\u0027, group\u003d\u0027upgrade_levels\u0027)"},{"line_number":633,"context_line":"        self._test_compute_api(\u0027rollback_live_migration_at_destination\u0027,"},{"line_number":634,"context_line":"                \u0027cast\u0027, instance\u003dself.fake_instance, host\u003d\u0027host\u0027,"},{"line_number":635,"context_line":"                destroy_disks\u003dTrue, migrate_data\u003dNone, version\u003d\u00272.0\u0027)"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"    def test_run_instance(self):"},{"line_number":638,"context_line":"        self._test_compute_api(\u0027run_instance\u0027, \u0027cast\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_b22a572a","line":635,"updated":"2014-05-02 21:05:51.000000000","message":"this should be 3.26, no?","commit_id":"92cd036c36cf3c3d6c46b225a2872db825b7f9bd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"807ca71e0dd89e7c570d3453ef66d54092a5f052","unresolved":false,"context_lines":[{"line_number":632,"context_line":"        self.flags(compute\u003d\u0027havana\u0027, group\u003d\u0027upgrade_levels\u0027)"},{"line_number":633,"context_line":"        self._test_compute_api(\u0027rollback_live_migration_at_destination\u0027,"},{"line_number":634,"context_line":"                \u0027cast\u0027, instance\u003dself.fake_instance, host\u003d\u0027host\u0027,"},{"line_number":635,"context_line":"                destroy_disks\u003dTrue, migrate_data\u003dNone, version\u003d\u00272.0\u0027)"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"    def test_run_instance(self):"},{"line_number":638,"context_line":"        self._test_compute_api(\u0027run_instance\u0027, \u0027cast\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_1fec9ff2","line":635,"updated":"2014-05-06 16:46:35.000000000","message":"See here a unit test that is testing that the rpcapi works on both version 2.0 and version 3.26...","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"}],"nova/tests/virt/libvirt/test_libvirt.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0406be5d7114ab53f0477e46a9b8f8ab8180088b","unresolved":false,"context_lines":[{"line_number":3848,"context_line":"        conn._is_shared_instance_path(dest_check_data).AndReturn("},{"line_number":3849,"context_line":"                is_shared_instance_path)"},{"line_number":3850,"context_line":""},{"line_number":3851,"context_line":"        return (instance, dest_check_data, conn)"},{"line_number":3852,"context_line":""},{"line_number":3853,"context_line":"    def test_check_can_live_migrate_source_block_migration(self):"},{"line_number":3854,"context_line":"        instance, dest_check_data, conn \u003d self._mock_can_live_migrate_source("}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_6a51c2d7","line":3851,"updated":"2014-05-02 17:15:39.000000000","message":"Nice cleanup refactoring above. ++","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0406be5d7114ab53f0477e46a9b8f8ab8180088b","unresolved":false,"context_lines":[{"line_number":3963,"context_line":"            mock_get.return_value \u003d \u0027[{\"virt_disk_size\":2}]\u0027"},{"line_number":3964,"context_line":"            self.assertFalse(conn._is_shared_block_storage("},{"line_number":3965,"context_line":"                {\u0027name\u0027: \u0027instance_name\u0027}, {\u0027is_volume_backed\u0027: True}))"},{"line_number":3966,"context_line":"            mock_get.assert_called_with(\u0027instance_name\u0027)"},{"line_number":3967,"context_line":""},{"line_number":3968,"context_line":"    def test_is_shared_instance_path(self):"},{"line_number":3969,"context_line":"        conn \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_cab56eef","line":3966,"updated":"2014-05-02 17:15:39.000000000","message":"please use assert_called_once_with() to add just one more check around call usage.","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"0406be5d7114ab53f0477e46a9b8f8ab8180088b","unresolved":false,"context_lines":[{"line_number":3972,"context_line":"            mock_check.return_value \u003d True"},{"line_number":3973,"context_line":"            self.assertTrue(conn._is_shared_instance_path("},{"line_number":3974,"context_line":"                {\u0027filename\u0027: \u0027test\u0027}))"},{"line_number":3975,"context_line":"            mock_check.assert_called_with(\u0027test\u0027)"},{"line_number":3976,"context_line":""},{"line_number":3977,"context_line":"    def test_live_migration_raises_exception(self):"},{"line_number":3978,"context_line":"        # Confirms recover method is called when exceptions are raised."}],"source_content_type":"text/x-python","patch_set":2,"id":"9ad9bd40_eab872e7","line":3975,"updated":"2014-05-02 17:15:39.000000000","message":"ditto.","commit_id":"8d3280f90d8703c882f940afc5e767575c1def6d"}],"nova/tests/virt/test_ironic_api_contracts.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"66d8a608b1f664d2dfd1899d36d6319894a3a069","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                           \"ComputeDriver.destroy\","},{"line_number":94,"context_line":"                           [\u0027self\u0027, \u0027context\u0027, \u0027instance\u0027, \u0027network_info\u0027,"},{"line_number":95,"context_line":"                            \u0027block_device_info\u0027, \u0027destroy_disks\u0027,"},{"line_number":96,"context_line":"                            \u0027migrate_data\u0027])"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        self._check_method(driver.ComputeDriver.reboot,"},{"line_number":99,"context_line":"                           \"ComputeDriver.reboot\","}],"source_content_type":"text/x-python","patch_set":30,"id":"baada198_2ca1eba9","line":96,"updated":"2014-06-30 15:59:06.000000000","message":"Well I am not sure about this and what the process is nowadays, but this seems to me that all changes like this should go to ironic first, as now this test is not reflecting reality. Thoughts?","commit_id":"6eba979d76f451031ec6c184e7b66190092acbae"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"a719722cff24d0170a4a9cb5d6694c0589f370a3","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                           \"ComputeDriver.destroy\","},{"line_number":94,"context_line":"                           [\u0027self\u0027, \u0027context\u0027, \u0027instance\u0027, \u0027network_info\u0027,"},{"line_number":95,"context_line":"                            \u0027block_device_info\u0027, \u0027destroy_disks\u0027,"},{"line_number":96,"context_line":"                            \u0027migrate_data\u0027])"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        self._check_method(driver.ComputeDriver.reboot,"},{"line_number":99,"context_line":"                           \"ComputeDriver.reboot\","}],"source_content_type":"text/x-python","patch_set":30,"id":"baada198_8d8831e9","line":96,"in_reply_to":"baada198_2ca1eba9","updated":"2014-06-30 17:26:00.000000000","message":"Given that Ironic driver is currently maintained in a separate repository, I don\u0027t think you need to enforce a strict order of Ironic change getting merged first. As long as a commit reflecting this change is proposed for Ironic (and I\u0027ve +1\u0027d your commit that does that), it will be up to Ironic team to catch up with this change on the Nova side. Otherwise you\u0027d be setting a dangerous precedent for external driver maintainers to veto API changes in Nova.","commit_id":"6eba979d76f451031ec6c184e7b66190092acbae"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"fb8eec79fa0b7e5ea10df8581b67aa37cb4f2686","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                           \"ComputeDriver.destroy\","},{"line_number":94,"context_line":"                           [\u0027self\u0027, \u0027context\u0027, \u0027instance\u0027, \u0027network_info\u0027,"},{"line_number":95,"context_line":"                            \u0027block_device_info\u0027, \u0027destroy_disks\u0027,"},{"line_number":96,"context_line":"                            \u0027migrate_data\u0027])"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        self._check_method(driver.ComputeDriver.reboot,"},{"line_number":99,"context_line":"                           \"ComputeDriver.reboot\","}],"source_content_type":"text/x-python","patch_set":30,"id":"baada198_6e04b015","line":96,"in_reply_to":"baada198_8d8831e9","updated":"2014-06-30 20:00:20.000000000","message":"I agree with you but they do seem to be quite close to graduating so no need to make it worse. You are right that we do not really need to block nova patches on this tho.","commit_id":"6eba979d76f451031ec6c184e7b66190092acbae"}],"nova/virt/driver.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2069960bd89e79ad25daabc733f6dd6e59b8f241","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        :param instance_ref: instance object that was being migrated"},{"line_number":670,"context_line":"        :param network_info: instance network information"},{"line_number":671,"context_line":"        :param block_device_info: instance block device information"},{"line_number":672,"context_line":"        :param migrate_data: implementation specific params"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"        \"\"\""},{"line_number":675,"context_line":"        raise NotImplementedError()"}],"source_content_type":"text/x-python","patch_set":3,"id":"9ad9bd40_c3fd543d","line":672,"updated":"2014-05-02 20:04:56.000000000","message":"You missed a docstring for the new destroy_disks param :)","commit_id":"92cd036c36cf3c3d6c46b225a2872db825b7f9bd"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3138ba8c6c87730807f0409d83bd989f6062725d","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        :param network_info:"},{"line_number":330,"context_line":"           :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info`"},{"line_number":331,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":332,"context_line":"                                  be detached from the instance."},{"line_number":333,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_3bafc595","line":332,"updated":"2014-05-06 09:21:33.000000000","message":"Why not add migrate data to the docstring while you\u0027re at it?","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"807ca71e0dd89e7c570d3453ef66d54092a5f052","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        :param network_info:"},{"line_number":330,"context_line":"           :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info`"},{"line_number":331,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":332,"context_line":"                                  be detached from the instance."},{"line_number":333,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_3fbc03dc","line":332,"in_reply_to":"9ad9bd40_3bafc595","updated":"2014-05-06 16:46:35.000000000","message":"++","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"d62eaf44baefd45ed8633ae187852f8d5ffef550","unresolved":false,"context_lines":[{"line_number":329,"context_line":"        :param network_info:"},{"line_number":330,"context_line":"           :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info`"},{"line_number":331,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":332,"context_line":"                                  be detached from the instance."},{"line_number":333,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_e0ae7c2d","line":332,"in_reply_to":"9ad9bd40_3fbc03dc","updated":"2014-05-06 19:45:34.000000000","message":"Done","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"73ee4c4cab900a3e91f11514dd1a2801bc4db6bb","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        raise NotImplementedError()"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def destroy(self, context, instance, network_info, block_device_info\u003dNone,"},{"line_number":303,"context_line":"                destroy_disks\u003dTrue, migrate_data\u003dNone):"},{"line_number":304,"context_line":"        \"\"\"Destroy the specified instance from the Hypervisor."},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        If the instance is not found (for example if networking failed), this"}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_70df880d","line":303,"updated":"2014-06-04 15:59:43.000000000","message":"You\u0027ve added a new parameter here, but only updated the libvirt + fake driver subclasses. You need to add this to all virt driver subclasses.\n\nLikewise for the other parameters added below.","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"56bf93d0d2a3c4a1265639bd128ea87f844b2cf5","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        raise NotImplementedError()"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def destroy(self, context, instance, network_info, block_device_info\u003dNone,"},{"line_number":303,"context_line":"                destroy_disks\u003dTrue, migrate_data\u003dNone):"},{"line_number":304,"context_line":"        \"\"\"Destroy the specified instance from the Hypervisor."},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        If the instance is not found (for example if networking failed), this"}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_ca70a8de","line":303,"in_reply_to":"1ae5cdf2_70df880d","updated":"2014-06-04 17:47:23.000000000","message":"+1","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"02074d532be86681cbcc9c4c6f471ed33e4625bd","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        raise NotImplementedError()"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def destroy(self, context, instance, network_info, block_device_info\u003dNone,"},{"line_number":303,"context_line":"                destroy_disks\u003dTrue, migrate_data\u003dNone):"},{"line_number":304,"context_line":"        \"\"\"Destroy the specified instance from the Hypervisor."},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        If the instance is not found (for example if networking failed), this"}],"source_content_type":"text/x-python","patch_set":21,"id":"1ae5cdf2_c05de97a","line":303,"in_reply_to":"1ae5cdf2_ca70a8de","updated":"2014-06-06 18:37:52.000000000","message":"Done","commit_id":"37dbbe667a5106e45b946be968f374750ea9be4c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f6ebcd93905c23207b0a62958e9c3a386bc05f07","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":315,"context_line":"                                  be detached from the instance."},{"line_number":316,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":317,"context_line":"        :param migrate_data: implementation specific params"},{"line_number":318,"context_line":"        \"\"\""},{"line_number":319,"context_line":"        raise NotImplementedError()"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_2f588831","line":317,"updated":"2014-06-10 17:19:58.000000000","message":"The more I look at this patch, the less I like the idea of adding these opaque, arbitrary information, \u0027migrate_data\u0027 parameters to the destroy \u0026 cleanup methods.\n\nUltimately the libvirt driver is the only thing that uses this, and it only reads one single field from it.\n\nI feel like we\u0027d be better killing this migrate_data parameter and having another boolean parameter that explicitly represents the logic we\u0027re wanting eg something like  \"delete_nonshared_disks\"","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"c5c02794903b563c92641b38680bbb6d9b150944","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":315,"context_line":"                                  be detached from the instance."},{"line_number":316,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":317,"context_line":"        :param migrate_data: implementation specific params"},{"line_number":318,"context_line":"        \"\"\""},{"line_number":319,"context_line":"        raise NotImplementedError()"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_d40b1dd4","line":317,"in_reply_to":"1ae5cdf2_057d6de7","updated":"2014-06-11 09:11:33.000000000","message":"@matt I realize the migration methods already have this migrate_data object, and that is unfortunate since, but the destroy / cleanup methods are really not migration methods and thus I don\u0027t think we should pollute them with this known bad approach too. If cleanup/destroy need further parameters in order to support migration we should add them in a style which is not specific to migration, so they can be used by other code too if needed.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e020b0b741ae492193c082e8d91f8e3f7c744f3c","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":315,"context_line":"                                  be detached from the instance."},{"line_number":316,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":317,"context_line":"        :param migrate_data: implementation specific params"},{"line_number":318,"context_line":"        \"\"\""},{"line_number":319,"context_line":"        raise NotImplementedError()"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_455cd5ed","line":317,"in_reply_to":"1ae5cdf2_2f588831","updated":"2014-06-10 18:14:45.000000000","message":"Tend to agree, but migrate_data does show up in the other methods related to live migration so there is some precedent.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"7bc5747a23ae0e21b0dca7195e71d661ab4b629c","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":315,"context_line":"                                  be detached from the instance."},{"line_number":316,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":317,"context_line":"        :param migrate_data: implementation specific params"},{"line_number":318,"context_line":"        \"\"\""},{"line_number":319,"context_line":"        raise NotImplementedError()"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_057d6de7","line":317,"in_reply_to":"1ae5cdf2_2f588831","updated":"2014-06-10 19:09:47.000000000","message":"That sentiment contradicts your comment to compute/manager.py:4616. Over there, you\u0027re looking to get rid of driver specific logic in common code paths. Here, you\u0027re asking to expose driver specific flags in common RPC API.\n\nMy point made in compute manager comments also applies here: it would be nice to refactor this code much more than this patch already did, but it\u0027s outside of scope of the bug this patch is fixing. We need to go through blueprint process to discuss, design, and review a plan to improve modularity and maintainability of live migrations code.\n\nRe single one field: If you have another look at compute manager, you can see that live migration methods use at least two fields in migrate_data, not just one. And your comment to the libvirt driver adds yet another argument on why we might have to pass even more driver specific information around via RPC.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"ef73a88b2e0ab140c64f1dc6e35468ae352fa655","unresolved":false,"context_lines":[{"line_number":332,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":333,"context_line":"                                  be detached from the instance."},{"line_number":334,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":335,"context_line":"        :param migrate_data: implementation specific params"},{"line_number":336,"context_line":"        \"\"\""},{"line_number":337,"context_line":"        raise NotImplementedError()"},{"line_number":338,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"baada198_9d7a6e75","line":335,"updated":"2014-07-04 16:32:06.000000000","message":"I\u0027m really not happy with polluting the destroy + cleanup methods with arbitrary driver specific data blob like this.\n\nThe pre-existing \u0027destroy_disks\u0027 parameter here is involved in migration and set appropriately by the compute manager. I really think a similar approach is require here. ie any new parameter to \u0027cleanup\u0027 should take the form of a functional instruction of what the required behaviour of the cleanup method is.\n\nie IMHO this should be \u0027delete_instance_files\u003dTrue\u0027 as a parameter, since the semantics of that nicely match the standard \u0027delete_instance_files\u0027 method in the virt driver API.","commit_id":"65ec0993d17aeb34b02662295206bb9888a437c4"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"2b86c9265e08b2aaa96772fb900d067300eaa4b5","unresolved":false,"context_lines":[{"line_number":332,"context_line":"        :param block_device_info: Information about block devices that should"},{"line_number":333,"context_line":"                                  be detached from the instance."},{"line_number":334,"context_line":"        :param destroy_disks: Indicates if disks should be destroyed"},{"line_number":335,"context_line":"        :param migrate_data: implementation specific params"},{"line_number":336,"context_line":"        \"\"\""},{"line_number":337,"context_line":"        raise NotImplementedError()"},{"line_number":338,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"baada198_b69e0b06","line":335,"in_reply_to":"baada198_9d7a6e75","updated":"2014-07-09 02:33:56.000000000","message":"We already had this discussion in patch set 22, the conclusion was that, even if what you\u0027re asking for were a valid approach, it would expand the scope of this patch too far beyond the bug in question.\n\nIf you really insist on pulling that discussion out of the freezer after a month and a dozen patch revisions (which by itself is another indicator that the discussion belongs in a blueprint, not bugfix review comments), here\u0027s where we left it, from my perspective.\n\nI think that polluting signatures of methods of the parent class with implementation specific parameters (and delete_instance_files is as libvirt specific as it gets) is a recipe for the shotgun surgery anti-pattern. You don\u0027t have to go further than this very patch to observe the impact of that anti-pattern in how (at your own request in patch set 21) I had to touch all virt drivers to fix a libvirt specific problem.\n\nAnother argument I already made in patch set 22 that is worth reiterating is that the amount of driver specific information passed over RPC is likely to grow as drivers mature. And every time you need to change what you pass between two instances of the same driver, you\u0027d have same amount of shotgun surgery all over the place, unless you encapsulate implementation specific details away from upper layers so that only the driver itself has to be changed.","commit_id":"65ec0993d17aeb34b02662295206bb9888a437c4"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3138ba8c6c87730807f0409d83bd989f6062725d","unresolved":false,"context_lines":[{"line_number":283,"context_line":"DISABLE_REASON_UNDEFINED \u003d \u0027None\u0027"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# Backends that put images on block storage shared between compute nodes"},{"line_number":286,"context_line":"SHARED_IMAGE_TYPES \u003d (\u0027rbd\u0027,)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"def patch_tpool_proxy():"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_7b76add8","line":286,"updated":"2014-05-06 09:21:33.000000000","message":"Why define this here? We have a _whole class_ in imagebackend.py that represents this entity - why not make this a property of the object and part of it\u0027s public API so that data is kept in one place.","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"0b8bf588778cf699cf3b0935bb8405335d8e1bdb","unresolved":false,"context_lines":[{"line_number":283,"context_line":"DISABLE_REASON_UNDEFINED \u003d \u0027None\u0027"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# Backends that put images on block storage shared between compute nodes"},{"line_number":286,"context_line":"SHARED_IMAGE_TYPES \u003d (\u0027rbd\u0027,)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"def patch_tpool_proxy():"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_e8465ebf","line":286,"in_reply_to":"9ad9bd40_74aead99","updated":"2014-05-06 21:33:51.000000000","message":"Done","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"807ca71e0dd89e7c570d3453ef66d54092a5f052","unresolved":false,"context_lines":[{"line_number":283,"context_line":"DISABLE_REASON_UNDEFINED \u003d \u0027None\u0027"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# Backends that put images on block storage shared between compute nodes"},{"line_number":286,"context_line":"SHARED_IMAGE_TYPES \u003d (\u0027rbd\u0027,)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"def patch_tpool_proxy():"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_ffc53b5c","line":286,"in_reply_to":"9ad9bd40_7b76add8","updated":"2014-05-06 16:46:35.000000000","message":"Agreed. Though this was likely due to Dmitry not knowing about that code...","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"d62eaf44baefd45ed8633ae187852f8d5ffef550","unresolved":false,"context_lines":[{"line_number":283,"context_line":"DISABLE_REASON_UNDEFINED \u003d \u0027None\u0027"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"# Backends that put images on block storage shared between compute nodes"},{"line_number":286,"context_line":"SHARED_IMAGE_TYPES \u003d (\u0027rbd\u0027,)"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"def patch_tpool_proxy():"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_74aead99","line":286,"in_reply_to":"9ad9bd40_ffc53b5c","updated":"2014-05-06 19:45:34.000000000","message":"Jay, you must have missed the part where I\u0027ve been tracking this patch along with the rest of ephemeral rbd clone series through 3 OpenStack versions now ;-)\n\nI actually considered making use of Image classes here, but decided against it, primarily to keep scope creep away. Nikola is making a fair point, I\u0027ll give it a try.","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3138ba8c6c87730807f0409d83bd989f6062725d","unresolved":false,"context_lines":[{"line_number":4215,"context_line":"        filename \u003d self._create_shared_storage_test_file()"},{"line_number":4216,"context_line":""},{"line_number":4217,"context_line":"        return {\"filename\": filename,"},{"line_number":4218,"context_line":"                \"image_type\": CONF.libvirt.images_type,"},{"line_number":4219,"context_line":"                \"block_migration\": block_migration,"},{"line_number":4220,"context_line":"                \"disk_over_commit\": disk_over_commit,"},{"line_number":4221,"context_line":"                \"disk_available_mb\": disk_available_mb}"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_d6d5d01c","line":4218,"updated":"2014-05-06 09:21:33.000000000","message":"This is not used anywhere afaict so we probably don\u0027t need to return it. Also this information leaving the libvirt driver probably means we are doing something wrong anyway as no one else should be concerned with this info.","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"d62eaf44baefd45ed8633ae187852f8d5ffef550","unresolved":false,"context_lines":[{"line_number":4215,"context_line":"        filename \u003d self._create_shared_storage_test_file()"},{"line_number":4216,"context_line":""},{"line_number":4217,"context_line":"        return {\"filename\": filename,"},{"line_number":4218,"context_line":"                \"image_type\": CONF.libvirt.images_type,"},{"line_number":4219,"context_line":"                \"block_migration\": block_migration,"},{"line_number":4220,"context_line":"                \"disk_over_commit\": disk_over_commit,"},{"line_number":4221,"context_line":"                \"disk_available_mb\": disk_available_mb}"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_f470fd41","line":4218,"in_reply_to":"9ad9bd40_d6d5d01c","updated":"2014-05-06 19:45:34.000000000","message":"It\u0027s used to check whether the destination has the same image backend as the source, down in _is_shared_block_storage(). As per above, I\u0027ll see if I can make use of Image here instead of CONF.","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"0b8bf588778cf699cf3b0935bb8405335d8e1bdb","unresolved":false,"context_lines":[{"line_number":4215,"context_line":"        filename \u003d self._create_shared_storage_test_file()"},{"line_number":4216,"context_line":""},{"line_number":4217,"context_line":"        return {\"filename\": filename,"},{"line_number":4218,"context_line":"                \"image_type\": CONF.libvirt.images_type,"},{"line_number":4219,"context_line":"                \"block_migration\": block_migration,"},{"line_number":4220,"context_line":"                \"disk_over_commit\": disk_over_commit,"},{"line_number":4221,"context_line":"                \"disk_available_mb\": disk_available_mb}"}],"source_content_type":"text/x-python","patch_set":6,"id":"9ad9bd40_a840d6a5","line":4218,"in_reply_to":"9ad9bd40_f470fd41","updated":"2014-05-06 21:33:51.000000000","message":"Considering that image_backend just holds the factory object (imagebackend.Backend) and is used to generate Image objects on the fly base don image_type (which is initialized from CONF all over the place), we don\u0027t gain anything by generating that object here, too. I think it\u0027s enough to pass image_type from destination to source.","commit_id":"8e177c51a26535979ce1655d8cbb6666dd0c5982"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cbdbf2e6be690a82794fc80b4fe3133919b63ae6","unresolved":false,"context_lines":[{"line_number":4212,"context_line":"        filename \u003d self._create_shared_storage_test_file()"},{"line_number":4213,"context_line":""},{"line_number":4214,"context_line":"        return {\"filename\": filename,"},{"line_number":4215,"context_line":"                \"image_type\": CONF.libvirt.images_type,"},{"line_number":4216,"context_line":"                \"block_migration\": block_migration,"},{"line_number":4217,"context_line":"                \"disk_over_commit\": disk_over_commit,"},{"line_number":4218,"context_line":"                \"disk_available_mb\": disk_available_mb}"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_c4fe9238","line":4215,"updated":"2014-05-07 15:26:10.000000000","message":"See comment below - I hate that something like this gets leaked over RPC at least in this form. Could live with it since we also have things like \u0027filename\u0027 in dest_check_data, which make sense only to libvirt.","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"830a70393a32fd9e88c50f6853b89780c79b32e3","unresolved":false,"context_lines":[{"line_number":4212,"context_line":"        filename \u003d self._create_shared_storage_test_file()"},{"line_number":4213,"context_line":""},{"line_number":4214,"context_line":"        return {\"filename\": filename,"},{"line_number":4215,"context_line":"                \"image_type\": CONF.libvirt.images_type,"},{"line_number":4216,"context_line":"                \"block_migration\": block_migration,"},{"line_number":4217,"context_line":"                \"disk_over_commit\": disk_over_commit,"},{"line_number":4218,"context_line":"                \"disk_available_mb\": disk_available_mb}"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_281aa46e","line":4215,"in_reply_to":"7adec928_c4fe9238","updated":"2014-05-07 19:18:04.000000000","message":"I suspect that implementation specific data will keep leaking into this dict as we get live migrations to work with more drivers. At some point we\u0027ll have to figure out a cleaner way to pass it around, but hopefully not as part of this review. As you said, it\u0027s already too big for its own good.","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cbdbf2e6be690a82794fc80b4fe3133919b63ae6","unresolved":false,"context_lines":[{"line_number":4258,"context_line":"                                    dest_check_data[\u0027disk_over_commit\u0027])"},{"line_number":4259,"context_line":""},{"line_number":4260,"context_line":"        elif not (dest_check_data[\u0027is_shared_block_storage\u0027] or"},{"line_number":4261,"context_line":"                  dest_check_data[\u0027is_shared_instance_path\u0027]):"},{"line_number":4262,"context_line":"            reason \u003d _(\"Live migration can not be used \""},{"line_number":4263,"context_line":"                       \"without shared storage.\")"},{"line_number":4264,"context_line":"            raise exception.InvalidSharedStorage(reason\u003dreason, path\u003dsource)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_4dd54542","line":4261,"updated":"2014-05-07 15:26:10.000000000","message":"Even though this looks correct - the refactoring and compacting of cases totally obscures the intent here (which was allow migration of PURELY volume backed instances even without shared storage).\n\nThis is actually a useful feature as there are people running purely volume based vms and like this it\u0027s likely to get broken in the future.\n\nAt least needs a comment","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"830a70393a32fd9e88c50f6853b89780c79b32e3","unresolved":false,"context_lines":[{"line_number":4258,"context_line":"                                    dest_check_data[\u0027disk_over_commit\u0027])"},{"line_number":4259,"context_line":""},{"line_number":4260,"context_line":"        elif not (dest_check_data[\u0027is_shared_block_storage\u0027] or"},{"line_number":4261,"context_line":"                  dest_check_data[\u0027is_shared_instance_path\u0027]):"},{"line_number":4262,"context_line":"            reason \u003d _(\"Live migration can not be used \""},{"line_number":4263,"context_line":"                       \"without shared storage.\")"},{"line_number":4264,"context_line":"            raise exception.InvalidSharedStorage(reason\u003dreason, path\u003dsource)"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_887c9874","line":4261,"in_reply_to":"7adec928_4dd54542","updated":"2014-05-07 19:18:04.000000000","message":"My bad, I was staring at this code long enough to internalize the assumption that \"volume backed\" is a subset of \"shared block storage\", I will expand docstring for _is_shared_block_storage() to explicitly mention it.","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cbdbf2e6be690a82794fc80b4fe3133919b63ae6","unresolved":false,"context_lines":[{"line_number":4278,"context_line":"        \u0027\u0027\u0027"},{"line_number":4279,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4280,"context_line":"                self.image_backend.backend().is_shared_block_storage()):"},{"line_number":4281,"context_line":"            # Image type is the same on source and destination and stores"},{"line_number":4282,"context_line":"            # images in a shared block storage"},{"line_number":4283,"context_line":"            return True"},{"line_number":4284,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_4409a294","line":4281,"updated":"2014-05-07 15:26:10.000000000","message":"Right - so image_type is checked here, I missed that on the first review.\n\nI guess this can cause issues if we actually have different parts of the cluster backed by different ceph deployments - which may or may not be outside of the scope fo this patch - but makes me wonder if we should maybe push all of these things into the image_backend and then for example have:\n\nclass Image(object):\n  ...\n  def shared_storage(data_from_the_other_node):\n       return False # in the general case or actually see if data makes sense to me\n\nAnd then data_format (that will go over RPC in this case) can be a bit less libvirt specific, and we\u0027ll also be able to easily implement in subclasess a \"yes but on a different ceph deployment so no\" scenario","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"830a70393a32fd9e88c50f6853b89780c79b32e3","unresolved":false,"context_lines":[{"line_number":4278,"context_line":"        \u0027\u0027\u0027"},{"line_number":4279,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4280,"context_line":"                self.image_backend.backend().is_shared_block_storage()):"},{"line_number":4281,"context_line":"            # Image type is the same on source and destination and stores"},{"line_number":4282,"context_line":"            # images in a shared block storage"},{"line_number":4283,"context_line":"            return True"},{"line_number":4284,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_7b41d80a","line":4281,"in_reply_to":"7adec928_4409a294","updated":"2014-05-07 19:18:04.000000000","message":"You\u0027re absolutely right, there\u0027s more work to be done here. I actually tried to go down that road yesterday, but it landed me in a rabbit hole.\n\nProblem #1: there\u0027s nothing in the data passed down the live migrations code path that refers to the image backend the instance\u0027s drives were provisioned with.\n\nProblem #2: driver itself doesn\u0027t keep a reference of Image object around, it uses a factory object to create a new Image instance every time it\u0027s needed. That\u0027s based on images_type, which means that creating an Image object here would be semantically identical to checking image_type directly, just with more code.\n\nI think this needs to go all the way to the Instance object that should encapsulate enough information about all its drives to make live migration decisions. Generally speaking, it should have a reference to the backend (name of image backend class?) and a dict with backend-specific information that can be passed to the backend\u0027s _can_live_migrate() method on the destination. In case of rbd, the backend then would compare fsid, or simply check if the rbd URL is accessible and writeable.\n\nAs I said, a rabbit hole.","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cbdbf2e6be690a82794fc80b4fe3133919b63ae6","unresolved":false,"context_lines":[{"line_number":4564,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"},{"line_number":4565,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4566,"context_line":"                    \u0027is_shared_instance_path\u0027, True)"},{"line_number":4567,"context_line":"            is_volume_backed \u003d migrate_data.get(\u0027is_volume_backed\u0027, False)"},{"line_number":4568,"context_line":"            is_block_migration \u003d migrate_data.get(\u0027block_migration\u0027, True)"},{"line_number":4569,"context_line":"            instance_relative_path \u003d migrate_data.get(\u0027instance_relative_path\u0027)"},{"line_number":4570,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_0774fcfb","line":4567,"updated":"2014-05-07 15:26:10.000000000","message":"Micronit: we don\u0027t need it anymore.","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"830a70393a32fd9e88c50f6853b89780c79b32e3","unresolved":false,"context_lines":[{"line_number":4564,"context_line":"                    \u0027is_shared_block_storage\u0027, True)"},{"line_number":4565,"context_line":"            is_shared_instance_path \u003d migrate_data.get("},{"line_number":4566,"context_line":"                    \u0027is_shared_instance_path\u0027, True)"},{"line_number":4567,"context_line":"            is_volume_backed \u003d migrate_data.get(\u0027is_volume_backed\u0027, False)"},{"line_number":4568,"context_line":"            is_block_migration \u003d migrate_data.get(\u0027block_migration\u0027, True)"},{"line_number":4569,"context_line":"            instance_relative_path \u003d migrate_data.get(\u0027instance_relative_path\u0027)"},{"line_number":4570,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_fe195623","line":4567,"in_reply_to":"7adec928_0774fcfb","updated":"2014-05-07 19:18:04.000000000","message":"Done","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cbdbf2e6be690a82794fc80b4fe3133919b63ae6","unresolved":false,"context_lines":[{"line_number":4594,"context_line":"            self._create_images_and_backing(context, instance, instance_dir,"},{"line_number":4595,"context_line":"                                            disk_info)"},{"line_number":4596,"context_line":""},{"line_number":4597,"context_line":"        if not (is_block_migration or is_shared_instance_path):"},{"line_number":4598,"context_line":"            # Touch the console.log file, required by libvirt."},{"line_number":4599,"context_line":"            console_file \u003d self._get_console_log_path(instance)"},{"line_number":4600,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_b288e8c7","line":4597,"updated":"2014-05-07 15:26:10.000000000","message":"Hmm this does not seem right! \n\nThis will happen now for some non volume backed instances, while previously it was a pre-req for this branch to happen. \n\nSeems like the new logic is flawed, or was previously flawed and you\u0027re fixing it? But in that case it really should be at least mentioned but better a separate patch (although does not seem to be - we need kernel and ramdisk if we have a volume backed instance sine _create_images will not be happening)","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"830a70393a32fd9e88c50f6853b89780c79b32e3","unresolved":false,"context_lines":[{"line_number":4594,"context_line":"            self._create_images_and_backing(context, instance, instance_dir,"},{"line_number":4595,"context_line":"                                            disk_info)"},{"line_number":4596,"context_line":""},{"line_number":4597,"context_line":"        if not (is_block_migration or is_shared_instance_path):"},{"line_number":4598,"context_line":"            # Touch the console.log file, required by libvirt."},{"line_number":4599,"context_line":"            console_file \u003d self._get_console_log_path(instance)"},{"line_number":4600,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_fef076cb","line":4597,"in_reply_to":"7adec928_b288e8c7","updated":"2014-05-07 19:18:04.000000000","message":"Yes, the logic here was flawed, this needs to be done for rbd backed instances, too. I\u0027ve added a comment to explain the new logic.","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"930a0ff8554e0381dcbd659e38c080ff41659c66","unresolved":false,"context_lines":[{"line_number":4291,"context_line":"        \u0027\u0027\u0027"},{"line_number":4292,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4293,"context_line":"                self.image_backend.backend().is_shared_block_storage()):"},{"line_number":4294,"context_line":"            return True"},{"line_number":4295,"context_line":""},{"line_number":4296,"context_line":"        if (dest_check_data.get(\u0027is_volume_backed\u0027) and"},{"line_number":4297,"context_line":"                not bool(jsonutils.loads("}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_fe4dfd3e","line":4294,"updated":"2014-05-19 17:39:56.000000000","message":"OK so looking at this here - I think I\u0027ve spotted a bug. This is OK to assume UNLESS we have a config drive attachesd. Looking at the libvirt code - the config drive is NOT created using imagebackend and thus will be on a non\u003dshared storage (local disc) so this assumption is unsafe.\n\nMaybe we should be checking disks here as well?","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"930a0ff8554e0381dcbd659e38c080ff41659c66","unresolved":false,"context_lines":[{"line_number":4580,"context_line":"            # NOTE(mikal): block migration of instances using config drive is"},{"line_number":4581,"context_line":"            # not supported because of a bug in libvirt (read only devices"},{"line_number":4582,"context_line":"            # are not copied by libvirt). See bug/1246201"},{"line_number":4583,"context_line":"            if configdrive.required_by(instance):"},{"line_number":4584,"context_line":"                raise exception.NoBlockMigrationForConfigDriveInLibVirt()"},{"line_number":4585,"context_line":""},{"line_number":4586,"context_line":"        if not is_shared_instance_path:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_9ec07951","line":4583,"updated":"2014-05-19 17:39:56.000000000","message":"Ah so we guard against the bug spotted above, but this is not enough. The situation where we do have shared_storage but not instance_path is also a no go for config drive. We need that check too now.","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"019e819facc3a801e2687d56c7a737837c5f4ff3","unresolved":false,"context_lines":[{"line_number":4580,"context_line":"            # NOTE(mikal): block migration of instances using config drive is"},{"line_number":4581,"context_line":"            # not supported because of a bug in libvirt (read only devices"},{"line_number":4582,"context_line":"            # are not copied by libvirt). See bug/1246201"},{"line_number":4583,"context_line":"            if configdrive.required_by(instance):"},{"line_number":4584,"context_line":"                raise exception.NoBlockMigrationForConfigDriveInLibVirt()"},{"line_number":4585,"context_line":""},{"line_number":4586,"context_line":"        if not is_shared_instance_path:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_e15b8e51","line":4583,"in_reply_to":"7adec928_45c29553","updated":"2014-05-20 12:36:40.000000000","message":"Yes actually you are right - still leaves us with a bogus exception tho - since we may not be using block migration, and confusing poor operators who will see misleading messages in their logs","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"1281d87bc10a47876d98bc73b71f25a052ab92e8","unresolved":false,"context_lines":[{"line_number":4580,"context_line":"            # NOTE(mikal): block migration of instances using config drive is"},{"line_number":4581,"context_line":"            # not supported because of a bug in libvirt (read only devices"},{"line_number":4582,"context_line":"            # are not copied by libvirt). See bug/1246201"},{"line_number":4583,"context_line":"            if configdrive.required_by(instance):"},{"line_number":4584,"context_line":"                raise exception.NoBlockMigrationForConfigDriveInLibVirt()"},{"line_number":4585,"context_line":""},{"line_number":4586,"context_line":"        if not is_shared_instance_path:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_45c29553","line":4583,"in_reply_to":"7adec928_9ec07951","updated":"2014-05-19 22:43:46.000000000","message":"Actually if is_shared_instance_path \u003d\u003d False and is_shared_block_storage \u003d\u003d True, then (is_shared_instance_path and is_shared_block_storage) \u003d\u003d False, negation of that will evaluate to True, and the config drive check will be invoked.","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"1f91e3cb2fb47debfe1d5e120983ab3d2029aa64","unresolved":false,"context_lines":[{"line_number":4580,"context_line":"            # NOTE(mikal): block migration of instances using config drive is"},{"line_number":4581,"context_line":"            # not supported because of a bug in libvirt (read only devices"},{"line_number":4582,"context_line":"            # are not copied by libvirt). See bug/1246201"},{"line_number":4583,"context_line":"            if configdrive.required_by(instance):"},{"line_number":4584,"context_line":"                raise exception.NoBlockMigrationForConfigDriveInLibVirt()"},{"line_number":4585,"context_line":""},{"line_number":4586,"context_line":"        if not is_shared_instance_path:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_8adaa1e7","line":4583,"in_reply_to":"7adec928_bddc141a","updated":"2014-05-22 06:09:03.000000000","message":"Maybe a bit, but if we left that in it would actually cause an exception that can carry wrong info. Thanks for fixing it.","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"ef2e8b20b189bae90ca3fc35ae7b3bd5d10674e2","unresolved":false,"context_lines":[{"line_number":4580,"context_line":"            # NOTE(mikal): block migration of instances using config drive is"},{"line_number":4581,"context_line":"            # not supported because of a bug in libvirt (read only devices"},{"line_number":4582,"context_line":"            # are not copied by libvirt). See bug/1246201"},{"line_number":4583,"context_line":"            if configdrive.required_by(instance):"},{"line_number":4584,"context_line":"                raise exception.NoBlockMigrationForConfigDriveInLibVirt()"},{"line_number":4585,"context_line":""},{"line_number":4586,"context_line":"        if not is_shared_instance_path:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_bddc141a","line":4583,"in_reply_to":"7adec928_e15b8e51","updated":"2014-05-21 20:33:29.000000000","message":"Well, you\u0027ve just slightly increased the scope of this commit even further :)\n\nI\u0027ve updated that exception to refer to live migrations instead of only block migrations.","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"bb1cd3243baa553f355ac6d7cccf29a1fccadabb","unresolved":false,"context_lines":[{"line_number":4314,"context_line":"        Returns true if the instance is volume backed and has no local disks,"},{"line_number":4315,"context_line":"        or if the image backend is the same on source and destination and the"},{"line_number":4316,"context_line":"        backend shares block storage between compute nodes."},{"line_number":4317,"context_line":"        \u0027\u0027\u0027"},{"line_number":4318,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4319,"context_line":"                self.image_backend.backend().is_shared_block_storage):"},{"line_number":4320,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":13,"id":"7adec928_248b5d69","line":4317,"updated":"2014-05-23 05:48:21.000000000","message":"Double quotes for doc strings.","commit_id":"aae89669a7d7a573c937042da00391b1c947cdc3"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"bb1cd3243baa553f355ac6d7cccf29a1fccadabb","unresolved":false,"context_lines":[{"line_number":4329,"context_line":"    def _is_shared_instance_path(self, dest_check_data):"},{"line_number":4330,"context_line":"        \u0027\u0027\u0027Check if instance path is shared between source and"},{"line_number":4331,"context_line":"        destination of a live migration."},{"line_number":4332,"context_line":"        \u0027\u0027\u0027"},{"line_number":4333,"context_line":"        return self._check_shared_storage_test_file("},{"line_number":4334,"context_line":"                    dest_check_data[\"filename\"])"},{"line_number":4335,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7adec928_84f35109","line":4332,"updated":"2014-05-23 05:48:21.000000000","message":"Ditto.","commit_id":"aae89669a7d7a573c937042da00391b1c947cdc3"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f6ebcd93905c23207b0a62958e9c3a386bc05f07","unresolved":false,"context_lines":[{"line_number":4284,"context_line":"        dest_check_data.update({\u0027is_shared_block_storage\u0027:"},{"line_number":4285,"context_line":"                self._is_shared_block_storage(instance, dest_check_data)})"},{"line_number":4286,"context_line":"        dest_check_data.update({\u0027is_shared_instance_path\u0027:"},{"line_number":4287,"context_line":"                self._is_shared_instance_path(dest_check_data)})"},{"line_number":4288,"context_line":""},{"line_number":4289,"context_line":"        if dest_check_data[\u0027block_migration\u0027]:"},{"line_number":4290,"context_line":"            if (dest_check_data[\u0027is_shared_block_storage\u0027] or"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_2f424810","line":4287,"updated":"2014-06-10 17:19:58.000000000","message":"The data populated here is being passed from host to host via the RPC service, via the virt driver specific migrate_data dict. By renaming the dict keys here you have basically changed the RPC API contract, but not changed the RPC version. So now old host can\u0027t properly migrate to new host and vica-verca. I think we must never change this data to avoid compat problems.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"dc1f67401e55df1f082c6a95e50ac459a47d4a80","unresolved":false,"context_lines":[{"line_number":4284,"context_line":"        dest_check_data.update({\u0027is_shared_block_storage\u0027:"},{"line_number":4285,"context_line":"                self._is_shared_block_storage(instance, dest_check_data)})"},{"line_number":4286,"context_line":"        dest_check_data.update({\u0027is_shared_instance_path\u0027:"},{"line_number":4287,"context_line":"                self._is_shared_instance_path(dest_check_data)})"},{"line_number":4288,"context_line":""},{"line_number":4289,"context_line":"        if dest_check_data[\u0027block_migration\u0027]:"},{"line_number":4290,"context_line":"            if (dest_check_data[\u0027is_shared_block_storage\u0027] or"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_fa14854e","line":4287,"in_reply_to":"1ae5cdf2_05cbad1e","updated":"2014-06-11 08:20:15.000000000","message":"This is the problem of versioning the return values of rpc calls and I think there is no good way to do this without objects (just looking at the code now it will work for backporting to pre-object data too, but it\u0027s a bit contrived and dirty - the whole magic is in the NovaObjectSerializer class - check it out) at the moment, so really you would need to make dest_check_data object for this to work, which is not totally a horrible idea and definitely better than writing yet another (poor) implementation of objects versioning code here :)","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ad81e7f5caf7f3a26521e7e94cc53e6ad41e3ca4","unresolved":false,"context_lines":[{"line_number":4284,"context_line":"        dest_check_data.update({\u0027is_shared_block_storage\u0027:"},{"line_number":4285,"context_line":"                self._is_shared_block_storage(instance, dest_check_data)})"},{"line_number":4286,"context_line":"        dest_check_data.update({\u0027is_shared_instance_path\u0027:"},{"line_number":4287,"context_line":"                self._is_shared_instance_path(dest_check_data)})"},{"line_number":4288,"context_line":""},{"line_number":4289,"context_line":"        if dest_check_data[\u0027block_migration\u0027]:"},{"line_number":4290,"context_line":"            if (dest_check_data[\u0027is_shared_block_storage\u0027] or"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_2913230e","line":4287,"in_reply_to":"1ae5cdf2_2f424810","updated":"2014-06-10 18:31:23.000000000","message":"Agreed, should be handling legacy items here for backwards compatibility.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"7bc5747a23ae0e21b0dca7195e71d661ab4b629c","unresolved":false,"context_lines":[{"line_number":4284,"context_line":"        dest_check_data.update({\u0027is_shared_block_storage\u0027:"},{"line_number":4285,"context_line":"                self._is_shared_block_storage(instance, dest_check_data)})"},{"line_number":4286,"context_line":"        dest_check_data.update({\u0027is_shared_instance_path\u0027:"},{"line_number":4287,"context_line":"                self._is_shared_instance_path(dest_check_data)})"},{"line_number":4288,"context_line":""},{"line_number":4289,"context_line":"        if dest_check_data[\u0027block_migration\u0027]:"},{"line_number":4290,"context_line":"            if (dest_check_data[\u0027is_shared_block_storage\u0027] or"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_05cbad1e","line":4287,"in_reply_to":"1ae5cdf2_2f424810","updated":"2014-06-10 19:09:47.000000000","message":"I did bump the RPC API version, but by the time these fields are examined it\u0027s too late to compare API versions. As discussed with mriedem on IRC, the only way to make this backwards compatible is to account for both old and new fields. Please let me know if you think there\u0027s a better way.","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f6ebcd93905c23207b0a62958e9c3a386bc05f07","unresolved":false,"context_lines":[{"line_number":4320,"context_line":"        backend shares block storage between compute nodes."},{"line_number":4321,"context_line":"        \"\"\""},{"line_number":4322,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4323,"context_line":"                self.image_backend.backend().is_shared_block_storage):"},{"line_number":4324,"context_line":"            return True"},{"line_number":4325,"context_line":""},{"line_number":4326,"context_line":"        if (dest_check_data.get(\u0027is_volume_backed\u0027) and"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_cf9ac46d","line":4323,"updated":"2014-06-10 17:19:58.000000000","message":"Can\u0027t help thinking that just comparing images_type is rather too simplistic. Both hosts can be configured with RBD images type but there\u0027s nothing saying they\u0027re pointing to the same RBD server / pool","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"7bc5747a23ae0e21b0dca7195e71d661ab4b629c","unresolved":false,"context_lines":[{"line_number":4320,"context_line":"        backend shares block storage between compute nodes."},{"line_number":4321,"context_line":"        \"\"\""},{"line_number":4322,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4323,"context_line":"                self.image_backend.backend().is_shared_block_storage):"},{"line_number":4324,"context_line":"            return True"},{"line_number":4325,"context_line":""},{"line_number":4326,"context_line":"        if (dest_check_data.get(\u0027is_volume_backed\u0027) and"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ae5cdf2_2cec118f","line":4323,"in_reply_to":"1ae5cdf2_cf9ac46d","updated":"2014-06-10 19:09:47.000000000","message":"I\u0027ve thought about that, but that would require passing even more driver specific (imagebackend specific even) data in the migrate_data dict. Given that the use case in question (\"rbd driver with different clusters/pools on different nodes\") is a strict subset of the \"rbd driver\" use-case, I think fixing just the \"rbd driver with the same cluster\u0026pool\" sub-case is a reasonable trade-off.\n\nAlso, the impact of leaving the \"different clusters/pools\" sub-case unaccounted for here is that ephemeral disks will not be cleaned up from Ceph on source (meaning loss of disk space + possible failure to migrate back), which is significantly better than the current behaviour in the more common \"same cluster\u0026pool\" sub-case: ephemeral disks get erased from Ceph on live migration (loss of instance + possible data loss).","commit_id":"7f245b4b3eecd20bb400215270cde99b4e3a462a"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"6e6184f2824c93da220fa9f243aa20641009dda4","unresolved":false,"context_lines":[{"line_number":4283,"context_line":"        backend shares block storage between compute nodes."},{"line_number":4284,"context_line":"        \"\"\""},{"line_number":4285,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4286,"context_line":"                self.image_backend.backend().is_shared_block_storage):"},{"line_number":4287,"context_line":"            return True"},{"line_number":4288,"context_line":""},{"line_number":4289,"context_line":"        if (dest_check_data.get(\u0027is_volume_backed\u0027) and"}],"source_content_type":"text/x-python","patch_set":31,"id":"baada198_ffd1f757","line":4286,"updated":"2014-07-02 16:26:01.000000000","message":"backend() returns the class not the object so property is a property object that evaluates to true and breaks block live mig!","commit_id":"57f87d93ea5d41a8f54273e9e73ddc7e09f39b82"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"006bc55f3f3947db5f00a1ea4435c20e7e44cf8a","unresolved":false,"context_lines":[{"line_number":4283,"context_line":"        backend shares block storage between compute nodes."},{"line_number":4284,"context_line":"        \"\"\""},{"line_number":4285,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4286,"context_line":"                self.image_backend.backend().is_shared_block_storage):"},{"line_number":4287,"context_line":"            return True"},{"line_number":4288,"context_line":""},{"line_number":4289,"context_line":"        if (dest_check_data.get(\u0027is_volume_backed\u0027) and"}],"source_content_type":"text/x-python","patch_set":31,"id":"baada198_027652a4","line":4286,"in_reply_to":"baada198_ffd1f757","updated":"2014-07-02 16:43:57.000000000","message":"Actual evidence that too much review is just as bad as not enough :) You were the one to suggest to change this from staticmethod to property in patch set 10... Done.","commit_id":"57f87d93ea5d41a8f54273e9e73ddc7e09f39b82"},{"author":{"_account_id":8874,"name":"ling-yun","email":"YunlingZeng@hotmail.com","username":"ling-yun"},"change_message_id":"6c2405342b75e22d36fa9c7d600b094ac9f88d34","unresolved":false,"context_lines":[{"line_number":4269,"context_line":"        \"\"\""},{"line_number":4270,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4271,"context_line":"                self.image_backend.backend().is_shared_block_storage()):"},{"line_number":4272,"context_line":"            return True"},{"line_number":4273,"context_line":""},{"line_number":4274,"context_line":"        if (dest_check_data.get(\u0027is_volume_backed\u0027) and"},{"line_number":4275,"context_line":"                not bool(jsonutils.loads("}],"source_content_type":"text/x-python","patch_set":33,"id":"baada198_451f4d08","line":4272,"updated":"2014-07-03 14:26:53.000000000","message":"If src compute host and dest compute host connect to different ceph rbd pool or different ceph rados cluster, it would cause live-migrate fail.\n\nI think dest compute host should import a file into ceph rbd pool A and src compute host check whether the file(a rbd image) exist in ceph rbd pool A, so as to determine whehter src compute host and dest compute connect to the same ceph rbd pool.","commit_id":"d7ba8853524a9518802f4fd77b336096a85101c4"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"81e3b2e567d4fe289d913e465c186993b5cf7c79","unresolved":false,"context_lines":[{"line_number":4269,"context_line":"        \"\"\""},{"line_number":4270,"context_line":"        if (CONF.libvirt.images_type \u003d\u003d dest_check_data.get(\u0027image_type\u0027) and"},{"line_number":4271,"context_line":"                self.image_backend.backend().is_shared_block_storage()):"},{"line_number":4272,"context_line":"            return True"},{"line_number":4273,"context_line":""},{"line_number":4274,"context_line":"        if (dest_check_data.get(\u0027is_volume_backed\u0027) and"},{"line_number":4275,"context_line":"                not bool(jsonutils.loads("}],"source_content_type":"text/x-python","patch_set":33,"id":"baada198_0090dc48","line":4272,"in_reply_to":"baada198_451f4d08","updated":"2014-07-03 17:10:56.000000000","message":"This is outside of scope of this fix, see the discussion in previous patch sets, for example patch set 8.\n\nThis patch is fixing a data loss problem in case of live migration over RBD or other shared storage except NFS. In the case of different Ceph clusters between source and destination, live migration is not possible to begin with. Live migration between different RBD pools of the same Ceph cluster is theoretically possible, but without this patch it would lead to VM\u0027s disks being erased from Ceph. In other words, both outcomes are not as critical as the fact that with this fix, possibility of data loss is eliminated.\n\nI also disagree with the proposed solution. We have to check shared storage this way with NFS simply because there\u0027s no reliable way to identify NFS servers. With Ceph, each cluster has a globally unique fsid, all we need is to pass it around as part of instance metadata.","commit_id":"d7ba8853524a9518802f4fd77b336096a85101c4"},{"author":{"_account_id":7494,"name":"Guangya Liu","email":"gyliu513@gmail.com","username":"JayLau"},"change_message_id":"fa8aaf81cce6c692f74ad906cd2e76480d4b4c3f","unresolved":false,"context_lines":[{"line_number":4657,"context_line":"        if not is_shared_block_storage:"},{"line_number":4658,"context_line":"            # Ensure images and backing files are present."},{"line_number":4659,"context_line":"            self._create_images_and_backing(context, instance,"},{"line_number":4660,"context_line":"                                            instance_dir, disk_info)"},{"line_number":4661,"context_line":""},{"line_number":4662,"context_line":"        if not (is_block_migration or is_shared_instance_path):"},{"line_number":4663,"context_line":"            # NOTE(angdraug): when block storage is shared between source and"}],"source_content_type":"text/x-python","patch_set":37,"id":"baada198_1989ce25","line":4660,"updated":"2014-07-22 07:41:09.000000000","message":"There seems to be some logic error here: take a look at instance_dir, if I was using shared instance path with QCOW image, then instance_dir will never be defined and _create_images_and_backing will throw exception.","commit_id":"bc45c56f102cdef58840e02b609a89f5278e8cce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d02d8cf1da6ff3a13dc3f8c044882c1e05f61f4a","unresolved":false,"context_lines":[{"line_number":4657,"context_line":"        if not is_shared_block_storage:"},{"line_number":4658,"context_line":"            # Ensure images and backing files are present."},{"line_number":4659,"context_line":"            self._create_images_and_backing(context, instance,"},{"line_number":4660,"context_line":"                                            instance_dir, disk_info)"},{"line_number":4661,"context_line":""},{"line_number":4662,"context_line":"        if not (is_block_migration or is_shared_instance_path):"},{"line_number":4663,"context_line":"            # NOTE(angdraug): when block storage is shared between source and"}],"source_content_type":"text/x-python","patch_set":37,"id":"baada198_6fa72e51","line":4660,"in_reply_to":"baada198_1989ce25","updated":"2014-07-22 20:50:31.000000000","message":"Yeah it\u0027s a regression, see:\n\nhttps://review.openstack.org/#/c/108404/","commit_id":"bc45c56f102cdef58840e02b609a89f5278e8cce"}],"nova/virt/libvirt/imagebackend.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"cbdbf2e6be690a82794fc80b4fe3133919b63ae6","unresolved":false,"context_lines":[{"line_number":306,"context_line":"        return driver_format"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    @staticmethod"},{"line_number":309,"context_line":"    def is_shared_block_storage():"},{"line_number":310,"context_line":"        \u0027\u0027\u0027Return True if the backend puts images on a shared block storage"},{"line_number":311,"context_line":"        \u0027\u0027\u0027"},{"line_number":312,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":8,"id":"7adec928_843aca34","line":309,"updated":"2014-05-07 15:26:10.000000000","message":"Nice - but see my comment in libvirt/driver.py","commit_id":"fdbdce4f03d727f1094df206f6241f5ca2d2cd23"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"930a0ff8554e0381dcbd659e38c080ff41659c66","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            raise exception.DiskInfoReadWriteFail(reason\u003dunicode(e))"},{"line_number":306,"context_line":"        return driver_format"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    @staticmethod"},{"line_number":309,"context_line":"    def is_shared_block_storage():"},{"line_number":310,"context_line":"        \u0027\u0027\u0027Return True if the backend puts images on a shared block storage"},{"line_number":311,"context_line":"        \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_704d9c99","line":308,"updated":"2014-05-19 17:39:56.000000000","message":"Nit: we could make this a propery instead maybe","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":8787,"name":"Dmitry Borodaenko","email":"angdraug@gmail.com","username":"angdraug"},"change_message_id":"ef2e8b20b189bae90ca3fc35ae7b3bd5d10674e2","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            raise exception.DiskInfoReadWriteFail(reason\u003dunicode(e))"},{"line_number":306,"context_line":"        return driver_format"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    @staticmethod"},{"line_number":309,"context_line":"    def is_shared_block_storage():"},{"line_number":310,"context_line":"        \u0027\u0027\u0027Return True if the backend puts images on a shared block storage"},{"line_number":311,"context_line":"        \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_dd93a04d","line":308,"in_reply_to":"7adec928_704d9c99","updated":"2014-05-21 20:33:29.000000000","message":"Done","commit_id":"15b973693be65880dc67cf6a5fef65500bbcb052"},{"author":{"_account_id":1313,"name":"Yaguang Tang","email":"heut2008@gmail.com","username":"heut2008"},"change_message_id":"e908cea1c042983c28aa2cfffabee7dca3b688b1","unresolved":false,"context_lines":[{"line_number":694,"context_line":""},{"line_number":695,"context_line":"    @property"},{"line_number":696,"context_line":"    def is_shared_block_storage(self):"},{"line_number":697,"context_line":"        return True"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"class Backend(object):"}],"source_content_type":"text/x-python","patch_set":12,"id":"7adec928_c49e6063","line":697,"updated":"2014-05-22 03:07:14.000000000","message":"I think this method should be added in Rbd image backend or live migration with rbd backend still fails duo to pre check.","commit_id":"13db90e0f18461d593604269b0fcdb0794f98670"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"ecc84b1d6ad6f03b2977f90456ba9cc17e8ea01e","unresolved":false,"context_lines":[{"line_number":694,"context_line":""},{"line_number":695,"context_line":"    @property"},{"line_number":696,"context_line":"    def is_shared_block_storage(self):"},{"line_number":697,"context_line":"        return True"},{"line_number":698,"context_line":""},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"class Backend(object):"}],"source_content_type":"text/x-python","patch_set":12,"id":"7adec928_2a650d45","line":697,"in_reply_to":"7adec928_c49e6063","updated":"2014-05-22 06:42:45.000000000","message":"Ummm... this _is_ the Rbd image backend?!","commit_id":"13db90e0f18461d593604269b0fcdb0794f98670"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bcfbb468e7f696983cd0af45da9f5e96c3a03a20","unresolved":false,"context_lines":[{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    @property"},{"line_number":309,"context_line":"    def is_shared_block_storage(self):"},{"line_number":310,"context_line":"        \u0027\u0027\u0027True if the backend puts images on a shared block storage"},{"line_number":311,"context_line":"        \u0027\u0027\u0027"},{"line_number":312,"context_line":"        return False"},{"line_number":313,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"1ae5cdf2_f3797494","line":310,"updated":"2014-05-30 17:21:06.000000000","message":"single quotes here but we can fix it later","commit_id":"3bf541c6351640608e868ed90e49a30f38ffe7c1"}],"nova/virt/vmwareapi/driver.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"e635c4c3bc942978650dd6a53874bffc3932e421","unresolved":false,"context_lines":[{"line_number":661,"context_line":"        \"\"\"Reboot VM instance.\"\"\""},{"line_number":662,"context_line":"        _vmops \u003d self._get_vmops_for_compute_node(instance[\u0027node\u0027])"},{"line_number":663,"context_line":"        _vmops.reboot(instance, network_info)"},{"line_number":664,"context_line":""},{"line_number":665,"context_line":"    def destroy(self, context, instance, network_info, block_device_info\u003dNone,"},{"line_number":666,"context_line":"                destroy_disks\u003dTrue):"},{"line_number":667,"context_line":"        \"\"\"Destroy VM instance.\"\"\""}],"source_content_type":"text/x-python","patch_set":32,"id":"baada198_dc55fdfa","line":664,"updated":"2014-07-03 12:01:19.000000000","message":"this API needs to be updated. same for the one below","commit_id":"73739035536a5fdad211edd9c2d4adf160bbc2fa"}]}