)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0636c72a3f44c1257cb482bdb977b2fc910d28ac","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"d70afb4e_931315d0","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"updated":"2023-10-17 10:47:23.000000000","message":"i commented on this in a previosu change flat is valid to use with use_cow_images\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/libvirt/imagebackend.py#L531-L536\n\n\n \"\"\"The Flat backend uses either raw or qcow2 storage. It never uses\n    a backing store, so when using qcow2 it copies an image rather than\n    creating an overlay. By default it creates raw files, but will use qcow2\n    when creating a disk from a qcow2 if force_raw_images is not set in config.\n    \"\"\"\n\n\nFlat is not refering to using a flat/raw file it refering to the fact that it disabled the image cache and does not use a shared backing file.\n\nso flat + use_cow_images\u003dtrue means use qcow files for the vms\nflat + use_cow_images\u003dfalse means use raw images.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"a7442f87a9ff4896b264761efd73fa0fcc07fc3e","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"18b1616e_959168c2","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"0fdd058c_ab43c54c","updated":"2023-10-20 16:25:07.000000000","message":"\u003e use_cow_images if i recall correctly also enampes image convertion in nova\n\nit is actually used in 2 places only in libvirt driver:\n\n1. choose Flat or Qcow2 image backend if `libvirt.images_type \u003d default` https://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/driver.py#L415\n2. this very conversion of raw to qcow in finish_migration https://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/driver.py#L11737-L11739\n\nhttps://codesearch.openstack.org/?q\u003duse_cow_images\u0026i\u003dnope\u0026literal\u003dnope\u0026files\u003d\u0026excludeFiles\u003dtest\u0026repos\u003dairship%2Fapis%2Copenstack%2Fnova\n\nthat\u0027s why it is so confusing, by the code it should not do anything else.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"35431f9faaea7c560b155b64528e34bfcbd49751","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"328caa3c_afb12ee8","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"18b1616e_959168c2","updated":"2023-10-23 12:59:40.000000000","message":"I did the testing with full set of permutations of:\n\n- libvirt.images_type \u003d default | qcow2 | flat | raw\n- use_cow_images \u003d True | False\n- force_raw_images \u003d True | False\n- Glance image \u003d qcow2 | raw\n\nI was booting an instance and checking the `qemu-img info $nova_state_path/instances/\u003cinstance_id\u003e/disk` for the image format, if the backing file is used and the backing file format.\n\nResults: https://paste.opendev.org/show/bAMQ9pR9tOewEDKB4roK/\n\nSummary:\n\n- `libvirt.images_type \u003d raw` behaves exactly the same as `flat`\n- `use_cow_images` has any effect only with `libvirt.images_type \u003d default`\n\n  `images_type \u003d default` + `use_cow_images \u003d True` \u003d\u003d `images_type \u003d qcow2`\n\n  `images_type \u003d default` + `use_cow_images \u003d False` \u003d\u003d `images_type \u003d flat`\n\n- when non-default `images_type` is configured, use_cow_images has no effect at all\n\nThat\u0027s it, it plays no other role with libvirt driver when booting the instance (is used however is several places with hyperv though).\n\nIt is however used in finish_migration to decide if to convert the image during cold migration. I believe this is a leftover from the times when there was no separation between qcow2 and \u0027raw/flat\u0027, and this option was the only one on which the format of the instance disk was chosen on.\nAFAIK since we generate the XML on the target node from scratch but telling it to use existing copied from src host image. The problem is when the XML we generate based on settings of current (target) host says one thing, but the actual file format is something else - if we think it is qcow2, but it is raw, we have security vulnerability, if we think it is raw but it is qcow2 - instance does not start at all.\nSo it seems this check in finish_migration is broken now, it does not account for all the possible ways how this vulnerability may be triggered, and more over, breaks other legitimate scenarios.\n\nWhat should be done IMO:\n1. fix the check in finish_migration to not rely on `use_cow_images`\n2. deprecate the usage of `use_cow_images` with libvirt driver\n3. deprecate `default` value for `libvirt.images_type`\n\nwe could start by emitting a startup warning in libvirt driver when use_cow_images \u003d False and images_type \u003d default, to warn those relying on that behavior of choosing \u0027flat\u0027 that this is deprecated, and they rather must set it explicitly.\n\nat the same time, we should deprecate the use_cow_images and copy it over to hyperv.use_cow_images, and use that in the hyperv driver.\n\nI\u0027ll copy this comment over to the LP bug, and will continue using this patch for option deprecation changes, and rebase it on top of patch that fixes the finish_migration check.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c6b19d531feac1e00c92c3dcc95ab5cf337a7c1","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"a72957aa_dbc38086","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"328caa3c_afb12ee8","updated":"2023-10-23 20:04:33.000000000","message":"so the expecte behvior for flat is to mirrof default\n\n\n\nimages_type \u003d default + use_cow_images \u003d True \u003d\u003d images_type \u003d qcow2\n\nimages_type \u003d default + use_cow_images \u003d False \u003d\u003d images_type \u003d flat\n\nexcept the qcow iamges shoudl not have a backing file.\n\nthat is obvoulsy not what is happening based on yoru testing but that is the expection.\n\nironicly the cause of some of this confution might be \n\nhttps://github.com/openstack/nova/commit/cff4d78a4a7ddbb18cb875c66b3c56f04a6caea3\n\nit renamed raw to flat since orfignally the use_cow_images and force_raw_images flags were ment to contol image convertion.\n\nso use_cow_images\u003dtrue, force_raw_images\u003dfalse was ment to confvert image to qcow\nuse_cow_images\u003dfalse, force_raw_images\u003dtrue was ment to convert to raw\n\nuse_cow_images\u003dfalse, force_raw_images\u003dfalse woudl do no image conversion\nand use_cow_images\u003dtrue, force_raw_images\u003dtrue was ment to resutlt in usign the qcow backet with a raw backing file.\n\n\nim not nessiarly agaisnt deprecation of defautl but i dont think its that simple.\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#libvirt.virt_type\nif virt_type is lxc or parlallels the defautl image type is a different format.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"a7a98a7508b6edc4749c1f318675e2ef667972cf","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ee2b4193_987c2faf","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"3a7f0f94_ce18f3da","updated":"2023-10-20 09:57:52.000000000","message":"\u003e flat and use_cow_images\u003dtrue should result in the vm booting with qcow fromat and then migrating with cow format.\n\nthen this is what is not working, the VM was booted with raw disk and a backing file..","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"fc16d5a82077d6a86da63a732c4267ce2f09aef4","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3a7f0f94_ce18f3da","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"a2304b14_fa93e7f1","updated":"2023-10-17 14:26:44.000000000","message":"yes, I was using raw image in glance. With images_type \u003d flat (or raw, behaves the same), and the rest defaults (so use_cow_images \u003d True), the instance is created with its disk being in \u0027raw\u0027 format, but after cold migration it is converted to qcow w/o changing XML, breaking instance.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1101b7066d71db23023f5c25201506f8e328026e","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"a2304b14_fa93e7f1","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"a822c0ca_e3120742","updated":"2023-10-17 11:26:49.000000000","message":"flat and use_cow_images\u003dtrue should result in the vm booting with qcow fromat and then migrating with cow format.\n \ni think there is also a sperate issue where you orginally tried to fix the issue in your first patch.\n\n https://review.opendev.org/c/openstack/nova/+/897842/4/nova/virt/libvirt/driver.py#b11738\n``` \n             if (disk_name !\u003d \u0027disk.config\u0027 and\n                 info[\u0027type\u0027] \u003d\u003d \u0027raw\u0027 and CONF.use_cow_images):\n                self._disk_raw_to_qcow2(info[\u0027path\u0027])\n```             \nwhen you tested this with images_type\u003dflat and use_cow_images\u003dtrue did you use a raw image in glance or a qcow2?\n\nmy guess is the  info[\u0027type\u0027] \u003d\u003d \u0027raw\u0027 check is wrong because its not taking into account the actual file format of the file on disk but rather the file format it has in glance. so its not  chekcing if the image has already been converted to qcow.\n\nwhat we likely need to do is use qemu image to check the format on the filesystem\nhttps://github.com/openstack/nova/blob/master/nova/virt/images.py#L42C12-L42C12\n\n```\nfile_format \u003d jsonutils.loads(qemu_img_info(path, \u0027json\u0027)).get(\u0027file_format\u0027, info[\u0027type\u0027])\n```\n\nthen update that if to \n\n```\n if (disk_name !\u003d \u0027disk.config\u0027 and\n     file_format \u003d\u003d \u0027raw\u0027 and CONF.use_cow_images):\n         self._disk_raw_to_qcow2(info[\u0027path\u0027])\n```","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"0f84ba5e940ec51afe5d080b3536fd632c045287","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"a822c0ca_e3120742","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"d70afb4e_931315d0","updated":"2023-10-17 11:01:59.000000000","message":"I am a bit puzzled then where this difference is playing out. The \u0027raw\u0027 is deprecated and \u0027flat\u0027 should be used instead. When you use the `libvirt.images_type \u003d raw`, the exact same `class Flat` is used - https://github.com/openstack/nova/blob/master/nova/virt/libvirt/imagebackend.py#L1325-L1326 so it should be exactly the same.\n\nI can perfectly reproduce the original issue with migrations with flat backend as well, when images_type is flat and use_cow_images\u003dtrue.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7bd10bfeae46a9a6c94e52d8c751ee306e272342","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"0fdd058c_ab43c54c","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"eade2fc3_6a1d6a0d","updated":"2023-10-20 11:46:30.000000000","message":"the full text of use_cow_images is \n\n```\nuse_cow_images¶\n\n    Type:\n\n        boolean\n    Default:\n\n        True\n\n    Enable use of copy-on-write (cow) images.\n\n    QEMU/KVM allow the use of qcow2 as backing files. By disabling this, backing files will not be used.\n```\n\n\"Enable use of copy-on-write (cow) images.\" is refering to using qcow2 images for the vm disk irrespective fo if backing files are used.\n\n\"By disabling this, backing files will not be used\" is actuly a implemnation detail that raw files cant have backing files.\n\nuse_cow_images if i recall correctly also enampes image convertion in nova\n\ni.e. when the file is downsload form glance if its not in qcow format we use qemu image to convert it\n\nforce_raw_images is explcitly about the backing file\n\n```\nforce_raw_images\n\n    Type:\n\n        boolean\n    Default:\n\n        True\n\n    Force conversion of backing images to raw format.\n\n    Possible values:\n\n        True: Backing image files will be converted to raw image format\n\n        False: Backing image files will not be converted\n\n    Related options:\n\n        compute_driver: Only the libvirt driver uses this option.\n\n        [libvirt]/images_type: If images_type is rbd, setting this option to False is not allowed. See the bug https://bugs.launchpad.net/nova/+bug/1816686 for more details.\n```\n\nyou can have both use_cow_images\u003dtrue and force_raw_images\u003dtrue\n\nto use a raw image as the backing file with a qcow guest disk as an overlay\n\n\nwithout that if the glance iamge is a qcow if you have use_cow_images enabled then the backing file will be a qcow as well.\n\n\nflat jsut disables backing fiels entirely\n\nso every vms will have its own image with no layers.\n\nwhen use_cow_images\u003dtrue i belive the intent is that the image will be converted to qcow2 format\n\nand if use_cow_images\u003dfalse it should not be converted.\n\nbut this is where the docs are not precise enough sicne they dont actully descrpbe the flat behavior at all.\n\n\n\ni dont have time to replicate this today but ill see if i can find time during the ptg to configre diffent optiosn and try and see if we can build a matix of the curret behavior of the  vm spwan case. then we can figure out the requried behavior for migration.\n\n\nif you have time to do that adn comment back that woudl also help\n\nthe varibles we need to consider are.\n\nsource iamge, force_raw_images, use_cow_images, images_type, output_image_format, backing_file_format(if presnet)\n\nfor souce iamge we need to test raw and qcow images in galnce.\nimages_type we need to test flat,raw,qcow\n\nwith that data we can figure out what the intend behavior should be and update the docs for the releven options and then determin a fix fo rthe migration case.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"8178a0a3117873005d45771dc27f8da33d6969b5","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"the combination of"},{"line_number":10,"context_line":"`[DEFAULT]/use_cow_images \u003d True` and"},{"line_number":11,"context_line":"`[libvirt]/images_type \u003d flat` (or `raw`)"},{"line_number":12,"context_line":"makes no sense and can lead to instances breaking after migration."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Add a check in libvirt driver\u0027s init_host that fails to start"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"eade2fc3_6a1d6a0d","line":11,"range":{"start_line":11,"start_character":25,"end_line":11,"end_character":29},"in_reply_to":"ee2b4193_987c2faf","updated":"2023-10-20 11:22:14.000000000","message":"btw, when I read the help for the use_cow_images options, it seems to relate only to create backing file or not\n\n\u003e QEMU/KVM allow the use of qcow2 as backing files. By disabling this,\nbacking files will not be used.\n\nAt the same time, Flat is also described as \u0027never creating backing files\u0027\nhttps://docs.openstack.org/nova/latest/admin/configuration/hypervisor-kvm.html#configure-compute-backing-storage\n\n\u003e  The Flat back end uses either raw or QCOW2 storage. It never uses a backing store, so when using QCOW2 it copies an image rather than creating an overlay.  By default, it creates raw files but will use QCOW2 when creating a disk from a QCOW2 if force_raw_images is not set in configuration.\n\nThis only mentions that force_raw_images is used to determine if qcow2 disk will be created by Flat backend, no mentions of use_cow_images too.\n\nSo it would seem that Flat and use_cow_images\u003dTrue do indeed contradict each other - \"Flat\" means \u0027use no backing file\" , use_cow_images means \"do use backing file\"..","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c6b19d531feac1e00c92c3dcc95ab5cf337a7c1","unresolved":true,"context_lines":[{"line_number":11,"context_line":"confusing and leads to misconfiguration errors."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Instead, deprecate the \u0027default\u0027 value for the libvirt.images_type"},{"line_number":14,"context_line":"option (probably will be changed to \u0027qcow2\u0027 later),"},{"line_number":15,"context_line":"and ask operators to set it explicitly for now."},{"line_number":16,"context_line":"Also, move the use_cow_images option to [hyperv] section as this is the"},{"line_number":17,"context_line":"only driver where it is actually used for real."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"2f3c1fa8_9d6536cf","line":14,"updated":"2023-10-23 20:04:33.000000000","message":"it is actully qcow today id fyou set use all of our defaults.\n\ni.e. default + all the other options set at there default will result in a vm booting with cow.","commit_id":"e5edef05d3cebf48d0358310546134e79ad7854b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7c6b19d531feac1e00c92c3dcc95ab5cf337a7c1","unresolved":true,"context_lines":[{"line_number":14,"context_line":"option (probably will be changed to \u0027qcow2\u0027 later),"},{"line_number":15,"context_line":"and ask operators to set it explicitly for now."},{"line_number":16,"context_line":"Also, move the use_cow_images option to [hyperv] section as this is the"},{"line_number":17,"context_line":"only driver where it is actually used for real."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: #2038898"},{"line_number":20,"context_line":"Change-Id: Ifa3da501fc6ebfd9e560118f27b38f4692b8e7c1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"90dfebe3_5a8cd019","line":17,"updated":"2023-10-23 20:04:33.000000000","message":"thanks for building https://paste.opendev.org/show/bAMQ9pR9tOewEDKB4roK/\n\nso this feels like we had a regrssion at some point and didnt notice becuase it was possibe to disable image cachign and have qcow images in the past.\n\nlookign at the tabel \n\nimages_type\u003dflat + use_qow_images \u003d true is expected to produce a qcow backed instance form a raw glance image.\n\nlibvirt.images_type \u003d raw is also ment to convert qcow images to raw\n\nbaiscally images_type\u003dqcow is ment to mean use qcow iamges for the vm disk \nadn images_type\u003draw is ment to be short hand for user raw iamges regardless of the souce format\n\nflat is ment to be no backing file but dont convertr the image.\n\n\nso for ## libvirt.images_type \u003d raw\n```\n### use_qow_images \u003d true + force_raw_images \u003d false\n#### qcow2 glance image\ninstance disk: qcow2\nbacking file:  none\n```\nand \n```\n### use_qow_images \u003d false + force_raw_images \u003d false\n#### qcow2 glance image\ninstance disk: qcow2\nbacking file:  none\n```\nis unexpected\n\nas is \n\n```\n## libvirt.images_type \u003d default\n### use_qow_images \u003d false + force_raw_images \u003d false\nimages type effectively is `flat` because of `use_qow_images`\n#### qcow2 glance image\ninstance disk: qcow2\nbacking file:  none\n#### raw glance image\ninstance disk: raw\nbacking file:  none\n```\n\nand\n\n```\n## libvirt.images_type \u003d flat\n### use_qow_images \u003d false + force_raw_images \u003d true\n#### qcow2 glance image\ninstance disk: raw\nbacking file:  none\n### use_qow_images \u003d true + force_raw_images \u003d false\n#### raw glance image\ninstance disk: raw\nbacking file:  none\n### use_qow_images \u003d true + force_raw_images \u003d true\n#### qcow2 glance image\ninstance disk: raw\nbacking file:  none\n```\n\nwether the behavior you have found is desireable or not is a seperate topci but its surprising based on how i expect those option to interact.\n\nso there is clearly a bug or sevelera in this code but im not sure what the best path forward is.\n\nim going to add this to the ptg adgenda to get some more attention on this this week.\n\nim concerned with altering the behavior here as a bug fix and i woudl liek wider disussion on this before we proceed.","commit_id":"e5edef05d3cebf48d0358310546134e79ad7854b"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"e076325e7144d9f0ff75fad153f3dc74f8d400ef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b398dae3_2d7ced14","updated":"2023-10-13 13:13:51.000000000","message":"needs unit test","commit_id":"ff0c881395d5a696c7d19d8b992e342d4af9beff"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"cda254924dc8f6d675359f92dabeca3fd891ac39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ec8a3837_b8c13d8e","updated":"2023-10-13 14:17:50.000000000","message":"added unit test and note in the libvirt.images_type option help","commit_id":"7452f749f7c1ecf61cb0fcbbafbc29a8bc78f80e"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"4911802bb4672309acc8dc78207fb4d4185ad7b8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f7a18ab2_b053d1b4","updated":"2023-10-14 18:52:32.000000000","message":"need some changes to functional tests apparently","commit_id":"7452f749f7c1ecf61cb0fcbbafbc29a8bc78f80e"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"35431f9faaea7c560b155b64528e34bfcbd49751","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"52057362_fbaf1e57","updated":"2023-10-23 12:59:40.000000000","message":"@sean","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"8178a0a3117873005d45771dc27f8da33d6969b5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c6745df7_4222bc08","updated":"2023-10-20 11:22:14.000000000","message":"@sean","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79b3113f29a7d6dc0be1741b7f996da5c5737f26","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"09cfbb0c_7ca51e7c","updated":"2023-10-17 10:54:44.000000000","message":"it should not be possible to cause the\nincorrect invocation of the confvertion now but you could extend\nhttps://opendev.org/openstack/nova/src/branch/stable/2023.2/nova/tests/functional/libvirt/test_live_migration.py\n\nwith a new test to prove that too although its not strictly requried with this approch.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"eec043094d0c83d6bd6ed87e839b4d376728ed80","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"076c6ee6_9622be85","updated":"2023-10-23 20:42:47.000000000","message":"i tried to summerise your finding in the ptg adgenda here\nhttps://etherpad.opendev.org/p/nova-caracal-ptg#L444\n\nif you are free to attend then that always welcome but if not ill just highlight his issue to the wider nova comunity and see if i cna drive some agreement on how to fix this.\n\nas a side note im plannign to delte the hyperv driver this cycle and have the patche up already so we might jsut be able to deprecate and delete the use_qow_images config option instead.","commit_id":"e5edef05d3cebf48d0358310546134e79ad7854b"}],"nova/conf/libvirt.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79b3113f29a7d6dc0be1741b7f996da5c5737f26","unresolved":true,"context_lines":[{"line_number":930,"context_line":"If default is specified, then use_cow_images flag is used instead of this"},{"line_number":931,"context_line":"one."},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"Note that when using \u0027flat\u0027 or \u0027raw\u0027 images_type, \u0027use_cow_images\u0027 option"},{"line_number":934,"context_line":"must be set to False, otherwise nova-compute will fail to start."},{"line_number":935,"context_line":""},{"line_number":936,"context_line":"Related options:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1af3d45b_fc7ac857","line":933,"range":{"start_line":933,"start_character":22,"end_line":933,"end_character":30},"updated":"2023-10-17 10:54:44.000000000","message":"delete","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"}],"nova/tests/functional/libvirt/test_evacuate.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79b3113f29a7d6dc0be1741b7f996da5c5737f26","unresolved":true,"context_lines":[{"line_number":154,"context_line":"        super(_FlatTest, self).setUp()"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"        self.flags(group\u003d\u0027libvirt\u0027, images_type\u003d\u0027flat\u0027)"},{"line_number":157,"context_line":"        self.flags(use_cow_images\u003dFalse)"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        def fake_create_image(_self, *args, **kwargs):"},{"line_number":160,"context_line":"            # Simply ensure the file exists"}],"source_content_type":"text/x-python","patch_set":3,"id":"ea6cad7e_8bdfc7d8","line":157,"updated":"2023-10-17 10:54:44.000000000","message":"this can be true or false.","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79b3113f29a7d6dc0be1741b7f996da5c5737f26","unresolved":true,"context_lines":[{"line_number":21381,"context_line":"        self.assertFalse("},{"line_number":21382,"context_line":"            driver.capabilities.get(\u0027supports_address_space_emulated\u0027))"},{"line_number":21383,"context_line":""},{"line_number":21384,"context_line":"    def test_fail_on_use_cow_images_with_flat_images_type(self):"},{"line_number":21385,"context_line":"        self.flags(use_cow_images\u003dTrue)"},{"line_number":21386,"context_line":"        self.flags(images_type\u003d\"flat\", group\u003d\u0027libvirt\u0027)"},{"line_number":21387,"context_line":"        driver \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9701f8cf_bd524289","line":21384,"updated":"2023-10-17 10:54:44.000000000","message":"this is incorrect","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79b3113f29a7d6dc0be1741b7f996da5c5737f26","unresolved":true,"context_lines":[{"line_number":21388,"context_line":"        self.assertRaises(exception.InvalidConfiguration,"},{"line_number":21389,"context_line":"                        driver.init_host, \u0027dummyhost\u0027)"},{"line_number":21390,"context_line":""},{"line_number":21391,"context_line":"    def test_fail_on_use_cow_images_with_raw_images_type(self):"},{"line_number":21392,"context_line":"        self.flags(use_cow_images\u003dTrue)"},{"line_number":21393,"context_line":"        self.flags(images_type\u003d\"raw\", group\u003d\u0027libvirt\u0027)"},{"line_number":21394,"context_line":"        driver \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f721ae27_dea64fa7","line":21391,"updated":"2023-10-17 10:54:44.000000000","message":"this is valid","commit_id":"8fe9c0049b0551713af9d0101faaef08f8aa8d32"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e05905d4e553db53226ea1388b388a32b6fade9","unresolved":true,"context_lines":[{"line_number":823,"context_line":"        # raw/flat + use_cow_images makes no sense, and can lead to"},{"line_number":824,"context_line":"        # things like instance breaking after cold migration,"},{"line_number":825,"context_line":"        # see https://bugs.launchpad.net/nova/+bug/2038898 for more details"},{"line_number":826,"context_line":"        if CONF.libvirt.images_type in (\"flat\", \"raw\"):"},{"line_number":827,"context_line":"            if CONF.use_cow_images:"},{"line_number":828,"context_line":"                msg \u003d _(\"\u0027[DEFAULT]/use_cow_images \u003d True\u0027 is not \""},{"line_number":829,"context_line":"                        \"allowed with \u0027[libvirt]/images_type\u0027 set to \u0027raw\u0027 \""}],"source_content_type":"text/x-python","patch_set":1,"id":"14ee9e44_b614600f","line":826,"updated":"2023-10-13 14:03:35.000000000","message":"flat shoudl be ok as flat jost means dont create a backign file\nso flat with qcow format shoudl be valid i think.","commit_id":"ff0c881395d5a696c7d19d8b992e342d4af9beff"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"cda254924dc8f6d675359f92dabeca3fd891ac39","unresolved":true,"context_lines":[{"line_number":823,"context_line":"        # raw/flat + use_cow_images makes no sense, and can lead to"},{"line_number":824,"context_line":"        # things like instance breaking after cold migration,"},{"line_number":825,"context_line":"        # see https://bugs.launchpad.net/nova/+bug/2038898 for more details"},{"line_number":826,"context_line":"        if CONF.libvirt.images_type in (\"flat\", \"raw\"):"},{"line_number":827,"context_line":"            if CONF.use_cow_images:"},{"line_number":828,"context_line":"                msg \u003d _(\"\u0027[DEFAULT]/use_cow_images \u003d True\u0027 is not \""},{"line_number":829,"context_line":"                        \"allowed with \u0027[libvirt]/images_type\u0027 set to \u0027raw\u0027 \""}],"source_content_type":"text/x-python","patch_set":1,"id":"a111b81a_0d4e9976","line":826,"in_reply_to":"14ee9e44_b614600f","updated":"2023-10-13 14:17:50.000000000","message":"they are treated completely equally in the code as far as I can see\n\nhttps://opendev.org/openstack/nova/src/branch/stable/2023.2/nova/virt/libvirt/imagebackend.py#L1325-L1326","commit_id":"ff0c881395d5a696c7d19d8b992e342d4af9beff"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79b3113f29a7d6dc0be1741b7f996da5c5737f26","unresolved":true,"context_lines":[{"line_number":823,"context_line":"        # raw/flat + use_cow_images makes no sense, and can lead to"},{"line_number":824,"context_line":"        # things like instance breaking after cold migration,"},{"line_number":825,"context_line":"        # see https://bugs.launchpad.net/nova/+bug/2038898 for more details"},{"line_number":826,"context_line":"        if CONF.libvirt.images_type in (\"flat\", \"raw\"):"},{"line_number":827,"context_line":"            if CONF.use_cow_images:"},{"line_number":828,"context_line":"                msg \u003d _(\"\u0027[DEFAULT]/use_cow_images \u003d True\u0027 is not \""},{"line_number":829,"context_line":"                        \"allowed with \u0027[libvirt]/images_type\u0027 set to \u0027raw\u0027 \""}],"source_content_type":"text/x-python","patch_set":1,"id":"04c75796_4f26fd37","line":826,"in_reply_to":"a111b81a_0d4e9976","updated":"2023-10-17 10:54:44.000000000","message":"that is not correct so raw used to be its own backend and it got collapesed into flat\n\n\nraw is shorthand for flat+use_cow_images\u003dfalse","commit_id":"ff0c881395d5a696c7d19d8b992e342d4af9beff"}]}