)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8d4ed4190687f82acc5501861dbc7f3b480c622","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0a3cad23_5bf13a82","updated":"2024-07-04 21:03:08.000000000","message":"Without checking your actual work to detect and parse the header (or trying it locally yet) this looks mostly good to me other than the assert stuff.\n\nThanks!","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"babc6be26df34a13284f2cf8aec04deaccc3fa1f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ad20222e_63eff4dc","updated":"2024-07-05 16:54:44.000000000","message":"First thing I found while testing with devstack:\n```\nJul 05 16:52:34 jammy nova-compute[525108]: ERROR nova.compute.manager [instance: 72d902f3-da4e-4e32-9bb8-c1608e29ffda] nova.exception.InvalidDiskInfo: Disk info file is invalid: qemu-img failed to execute on /opt/stack/data/nova/instances/_base/98ceb46531ca8deb15bfed58b3297eb69f76bde1.part : Unexpected error while running command.\nJul 05 16:52:34 jammy nova-compute[525108]: ERROR nova.compute.manager [instance: 72d902f3-da4e-4e32-9bb8-c1608e29ffda] Command: /opt/stack/data/venv/bin/python3.10 -m oslo_concurrency.prlimit --as\u003d1073741824 --cpu\u003d30 -- env LC_ALL\u003dC LANG\u003dC qemu-img info /opt/stack/data/nova/instances/_base/98ceb46531ca8deb15bfed58b3297eb69f76bde1.part --force-share --output\u003djson -f iso\nJul 05 16:52:34 jammy nova-compute[525108]: ERROR nova.compute.manager [instance: 72d902f3-da4e-4e32-9bb8-c1608e29ffda] Exit code: 1\nJul 05 16:52:34 jammy nova-compute[525108]: ERROR nova.compute.manager [instance: 72d902f3-da4e-4e32-9bb8-c1608e29ffda] Stdout: \u0027\u0027\nJul 05 16:52:34 jammy nova-compute[525108]: ERROR nova.compute.manager [instance: 72d902f3-da4e-4e32-9bb8-c1608e29ffda] Stderr: \"qemu-img: Could not open \u0027/opt/stack/data/nova/instances/_base/98ceb46531ca8deb15bfed58b3297eb69f76bde1.part\u0027: Unknown driver \u0027iso\u0027\\n\"\n```\n\nWe need to coerce iso to raw I think before we call `qemu-img` on it, but after we safety-check it.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"20f01df7e620c93b95f612632d2eab5afd194b48","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"63b611ed_e62670e1","updated":"2024-07-05 17:20:41.000000000","message":"Yeah, adding that makes it work as expected for me. Also confirmed the \"multiple formats detected\" does the right thing. I\u0027ll push up the two-line revision to this so it\u0027s available. Still haven\u0027t reviewed the current version yet.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"907ee5b0_b63a68ed","updated":"2024-07-05 17:46:35.000000000","message":"I\u0027m going to write a test and make sure I didn\u0027t break any others, but otherwise this looks good to me and it tests correct locally in devstack with my iso-\u003eraw fix.","commit_id":"4f1d55a64f233eab9e593f4e532540baccd7752a"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"a27b5bdc45385c72b9f909e806b1550ef42757e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"8023cc69_98f8e6cd","updated":"2024-07-05 18:52:59.000000000","message":"note that booting ISO is anyway broken ATM\nhttps://bugs.launchpad.net/nova/+bug/2054446\nhttps://review.opendev.org/c/openstack/nova/+/909611","commit_id":"f6d27a9bd4860a0a7bbf0216c63b6d7c95c6453b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f04b095e146833b51b12fd9166536549572c9450","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"69517c5a_cb141950","updated":"2024-07-05 21:22:36.000000000","message":"I\u0027m out of time today, but note that I was testing this for boot and download identification of the iso stuff. I have not actually replicated the rescue scenario that the bug reported (and I\u0027m guessing that isn\u0027t affected by the other bug pavlo reported earlier). So we should confirm that this actually fixes the report in the bug.","commit_id":"448be1c4462b2743cb40bb7fe0a3bd84570386ae"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4154c7b178188198a27bf44899d038c79f41365","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ec3f85cf_8ab68ae5","updated":"2024-07-08 09:57:29.000000000","message":"Patch looks good to me, but we still have test timeouts.","commit_id":"448be1c4462b2743cb40bb7fe0a3bd84570386ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fe88aebccf8a3853fc412705cb39f2f6c0d4c70f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"4f94caaa_3e8618ed","in_reply_to":"69517c5a_cb141950","updated":"2024-07-08 12:34:02.000000000","message":"i have deployed this locally with devstack.\n\ni can confirm this fixes the rescue case\n\ni booted a vm from the cirros iamge using the default qcow backend\nthen i uploaded an alpine image\n\nhttps://dl-cdn.alpinelinux.org/alpine/v3.20/releases/x86_64/alpine-standard-3.20.1-x86_64.iso\n\nand used that as a resuce image. \n\nit not only boots but i could login and mount the orginal cirros image ectra\nso this usecase appares to be fixed by this patch.\n\nunrescue also works as expected and the vm booted form the orgial cirros root disk.","commit_id":"448be1c4462b2743cb40bb7fe0a3bd84570386ae"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7d26c295d41bf3369524fbbc0192e4e00828b891","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"5604d36a_dc443a36","updated":"2024-07-08 16:21:37.000000000","message":"Tested locally with real ISOs and works for me. I had only a few small tweaks in here but otherwise this was all Sean, so I\u0027m approving.","commit_id":"b1cc39848ebe9b9cb63141a647bda52a2842ee4b"}],"nova/image/format_inspector.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8d4ed4190687f82acc5501861dbc7f3b480c622","unresolved":true,"context_lines":[{"line_number":882,"context_line":"        if not self.region(\u0027system_area\u0027).complete:"},{"line_number":883,"context_line":"            return False"},{"line_number":884,"context_line":"        if not self.region(\u0027header\u0027).complete:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        signature \u003d self.region(\u0027header\u0027).data[1:6]"},{"line_number":888,"context_line":"        assert len(signature) \u003d\u003d 5"}],"source_content_type":"text/x-python","patch_set":2,"id":"75ddc439_ba760773","line":885,"updated":"2024-07-04 21:03:08.000000000","message":"Note you can use `self.complete` for \"all my regions are complete\" as it\u0027s shorter, but this is of course not wrong.","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2c14500f536312ba9bfd88441a27207baadf7f48","unresolved":true,"context_lines":[{"line_number":882,"context_line":"        if not self.region(\u0027system_area\u0027).complete:"},{"line_number":883,"context_line":"            return False"},{"line_number":884,"context_line":"        if not self.region(\u0027header\u0027).complete:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        signature \u003d self.region(\u0027header\u0027).data[1:6]"},{"line_number":888,"context_line":"        assert len(signature) \u003d\u003d 5"}],"source_content_type":"text/x-python","patch_set":2,"id":"c3e677da_1096a6c4","line":885,"in_reply_to":"75ddc439_ba760773","updated":"2024-07-05 00:33:53.000000000","message":"thanks ya that will make this cleaner.","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":false,"context_lines":[{"line_number":882,"context_line":"        if not self.region(\u0027system_area\u0027).complete:"},{"line_number":883,"context_line":"            return False"},{"line_number":884,"context_line":"        if not self.region(\u0027header\u0027).complete:"},{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        signature \u003d self.region(\u0027header\u0027).data[1:6]"},{"line_number":888,"context_line":"        assert len(signature) \u003d\u003d 5"}],"source_content_type":"text/x-python","patch_set":2,"id":"d337c985_63b62188","line":885,"in_reply_to":"c3e677da_1096a6c4","updated":"2024-07-05 17:46:35.000000000","message":"Done","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f8d4ed4190687f82acc5501861dbc7f3b480c622","unresolved":true,"context_lines":[{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        signature \u003d self.region(\u0027header\u0027).data[1:6]"},{"line_number":888,"context_line":"        assert len(signature) \u003d\u003d 5"},{"line_number":889,"context_line":"        return signature in (b\u0027CD001\u0027, b\u0027NSR02\u0027, b\u0027NSR03\u0027)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"6ec30447_5fe7f149","line":888,"updated":"2024-07-04 21:03:08.000000000","message":"We should use actual checks and not asserts:\n\nhttps://snyk.io/blog/the-dangers-of-assert-in-python/\n\nPython with optimize turned on will ignore them, and it\u0027s common in packaged/frozen code (more common on macos and windows, granted) to do that as part of the build process. I\u0027m definitely guilty of using assert myself, but lately I\u0027ve been trying to follow this rule.","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2c14500f536312ba9bfd88441a27207baadf7f48","unresolved":true,"context_lines":[{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        signature \u003d self.region(\u0027header\u0027).data[1:6]"},{"line_number":888,"context_line":"        assert len(signature) \u003d\u003d 5"},{"line_number":889,"context_line":"        return signature in (b\u0027CD001\u0027, b\u0027NSR02\u0027, b\u0027NSR03\u0027)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"ddbc6c8d_e06d74b0","line":888,"in_reply_to":"6ec30447_5fe7f149","updated":"2024-07-05 00:33:53.000000000","message":"i used assert because this is not actually needed at runtime and is useful for debugging. I added them purely to ensure i did the math correctly on the slicing\n\nso if you like I can entirely drop them or i can add an assert message to callout why they are there but it would be incorrect to use if here here.\nthese are here to ensure if someone ever touches this something will fail if they change the splice in a refactor.\n\nI am explicitly not using them for error handling or control flow here.\ni never use them for that an only ever use them for invaiants like this.\nim using them here because i had an off-by-one error and originally wrote [1:5]\n\nall the other assert is did the slice correctly but i put them in just to be extra sure i wrote it correclty.\n\ni would consider this a safe/correct use of assert based on https://snyk.io/blog/the-dangers-of-assert-in-python/\n\nim pretty sure i have read that before.\n\nif you want me to drop them\ni could rewrite all the slices as \n```\nval \u003d data[offset : (offset+lenght)]\n```\nor \n```\nstart \u003d offset\nend \u003d offset + lenght\nval \u003d data[start:end]\n```\ni can but i tought this was cleaner to juset directly slice with the bytes and assert the lenth of the returned slice was the size i exepected.","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":false,"context_lines":[{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        signature \u003d self.region(\u0027header\u0027).data[1:6]"},{"line_number":888,"context_line":"        assert len(signature) \u003d\u003d 5"},{"line_number":889,"context_line":"        return signature in (b\u0027CD001\u0027, b\u0027NSR02\u0027, b\u0027NSR03\u0027)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"014181d3_3e604c60","line":888,"in_reply_to":"b85e3201_5bdf1a7d","updated":"2024-07-05 17:46:35.000000000","message":"Acknowledged","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e7316336029bff374792d54af5dcaa7c03670bc9","unresolved":true,"context_lines":[{"line_number":885,"context_line":"            return False"},{"line_number":886,"context_line":""},{"line_number":887,"context_line":"        signature \u003d self.region(\u0027header\u0027).data[1:6]"},{"line_number":888,"context_line":"        assert len(signature) \u003d\u003d 5"},{"line_number":889,"context_line":"        return signature in (b\u0027CD001\u0027, b\u0027NSR02\u0027, b\u0027NSR03\u0027)"},{"line_number":890,"context_line":""},{"line_number":891,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":2,"id":"b85e3201_5bdf1a7d","line":888,"in_reply_to":"ddbc6c8d_e06d74b0","updated":"2024-07-05 13:55:20.000000000","message":"That\u0027s fine, I just wanted to point it out in case you were covering for ambiguities in the spec, or for values that must be something specific for the assumptions you\u0027re making. If it\u0027s just for debug sanity checking then that\u0027s what assert is for, all good. This is security-sensitive stuff and complex parsing, so I just wanted to make sure.","commit_id":"2147605b0baf291079444dcc515c5fef36f05ee9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"494e6b6ea2150403f890a733d78d2a74f1abb497","unresolved":true,"context_lines":[{"line_number":851,"context_line":"    to look for the ISO 9660 or UDF signature."},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    http://wiki.osdev.org/ISO_9660"},{"line_number":854,"context_line":"    http://wiki.osdev.org/UDFmkisofs --help  | grep udf"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"    The Universal Disc Format or UDF is the filesystem used on DVDs and"},{"line_number":857,"context_line":"    Blu-Ray discs.UDF is an extension of ISO 9660 and shares the same"}],"source_content_type":"text/x-python","patch_set":3,"id":"cb042f9e_11a0ac47","line":854,"updated":"2024-07-05 14:15:51.000000000","message":"nit: missing linebreak between the URL and the CLI command","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":true,"context_lines":[{"line_number":851,"context_line":"    to look for the ISO 9660 or UDF signature."},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    http://wiki.osdev.org/ISO_9660"},{"line_number":854,"context_line":"    http://wiki.osdev.org/UDFmkisofs --help  | grep udf"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"    The Universal Disc Format or UDF is the filesystem used on DVDs and"},{"line_number":857,"context_line":"    Blu-Ray discs.UDF is an extension of ISO 9660 and shares the same"}],"source_content_type":"text/x-python","patch_set":3,"id":"ed8e1f29_0bfeb566","line":854,"in_reply_to":"0a4d469a_7e788507","updated":"2024-07-05 17:46:35.000000000","message":"Oops I missed this in my respin too.","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"16d81f7e6451c2dbceeb24f0685194a6def93b51","unresolved":true,"context_lines":[{"line_number":851,"context_line":"    to look for the ISO 9660 or UDF signature."},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    http://wiki.osdev.org/ISO_9660"},{"line_number":854,"context_line":"    http://wiki.osdev.org/UDFmkisofs --help  | grep udf"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"    The Universal Disc Format or UDF is the filesystem used on DVDs and"},{"line_number":857,"context_line":"    Blu-Ray discs.UDF is an extension of ISO 9660 and shares the same"}],"source_content_type":"text/x-python","patch_set":3,"id":"0a4d469a_7e788507","line":854,"in_reply_to":"cb042f9e_11a0ac47","updated":"2024-07-05 15:44:38.000000000","message":"ack i missed that in my last resping i also need to adress your comment on the proceed patch so ill do both  shortly after i star setting up a devstack to test this locally.","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"872f08727e350ee0426a4525b58a517c919e0995","unresolved":false,"context_lines":[{"line_number":851,"context_line":"    to look for the ISO 9660 or UDF signature."},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    http://wiki.osdev.org/ISO_9660"},{"line_number":854,"context_line":"    http://wiki.osdev.org/UDFmkisofs --help  | grep udf"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"    The Universal Disc Format or UDF is the filesystem used on DVDs and"},{"line_number":857,"context_line":"    Blu-Ray discs.UDF is an extension of ISO 9660 and shares the same"}],"source_content_type":"text/x-python","patch_set":3,"id":"e5f8ef73_50fdd4dc","line":854,"in_reply_to":"ed8e1f29_0bfeb566","updated":"2024-07-05 18:02:22.000000000","message":"Done","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"494e6b6ea2150403f890a733d78d2a74f1abb497","unresolved":true,"context_lines":[{"line_number":899,"context_line":"        # the rest of the header contains type specific data is 2041 bytes"},{"line_number":900,"context_line":"        # see http://wiki.osdev.org/ISO_9660#The_Primary_Volume_Descriptor"},{"line_number":901,"context_line":""},{"line_number":902,"context_line":"        # we need to check that the descriptor type is 1"},{"line_number":903,"context_line":"        # to ensure that this is a primary volume descriptor"},{"line_number":904,"context_line":"        descriptor_type \u003d self.region(\u0027header\u0027).data[0]"},{"line_number":905,"context_line":"        if descriptor_type !\u003d 1:"},{"line_number":906,"context_line":"            return 0"},{"line_number":907,"context_line":"        # The size in bytes of a logical block is stored at offset 128"},{"line_number":908,"context_line":"        # and is 2 bytes long encoded in both little and big endian"},{"line_number":909,"context_line":"        # int16_LSB-MSB so the field is 4 bytes long"}],"source_content_type":"text/x-python","patch_set":3,"id":"295d0158_7cfbd472","line":906,"range":{"start_line":902,"start_character":1,"end_line":906,"end_character":20},"updated":"2024-07-05 14:15:51.000000000","message":"does returning 0 indicates error? If not then I feel we need to signal error at this case when we cannot determine the virtual_size otherwise we allow a potentially invalid image to be used. I.e. having a valid ISO with size 0 is different than having an invalid ISO where the first volume is not a primary one.","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dbd12ced99cf8ed4aa4c507f97525b74a4128eac","unresolved":true,"context_lines":[{"line_number":899,"context_line":"        # the rest of the header contains type specific data is 2041 bytes"},{"line_number":900,"context_line":"        # see http://wiki.osdev.org/ISO_9660#The_Primary_Volume_Descriptor"},{"line_number":901,"context_line":""},{"line_number":902,"context_line":"        # we need to check that the descriptor type is 1"},{"line_number":903,"context_line":"        # to ensure that this is a primary volume descriptor"},{"line_number":904,"context_line":"        descriptor_type \u003d self.region(\u0027header\u0027).data[0]"},{"line_number":905,"context_line":"        if descriptor_type !\u003d 1:"},{"line_number":906,"context_line":"            return 0"},{"line_number":907,"context_line":"        # The size in bytes of a logical block is stored at offset 128"},{"line_number":908,"context_line":"        # and is 2 bytes long encoded in both little and big endian"},{"line_number":909,"context_line":"        # int16_LSB-MSB so the field is 4 bytes long"}],"source_content_type":"text/x-python","patch_set":3,"id":"61c1223a_f692b173","line":906,"range":{"start_line":902,"start_character":1,"end_line":906,"end_character":20},"in_reply_to":"285cdebd_e1fe1fe6","updated":"2024-07-05 16:44:16.000000000","message":"Yeah, this came from glance remember, and was designed to sit in the upload pipeline and grab some metadata as it flew by. It was designed to never ever fail, since it was just informational. Since virtual_size\u003d0 if we don\u0027t know the size, that\u0027s why we return it here. However, now we do need it to fail in places where we want to assert things like the safety check. Thus when I move this to oslo, this will likely be something to revisit. Glance will want to start rejecting images that do match and fail the safety check, even if it aborts the upload.","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"16d81f7e6451c2dbceeb24f0685194a6def93b51","unresolved":true,"context_lines":[{"line_number":899,"context_line":"        # the rest of the header contains type specific data is 2041 bytes"},{"line_number":900,"context_line":"        # see http://wiki.osdev.org/ISO_9660#The_Primary_Volume_Descriptor"},{"line_number":901,"context_line":""},{"line_number":902,"context_line":"        # we need to check that the descriptor type is 1"},{"line_number":903,"context_line":"        # to ensure that this is a primary volume descriptor"},{"line_number":904,"context_line":"        descriptor_type \u003d self.region(\u0027header\u0027).data[0]"},{"line_number":905,"context_line":"        if descriptor_type !\u003d 1:"},{"line_number":906,"context_line":"            return 0"},{"line_number":907,"context_line":"        # The size in bytes of a logical block is stored at offset 128"},{"line_number":908,"context_line":"        # and is 2 bytes long encoded in both little and big endian"},{"line_number":909,"context_line":"        # int16_LSB-MSB so the field is 4 bytes long"}],"source_content_type":"text/x-python","patch_set":3,"id":"285cdebd_e1fe1fe6","line":906,"range":{"start_line":902,"start_character":1,"end_line":906,"end_character":20},"in_reply_to":"295d0158_7cfbd472","updated":"2024-07-05 15:44:38.000000000","message":"that seems to be the patteren elsewhere we retrun 0 when the formats dont match for example\n\nthe docstrign for this method in the base file insepctor notes\n\n \"\"\"Returns the virtual size of the disk image, or zero if unknown.\"\"\"\n \n so if we have an image with a descriptior that does not match our expections\n then we woudl return 0 indicating unknown which i belive is the expected behavior.","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a283e4eaac64d5e2b8d3992d26babc5dcd26245","unresolved":false,"context_lines":[{"line_number":899,"context_line":"        # the rest of the header contains type specific data is 2041 bytes"},{"line_number":900,"context_line":"        # see http://wiki.osdev.org/ISO_9660#The_Primary_Volume_Descriptor"},{"line_number":901,"context_line":""},{"line_number":902,"context_line":"        # we need to check that the descriptor type is 1"},{"line_number":903,"context_line":"        # to ensure that this is a primary volume descriptor"},{"line_number":904,"context_line":"        descriptor_type \u003d self.region(\u0027header\u0027).data[0]"},{"line_number":905,"context_line":"        if descriptor_type !\u003d 1:"},{"line_number":906,"context_line":"            return 0"},{"line_number":907,"context_line":"        # The size in bytes of a logical block is stored at offset 128"},{"line_number":908,"context_line":"        # and is 2 bytes long encoded in both little and big endian"},{"line_number":909,"context_line":"        # int16_LSB-MSB so the field is 4 bytes long"}],"source_content_type":"text/x-python","patch_set":3,"id":"81488eeb_03da9375","line":906,"range":{"start_line":902,"start_character":1,"end_line":906,"end_character":20},"in_reply_to":"61c1223a_f692b173","updated":"2024-07-08 10:17:01.000000000","message":"Ack, ill keep the logic consistent for now and when this is moved to oslow we could perhaps raise and exception here instead and allow the caller to decide how to proceed.\n\nfor glance that might be treat as 0 for us I\u0027m not sure.\nwe dont nessisarly need the virtual size other than to check that the flavour is larger then the firtual size.\n\nthe side effect of not crating a disk large enough is the guest will get an io error before it believes it should be out of space the same as if we were on oversubscribed storage.\n\nthats not ideal but for iso which will generally be read-only anyway it\u0027s probably acceptable.","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4154c7b178188198a27bf44899d038c79f41365","unresolved":false,"context_lines":[{"line_number":899,"context_line":"        # the rest of the header contains type specific data is 2041 bytes"},{"line_number":900,"context_line":"        # see http://wiki.osdev.org/ISO_9660#The_Primary_Volume_Descriptor"},{"line_number":901,"context_line":""},{"line_number":902,"context_line":"        # we need to check that the descriptor type is 1"},{"line_number":903,"context_line":"        # to ensure that this is a primary volume descriptor"},{"line_number":904,"context_line":"        descriptor_type \u003d self.region(\u0027header\u0027).data[0]"},{"line_number":905,"context_line":"        if descriptor_type !\u003d 1:"},{"line_number":906,"context_line":"            return 0"},{"line_number":907,"context_line":"        # The size in bytes of a logical block is stored at offset 128"},{"line_number":908,"context_line":"        # and is 2 bytes long encoded in both little and big endian"},{"line_number":909,"context_line":"        # int16_LSB-MSB so the field is 4 bytes long"}],"source_content_type":"text/x-python","patch_set":3,"id":"d3495ce3_1a63894e","line":906,"range":{"start_line":902,"start_character":1,"end_line":906,"end_character":20},"in_reply_to":"61c1223a_f692b173","updated":"2024-07-08 09:57:29.000000000","message":"OK. I agree that this can be improved separately.","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6721c0a65cebfb866e71b81ad40681d5a150d997","unresolved":true,"context_lines":[{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"    :param filename: The path to the file to inspect."},{"line_number":1024,"context_line":"    :returns: A list of FormatInspector instances that matched."},{"line_number":1025,"context_line":"    if no match is found, a single raw inspector is returned."},{"line_number":1026,"context_line":"    \"\"\""},{"line_number":1027,"context_line":"    inspectors \u003d {k: v() for k, v in ALL_FORMATS.items()}"},{"line_number":1028,"context_line":"    detections \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"f1181d9a_d0bb7bb6","line":1025,"updated":"2024-07-05 14:04:05.000000000","message":"This is a delta from the others, which we _can_ handle, but I think we should try to avoid if possible. I also think the effect it has on every other place this is used is kinda unfortunate.\n\nIMHO, it would be fine for us to raise `ImageFormatError(\u0027ambiguous file format\u0027)` or something here, right?","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":false,"context_lines":[{"line_number":1022,"context_line":""},{"line_number":1023,"context_line":"    :param filename: The path to the file to inspect."},{"line_number":1024,"context_line":"    :returns: A list of FormatInspector instances that matched."},{"line_number":1025,"context_line":"    if no match is found, a single raw inspector is returned."},{"line_number":1026,"context_line":"    \"\"\""},{"line_number":1027,"context_line":"    inspectors \u003d {k: v() for k, v in ALL_FORMATS.items()}"},{"line_number":1028,"context_line":"    detections \u003d []"}],"source_content_type":"text/x-python","patch_set":3,"id":"a575fa85_43809890","line":1025,"in_reply_to":"f1181d9a_d0bb7bb6","updated":"2024-07-05 17:46:35.000000000","message":"Done","commit_id":"8865bda4ddcbbdeed46de62749eb3e1899204b42"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":true,"context_lines":[{"line_number":926,"context_line":"        # can use the first 4 bytes which are the little endian part"},{"line_number":927,"context_line":"        volume_space_size, \u003d struct.unpack(\u0027\u003cL\u0027, volume_space_size_data[:4])"},{"line_number":928,"context_line":"        # the virtual size is the volume space size * logical block size"},{"line_number":929,"context_line":"        return volume_space_size * logical_block_size"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"    def __str__(self):"},{"line_number":932,"context_line":"        return \u0027iso\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf685a7d_4a7446f9","line":929,"updated":"2024-07-05 17:46:35.000000000","message":"This will be a nice addition for glance as well when it moves back there.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a283e4eaac64d5e2b8d3992d26babc5dcd26245","unresolved":false,"context_lines":[{"line_number":926,"context_line":"        # can use the first 4 bytes which are the little endian part"},{"line_number":927,"context_line":"        volume_space_size, \u003d struct.unpack(\u0027\u003cL\u0027, volume_space_size_data[:4])"},{"line_number":928,"context_line":"        # the virtual size is the volume space size * logical block size"},{"line_number":929,"context_line":"        return volume_space_size * logical_block_size"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":"    def __str__(self):"},{"line_number":932,"context_line":"        return \u0027iso\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"29cc6c4a_6f6cd12d","line":929,"in_reply_to":"bf685a7d_4a7446f9","updated":"2024-07-08 10:17:01.000000000","message":"Acknowledged","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":true,"context_lines":[{"line_number":1025,"context_line":"                        format !\u003d \u0027raw\u0027):"},{"line_number":1026,"context_line":"                    # record all match (other than raw)"},{"line_number":1027,"context_line":"                    detections.append(inspector)"},{"line_number":1028,"context_line":"                    inspectors.pop(format)"},{"line_number":1029,"context_line":"            if all(i.complete for i in inspectors.values()):"},{"line_number":1030,"context_line":"                # If all the inspectors are sure they are not a match, avoid"},{"line_number":1031,"context_line":"                # reading to the end of the file to settle on \u0027raw\u0027."}],"source_content_type":"text/x-python","patch_set":4,"id":"1ec7b4aa_88d079d0","line":1028,"updated":"2024-07-05 17:46:35.000000000","message":"Oh, nice optimization here.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a283e4eaac64d5e2b8d3992d26babc5dcd26245","unresolved":false,"context_lines":[{"line_number":1025,"context_line":"                        format !\u003d \u0027raw\u0027):"},{"line_number":1026,"context_line":"                    # record all match (other than raw)"},{"line_number":1027,"context_line":"                    detections.append(inspector)"},{"line_number":1028,"context_line":"                    inspectors.pop(format)"},{"line_number":1029,"context_line":"            if all(i.complete for i in inspectors.values()):"},{"line_number":1030,"context_line":"                # If all the inspectors are sure they are not a match, avoid"},{"line_number":1031,"context_line":"                # reading to the end of the file to settle on \u0027raw\u0027."}],"source_content_type":"text/x-python","patch_set":4,"id":"5e9533af_dda77ebc","line":1028,"in_reply_to":"1ec7b4aa_88d079d0","updated":"2024-07-08 10:17:01.000000000","message":"in the previous behaviour because it early outs with a return it effectively did the same but ya this is the need to not keep checking all formats that already matched for the full 32KB of the iso system area.\n\nthe more formats we know how to inspect the more important that is.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":true,"context_lines":[{"line_number":1034,"context_line":"    if len(detections) \u003e 1:"},{"line_number":1035,"context_line":"        all_formats \u003d [str(inspector) for inspector in detections]"},{"line_number":1036,"context_line":"        raise exception.InvalidDiskInfo("},{"line_number":1037,"context_line":"            \u0027Multiple formats detected: %s\u0027 % \u0027, \u0027.join(all_formats))"},{"line_number":1038,"context_line":""},{"line_number":1039,"context_line":"    return inspectors[\u0027raw\u0027] if not detections else detections[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"253acbbd_4028f0b7","line":1037,"updated":"2024-07-05 17:46:35.000000000","message":"This is good, and I tested locally in devstack confirming it works.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a283e4eaac64d5e2b8d3992d26babc5dcd26245","unresolved":false,"context_lines":[{"line_number":1034,"context_line":"    if len(detections) \u003e 1:"},{"line_number":1035,"context_line":"        all_formats \u003d [str(inspector) for inspector in detections]"},{"line_number":1036,"context_line":"        raise exception.InvalidDiskInfo("},{"line_number":1037,"context_line":"            \u0027Multiple formats detected: %s\u0027 % \u0027, \u0027.join(all_formats))"},{"line_number":1038,"context_line":""},{"line_number":1039,"context_line":"    return inspectors[\u0027raw\u0027] if not detections else detections[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"c96672f1_0b1c0094","line":1037,"in_reply_to":"253acbbd_4028f0b7","updated":"2024-07-08 10:17:01.000000000","message":"Acknowledged","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6e75b6e6a1aaa7c2ec262033f12f28b7d32f00c4","unresolved":true,"context_lines":[{"line_number":1034,"context_line":""},{"line_number":1035,"context_line":"    if len(detections) \u003e 1:"},{"line_number":1036,"context_line":"        all_formats \u003d [str(inspector) for inspector in detections]"},{"line_number":1037,"context_line":"        raise exception.InvalidDiskInfo("},{"line_number":1038,"context_line":"            \u0027Multiple formats detected: %s\u0027 % \u0027, \u0027.join(all_formats))"},{"line_number":1039,"context_line":""},{"line_number":1040,"context_line":"    return inspectors[\u0027raw\u0027] if not detections else detections[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"8fd63b37_a3b36836","line":1037,"updated":"2024-07-05 19:19:35.000000000","message":"Also, if we can, let\u0027s raise a format_inspector-specific exception here so that we\u0027re not adding too much nova into this that will have to be un-done later. I think `ImageFormatError` is fine and any exception handlers that look for that will not have to change once this moves back out of the tree.","commit_id":"f6d27a9bd4860a0a7bbf0216c63b6d7c95c6453b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37ef3225f5048558be8ffba4fa923c540a942a6d","unresolved":false,"context_lines":[{"line_number":1034,"context_line":""},{"line_number":1035,"context_line":"    if len(detections) \u003e 1:"},{"line_number":1036,"context_line":"        all_formats \u003d [str(inspector) for inspector in detections]"},{"line_number":1037,"context_line":"        raise exception.InvalidDiskInfo("},{"line_number":1038,"context_line":"            \u0027Multiple formats detected: %s\u0027 % \u0027, \u0027.join(all_formats))"},{"line_number":1039,"context_line":""},{"line_number":1040,"context_line":"    return inspectors[\u0027raw\u0027] if not detections else detections[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"bca48a37_89d3c377","line":1037,"in_reply_to":"8fd63b37_a3b36836","updated":"2024-07-05 19:33:49.000000000","message":"Done","commit_id":"f6d27a9bd4860a0a7bbf0216c63b6d7c95c6453b"}],"nova/tests/unit/image/test_format_inspector.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":true,"context_lines":[{"line_number":268,"context_line":"            shell\u003dTrue)"},{"line_number":269,"context_line":"        subprocess.check_output("},{"line_number":270,"context_line":"            \u0027dd if\u003d%s of\u003d%s bs\u003d32K skip\u003d1 seek\u003d1\u0027 % (iso, fn),"},{"line_number":271,"context_line":"            shell\u003dTrue)"},{"line_number":272,"context_line":"        return qcow, iso, fn"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def test_bad_iso_qcow2(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"f2b8fd96_3b8a0d47","line":271,"updated":"2024-07-05 17:46:35.000000000","message":"Might be better to just do this ourselves instead of shelling out to dd, but no reason to change this now. I may do that in the great move to oslo.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79277d7adbc1a962bb18dd704393f405b78be89f","unresolved":false,"context_lines":[{"line_number":268,"context_line":"            shell\u003dTrue)"},{"line_number":269,"context_line":"        subprocess.check_output("},{"line_number":270,"context_line":"            \u0027dd if\u003d%s of\u003d%s bs\u003d32K skip\u003d1 seek\u003d1\u0027 % (iso, fn),"},{"line_number":271,"context_line":"            shell\u003dTrue)"},{"line_number":272,"context_line":"        return qcow, iso, fn"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"    def test_bad_iso_qcow2(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"c8d5b256_d9b12fde","line":271,"in_reply_to":"f2b8fd96_3b8a0d47","updated":"2024-07-08 10:41:57.000000000","message":"ack ill leave this for that refactor.","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":true,"context_lines":[{"line_number":290,"context_line":"        e \u003d self.assertRaises("},{"line_number":291,"context_line":"            exception.InvalidDiskInfo,"},{"line_number":292,"context_line":"            format_inspector.detect_file_format, fn)"},{"line_number":293,"context_line":"        self.assertIn(\u0027Multiple formats detected\u0027, str(e))"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def test_vhd(self):"},{"line_number":296,"context_line":"        self._test_format(\u0027vhd\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"617f160b_971150b1","line":293,"updated":"2024-07-05 17:46:35.000000000","message":"++ nice","commit_id":"c15753e8acef507277c3e281061fcd2a434dae30"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e4154c7b178188198a27bf44899d038c79f41365","unresolved":true,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        # Read the format in various sizes, some of which will read whole"},{"line_number":205,"context_line":"        # sections in a single read, others will be completely unaligned, etc."},{"line_number":206,"context_line":"        block_sizes \u003d (64 * units.Ki, 512, 1 * units.Mi)"},{"line_number":207,"context_line":"        # ISO images have a 32KB system area at the beginning of the image"},{"line_number":208,"context_line":"        # reading that in 17 byte blocks takes too long, so skip it"},{"line_number":209,"context_line":"        if format_name !\u003d \u0027iso\u0027:"},{"line_number":210,"context_line":"            block_sizes +\u003d (17,)"},{"line_number":211,"context_line":"        for block_size in block_sizes:"},{"line_number":212,"context_line":"            fmt \u003d self._test_format_at_block_size(format_name, img, block_size)"},{"line_number":213,"context_line":"            self.assertTrue(fmt.format_match,"},{"line_number":214,"context_line":"                            \u0027Failed to match %s at size %i block %i\u0027 % ("},{"line_number":215,"context_line":"                                format_name, image_size, block_size))"}],"source_content_type":"text/x-python","patch_set":7,"id":"56a65037_94096f74","line":212,"range":{"start_line":206,"start_character":0,"end_line":212,"end_character":79},"updated":"2024-07-08 09:57:29.000000000","message":"This still times out in the coverage job: https://zuul.opendev.org/t/openstack/build/b62d98c841a84ca8b56ec53cc1a0c23b/log/job-output.txt#22086","commit_id":"448be1c4462b2743cb40bb7fe0a3bd84570386ae"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"79277d7adbc1a962bb18dd704393f405b78be89f","unresolved":true,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        # Read the format in various sizes, some of which will read whole"},{"line_number":205,"context_line":"        # sections in a single read, others will be completely unaligned, etc."},{"line_number":206,"context_line":"        block_sizes \u003d (64 * units.Ki, 512, 1 * units.Mi)"},{"line_number":207,"context_line":"        # ISO images have a 32KB system area at the beginning of the image"},{"line_number":208,"context_line":"        # reading that in 17 byte blocks takes too long, so skip it"},{"line_number":209,"context_line":"        if format_name !\u003d \u0027iso\u0027:"},{"line_number":210,"context_line":"            block_sizes +\u003d (17,)"},{"line_number":211,"context_line":"        for block_size in block_sizes:"},{"line_number":212,"context_line":"            fmt \u003d self._test_format_at_block_size(format_name, img, block_size)"},{"line_number":213,"context_line":"            self.assertTrue(fmt.format_match,"},{"line_number":214,"context_line":"                            \u0027Failed to match %s at size %i block %i\u0027 % ("},{"line_number":215,"context_line":"                                format_name, image_size, block_size))"}],"source_content_type":"text/x-python","patch_set":7,"id":"6b1838e1_5c058097","line":212,"range":{"start_line":206,"start_character":0,"end_line":212,"end_character":79},"in_reply_to":"56a65037_94096f74","updated":"2024-07-08 10:41:57.000000000","message":"im updating this too\n\n```\n        # Read the format in various sizes, some of which will read whole\n        # sections in a single read, others will be completely unaligned, etc.\n        block_sizes \u003d [64 * units.Ki, 1 * units.Mi]\n        # ISO images have a 32KB system area at the beginning of the image\n        # as a result reading that in 17 or 512 byte blocks takes too long,\n        # causing the test to fail. The 64KiB block size is enough to read\n        # the system area and header in a single read. the 1MiB block size\n        # adds very little time to the test so we include it.\n        if format_name !\u003d \u0027iso\u0027:\n            block_sizes.extend([17, 512])\n```\n\nlocally that reduces the test time to 1/4 vs including 512\n\ni thought we might get away with 512 but apparently not.","commit_id":"448be1c4462b2743cb40bb7fe0a3bd84570386ae"}],"nova/tests/unit/virt/test_images.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3471ea5ed1671408b1bd1a8c2f547b8035f657fd","unresolved":true,"context_lines":[{"line_number":246,"context_line":"        inspector \u003d mock_gi.return_value.from_file.return_value"},{"line_number":247,"context_line":"        inspector.safety_check.return_value \u003d True"},{"line_number":248,"context_line":"        info \u003d {\u0027format\u0027: \u0027raw\u0027,"},{"line_number":249,"context_line":"                \u0027format-specific\u0027: {"},{"line_number":250,"context_line":"                    \u0027type\u0027: \u0027vmdk\u0027,"},{"line_number":251,"context_line":"                    \u0027data\u0027: {"},{"line_number":252,"context_line":"                        \u0027create-type\u0027: \u0027monolithicFlat\u0027,"},{"line_number":253,"context_line":"                }}}"},{"line_number":254,"context_line":"        mock_info.return_value \u003d jsonutils.dumps(info)"},{"line_number":255,"context_line":"        with mock.patch(\u0027os.path.exists\u0027, return_value\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":6,"id":"55d19fcd_5e53d1e7","line":252,"range":{"start_line":249,"start_character":0,"end_line":252,"end_character":56},"updated":"2024-07-05 19:05:14.000000000","message":"oops, should have snipped this out of what I copied it from, but it doesn\u0027t hurt. Remove if respin.","commit_id":"f6d27a9bd4860a0a7bbf0216c63b6d7c95c6453b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a283e4eaac64d5e2b8d3992d26babc5dcd26245","unresolved":true,"context_lines":[{"line_number":246,"context_line":"        inspector \u003d mock_gi.return_value.from_file.return_value"},{"line_number":247,"context_line":"        inspector.safety_check.return_value \u003d True"},{"line_number":248,"context_line":"        info \u003d {\u0027format\u0027: \u0027raw\u0027,"},{"line_number":249,"context_line":"                \u0027format-specific\u0027: {"},{"line_number":250,"context_line":"                    \u0027type\u0027: \u0027vmdk\u0027,"},{"line_number":251,"context_line":"                    \u0027data\u0027: {"},{"line_number":252,"context_line":"                        \u0027create-type\u0027: \u0027monolithicFlat\u0027,"},{"line_number":253,"context_line":"                }}}"},{"line_number":254,"context_line":"        mock_info.return_value \u003d jsonutils.dumps(info)"},{"line_number":255,"context_line":"        with mock.patch(\u0027os.path.exists\u0027, return_value\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":6,"id":"877655bd_005d0a15","line":252,"range":{"start_line":249,"start_character":0,"end_line":252,"end_character":56},"in_reply_to":"55d19fcd_5e53d1e7","updated":"2024-07-08 10:17:01.000000000","message":"i need to recheck this patch anyway and gibi had a few nits on the previous patch so ill just respin the patch and also update this\n\nthanks for adding the testcase.","commit_id":"f6d27a9bd4860a0a7bbf0216c63b6d7c95c6453b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fe88aebccf8a3853fc412705cb39f2f6c0d4c70f","unresolved":false,"context_lines":[{"line_number":246,"context_line":"        inspector \u003d mock_gi.return_value.from_file.return_value"},{"line_number":247,"context_line":"        inspector.safety_check.return_value \u003d True"},{"line_number":248,"context_line":"        info \u003d {\u0027format\u0027: \u0027raw\u0027,"},{"line_number":249,"context_line":"                \u0027format-specific\u0027: {"},{"line_number":250,"context_line":"                    \u0027type\u0027: \u0027vmdk\u0027,"},{"line_number":251,"context_line":"                    \u0027data\u0027: {"},{"line_number":252,"context_line":"                        \u0027create-type\u0027: \u0027monolithicFlat\u0027,"},{"line_number":253,"context_line":"                }}}"},{"line_number":254,"context_line":"        mock_info.return_value \u003d jsonutils.dumps(info)"},{"line_number":255,"context_line":"        with mock.patch(\u0027os.path.exists\u0027, return_value\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":6,"id":"55ad36cf_b8eeb320","line":252,"range":{"start_line":249,"start_character":0,"end_line":252,"end_character":56},"in_reply_to":"877655bd_005d0a15","updated":"2024-07-08 12:34:02.000000000","message":"Done","commit_id":"f6d27a9bd4860a0a7bbf0216c63b6d7c95c6453b"}],"nova/virt/images.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37951420c9ab22b1b63a34f0740bb49af5dea9a6","unresolved":true,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    if disk_format \u003d\u003d \u0027iso\u0027:"},{"line_number":179,"context_line":"        # ISO image passed safety check; qemu will treat this as raw from here"},{"line_number":180,"context_line":"        disk_format \u003d \u0027raw\u0027"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    return disk_format"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"388467f1_0ac1c824","line":180,"updated":"2024-07-05 17:46:35.000000000","message":"Oops, I need to write a test for this.","commit_id":"4f1d55a64f233eab9e593f4e532540baccd7752a"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"872f08727e350ee0426a4525b58a517c919e0995","unresolved":false,"context_lines":[{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    if disk_format \u003d\u003d \u0027iso\u0027:"},{"line_number":179,"context_line":"        # ISO image passed safety check; qemu will treat this as raw from here"},{"line_number":180,"context_line":"        disk_format \u003d \u0027raw\u0027"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    return disk_format"},{"line_number":183,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"2f362557_472d2fdf","line":180,"in_reply_to":"388467f1_0ac1c824","updated":"2024-07-05 18:02:22.000000000","message":"Done","commit_id":"4f1d55a64f233eab9e593f4e532540baccd7752a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a283e4eaac64d5e2b8d3992d26babc5dcd26245","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                reason\u003d_(\u0027Image not in a supported format\u0027))"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    if disk_format \u003d\u003d \u0027iso\u0027:"},{"line_number":179,"context_line":"        # ISO image passed safety check; qemu will treat this as raw from here"},{"line_number":180,"context_line":"        disk_format \u003d \u0027raw\u0027"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    return disk_format"}],"source_content_type":"text/x-python","patch_set":7,"id":"4df9fb2f_98765557","line":179,"updated":"2024-07-08 10:17:01.000000000","message":"as an aside it will but i would argue that is a bug in Qemu and it should not\nbe treating iso files  as raw. it really should treat them separately because they are a very different format especially if they are UDF format.\n\nyou can kind of get away with just DDing /dev/sda to a file and calling it an ISO in many cases  at least if you put the primary header at teh 32K offset but qemu should not be as fast and losse with what it considers a valid iso. its not the same as raw and it should eventurally be enchanced to know about ISOs.\n\nsince we cant rely on there tooling for security reason however this does not change the fact nova has to also know about ISOs so this is just a rant and has no real impact on the patch so marking resolved. im just annoyed they consider it to be raw and we need to do this as a result.","commit_id":"448be1c4462b2743cb40bb7fe0a3bd84570386ae"}]}
