)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0ea3be81f7dae1a68ad957806d3686c9c5a26329","unresolved":true,"context_lines":[{"line_number":18,"context_line":"place that are required for the test to actually run. So, this adds"},{"line_number":19,"context_line":"mocks for qemu_img_info() calls that actually try to read the file on"},{"line_number":20,"context_line":"disk, as well as the privsep chown() that attempts to run after."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ie731045629f0899840a4680d21793a16ade9b98e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8ecaaf4f_155b34f7","line":21,"updated":"2024-07-25 11:39:39.000000000","message":"if you do respin we proably shoudl also have a bug to track this by the way\nim fine with calling this related to the cve as it is but just noting it woudl be nice ot have something in the commit to explicitly state taht or a separate tracker.\n\nthats not enouch to hold this however.","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"343e8576527b4cc738b12a131cca760440b78d54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1c95be7f_c0d6a267","updated":"2024-07-24 22:37:37.000000000","message":"+1 for now\nthis is uploading images with the correct format when tatkign snapshots now\n\nthe issue i thought i hit was actully caused by exceeding the quota limit for total images\n\nwhich si why it worked when i deleted the snapshot and failed if i created it\n\nill review this again tomorrow when im less tired\n\nwe proably shoudl fix the incorect loging for over quota but that an exsiting seperate bug","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e9fb043031595bfd6d9723a252c3165052c32166","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1c14e03b_bf90a1b2","updated":"2024-07-25 07:44:39.000000000","message":"As far as I understand this change makes sense. Also I saw Sean was able to verify it.","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bcb6420593bee937998251e334658ae6705706ad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"89cd1cda_0e66a82c","updated":"2024-07-25 15:28:18.000000000","message":"Thanks for this patch.","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6386e28b528c27ad1507bf5228eebfc272a1f222","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bc11068a_d2bc305a","updated":"2024-07-25 13:48:26.000000000","message":"We can related-bug this: https://bugs.launchpad.net/nova/+bug/2073944\n\nI can respin if you think it\u0027s important.","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ebe7a8aa2936b838f399683ba3866d9ea4c6513d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"892099dd_24ee8085","updated":"2024-07-25 14:12:29.000000000","message":"i have aslo tested setting the snapshot_image_format to qcow \nand use_cow_iamge \u003d false and force_raw_iamages\u003dfalse images_type\u003dflat\n\n\nguest originally booted form iso or ami result in the creation fo qcow snapshots as expected.\n\nrebuilding a guest form the snaphost or shelving and unshlevign results in the format chaging to qcow as expected\n\nas does shelve/unshelve.\n\nso im pretty happy that this close the regression that were introduced \nwith the previously noted exception regarding iso+flat.","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f4e6af6ad44cd2548cde547cda1caa5283210211","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"535302b5_fd9b98d2","updated":"2024-07-25 10:22:40.000000000","message":"i have tested this manuallly using 3 differnt config\n\nuse_cow_iamge \u003d true and force_raw_iamages\u003dtrue\nuse_cow_iamge \u003d false and force_raw_iamages\u003dtrue\nuse_cow_iamge \u003d false and force_raw_iamages\u003dtrue images_type\u003dflat\n\nin all 3 cases snapshot, shelve, boot form snapshot and rebuild all work as expected when using a glace ami image\n\nin all 3 cases the format of the snapshot uploaded ot glance matches the format of the root disk of the vm (after it converted if configured)\n\ni also using a glance qcow image and that also works as expected.\n\nwhen testing with a glance iso image\n\nuse_cow_iamge \u003d false and force_raw_iamages\u003dtrue images_type\u003dflat\nfails\n\nthat is because we dont have code to detect that case and upload it as iso instead of as raw. the upload works but creating a an instance form it or rebuilding failes because we detact that its really an iso in nova.\n\nThis is a latent bug in the flat backend because qemu-img does not support iso.\nim incliend to say we can adress that gap seperatly but if we want to very quickly add an overried for the iso case where we removed the ami one i think that would be simple and approitate.\n\nthe iso edgecase is and edgecase of and edgecase so im not going to +w to allow dan to see this but ill leave him either respin or +w based on there opinion.\n\ni have a slight perfernce to fix it in this patch but not enouch to -1","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"7beb7128eb051ae2961e1d336f33d449fad1f90d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0b690934_23cd9e57","updated":"2024-07-25 12:41:07.000000000","message":"thx","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dc4fbe4b9ce875b3ddb5133d3e5ff4e48e0d8d17","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"37677d3e_143568c7","updated":"2024-07-25 14:15:28.000000000","message":"upgrading to +w based on local testing","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6386e28b528c27ad1507bf5228eebfc272a1f222","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a656282c_187181dd","in_reply_to":"06dd9bd7_5e8b08cb","updated":"2024-07-25 13:48:26.000000000","message":"So, I\u0027m honestly not sure what to do about that problem, but I\u0027m thinking it\u0027s maybe best solved elsewhere. Meaning, at this point in the code, if we think the image is a raw, we should upload it as a raw. The problem really came in from us taking something like an iso and faking that it was a raw internally because \"anything that qemu doesn\u0027t support is a raw\" which is not really the correct thing to do. Really, we should keep track of the format separately from what we tell qemu-img to do with it, but we don\u0027t have that level of tracking.\n\nAnother way to look at it is, if I own the instance and I write a qcow2 or iso header to my root disk right before I snapshot, nova is correct to say \"hey, you said this was raw, but it\u0027s not.\"","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"80a08c81e717c9f1ae7e697d34c89103a800f139","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"06dd9bd7_5e8b08cb","in_reply_to":"535302b5_fd9b98d2","updated":"2024-07-25 12:52:06.000000000","message":"copy paste error\n`use_cow_iamge \u003d false and force_raw_iamages\u003dtrue images_type\u003dflat`\nshoudl have been \n\n`use_cow_iamge \u003d false and force_raw_iamages\u003dfalse images_type\u003dflat`","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dc4fbe4b9ce875b3ddb5133d3e5ff4e48e0d8d17","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ed1bde26_a90191e4","in_reply_to":"a656282c_187181dd","updated":"2024-07-25 14:15:28.000000000","message":"ya so i dont think nova is incorrect to block it\nim gong to take the execitiv desiosn to declare this out of scope fo this patch and appove this change.\n\nwe can reflect on what to do about this as part of the general make \"iamge handling sane\" followups.","commit_id":"d5a631ba7791b37e49213707e4ea650a56d2ed9e"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e9fb043031595bfd6d9723a252c3165052c32166","unresolved":true,"context_lines":[{"line_number":3110,"context_line":"        if instance.os_type:"},{"line_number":3111,"context_line":"            metadata[\u0027properties\u0027][\u0027os_type\u0027] \u003d instance.os_type"},{"line_number":3112,"context_line":""},{"line_number":3113,"context_line":"        # NOTE(vish): glance forces ami disk format to be ami"},{"line_number":3114,"context_line":"        if image_meta.disk_format \u003d\u003d \u0027ami\u0027:"},{"line_number":3115,"context_line":"            metadata[\u0027disk_format\u0027] \u003d \u0027ami\u0027"},{"line_number":3116,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"f24decf4_30cbcf4d","side":"PARENT","line":3113,"updated":"2024-07-25 07:44:39.000000000","message":"This originates from the fix for https://bugs.launchpad.net/nova/+bug/873156 As far as understand this old bug the fix wasn\u0027t really about glance failing on ami but qemu-img failing to covert to it.","commit_id":"df39222b106326a4c28dee26b7127a61174d6b51"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fdb51b5009744319802aeb5634a2b777a7153d01","unresolved":true,"context_lines":[{"line_number":3110,"context_line":"        if instance.os_type:"},{"line_number":3111,"context_line":"            metadata[\u0027properties\u0027][\u0027os_type\u0027] \u003d instance.os_type"},{"line_number":3112,"context_line":""},{"line_number":3113,"context_line":"        # NOTE(vish): glance forces ami disk format to be ami"},{"line_number":3114,"context_line":"        if image_meta.disk_format \u003d\u003d \u0027ami\u0027:"},{"line_number":3115,"context_line":"            metadata[\u0027disk_format\u0027] \u003d \u0027ami\u0027"},{"line_number":3116,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b485d3d9_8fd448cf","side":"PARENT","line":3113,"in_reply_to":"f24decf4_30cbcf4d","updated":"2024-07-25 10:31:30.000000000","message":"as noted in the top level comment this is not requried or really correct to do for ami image however it is requried for one specific case of using iso images with no fromat conversion enabled in nova.\n\ni think we would need to handel any format that glance know about but qemu does not in a similar way.\n\ni also came aross https://docs.openstack.org/nova/latest/configuration/config.html#libvirt.snapshot_image_format in my review of the the config option\n\nin principal i think that is ment to override the behavior where we defautl to the same format as the root disk and allow you to standardise on a specific format\n\ni am going to try setting that ot diffent values and see what the havior is\n\nit default to None so it not used in genral and i suspsect this is very uncommin in pratics but i can see usecases for it. i.e. forcing raw for perfoamcne reasons but uploading qcow for storage efficency.","commit_id":"df39222b106326a4c28dee26b7127a61174d6b51"}]}