)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f27433d4eee5a51d83746c60708455b08860bfa0","unresolved":true,"context_lines":[{"line_number":11,"context_line":"successfully connect to the rbd disk in case the mons have"},{"line_number":12,"context_line":"changed during the lifetime of the instance."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #1741364"},{"line_number":15,"context_line":"Change-Id: Ia9a5cc60269436a29bdb9f6368fa5b0b754d8367"},{"line_number":16,"context_line":"Signed-off-by: Michel Nederlof \u003cmichel.nederlof@nl.team.blue\u003e"},{"line_number":17,"context_line":"Signed-off-by: Erlon R. Cruz \u003cerlon@canonical.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9d0a9744_1ea3b32a","line":14,"updated":"2026-03-26 08:17:22.000000000","message":"i would generally not condier this to be a bug its a feature request as we intentionally do not support mon ip change in nova.\n\nby modifying them you are steping outside of what is expected to work.\n\nwe can hardend nova ot operator error but we should log a warning at a minim if we ever need to update the mon ips as that means you did an unsupproted action.","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c8018968d3649dd91a18480ca65eaa512fb914db","unresolved":true,"context_lines":[{"line_number":11,"context_line":"successfully connect to the rbd disk in case the mons have"},{"line_number":12,"context_line":"changed during the lifetime of the instance."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #1741364"},{"line_number":15,"context_line":"Change-Id: Ia9a5cc60269436a29bdb9f6368fa5b0b754d8367"},{"line_number":16,"context_line":"Signed-off-by: Michel Nederlof \u003cmichel.nederlof@nl.team.blue\u003e"},{"line_number":17,"context_line":"Signed-off-by: Erlon R. Cruz \u003cerlon@canonical.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"68ca343b_894cc1ad","line":14,"in_reply_to":"9d0a9744_1ea3b32a","updated":"2026-03-26 20:01:04.000000000","message":"Given the IRC chat today [1] where someone experiencing the bug says live migration fails without this update, I am personally OK with this patch.\n\n[1] https://meetings.opendev.org/irclogs/%23openstack-nova/%23openstack-nova.2026-03-26.log.html#openstack-nova.2026-03-26.log.html#t2026-03-26T19:25:43\n\n------\n\nThis is a comment I wrote earlier before live migration fail was confirmed:\n\n------\n\nMy initial reaction to this patch was similar but after looking into it more, there are a couple of reasons I think I am OK with this as a bug fix.\n\ntl;dr I don\u0027t think this is actually doing anything different or out of scope from what we are already currently doing for other RBD disk XML generation.\n\nExisting behaviors:\n\n* Every time Nova generates XML for RBD local/ephemeral disks (e.g. hard reboot) it calls Ceph to obtain the mon IPs, so it will pick up changes in mon IPs [2]\n\n* During pre_live_migration, Cinder volumes backed by RBD pull fresh connection_info from Cinder which will pick up changes in mon IPs [3]\n\nInconsistencies:\n\n* When Nova generates XML for RBD Cinder volumes (e.g. hard reboot) connection_info is _not_ refreshed\n\n* For live migration, for RBD local/ephemeral disks Nova does _not_ call Ceph to obtain the mon IPs\n\nMy thinking is, this patch would address bullet 2 of current inconsistencies and it would do so without doing anything different than our existing code for XML generation (e.g. hard reboot). I have thoughts on bullet 1 also but that is out of scope of this patch review.\n\n[2] https://github.com/openstack/nova/blob/327c790ec87e95e81900118537b68ed94e8e4475/nova/virt/libvirt/imagebackend.py#L947\n[3] https://github.com/openstack/nova/blob/327c790ec87e95e81900118537b68ed94e8e4475/nova/compute/manager.py#L9538","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0ccad3be1a2224b040ccba38f80a1c49fd52757","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e833f780_917aa8f7","updated":"2026-03-25 20:24:08.000000000","message":"Fixed all review comments and added more unit tests.","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"68cd2fef151aa71838f3f57f8d0a06b9de03492e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"36ac5312_6d6fc6a7","updated":"2026-04-13 09:13:11.000000000","message":"Thanks for all the attention to this issue everyone!\n\nSo if i read correctly all comments, and summarizing it, it came down to three changes:\n- reverting the strip_brackets change (as libvirt does not want those brackets)\n- implement the new device alias behaviour as per libvirt-dev-alias spec\n- add a release note\n\nI added those three in the latest patchset.\n\nI also can confirm that if the ceph mons (defined in the xml upon creation of the instance) are not reachable, the destination compute node is not able to connect to the storage, causing an error during migration.\n\nAnd i agree wit melwitt here, this should be handled as a bug, as live migrations do not work if the rbd mons that were defined upon launch of the instance are not the correct mons anymore.\nAnd you want to be able to do live migrations for maintenance, without asking customers/endusers to do a hard reboot / cold migration in order to start with said maintenance.\n\nWe are already running the initial code in our production environment with success and i think it would be beneficial if this patch would merge.\n\nThe concern raised by sean about potential security risks are not any different than already implemented by hard-reboots and/or cinder volume updates during live migrations.","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f27433d4eee5a51d83746c60708455b08860bfa0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4ac168bc_a071882d","updated":"2026-03-26 08:17:22.000000000","message":"this likely should have a release note and i think we need better testing.\nspecificly before proceeding to a patch i thnk you shoudl create a functional regression test for this using the libvirt functional test class as a base.\n\nwe have several exmaple of this already\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_2139351.py is perhaps a good refence as that is asserting changes in the libvirt xml on migration.\n\nso you should first crete a repoducer patch like this\nhttps://github.com/openstack/nova/commit/ba24639b8dd34a19885298cf728e58dd7db9e703 and then rebase this review on top of it and exent this patch to update the repoducer\n\nhttps://github.com/openstack/nova/commit/53a613d9948826ec9a4cd4a502f7a5d1b2dc87d7#diff-e9e6f84bf222526d26d0a7dcdef41796e79cbe1b6778a804915a010fd12d78ec\n\nthat following our normal 2 step bug fix process\n\nhere is another refence for volumes \nhttps://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_2127196.py\n\nwe have a rbd mixin in the fucntional tests \n\nhttps://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_evacuate.py#L192\n\nthat likely should be moved into the test fixture but that allows you to configure the mons\n\nhttps://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_evacuate.py#L224\n\nso you could use that to create a vm in teh functional tests then update the return value and do a live migrate\n\nwithout thhis change we are expecting the xml will not be modified \nyoucan check that using the patters form the 2 regression tests above\n\nthen with your proposed fix you can update the test to show the new behvior and prevent future regressions.","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"0520e59c88dc9ddaab86b974731f1c4da54f57c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b7521497_62c33073","updated":"2026-04-13 13:20:28.000000000","message":"recheck, does not seem related to this change.","commit_id":"1517eb2763353cf74426c51ccfbba4070a6f9317"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bc5b2275c05d0b39ef1a12be43c30b0576e9a8c2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"0490a1f5_095ffba3","updated":"2026-04-28 20:19:29.000000000","message":"After a ton of trial and error, I was finally able to test Ceph live migration with IPv6 successfully and can confirm that this patch PS3 leaving brackets in the mon address causes a failure in libvirt [1]:\n```\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:     \u003cdisk type\u003d\"network\" device\u003d\"disk\"\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:       \u003cdriver name\u003d\"qemu\" type\u003d\"raw\" cache\u003d\"writeback\"/\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:       \u003cauth username\u003d\"cinder\"\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:         \u003csecret type\u003d\"ceph\" uuid\u003d\"d531d2d4-3937-429c-b0c2-658fe41e82aa\"/\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:       \u003c/auth\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:       \u003csource protocol\u003d\"rbd\" name\u003d\"vms/c1e9d0f8-b00f-40b2-be98-22f65a4e8213_disk\"\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:         \u003chost name\u003d\"[fd00::1]\" port\u003d\"6789\"/\u003e\u003c/source\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:       \u003ctarget dev\u003d\"vda\" bus\u003d\"virtio\"/\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:       \u003caddress type\u003d\"pci\" domain\u003d\"0x0000\" bus\u003d\"0x00\" slot\u003d\"0x04\" function\u003d\"0x0\"/\u003e\nApr 28 17:23:27.570540 np8b3b1bddd3dc4 nova-compute[45192]:     \u003c/disk\u003e\n[...]\nApr 28 17:23:32.150080 np8b3b1bddd3dc4 nova-compute[45192]: ERROR nova.virt.libvirt.driver [instance: c1e9d0f8-b00f-40b2-be98-22f65a4e8213] libvirt.libvirtError: internal error: process exited while connecting to monitor: server name not found: [fd00::1] (Name or service not known)\nApr 28 17:23:32.150080 np8b3b1bddd3dc4 nova-compute[45192]: ERROR nova.virt.libvirt.driver [instance: c1e9d0f8-b00f-40b2-be98-22f65a4e8213] unable to parse addrs in \u0027[[fd00::1]]:6789\u0027\n```\n\nWhereas this patch PS6 with brackets stripped works without errors [2].\n\nSeparately I will work on getting devstack/CI IPv6 related bits proposed to their respective repos so that we can test something like this more easily in the future.\n\n+1 for now until @smooney@redhat.com looks and confirms the dev alias or serial part is handled as they expected. It otherwise looks OK to me.\n\n[1] https://zuul.opendev.org/t/openstack/build/5ed3cedbf3b645f6a449fdcdff7fcf5d/log/compute1/logs/screen-n-cpu.txt#18888 from PS18 of https://review.opendev.org/c/openstack/nova/+/984623/18\n[2] https://zuul.opendev.org/t/openstack/build/5a133e818b57446b8e84cab6f62cf4c1/logs from PS17 of https://review.opendev.org/c/openstack/nova/+/984623/17","commit_id":"957eb0f79c1bf005d66d48964490af8912fe41ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"df79113d202fbcd0f887e9b622656a048e8fe9ce","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f7127a55_3040871a","updated":"2026-05-06 18:26:56.000000000","message":"directionally i think this is correct we just need to decided what level fo testing we expect to proceed with this. mel\u0027s dnm test certenly helps prove our some of this but we are not actully modifying the mon ips in that work just verifying ipv6 works in general.","commit_id":"957eb0f79c1bf005d66d48964490af8912fe41ae"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"3c7082e25a1fb1475f23a339ce95d34daa38f498","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"fe9788e3_f9933083","updated":"2026-04-14 07:42:24.000000000","message":"recheck","commit_id":"957eb0f79c1bf005d66d48964490af8912fe41ae"}],"nova/tests/unit/virt/libvirt/test_migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"df79113d202fbcd0f887e9b622656a048e8fe9ce","unresolved":true,"context_lines":[{"line_number":798,"context_line":"            migration._update_rbd_xml(doc, data),"},{"line_number":799,"context_line":"            encoding\u003d\u0027unicode\u0027"},{"line_number":800,"context_line":"        )"},{"line_number":801,"context_line":"        self.assertXmlEqual(res, xml)"},{"line_number":802,"context_line":""},{"line_number":803,"context_line":"    def test_update_volume_xml(self):"},{"line_number":804,"context_line":"        connection_info \u003d {"}],"source_content_type":"text/x-python","patch_set":6,"id":"710703de_9a526a9c","line":801,"updated":"2026-05-06 18:26:56.000000000","message":"it would be nice ot have an ipv6 version as well given the dicusion on the brakets`[]`","commit_id":"957eb0f79c1bf005d66d48964490af8912fe41ae"}],"nova/virt/libvirt/migration.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"6c918da9824ed1ff08514231dfc79b4e7a29589a","unresolved":true,"context_lines":[{"line_number":362,"context_line":"    if migrate_data.obj_attr_is_set(\"image_type\"):"},{"line_number":363,"context_line":"        image_type \u003d migrate_data.image_type"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    if image_type !\u003d \"rbd\":"},{"line_number":366,"context_line":"        # only when we use image_type rbd, we have rbd \u0027local\u0027 disks"},{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs()"},{"line_number":370,"context_line":"    rbd_mon_mapping \u003d [\u0027:\u0027.join(p) for p in zip(rbd_mons, rbd_ports)]"}],"source_content_type":"text/x-python","patch_set":2,"id":"56a520e0_136c8c2d","line":367,"range":{"start_line":365,"start_character":3,"end_line":367,"end_character":22},"updated":"2026-02-20 12:27:26.000000000","message":"Your unit tests do not cover some paths in this function. Specifically, when the image type is different from RBD, and in the two cases below where the serial source is not None.","commit_id":"512388ef405078305888b9fc6885083afc03e2c1"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0ccad3be1a2224b040ccba38f80a1c49fd52757","unresolved":false,"context_lines":[{"line_number":362,"context_line":"    if migrate_data.obj_attr_is_set(\"image_type\"):"},{"line_number":363,"context_line":"        image_type \u003d migrate_data.image_type"},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"    if image_type !\u003d \"rbd\":"},{"line_number":366,"context_line":"        # only when we use image_type rbd, we have rbd \u0027local\u0027 disks"},{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs()"},{"line_number":370,"context_line":"    rbd_mon_mapping \u003d [\u0027:\u0027.join(p) for p in zip(rbd_mons, rbd_ports)]"}],"source_content_type":"text/x-python","patch_set":2,"id":"ee2dc172_6bf44165","line":367,"range":{"start_line":365,"start_character":3,"end_line":367,"end_character":22},"in_reply_to":"56a520e0_136c8c2d","updated":"2026-03-25 20:24:08.000000000","message":"Done","commit_id":"512388ef405078305888b9fc6885083afc03e2c1"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"eab6d48b8b79b37c45097f43ad16292ec846af3f","unresolved":true,"context_lines":[{"line_number":366,"context_line":"        # only when we use image_type rbd, we have rbd \u0027local\u0027 disks"},{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs()"},{"line_number":370,"context_line":"    rbd_mon_mapping \u003d [\u0027:\u0027.join(p) for p in zip(rbd_mons, rbd_ports)]"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    # Update volume xml"}],"source_content_type":"text/x-python","patch_set":2,"id":"15b9a7df_c071f284","line":369,"updated":"2026-02-02 16:37:20.000000000","message":"you need strip_brackets\u003dFalse to keep [] surrounding IPv6 address.","commit_id":"512388ef405078305888b9fc6885083afc03e2c1"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0ccad3be1a2224b040ccba38f80a1c49fd52757","unresolved":false,"context_lines":[{"line_number":366,"context_line":"        # only when we use image_type rbd, we have rbd \u0027local\u0027 disks"},{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs()"},{"line_number":370,"context_line":"    rbd_mon_mapping \u003d [\u0027:\u0027.join(p) for p in zip(rbd_mons, rbd_ports)]"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    # Update volume xml"}],"source_content_type":"text/x-python","patch_set":2,"id":"6220b37b_aef999a2","line":369,"in_reply_to":"15b9a7df_c071f284","updated":"2026-03-25 20:24:08.000000000","message":"Done","commit_id":"512388ef405078305888b9fc6885083afc03e2c1"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"6c918da9824ed1ff08514231dfc79b4e7a29589a","unresolved":true,"context_lines":[{"line_number":374,"context_line":"    for _, disk_dev in enumerate(disk_nodes):"},{"line_number":375,"context_line":"        serial_source \u003d disk_dev.findtext(\"serial\")"},{"line_number":376,"context_line":"        if serial_source is not None:"},{"line_number":377,"context_line":"            # disks with serials are handled in _update_volume_xml below,"},{"line_number":378,"context_line":"            # as they are cinder volumes and get the connection_info fresh"},{"line_number":379,"context_line":"            # from cinder"},{"line_number":380,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":2,"id":"b2049a22_6d4140b6","line":377,"range":{"start_line":377,"start_character":67,"end_line":377,"end_character":72},"updated":"2026-02-20 12:27:26.000000000","message":"Drop the below since functions can move around overtime","commit_id":"512388ef405078305888b9fc6885083afc03e2c1"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0ccad3be1a2224b040ccba38f80a1c49fd52757","unresolved":false,"context_lines":[{"line_number":374,"context_line":"    for _, disk_dev in enumerate(disk_nodes):"},{"line_number":375,"context_line":"        serial_source \u003d disk_dev.findtext(\"serial\")"},{"line_number":376,"context_line":"        if serial_source is not None:"},{"line_number":377,"context_line":"            # disks with serials are handled in _update_volume_xml below,"},{"line_number":378,"context_line":"            # as they are cinder volumes and get the connection_info fresh"},{"line_number":379,"context_line":"            # from cinder"},{"line_number":380,"context_line":"            continue"}],"source_content_type":"text/x-python","patch_set":2,"id":"e2e56d5f_c0fc2b56","line":377,"range":{"start_line":377,"start_character":67,"end_line":377,"end_character":72},"in_reply_to":"b2049a22_6d4140b6","updated":"2026-03-25 20:24:08.000000000","message":"Done","commit_id":"512388ef405078305888b9fc6885083afc03e2c1"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"1a611b04e9160c9e774437207e10d866d50543dc","unresolved":true,"context_lines":[{"line_number":391,"context_line":"            if key not in rbd_mon_mapping:"},{"line_number":392,"context_line":"                source.remove(host)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        for mon_entry in set(rbd_mon_mapping) - active_mons:"},{"line_number":395,"context_line":"            mon_name, mon_port \u003d mon_entry.split(\":\")"},{"line_number":396,"context_line":"            host_elem \u003d etree.Element(\"host\", name\u003dmon_name, port\u003dmon_port)"},{"line_number":397,"context_line":"            source.append(host_elem)"},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"    return xml_doc"},{"line_number":400,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d747e77a_4fe5bb1a","line":397,"range":{"start_line":394,"start_character":0,"end_line":397,"end_character":36},"updated":"2026-03-12 19:27:55.000000000","message":"Since the only monitors that matters for us are the ones coming from get_mon_addrs(), you can simplify this 2 for loops, by just removing all the monitors from the XML, and adding then again.","commit_id":"512388ef405078305888b9fc6885083afc03e2c1"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3b5cf43fdf95461f36ab4aef0681f90f284bba1a","unresolved":true,"context_lines":[{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs("},{"line_number":370,"context_line":"        strip_brackets\u003dFalse)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    # Update volume xml"},{"line_number":373,"context_line":"    disk_nodes \u003d xml_doc.findall(\"./devices/disk\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"dbb5b0f3_7b0b8b0f","line":370,"range":{"start_line":370,"start_character":8,"end_line":370,"end_character":28},"updated":"2026-03-25 22:54:05.000000000","message":"I was curious about this so I investigated a little and it appears that libvirt might not support IPv6 address formatted with brackets in XML.\n\nFirst I looked at the git blame that added the strip_brackets kwarg and it said [1]:\n\u003e Brackets are removed from all IPv6 addresses when getting Ceph monitor addresses. This is correct for libvirt\u0027s XML files but not for libguestfs.\n\nThen I tried to find a way to validate this in libvirt and used the virt-xml-validate CLI [2] and got the following behavior:\n```\nubuntu@noble:~$ cat network.xml \n\u003cnetwork\u003e\n  \u003cname\u003edefault\u003c/name\u003e\n  \u003cbridge name\u003d\"virbr0\"/\u003e\n  \u003cforward mode\u003d\"nat\"/\u003e\n  \u003cip address\u003d\"192.168.122.1\" netmask\u003d\"255.255.255.0\"\u003e\n    \u003cdhcp\u003e\n      \u003crange start\u003d\"192.168.122.2\" end\u003d\"192.168.122.254\"/\u003e\n    \u003c/dhcp\u003e\n  \u003c/ip\u003e\n  \u003cip family\u003d\"ipv6\" address\u003d\"2001:db8:ca2:2::1\" prefix\u003d\"64\"/\u003e\n\u003c/network\u003e\nubuntu@noble:~$ virt-xml-validate network.xml \nnetwork.xml validates\nubuntu@noble:~$ cat network_ipv6_brackets.xml \n\u003cnetwork\u003e\n  \u003cname\u003edefault\u003c/name\u003e\n  \u003cbridge name\u003d\"virbr0\"/\u003e\n  \u003cforward mode\u003d\"nat\"/\u003e\n  \u003cip address\u003d\"192.168.122.1\" netmask\u003d\"255.255.255.0\"\u003e\n    \u003cdhcp\u003e\n      \u003crange start\u003d\"192.168.122.2\" end\u003d\"192.168.122.254\"/\u003e\n    \u003c/dhcp\u003e\n  \u003c/ip\u003e\n  \u003cip family\u003d\"ipv6\" address\u003d\"[2001:db8:ca2:2::1]\" prefix\u003d\"64\"/\u003e\n\u003c/network\u003e\nubuntu@noble:~$ virt-xml-validate network_ipv6_brackets.xml \nnetwork_ipv6_brackets.xml:10: element ip: Relax-NG validity error : Invalid attribute address for element ip\nRelax-NG validity error : Extra element ip in interleave\nnetwork_ipv6_brackets.xml:10: element ip: Relax-NG validity error : Element network failed to validate content\nnetwork_ipv6_brackets.xml fails to validate\n```\n\nSo I think use of strip_brackets\u003dFalse here might actually be a problem.\n\nI checked the \"usual\" code for generating RBD non-Cinder disks and saw it also calls get_mon_addrs() without strip_brackets\u003dFalse [3].\n\n[1] https://opendev.org/openstack/nova/commit/5d29dccaf91c697ffc12a7289ce16cd83e60b07f\n[2] https://wiki.libvirt.org/Common_XML_errors.html#validating-xml-against-libvirt-schemas\n[3] https://opendev.org/openstack/nova/src/commit/327c790ec87e95e81900118537b68ed94e8e4475/nova/virt/libvirt/imagebackend.py#L947","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e42834c6a88cceb71b198bac99410544f6a0ffd9","unresolved":true,"context_lines":[{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs("},{"line_number":370,"context_line":"        strip_brackets\u003dFalse)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    # Update volume xml"},{"line_number":373,"context_line":"    disk_nodes \u003d xml_doc.findall(\"./devices/disk\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"41d9d8b0_287a2a91","line":370,"range":{"start_line":370,"start_character":8,"end_line":370,"end_character":28},"in_reply_to":"6cbcc2ad_4847dfc8","updated":"2026-03-26 19:02:02.000000000","message":"i dont think nova has ipv6 only jobs today but other project do so yes we could\ndewploy tempest with ipv6 for the service adress used for ceph/rabbit ectra\n\nthat would include libvirt if we set the correct config options so defiantly possible upstream with work.\n\nhttps://github.com/openstack/devstack/tree/416d27e89e0c1891921fee2a692086eb8fcd0307/roles/devstack-ipv6-only-deployments-verification\n\nhttps://github.com/openstack/devstack/blob/416d27e89e0c1891921fee2a692086eb8fcd0307/openrc#L42-L56\n\nwe just need to move one of our ceph jobs to add the relevnet parmter like in \nhttps://github.com/openstack/devstack/blob/416d27e89e0c1891921fee2a692086eb8fcd0307/.zuul.yaml#L787\n\nhttps://github.com/openstack/devstack/blob/416d27e89e0c1891921fee2a692086eb8fcd0307/.zuul.yaml#L788C1-L798C1","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f27433d4eee5a51d83746c60708455b08860bfa0","unresolved":true,"context_lines":[{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs("},{"line_number":370,"context_line":"        strip_brackets\u003dFalse)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    # Update volume xml"},{"line_number":373,"context_line":"    disk_nodes \u003d xml_doc.findall(\"./devices/disk\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"ddd392f9_7070ba32","line":370,"range":{"start_line":370,"start_character":8,"end_line":370,"end_character":28},"in_reply_to":"dbb5b0f3_7b0b8b0f","updated":"2026-03-26 08:17:22.000000000","message":"hum thats an odd gap good catch.\n\n\nin general we dont typically like change to live migration that are only tested with unit tests.\n\nwe generally prefer requiring a functional regression test or stardard functional test to validate the behvior.\n\nin this case it would not have caght this but im wondering if we should require a momre complet set of testing here.\n\nfor exampel","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8f7c33105e88c1d76a2bc79253f1dede8cfc8d45","unresolved":true,"context_lines":[{"line_number":367,"context_line":"        return xml_doc"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    rbd_mons, rbd_ports \u003d rbd_utils.RBDDriver().get_mon_addrs("},{"line_number":370,"context_line":"        strip_brackets\u003dFalse)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    # Update volume xml"},{"line_number":373,"context_line":"    disk_nodes \u003d xml_doc.findall(\"./devices/disk\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"6cbcc2ad_4847dfc8","line":370,"range":{"start_line":370,"start_character":8,"end_line":370,"end_character":28},"in_reply_to":"ddd392f9_7070ba32","updated":"2026-03-26 17:04:23.000000000","message":"I agree, when I found this it was troubling to know that if it merged as-is, it would break live migration for XML that would include IPv6 addresses. Unit tests feel quite insufficient in a case like this.\n\nHow to improve it is however not clear to me though because as you said, a func test would also not catch this. I would like to cover something like IPv6 address in Tempest but would it be possible in upstream CI?","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f27433d4eee5a51d83746c60708455b08860bfa0","unresolved":true,"context_lines":[{"line_number":372,"context_line":"    # Update volume xml"},{"line_number":373,"context_line":"    disk_nodes \u003d xml_doc.findall(\"./devices/disk\")"},{"line_number":374,"context_line":"    for disk_dev in disk_nodes:"},{"line_number":375,"context_line":"        serial_source \u003d disk_dev.findtext(\"serial\")"},{"line_number":376,"context_line":"        if serial_source is not None:"},{"line_number":377,"context_line":"            # disks with serials are handled in _update_volume_xml,"},{"line_number":378,"context_line":"            # as they are cinder volumes and get the connection_info fresh"}],"source_content_type":"text/x-python","patch_set":3,"id":"89ff1de0_e3c88817","line":375,"updated":"2026-03-26 08:17:22.000000000","message":"finding the disk by serial is incorrect.\n\nthat assumes that for one that the disk will have a seiral (it wont if a cinder volume attach as device_type\u003dlun) and that its ok to handel some disking in _update_volume_xml and ohter here.\n\nthe latter might be ok it feels a bit od to treat cinder volumes and nova provisioned sotrage seperatly in this regard but i can see why you might want to keep it self contianed.\n\nspliting this logic feels wrong to me.\n\nif we want to correlate the disk we should be using the disks alias\nin general.\n\nhttps://cdn.maingear.com/project/specs.openstack.org/openstack/nova-specs/specs/2024.1/implemented/libvirt-dev-alias.html\n\non moderen install libvirt disk will alwasy have a serial set.\n\nadapting _update_volume_xml to not rely on serial is arguable more corect.\nthen special casing images_type\u003drbd in this way.\n\neven if we were to add this capability  it would not change the fact that nova dose not support changing the ceph mon adresses for a cluster used by nova.\n\nif you do that we provide no guarantee of any functionatliy of the vms.","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f27433d4eee5a51d83746c60708455b08860bfa0","unresolved":true,"context_lines":[{"line_number":383,"context_line":"        if source is None or source.get(\"protocol\") !\u003d \u0027rbd\u0027:"},{"line_number":384,"context_line":"            continue"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        for host in source.findall(\"host\"):"},{"line_number":387,"context_line":"            source.remove(host)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        for mon_name, mon_port in zip(rbd_mons, rbd_ports):"},{"line_number":390,"context_line":"            host_elem \u003d etree.Element(\"host\", name\u003dmon_name, port\u003dmon_port)"},{"line_number":391,"context_line":"            source.append(host_elem)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    return xml_doc"},{"line_number":394,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"96a838f5_e9af2077","line":391,"range":{"start_line":386,"start_character":7,"end_line":391,"end_character":36},"updated":"2026-03-26 08:17:22.000000000","message":"we shold not just alwasy update this.\n\nor at least we should be comparing the old value to new one and logging a warning\nsince modifying the mon ips is not supproted.\n\nnot only does chaning mon ips have direct impact on the runnign qemu instance but it can also cause the nova-compute agent to be unable to interact with the ceph cluster and cause an operational outage depening on how your /etc/ceph ectra is configured.","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8f7c33105e88c1d76a2bc79253f1dede8cfc8d45","unresolved":true,"context_lines":[{"line_number":383,"context_line":"        if source is None or source.get(\"protocol\") !\u003d \u0027rbd\u0027:"},{"line_number":384,"context_line":"            continue"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"        for host in source.findall(\"host\"):"},{"line_number":387,"context_line":"            source.remove(host)"},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"        for mon_name, mon_port in zip(rbd_mons, rbd_ports):"},{"line_number":390,"context_line":"            host_elem \u003d etree.Element(\"host\", name\u003dmon_name, port\u003dmon_port)"},{"line_number":391,"context_line":"            source.append(host_elem)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    return xml_doc"},{"line_number":394,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"08f09377_e15fbb0e","line":391,"range":{"start_line":386,"start_character":7,"end_line":391,"end_character":36},"in_reply_to":"96a838f5_e9af2077","updated":"2026-03-26 17:04:23.000000000","message":"This is an interesting point regarding a running guest. If I have understood correctly in the past, updates to Ceph mon IPs are automatically reflected for a running guest via its existing connection to Ceph. And when the guest experiences a life cycle event (e.g. hard reboot) for local/ephemeral disks Nova calls Ceph to get the new mon IPs and generates the XML with it [1].\n\nSo I\u0027m wondering, is this patch actually needed? I read through the launchpad bug report again and it does not actually say what breaks or what error happens in the described scenario. Is it just that people want to use live migration as a way to update the XML? If so, why is the existing XML generation during life cycle events not sufficient?\n\nAFAIK the guest should continue to function properly with the new mon IPs while it is running (due to its connection to Ceph and Ceph automatically handling failover) and if anything happens that would cause it to generate XML, that will be covered properly as well by Nova.\n\n[1] https://github.com/openstack/nova/blob/327c790ec87e95e81900118537b68ed94e8e4475/nova/virt/libvirt/imagebackend.py#L947","commit_id":"b72e31134b40c7ad468412b09cc8df8563ed486f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"df79113d202fbcd0f887e9b622656a048e8fe9ce","unresolved":true,"context_lines":[{"line_number":357,"context_line":"    return xml_doc"},{"line_number":358,"context_line":""},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"def _update_rbd_xml(xml_doc, migrate_data):"},{"line_number":361,"context_line":"    \"\"\"Update XML for rbd disk drives (ephemeral, root disk and config drive)"},{"line_number":362,"context_line":"    by checking if the set of mons is still up-to-date."},{"line_number":363,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"c3f30dec_1daf370f","line":360,"updated":"2026-05-06 18:26:56.000000000","message":"i was honestly expecting this to be missign test coverage\n\nhttps://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_e3a/openstack/e3ae08a737f741389a72bd5be8900417/cover/z_155e0da3f29b7c55_migration_py.html#t360\n\nbut i gusses its technically covered bar the ipv6 case we were investigating.\n\ni think melainie is going to continue with getting ci coverage fo tha tnow that she has a poc so we could proceed but i think that is the main testing gap on the unit test side.\n\nit would also be nice to have functional tests for this eventually.\n\nas i noted in my inital review\n\nhttps://review.opendev.org/c/openstack/nova/+/974032/comments/4ac168bc_a071882d","commit_id":"957eb0f79c1bf005d66d48964490af8912fe41ae"}]}