)]}'
{"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7dd7762edf710af8ba68c29a933718f37bccddbf","unresolved":false,"context_lines":[{"line_number":5750,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5751,"context_line":"                # MigrationContext on the Instance"},{"line_number":5752,"context_line":"                new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":5753,"context_line":"                    guest, migrate_data, self._get_volume_config)"},{"line_number":5754,"context_line":"            if self._host.has_min_version("},{"line_number":5755,"context_line":"                    MIN_LIBVIRT_BLOCK_LM_WITH_VOLUMES_VERSION):"},{"line_number":5756,"context_line":"                params \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_b53c86c0","line":5753,"range":{"start_line":5753,"start_character":41,"end_line":5753,"end_character":64},"updated":"2016-06-15 16:54:53.000000000","message":"I\u0027m pretty sure you meant to pass self here.","commit_id":"938af4a67844681d4b497c6763a482991cfa2d78"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9d707acdd7307ca55baf3e31fbc4c8d6c821b838","unresolved":false,"context_lines":[{"line_number":5750,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5751,"context_line":"                # MigrationContext on the Instance"},{"line_number":5752,"context_line":"                new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":5753,"context_line":"                    guest, migrate_data, self._get_volume_config)"},{"line_number":5754,"context_line":"            if self._host.has_min_version("},{"line_number":5755,"context_line":"                    MIN_LIBVIRT_BLOCK_LM_WITH_VOLUMES_VERSION):"},{"line_number":5756,"context_line":"                params \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_959d0a0d","line":5753,"in_reply_to":"7aa08908_55e72246","updated":"2016-06-15 17:11:55.000000000","message":"Yes.. I need some rest. As we have discussed on IRC your point to pass a datastructure with whatever is needed seem to be a clearly better idea. Thanks for you review.","commit_id":"938af4a67844681d4b497c6763a482991cfa2d78"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"aba73c1246f54dc9ca9fefc2e5a4e136bd1e62cb","unresolved":false,"context_lines":[{"line_number":5750,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5751,"context_line":"                # MigrationContext on the Instance"},{"line_number":5752,"context_line":"                new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":5753,"context_line":"                    guest, migrate_data, self._get_volume_config)"},{"line_number":5754,"context_line":"            if self._host.has_min_version("},{"line_number":5755,"context_line":"                    MIN_LIBVIRT_BLOCK_LM_WITH_VOLUMES_VERSION):"},{"line_number":5756,"context_line":"                params \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_e9029971","line":5753,"in_reply_to":"7aa08908_959d0a0d","updated":"2016-06-16 09:16:03.000000000","message":"Hum after look at it, it\u0027s little bit more complicated. If we take example of volumes; We need to go through dbms and for each retrieve the libvirtconfig associated. It\u0027s of a \"lot of logic\" to do before to construct the list of libvirtconfig the method update_volume_xml is needed for.\n\nI have an other idea based on your comment, we can use a named tuple to pass references of needed methods from the driver. Something like:\n\nDriverMigrationInterface \u003d collections.namedtuple(\n    \u0027DriverMigrationInterface\u0027, [\u0027get_volume_config\u0027, \u0027get_numa_topology_config\u0027])\n\nI\u0027m going to try in that way, and see with you what do you think.","commit_id":"938af4a67844681d4b497c6763a482991cfa2d78"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d03a744b05039c14242bc199f5909b108c798d5f","unresolved":false,"context_lines":[{"line_number":5750,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5751,"context_line":"                # MigrationContext on the Instance"},{"line_number":5752,"context_line":"                new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":5753,"context_line":"                    guest, migrate_data, self._get_volume_config)"},{"line_number":5754,"context_line":"            if self._host.has_min_version("},{"line_number":5755,"context_line":"                    MIN_LIBVIRT_BLOCK_LM_WITH_VOLUMES_VERSION):"},{"line_number":5756,"context_line":"                params \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_b563e6dc","line":5753,"range":{"start_line":5753,"start_character":41,"end_line":5753,"end_character":64},"in_reply_to":"7aa08908_b53c86c0","updated":"2016-06-15 16:55:46.000000000","message":"Rather than do this, why not just pass the volume config into the get_updated_guest_xml method? Passing the driver around to perform callbacks seems pretty fragile.","commit_id":"938af4a67844681d4b497c6763a482991cfa2d78"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4363bcdce42e1cc8e3b7c72b2725bc168b41a0b7","unresolved":false,"context_lines":[{"line_number":5750,"context_line":"                # recalculated NUMA/CPU pinning information that is stored in"},{"line_number":5751,"context_line":"                # MigrationContext on the Instance"},{"line_number":5752,"context_line":"                new_xml_str \u003d libvirt_migrate.get_updated_guest_xml("},{"line_number":5753,"context_line":"                    guest, migrate_data, self._get_volume_config)"},{"line_number":5754,"context_line":"            if self._host.has_min_version("},{"line_number":5755,"context_line":"                    MIN_LIBVIRT_BLOCK_LM_WITH_VOLUMES_VERSION):"},{"line_number":5756,"context_line":"                params \u003d {"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_55e72246","line":5753,"range":{"start_line":5753,"start_character":41,"end_line":5753,"end_character":64},"in_reply_to":"7aa08908_b563e6dc","updated":"2016-06-15 16:57:18.000000000","message":"Nevermind, I see why it\u0027s passed in.","commit_id":"938af4a67844681d4b497c6763a482991cfa2d78"}],"nova/virt/libvirt/migration.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4363bcdce42e1cc8e3b7c72b2725bc168b41a0b7","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            not bdm_info or not bdm_info.connection_info or"},{"line_number":96,"context_line":"            serial_source not in bdm_info_by_serial):"},{"line_number":97,"context_line":"            continue"},{"line_number":98,"context_line":"        conf \u003d driver._get_volume_config("},{"line_number":99,"context_line":"            bdm_info.connection_info, bdm_info.as_disk_info())"},{"line_number":100,"context_line":"        xml_doc2 \u003d etree.XML(conf.to_xml(), parser)"},{"line_number":101,"context_line":"        serial_dest \u003d xml_doc2.findtext(\u0027serial\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_158f5a76","line":98,"updated":"2016-06-15 16:57:18.000000000","message":"Oh I guess because we\u0027re enumerating a list of disk nodes here, which isn\u0027t something the driver does and could then pass in.","commit_id":"938af4a67844681d4b497c6763a482991cfa2d78"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9c79d14faa061f7647be43142fc3d2beed9c2827","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"DriverInterface \u003d namedtuple("},{"line_number":27,"context_line":"    \u0027DriverInterface\u0027, [\u0027get_volume_config\u0027, ])"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_15e9f6b7","line":26,"updated":"2016-06-16 10:24:28.000000000","message":"If when find a better name I\u0027m OK to change it for sure (in the hypothesis we are agree about it resolves the issue).","commit_id":"ac6b206749e2ba3b3878d7a3226ed75de3f663f4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"aa7a8894eb1c2c380c6083dcdbde660d7e05a2bd","unresolved":false,"context_lines":[{"line_number":34,"context_line":"libvirt \u003d None"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DriverInterface \u003d namedtuple("},{"line_number":37,"context_line":"    \u0027DriverInterface\u0027, [\u0027get_volume_config\u0027, ])"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def graphics_listen_addrs(migrate_data):"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_6646407c","line":37,"updated":"2016-08-01 16:38:24.000000000","message":"This is exceedingly odd coding style. What precisely are you attempting to do here? Even in the follow-up patch, which I +2\u0027d, it\u0027s really not clear how this makes the code any easier to read or follow.\n\nAre you attempting to define an internal API for migrations to follow? If so, this is really a poor way of doing that, especially with zero docstrings or comments indicating what it is supposed to be for.","commit_id":"5577095eac6341c4f7b253ec829a5c12db6f3b93"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d79419f43ee06444ccd12c16ec7194e6298cc95c","unresolved":false,"context_lines":[{"line_number":34,"context_line":"libvirt \u003d None"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DriverInterface \u003d namedtuple("},{"line_number":37,"context_line":"    \u0027DriverInterface\u0027, [\u0027get_volume_config\u0027, ])"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def graphics_listen_addrs(migrate_data):"}],"source_content_type":"text/x-python","patch_set":12,"id":"3ac371cc_15e36c34","line":37,"in_reply_to":"bacf61ea_6646407c","updated":"2016-08-17 09:44:51.000000000","message":"Some methods of this module are going to require methods from an instance of libvirt driver which are defined as private I wanted to avoid passing the reference and using them like that so I started to bring that idea which defines a tuple of methods passed to this module.\n\nPerhaps is not the best solution and I should just pass the driver reference and change the visibility of these methods. I really don\u0027t have a strong opinion.","commit_id":"5577095eac6341c4f7b253ec829a5c12db6f3b93"}]}