)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dca05a3f8dfb33a761ba35180fafc0629f57ade3","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Do not unnecessary convert images in finish_migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"only convert raw to qcow images when the images backend is"},{"line_number":10,"context_line":"actually the QCOW2 one, otherwise the image format will"},{"line_number":11,"context_line":"change from original one without updating the XML,"},{"line_number":12,"context_line":"breaking the instance after migration."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7fefbec1_2a73ca59","line":10,"updated":"2023-10-13 12:06:01.000000000","message":"you can use qcow format with flat image backend too.","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"6d458db120ef8375a1f266ee67581719b85eedf5","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Do not unnecessary convert images in finish_migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"only convert raw to qcow images when the images backend is"},{"line_number":10,"context_line":"actually the QCOW2 one, otherwise the image format will"},{"line_number":11,"context_line":"change from original one without updating the XML,"},{"line_number":12,"context_line":"breaking the instance after migration."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"b5c2e508_372cfc25","line":10,"in_reply_to":"7fefbec1_2a73ca59","updated":"2023-10-13 12:25:28.000000000","message":"you can, but you can also raw as well, so why convert the image that was raw on a identically configured compute to qcow after migration is the main question. The only reason may be if the backend on the destination compute is the `class Qcow2` one.","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dca05a3f8dfb33a761ba35180fafc0629f57ade3","unresolved":true,"context_lines":[{"line_number":11,"context_line":"change from original one without updating the XML,"},{"line_number":12,"context_line":"breaking the instance after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #2038898"},{"line_number":15,"context_line":"Change-Id: I96923807ae842a161db5cbfc328e1ccc19e1569c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7ae03ee8_e6147429","line":14,"updated":"2023-10-13 12:06:01.000000000","message":"idealy when adressign a bug you would add a repoducer functional test to dempostare the bug and hten a second patch to fix it.\n\nin this case its actully an optimisation not a defect but you shoudl still add unit test coverage to assert that we do the convertion when required and dotn do it when not required\n\nyou shoudl also add are release note for this chagne.","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"8fa5365e98a748c2c0378dd66143ce25af1ccef9","unresolved":true,"context_lines":[{"line_number":11,"context_line":"change from original one without updating the XML,"},{"line_number":12,"context_line":"breaking the instance after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #2038898"},{"line_number":15,"context_line":"Change-Id: I96923807ae842a161db5cbfc328e1ccc19e1569c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9848a2cc_c68225ae","line":14,"in_reply_to":"7ae03ee8_e6147429","updated":"2023-10-13 12:23:03.000000000","message":"that\u0027s not merely an optimization but an actual defect breaking instances after migration\n\nif you configure your hosts with\n```\n[DEFAULT]\n# these are in fact default settings\nuse_cow_images \u003d True\nforce_raw_images \u003d True\n[libvirt]\n# this one is not default\nimages_format \u003d raw\n```\nthat nova perfectly allows and does not complain about,\nif you migrate an instance booted from raw image, the image will be silently converted to qcow2 (even though on the source compute it was raw), and instance fails to start completely - plz read the whole story and repro scenario in the linked bug.","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"1e4a57c6e0a6a2610054d4686ed08258f4d80331","unresolved":true,"context_lines":[{"line_number":11,"context_line":"change from original one without updating the XML,"},{"line_number":12,"context_line":"breaking the instance after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #2038898"},{"line_number":15,"context_line":"Change-Id: I96923807ae842a161db5cbfc328e1ccc19e1569c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"75c523a7_7ea3cadb","line":14,"in_reply_to":"8fb6c399_5f2831af","updated":"2023-10-13 13:13:41.000000000","message":"plz check https://review.opendev.org/c/openstack/nova/+/898229 as the alt version with your proposal","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ba5a6c09f6226724e5278f1ad45e3fb383e21e0","unresolved":true,"context_lines":[{"line_number":11,"context_line":"change from original one without updating the XML,"},{"line_number":12,"context_line":"breaking the instance after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #2038898"},{"line_number":15,"context_line":"Change-Id: I96923807ae842a161db5cbfc328e1ccc19e1569c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"8fb6c399_5f2831af","line":14,"in_reply_to":"9848a2cc_c68225ae","updated":"2023-10-13 12:39:03.000000000","message":"right so that actully waht we shoudl be fixign \nnova shoudl not allwo you to set \n\nimages_type\u003draw + use_cow_images\u003dtrue\n\nwe shoudl be raising in init_host with a configuration error and refusing to start the comptue agent in that case.\n\n\n\nuse_cow_images \u003d True\nwith \nimages_type\u003dflat|qcow\n\nare all valid but not raw\n\nfor rbd or lvm use_cow_images is mostly ignored since in bothcase the images are always converted effectivly to raw when teh backing volume rbd or lvm are created.\n\nit ambigious what the operator intended in the \n\nuse_cow_images \u003d True and images_type\u003draw so that should be raised as an error","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"713e44764731ccee1ad8180a6958aa10d049c595","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fc25272a_6dfb86c1","updated":"2023-10-10 14:47:30.000000000","message":"not sure about implementation yet..","commit_id":"7a8bb409ba98bb834b29298fa70d771a3d7bf3ce"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"8fa5365e98a748c2c0378dd66143ce25af1ccef9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7f638556_f9da5bc2","updated":"2023-10-13 12:23:03.000000000","message":"@sean\n\nI\u0027ll definitely add some unit tests and release note, as for functional I have some questions:\n\nDoes nova have possibility to define and run functional tests that a) require and only run on multinode b) allow to arbitrary configure both nova computes, and c) upload arbitrary images to glance?","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4ba5a6c09f6226724e5278f1ad45e3fb383e21e0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d460a2ee_5c7d2023","in_reply_to":"7f638556_f9da5bc2","updated":"2023-10-13 12:39:03.000000000","message":"yes to a and b and for c we have a fixture that fakes out glance.\n\nour functional tests dont actully boot real vms or invoke external comands like qemu image we have fixtues that stub out libvirt neutron and other external deps like the db or message bus\n\nthe libvirt funcitonla test have support for emulating cold/live migratoin and can set cofnig option provided they are the saem on all host.","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"b2986d24f8196ead0f4ed2e48686fb03cce8cbf7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b729b4e1_91861f0b","in_reply_to":"d460a2ee_5c7d2023","updated":"2023-10-17 11:23:44.000000000","message":"I don\u0027t think that w/o actual calls to qemu and libvirt I will be able to clearly demonstrate the problem, which is:\n\nafter cold migration, the image is in qcow format, but the libvirt XML has it as RAW, which leads to instance being \u0027active\u0027 as per nova/libvirt (libvirt domain is running), but nothing is actually running inside the instance (presumably because qemu can not read the qcow disk as raw).","commit_id":"317a48c23fb7d4aa0e0a6ebc0ffd72c74c84d087"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"1361b067624ee2097de8cc1a57a0b4e3e9fb009b","unresolved":true,"context_lines":[{"line_number":11708,"context_line":"            # TODO(pas-ha) once usage use_cow_images and \u0027default\u0027 images_type"},{"line_number":11709,"context_line":"            # are removed, replace with simpler check based on config"},{"line_number":11710,"context_line":"            # options (libvirt.images_type and force_raw_images) only."},{"line_number":11711,"context_line":"            image \u003d self.image_backend.by_libvirt_path("},{"line_number":11712,"context_line":"                instance\u003dinstance, path\u003dpath)"},{"line_number":11713,"context_line":"            expect_qcow2 \u003d ("},{"line_number":11714,"context_line":"                image.driver_format \u003d\u003d \"qcow2\" or"},{"line_number":11715,"context_line":"                ("},{"line_number":11716,"context_line":"                    image_meta.disk_format \u003d\u003d \"qcow2\" and"},{"line_number":11717,"context_line":"                    image.driver_format \u003d\u003d \"raw\" and"},{"line_number":11718,"context_line":"                    # to distinguish from Lvm"},{"line_number":11719,"context_line":"                    not image.is_block_dev and"},{"line_number":11720,"context_line":"                    not CONF.force_raw_images"},{"line_number":11721,"context_line":"                )"},{"line_number":11722,"context_line":"            )"},{"line_number":11723,"context_line":"            # NOTE(mdbooth): The code below looks wrong, but is actually"},{"line_number":11724,"context_line":"            # required to prevent a security hole when migrating from a host"},{"line_number":11725,"context_line":"            # with use_cow_images\u003dFalse to one with use_cow_images\u003dTrue."}],"source_content_type":"text/x-python","patch_set":9,"id":"2589d2dd_81ed5c65","line":11722,"range":{"start_line":11711,"start_character":0,"end_line":11722,"end_character":13},"updated":"2023-10-26 16:19:45.000000000","message":"this can be replaced with quite a lengthy if statement, smth like\n\n```\nbackend_is_qcow2 \u003d (\n    CONF.libvirt.images_type \u003d\u003d \u0027qcow2\u0027 or\n    (\n        CONF.libvirt.images_type \u003d\u003d \u0027default\u0027 and\n        CONF.use_cow_images is True\n     )\n)\nbackend_is_flat \u003d (\n    CONF.libvirt.images_type in (\u0027raw\u0027, \u0027flat\u0027) or\n    (\n        CONF.libvirt.images_type \u003d\u003d \u0027default\u0027 and\n        CONF.use_cow_images is False\n     )\n)\nexpect_qcow2 \u003d (\n    backend_is_qcow2 or\n    (\n        backend_is_flat and\n        CONF.force_raw_images is False and\n        image_meta.disk_format \u003d\u003d \u0027qcow2\u0027\n        \n     )\n)\n```\nand brought out of for loop,\n\nbut somehow I feel this is duplicating the logic in the imagebackend.py a lot...\nhowever might be somewhat clearer I guess..?","commit_id":"311a56633c18d613447a5b828725e36d2703e90b"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"54bf7a54566d83e364792e828b2afdf41da923b2","unresolved":true,"context_lines":[{"line_number":11726,"context_line":"            # Imagebackend uses use_cow_images to select between the"},{"line_number":11727,"context_line":"            # atrociously-named-Raw and Qcow2 backends. The Qcow2 backend"},{"line_number":11728,"context_line":"            # writes to disk.info, but does not read it as it assumes qcow2."},{"line_number":11729,"context_line":"            # Therefore if we don\u0027t convert raw to qcow2 here, a raw disk will"},{"line_number":11730,"context_line":"            # be incorrectly assumed to be qcow2, which is a severe security"},{"line_number":11731,"context_line":"            # flaw. The reverse is not true, because the atrociously-named-Raw"},{"line_number":11732,"context_line":"            # backend supports both qcow2 and raw disks, and will choose"},{"line_number":11733,"context_line":"            # appropriately between them as long as disk.info exists and is"},{"line_number":11734,"context_line":"            # correctly populated, which it is because Qcow2 writes to"}],"source_content_type":"text/x-python","patch_set":9,"id":"c77fe446_9f597210","line":11731,"range":{"start_line":11729,"start_character":1,"end_line":11731,"end_character":19},"updated":"2023-10-26 18:49:15.000000000","message":"btw, it seems this is already mitigated on the libvirt/qemu level\n\nwhile testing some of the patchsets here, this was one of the errors when I by mistake was also filling the disk.info for the config drive as \"qcow2\" as thus the XML was generated as qcow while the disk.config was actually left \u0027raw\u0027\n\n```\nOct 24 22:13:42.930127 np0035581148 nova-compute[74517]: ERROR nova.compute.manager [instance: b71c74aa-ae2f-49dd-add1-55eb50d98c66] libvirt.libvirtError: internal error: process exited while connecting to monitor: 2023-10-24T22:13:42.806345Z qemu-system-x86_64: -blockdev {\"node-name\":\"libvirt-1-format\",\"read-only\":true,\"cache\":{\"direct\":true,\"no-flush\":false},\"driver\":\"qcow2\",\"file\":\"libvirt-1-storage\",\"backing\":null}: Image is not in qcow2 format\n```","commit_id":"311a56633c18d613447a5b828725e36d2703e90b"}]}