)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"25671fd8cb8e317226ab5189df42f2e40a8eab86","unresolved":false,"context_lines":[{"line_number":7,"context_line":"libvirt: clean serial listen addr to always deal with string"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the MigrateData objects have been introduced the field"},{"line_number":10,"context_line":"\u0027serial_listen_addr\u0027 could not exist on compute nodes which do not"},{"line_number":11,"context_line":"have been upgraded so an utility method has been introduced to verify"},{"line_number":12,"context_line":"if the field is set or return None. Now we should never be in the case"},{"line_number":13,"context_line":"where the field is not set and we do not want to deal with cases where"},{"line_number":14,"context_line":"that field could have been set to None or an empty string. This commit"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a77a97e_d21ef30a","line":11,"range":{"start_line":10,"start_character":54,"end_line":11,"end_character":18},"updated":"2016-11-18 17:39:26.000000000","message":"\"which have not been upgraded\"","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"6dadeb48effb79d1438a263323373db8f35989f0","unresolved":false,"context_lines":[{"line_number":12,"context_line":"if the field is set or return None. Now we should never be in the case"},{"line_number":13,"context_line":"where the field is not set and we do not want to deal with cases where"},{"line_number":14,"context_line":"that field could have been set to None or an empty string. This commit"},{"line_number":15,"context_line":"clean the code to always use an empty string."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ib3b2d96dc9063ad2e04bc9d70b3aa01de6db5158"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a77a97e_200de337","line":15,"range":{"start_line":15,"start_character":18,"end_line":15,"end_character":44},"updated":"2016-11-21 10:50:24.000000000","message":"Shouldn\u0027t that also include to remove the \"nullable\u003dTrue\" from the field \"serial_listen_addr\" in LibvirtLiveMigrateData? https://github.com/openstack/nova/blob/4eb89c206e68a7172ebad897ad24769036c7bdd6/nova/objects/migrate_data.py#L127-L127","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"fc3acee6fd776ac38835109637f42f37fd4ff69e","unresolved":false,"context_lines":[{"line_number":12,"context_line":"if the field is set or return None. Now we should never be in the case"},{"line_number":13,"context_line":"where the field is not set and we do not want to deal with cases where"},{"line_number":14,"context_line":"that field could have been set to None or an empty string. This commit"},{"line_number":15,"context_line":"clean the code to always use an empty string."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ib3b2d96dc9063ad2e04bc9d70b3aa01de6db5158"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a77a97e_275b3530","line":15,"in_reply_to":"7a77a97e_200de337","updated":"2016-11-21 12:16:32.000000000","message":"I guess no, we can still have in database rows with that field set to NULL","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"31344cdc491473045807844ea85e998d5631ca59","unresolved":false,"context_lines":[{"line_number":12,"context_line":"if the field is set or return None. Now we should never be in the case"},{"line_number":13,"context_line":"where the field is not set and we do not want to deal with cases where"},{"line_number":14,"context_line":"that field could have been set to None or an empty string. This commit"},{"line_number":15,"context_line":"clean the code to always use an empty string."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ib3b2d96dc9063ad2e04bc9d70b3aa01de6db5158"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a77a97e_679fed50","line":15,"in_reply_to":"7a77a97e_275b3530","updated":"2016-11-21 12:29:12.000000000","message":"Oh, I didn\u0027t know that the migration data objects get persisted in the database. I assumed the migration data object gets created one time, sent over via RPC and then dismissed. Thanks for the info.","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"25671fd8cb8e317226ab5189df42f2e40a8eab86","unresolved":false,"context_lines":[{"line_number":7737,"context_line":"            config \u003d libvirt_migrate.get_updated_guest_xml(guest,"},{"line_number":7738,"context_line":"                            objects.LibvirtLiveMigrateData("},{"line_number":7739,"context_line":"                                bdms\u003d[bdmi],"},{"line_number":7740,"context_line":"                                serial_listen_addr\u003d\u0027\u0027),"},{"line_number":7741,"context_line":"                            drvr._get_volume_config)"},{"line_number":7742,"context_line":"            parser \u003d etree.XMLParser(remove_blank_text\u003dTrue)"},{"line_number":7743,"context_line":"            config \u003d etree.fromstring(config, parser)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_92847b0b","line":7740,"updated":"2016-11-18 17:39:26.000000000","message":"When would this ever be the empty string?","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"6dadeb48effb79d1438a263323373db8f35989f0","unresolved":false,"context_lines":[{"line_number":7737,"context_line":"            config \u003d libvirt_migrate.get_updated_guest_xml(guest,"},{"line_number":7738,"context_line":"                            objects.LibvirtLiveMigrateData("},{"line_number":7739,"context_line":"                                bdms\u003d[bdmi],"},{"line_number":7740,"context_line":"                                serial_listen_addr\u003d\u0027\u0027),"},{"line_number":7741,"context_line":"                            drvr._get_volume_config)"},{"line_number":7742,"context_line":"            parser \u003d etree.XMLParser(remove_blank_text\u003dTrue)"},{"line_number":7743,"context_line":"            config \u003d etree.fromstring(config, parser)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_e07ccbed","line":7740,"in_reply_to":"7a77a97e_92847b0b","updated":"2016-11-21 10:50:24.000000000","message":"@Dan: `serial_listen_addr\u003d\u0027\u0027` should happen when a live-migration to a target host with `CONF.serial_console.enabled \u003d\u003d False` is happening. IOW, `serial_listen_addr\u003d\u0027\u0027` is the signal to abort the live-migration to that host by throwing a MigrationError in \nhttps://github.com/openstack/nova/blob/d4fc68d75d11b42639481941e231d1632f59ef53/nova/virt/libvirt/driver.py#L5907-L5914 \nduring `_live_migration_operation`. \n\nBe aware that this is currently a no-op because `listen_addrs` at \nhttps://github.com/openstack/nova/blob/d4fc68d75d11b42639481941e231d1632f59ef53/nova/virt/libvirt/driver.py#L5941-L5944 \nis *never* False, because this value is *always* a dict: \nhttps://github.com/openstack/nova/blob/4f33047d07f5a11b208c344fe206aba01cd8e6fe/nova/virt/libvirt/migration.py#L36-L47","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"241387e3e2d77f94c17b66e1ce6894e324a89294","unresolved":false,"context_lines":[{"line_number":7737,"context_line":"            config \u003d libvirt_migrate.get_updated_guest_xml(guest,"},{"line_number":7738,"context_line":"                            objects.LibvirtLiveMigrateData("},{"line_number":7739,"context_line":"                                bdms\u003d[bdmi],"},{"line_number":7740,"context_line":"                                serial_listen_addr\u003d\u0027\u0027),"},{"line_number":7741,"context_line":"                            drvr._get_volume_config)"},{"line_number":7742,"context_line":"            parser \u003d etree.XMLParser(remove_blank_text\u003dTrue)"},{"line_number":7743,"context_line":"            config \u003d etree.fromstring(config, parser)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_bd48408c","line":7740,"in_reply_to":"7a77a97e_92847b0b","updated":"2016-11-21 10:30:15.000000000","message":"We are not testing on that field that would be anything","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"84435f9df3e4ab2667b389132db746923299b38f","unresolved":false,"context_lines":[{"line_number":7737,"context_line":"            config \u003d libvirt_migrate.get_updated_guest_xml(guest,"},{"line_number":7738,"context_line":"                            objects.LibvirtLiveMigrateData("},{"line_number":7739,"context_line":"                                bdms\u003d[bdmi],"},{"line_number":7740,"context_line":"                                serial_listen_addr\u003d\u0027\u0027),"},{"line_number":7741,"context_line":"                            drvr._get_volume_config)"},{"line_number":7742,"context_line":"            parser \u003d etree.XMLParser(remove_blank_text\u003dTrue)"},{"line_number":7743,"context_line":"            config \u003d etree.fromstring(config, parser)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_f9bd5dd8","line":7740,"in_reply_to":"7a77a97e_e07ccbed","updated":"2016-11-21 15:23:37.000000000","message":"Sahid\u0027s point is fine I guess.","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"}],"nova/virt/libvirt/migration.py":[{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"121b0e763765e57620be4f7e55ade1eee2bd0229","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d str(migrate_data.serial_listen_addr)"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_9a421f67","line":75,"range":{"start_line":75,"start_character":18,"end_line":75,"end_character":22},"updated":"2016-11-17 10:24:24.000000000","message":"Isn\u0027t this string conversion redundant? serial_listen_addr is a string field and it is always populated with at least empty string.","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"84435f9df3e4ab2667b389132db746923299b38f","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d str(migrate_data.serial_listen_addr)"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_195779cb","line":75,"in_reply_to":"7a77a97e_3d0b502d","updated":"2016-11-21 15:23:37.000000000","message":"Are you saying you\u0027re removing this because you know that liberty-\u003eocata migrations won\u0027t happen? You can\u0027t really make that judgment because the major version of the object hasn\u0027t been bumped and the from_legacy compat code is still in place, so you really should continue to accept old objects, IMHO.","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"241387e3e2d77f94c17b66e1ce6894e324a89294","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d str(migrate_data.serial_listen_addr)"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_3d0b502d","line":75,"in_reply_to":"7a77a97e_72afdf7c","updated":"2016-11-21 10:30:15.000000000","message":"Actually it can\u0027t, serial_listen_addr has been introduced for Mitaka with the code to set it based on CONF.serial_console.proxyclient_address which is a string. We have added that guard because Liberty would have not set that field in case of a live migration from Mitaka to Liberty during an upgrade.\n\nOr do you see a case where it can still be not set ?","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"074002f6133fea875e22be749fbe7609a9961d76","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d str(migrate_data.serial_listen_addr)"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_bdb7d5f3","line":75,"range":{"start_line":75,"start_character":18,"end_line":75,"end_character":22},"in_reply_to":"7a77a97e_9a421f67","updated":"2016-11-17 10:32:23.000000000","message":"Agree, at least we don\u0027t end up with \"None\" string here by any chance","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e4773f13eafee8c29d3052638070d16b2919cae3","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d str(migrate_data.serial_listen_addr)"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_c304ae4b","line":75,"in_reply_to":"7a77a97e_bdb7d5f3","updated":"2016-11-17 11:31:20.000000000","message":"Ok I will change that point, thanks","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"25671fd8cb8e317226ab5189df42f2e40a8eab86","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d str(migrate_data.serial_listen_addr)"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_72afdf7c","line":75,"in_reply_to":"7a77a97e_c304ae4b","updated":"2016-11-18 17:39:26.000000000","message":"I don\u0027t understand why you\u0027re removing this. The serial_listen_addr can still be unset, so you still need to guard against it.","commit_id":"c21bb5a2ad82483a1deb95ae8282b72b1790209a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3800c1ed45f03833e00e426913cd3d767344174e","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d migrate_data.serial_listen_addr"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a77a97e_d9db4120","line":75,"updated":"2016-11-21 15:27:09.000000000","message":"Our own from_legacy_dict() code can return an object with nothing set here. That can happen based on the compute RPC version pin. Until we can bump that major (which we can do just before ocata releases), you may still get a legacy dict over the wire, converted to an object, and thus is may be unset here.","commit_id":"5d8cfb84f66f316f9d76315fed68c667efe5f56e"},{"author":{"_account_id":12299,"name":"Paweł Koniszewski","email":"pawel.koniszewski@gmail.com","username":"pawel.koniszewski"},"change_message_id":"002b6f5bce9751d40d7cc0436fb09468a1eadbfc","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"def _update_serial_xml(xml_doc, migrate_data):"},{"line_number":75,"context_line":"    listen_addr \u003d migrate_data.serial_listen_addr"},{"line_number":76,"context_line":"    for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":77,"context_line":"        if dev.get(\u0027host\u0027) is not None:"},{"line_number":78,"context_line":"            dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a77a97e_870e9430","line":75,"in_reply_to":"7a77a97e_d9db4120","updated":"2016-11-22 12:51:37.000000000","message":"Dan, I haven\u0027t found code path that would lead to such situation. Even if doing Libert-\u003eOcata migrations this code will not be executed, because the source node will be Liberty. If doing Ocata-\u003eLiberty (which won\u0027t work due to, e.g., hypervisor version incompatibility, we only allow to live migrate the same or newer version) it will also have serial_listen_addr populated, as Liberty always sets serial_listen_addr key in migrate_data [1] dictionary and our from_legacy_dict and to_legacy_dict always take care of setting this key/property in dict/object.\n\nHowever, I like your idea about passing None instead of the empty string, so this change does not make sense in its current form and, if needed, should be put on top of https://review.openstack.org/#/c/397276\n\n[1] https://github.com/openstack/nova/blob/7495bbeadbe61a6b96bd0f39514651d819d16189/nova/virt/libvirt/driver.py#L6380","commit_id":"5d8cfb84f66f316f9d76315fed68c667efe5f56e"}]}
